Re: [PATCH v2 2/3] net: add r9a06g032 MIIC driver

On 8/14/26 8:30 PM, Ralph Siemsen wrote:

[...]

> +++ b/drivers/net/Kconfig
> @@ -886,6 +886,12 @@ config RZN1_A5PSW
>   	  Support the Advanced 5-Port ethernet switch (A5PSW) found in
>   	  the Renesas RZ/N1 SoC.
>   
> +config RZN1_MIIC

config RENESAS_RZN1_MIIC

> +	bool "Renesas RZ/N1 MIIC ethernet accessory"
> +	help
> +	  Support the MIIC ethernet accessory found in Renesas RZ/N1 SoC.
> +	  These are media converters for MII/GMII/RGMII.
> +
>   config MPC8XX_FEC
>   	bool "Fast Ethernet Controller on MPC8XX"
>   	depends on MPC8xx
> diff --git a/drivers/net/Makefile b/drivers/net/Makefile
> index b8f270ae233..049212b1fd2 100644
> --- a/drivers/net/Makefile
> +++ b/drivers/net/Makefile
> @@ -95,6 +95,7 @@ obj-$(CONFIG_PIC32_ETH) += pic32_mdio.o pic32_eth.o
>   obj-$(CONFIG_RENESAS_ETHER_SWITCH) += rswitch.o
>   obj-$(CONFIG_RENESAS_RAVB) += ravb.o
>   obj-$(CONFIG_RZN1_A5PSW) += rzn1_a5psw.o
> +obj-$(CONFIG_RZN1_MIIC) += rzn1_miic.o
>   obj-$(CONFIG_RTL8139) += rtl8139.o
>   obj-$(CONFIG_RTL8169) += rtl8169.o
>   obj-$(CONFIG_SH_ETHER) += sh_eth.o
> diff --git a/drivers/net/rzn1_miic.c b/drivers/net/rzn1_miic.c
> new file mode 100644
> index 00000000000..491923d8511
> --- /dev/null
> +++ b/drivers/net/rzn1_miic.c
> @@ -0,0 +1,193 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * MII/GMII/RGMII converter support
> + *
> + * Based loosely on Linux drivers/net/pcs/pcs-rzn1-miic.c
> + * and the older driver in Renesas u-boot.

U-Boot

> + */

[...]

> +static void rzn1_rin_prot_writel(struct udevice *dev, u32 reg, u32 val)
> +{
> +	struct rzn1_miic_plat *plat = dev_get_plat(dev);
> +	struct regmap *regmap = plat->regmap;
> +
> +	/* RIN: Unprotect register writes */

What is "RIN" ?

> +	regmap_write(regmap, PRCMD, 0x00a5);
> +	regmap_write(regmap, PRCMD, 0x0001);
> +	regmap_write(regmap, PRCMD, 0xfffe);
> +	regmap_write(regmap, PRCMD, 0x0001);
> +
> +	/* Do the write */
> +	regmap_write(regmap, reg, val);
> +
> +	/* Re-enable protection */
> +	regmap_write(regmap, PRCMD, 0x0000);
> +}

[...]

> + * RIN RGMII/RMII Converter setup one
> + * RGMII/RMII Converter number: 0..4
> + * if_type: Type of PHY interface, see phy_interface_t
> + */
> +void rzn1_rgmii_rmii_conv_setup(struct udevice *dev, int phy,
> +				u32 if_type, int rmii_ref_clk_out)
> +{
> +	struct rzn1_miic_plat *plat = dev_get_plat(dev);
> +	struct regmap *regmap = plat->regmap;
> +	u32 val = 0;
> +	int speed = SPEED_100;

Reverse xmas tree.

Also, drop = 0 from val assignment and simply assign it below in the 
switch/case statement ...

> +
> +	switch (if_type) {
> +	case PHY_INTERFACE_MODE_RGMII:
> +	case PHY_INTERFACE_MODE_RGMII_ID:
> +	case PHY_INTERFACE_MODE_RGMII_RXID:
> +	case PHY_INTERFACE_MODE_RGMII_TXID:
> +		val |= CONVCTRL_RGMII;

val = CONVCTRL_RGMII;

> +		speed = SPEED_1000;
> +		break;
> +	case PHY_INTERFACE_MODE_RMII:
> +		val |= CONVCTRL_RMII;
> +		break;
> +	case PHY_INTERFACE_MODE_MII:
> +		val |= CONVCTRL_MII;
> +		break;

default:
  return -EINVAL;

or some such ?

> +	};
> +
> +	if (if_type == PHY_INTERFACE_MODE_RMII && rmii_ref_clk_out)
> +		val |= CONVCTRL_REF_CLK_OUT;
> +
> +	regmap_write(regmap, CONVCTRL(phy), val);
> +
> +	rzn1_rgmii_rmii_conv_speed(dev, phy, DUPLEX_FULL, speed);
> +
> +	/* reset */
> +	regmap_read(regmap, CONVRST, &val);
> +	val &= ~PHYIF_RST(phy);
> +	rzn1_rin_prot_writel(dev, CONVRST, val);
> +	udelay(1000);
> +	val |= PHYIF_RST(phy);
> +	rzn1_rin_prot_writel(dev, CONVRST, val);
> +}
> +
> +static int rzn1_miic_probe(struct udevice *dev)
> +{
> +	struct rzn1_miic_plat *plat = dev_get_plat(dev);
> +	int ret;
> +
> +	dev_warn(dev, "%s:%d\n", __func__, __LINE__);
> +
> +	/* Enable clocks */
> +	ret = clk_get_bulk(dev, &plat->bulk);
> +	if (ret) {
> +		dev_err(dev, "Failed to get clocks\n");

"Failed to get clocks (%d)\n", ret);

Please fix globally.

> +		return ret;
> +	}

Add newline here.

> +	ret = clk_enable_bulk(&plat->bulk);
> +	if (ret) {
> +		dev_err(dev, "Failed to enable clocks\n");
> +		return ret;
> +	}
[...]