[PATCH v2 4/5] net: sun8i-emac: Use common syscon setup for R40

[Ramon Fried]

On Mon, Jan 23, 2023 at 7:24 PM Andre Przywara <andre.przywara at arm.com> wrote:
>
> On 22/01/2023 22:51, Samuel Holland wrote:
> > While R40 puts the EMAC syscon register at a different address from
> > other variants, the relevant portion of the register's layout is the
> > same. Factor out the register offset so the same code can be shared
> > by all variants. This matches what the Linux driver does.
> >
> > This change provides two benefits beyond the simplification:
> >   - R40 boards now respect the RX delays from the devicetree
> >   - This resolves a warning on architectures where readl/writel
> >     expect the address to have a pointer type, not phys_addr_t.
> >
>
> Nice cleanup, and makes it obvious that the R40 isn't such a special
> snowflake after all.
>
> > Signed-off-by: Samuel Holland <samuel at sholland.org>
>
> Reviewed-by: Andre Przywara <andre.przywara at arm.com>
>
> Cheers,
> Andre
>
>
> > ---
> >
> > Changes in v2:
> >   - Add a structure for driver data, and put the syscon offset there
> >
> >   drivers/net/sun8i_emac.c | 32 +++++++++++++++-----------------
> >   1 file changed, 15 insertions(+), 17 deletions(-)
> >
> > diff --git a/drivers/net/sun8i_emac.c b/drivers/net/sun8i_emac.c
> > index 36cc2498b5..231aac19e3 100644
> > --- a/drivers/net/sun8i_emac.c
> > +++ b/drivers/net/sun8i_emac.c
> > @@ -137,6 +137,7 @@ enum emac_variant_id {
> >
> >   struct emac_variant {
> >       enum emac_variant_id    variant;
> > +     uint                    syscon_offset;
> >       bool                    soc_has_internal_phy;
> >       bool                    support_rmii;
> >   };
> > @@ -168,7 +169,7 @@ struct emac_eth_dev {
> >
> >       const struct emac_variant *variant;
> >       void *mac_reg;
> > -     phys_addr_t sysctl_reg;
> > +     void *sysctl_reg;
> >       struct phy_device *phydev;
> >       struct mii_dev *bus;
> >       struct clk tx_clk;
> > @@ -323,18 +324,7 @@ static int sun8i_emac_set_syscon(struct sun8i_eth_pdata *pdata,
> >   {
> >       u32 reg;
> >
> > -     if (priv->variant->variant == R40_GMAC) {
> > -             /* Select RGMII for R40 */
> > -             reg = readl(priv->sysctl_reg + 0x164);
> > -             reg |= SC_ETCS_INT_GMII |
> > -                    SC_EPIT |
> > -                    (CONFIG_GMAC_TX_DELAY << SC_ETXDC_OFFSET);
> > -
> > -             writel(reg, priv->sysctl_reg + 0x164);
> > -             return 0;
> > -     }
> > -
> > -     reg = readl(priv->sysctl_reg + 0x30);
> > +     reg = readl(priv->sysctl_reg);
> >
> >       reg = sun8i_emac_set_syscon_ephy(priv, reg);
> >
> > @@ -370,7 +360,7 @@ static int sun8i_emac_set_syscon(struct sun8i_eth_pdata *pdata,
> >               reg |= ((pdata->rx_delay_ps / 100) << SC_ERXDC_OFFSET)
> >                        & SC_ERXDC_MASK;
> >
> > -     writel(reg, priv->sysctl_reg + 0x30);
> > +     writel(reg, priv->sysctl_reg);
> >
> >       return 0;
> >   }
> > @@ -793,6 +783,7 @@ static int sun8i_emac_eth_of_to_plat(struct udevice *dev)
> >       struct sun8i_eth_pdata *sun8i_pdata = dev_get_plat(dev);
> >       struct eth_pdata *pdata = &sun8i_pdata->eth_pdata;
> >       struct emac_eth_dev *priv = dev_get_priv(dev);
> > +     phys_addr_t syscon_base;
> >       const fdt32_t *reg;
> >       int node = dev_of_offset(dev);
> >       int offset = 0;
> > @@ -838,13 +829,15 @@ static int sun8i_emac_eth_of_to_plat(struct udevice *dev)
> >                     __func__);
> >               return -EINVAL;
> >       }
> > -     priv->sysctl_reg = fdt_translate_address((void *)gd->fdt_blob,
> > -                                              offset, reg);
> > -     if (priv->sysctl_reg == FDT_ADDR_T_NONE) {
> > +
> > +     syscon_base = fdt_translate_address((void *)gd->fdt_blob, offset, reg);
> > +     if (syscon_base == FDT_ADDR_T_NONE) {
> >               debug("%s: Cannot find syscon base address\n", __func__);
> >               return -EINVAL;
> >       }
> >
> > +     priv->sysctl_reg = (void *)syscon_base + priv->variant->syscon_offset;
> > +
> >       pdata->phy_interface = -1;
> >       priv->phyaddr = -1;
> >       priv->use_internal_phy = false;
> > @@ -903,25 +896,30 @@ static int sun8i_emac_eth_of_to_plat(struct udevice *dev)
> >
> >   static const struct emac_variant emac_variant_a83t = {
> >       .variant                = A83T_EMAC,
> > +     .syscon_offset          = 0x30,
> >   };
> >
> >   static const struct emac_variant emac_variant_h3 = {
> >       .variant                = H3_EMAC,
> > +     .syscon_offset          = 0x30,
> >       .soc_has_internal_phy   = true,
> >       .support_rmii           = true,
> >   };
> >
> >   static const struct emac_variant emac_variant_r40 = {
> >       .variant                = R40_GMAC,
> > +     .syscon_offset          = 0x164,
> >   };
> >
> >   static const struct emac_variant emac_variant_a64 = {
> >       .variant                = A64_EMAC,
> > +     .syscon_offset          = 0x30,
> >       .support_rmii           = true,
> >   };
> >
> >   static const struct emac_variant emac_variant_h6 = {
> >       .variant                = H6_EMAC,
> > +     .syscon_offset          = 0x30,
> >       .support_rmii           = true,
> >   };
> >
>
Reviewed-by: Ramon Fried <rfried.dev at gmail.com>