| Message ID | 20231120070024.4079344-8-claudiu.beznea.uj@bp.renesas.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | renesas: rzg3s: Add support for Ethernet | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch |
Hi Claudiu, On Mon, Nov 20, 2023 at 8:01 AM Claudiu <claudiu.beznea@tuxon.dev> wrote: > From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> > > For Ethernet pins GPIO controller available on RZ/G3S (but also on RZ/G2L) > allows setting the power source. Based on the interface b/w Ethernet > controller and Ethernet PHY and board design specific power source need to > be selected. The GPIO controller allow 1.8V, 2.5V and 3.3V power source > selection for Ethernet pins and this could be selected though ETHX_POC > registers (X={0, 1}). > > Commit adjust the driver to support this and does proper instantiation > for RZ/G3S and RZ/G2L SoC. On RZ/G2L only get operation has been tested > at the moment. > > Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> Thanks for your patch! > --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c > +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c > @@ -623,7 +632,16 @@ static int rzg2l_get_power_source(struct rzg2l_pinctrl *pctrl, u32 pin, u32 caps > if (pwr_reg < 0) > return pwr_reg; > > - return (readl(pctrl->base + pwr_reg) & PVDD_MASK) ? 1800 : 3300; This removes the last user of PVDD_MASK. I guess it can be removed, as all unused register bits are documented to read as zeroes. > + val = readl(pctrl->base + pwr_reg); While these registers are documented to support access sizes of 8/16/32 bits on RZ/G3S, RZ/G2L is limited to 8 bits, so this should have used readb() from the beginning. > + if (val == PVDD_1800) > + return 1800; > + if (val == PVDD_2500) > + return 2500; > + if (val == PVDD_3300) > + return 3300; > + > + /* Should not happen. */ > + return -EINVAL; Please use a switch() statement. > } > > static int rzg2l_set_power_source(struct rzg2l_pinctrl *pctrl, u32 pin, u32 caps, u32 ps) > @@ -631,17 +649,27 @@ static int rzg2l_set_power_source(struct rzg2l_pinctrl *pctrl, u32 pin, u32 caps > const struct rzg2l_hwcfg *hwcfg = pctrl->data->hwcfg; > const struct rzg2l_register_offsets *regs = &hwcfg->regs; > int pwr_reg; > + u32 val; > > if (caps & PIN_CFG_SOFT_PS) { > pctrl->settings[pin].power_source = ps; > return 0; > } > > + if (ps == 1800) > + val = PVDD_1800; > + else if (ps == 2500) > + val = PVDD_2500; > + else if (ps == 3300) > + val = PVDD_3300; > + else > + return -EINVAL; Please use a switch() statement. > + > pwr_reg = rzg2l_caps_to_pwr_reg(regs, caps); > if (pwr_reg < 0) > return pwr_reg; > > - writel((ps == 1800) ? PVDD_1800 : PVDD_3300, pctrl->base + pwr_reg); > + writel(val, pctrl->base + pwr_reg); writeb() for RZ/G2L. > pctrl->settings[pin].power_source = ps; > > return 0; Gr{oetje,eeting}s, Geert
diff --git a/drivers/pinctrl/renesas/pinctrl-rzg2l.c b/drivers/pinctrl/renesas/pinctrl-rzg2l.c index 819698dacef0..1401bb215686 100644 --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c @@ -107,8 +107,10 @@ #define IEN(off) (0x1800 + (off) * 8) #define ISEL(off) (0x2C00 + (off) * 8) #define SD_CH(off, ch) ((off) + (ch) * 4) +#define ETH_POC(off, ch) ((off) + (ch) * 4) #define QSPI (0x3008) +#define PVDD_2500 2 /* I/O domain voltage 2.5V */ #define PVDD_1800 1 /* I/O domain voltage <= 1.8V */ #define PVDD_3300 0 /* I/O domain voltage >= 3.3V */ @@ -135,10 +137,12 @@ * struct rzg2l_register_offsets - specific register offsets * @pwpr: PWPR register offset * @sd_ch: SD_CH register offset + * @eth_poc: ETH_POC register offset */ struct rzg2l_register_offsets { u16 pwpr; u16 sd_ch; + u16 eth_poc; }; /** @@ -604,6 +608,10 @@ static int rzg2l_caps_to_pwr_reg(const struct rzg2l_register_offsets *regs, u32 return SD_CH(regs->sd_ch, 0); if (caps & PIN_CFG_IO_VMC_SD1) return SD_CH(regs->sd_ch, 1); + if (caps & PIN_CFG_IO_VMC_ETH0) + return ETH_POC(regs->eth_poc, 0); + if (caps & PIN_CFG_IO_VMC_ETH1) + return ETH_POC(regs->eth_poc, 1); if (caps & PIN_CFG_IO_VMC_QSPI) return QSPI; @@ -615,6 +623,7 @@ static int rzg2l_get_power_source(struct rzg2l_pinctrl *pctrl, u32 pin, u32 caps const struct rzg2l_hwcfg *hwcfg = pctrl->data->hwcfg; const struct rzg2l_register_offsets *regs = &hwcfg->regs; int pwr_reg; + u32 val; if (caps & PIN_CFG_SOFT_PS) return pctrl->settings[pin].power_source; @@ -623,7 +632,16 @@ static int rzg2l_get_power_source(struct rzg2l_pinctrl *pctrl, u32 pin, u32 caps if (pwr_reg < 0) return pwr_reg; - return (readl(pctrl->base + pwr_reg) & PVDD_MASK) ? 1800 : 3300; + val = readl(pctrl->base + pwr_reg); + if (val == PVDD_1800) + return 1800; + if (val == PVDD_2500) + return 2500; + if (val == PVDD_3300) + return 3300; + + /* Should not happen. */ + return -EINVAL; } static int rzg2l_set_power_source(struct rzg2l_pinctrl *pctrl, u32 pin, u32 caps, u32 ps) @@ -631,17 +649,27 @@ static int rzg2l_set_power_source(struct rzg2l_pinctrl *pctrl, u32 pin, u32 caps const struct rzg2l_hwcfg *hwcfg = pctrl->data->hwcfg; const struct rzg2l_register_offsets *regs = &hwcfg->regs; int pwr_reg; + u32 val; if (caps & PIN_CFG_SOFT_PS) { pctrl->settings[pin].power_source = ps; return 0; } + if (ps == 1800) + val = PVDD_1800; + else if (ps == 2500) + val = PVDD_2500; + else if (ps == 3300) + val = PVDD_3300; + else + return -EINVAL; + pwr_reg = rzg2l_caps_to_pwr_reg(regs, caps); if (pwr_reg < 0) return pwr_reg; - writel((ps == 1800) ? PVDD_1800 : PVDD_3300, pctrl->base + pwr_reg); + writel(val, pctrl->base + pwr_reg); pctrl->settings[pin].power_source = ps; return 0; @@ -1891,6 +1919,7 @@ static const struct rzg2l_hwcfg rzg2l_hwcfg = { .regs = { .pwpr = 0x3014, .sd_ch = 0x3000, + .eth_poc = 0x300c, }, .iolh_groupa_ua = { /* 3v3 power source */ @@ -1903,6 +1932,7 @@ static const struct rzg2l_hwcfg rzg3s_hwcfg = { .regs = { .pwpr = 0x3000, .sd_ch = 0x3004, + .eth_poc = 0x3010, }, .iolh_groupa_ua = { /* 1v8 power source */