| Message ID | f0ef3e00aec461beb33869ab69ccb44a23d78f51.1718378166.git.geert+renesas@glider.be (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | [PATCH/RFC] net: ravb: Add MII support for R-Car V4M | expand |
Hi Geert, Thanks for your work. On 2024-06-14 17:25:22 +0200, Geert Uytterhoeven wrote: > All EtherAVB instances on R-Car Gen3/Gen4 SoCs support the RGMII > interface. In addition, the first two EtherAVB instances on R-Car V4M > also support the MII interface, but this is not yet supported by the > driver. > > Add support for MII to the R-Car Gen3/Gen4-specific EMAC initialization > function, by selecting the MII clock instead of the RGMII clock when the > PHY interface is MII. Note that all implementations of EtherAVB on > R-Car Gen3/Gen4 SoCs have the APSR register, but only MII-capable > instances are documented to have the MIISELECT bit, which has a > documented value of zero when reserved. > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> > --- > Compile-tested only, as all AVB interfaces on the Gray Hawk Single > development board are connected to RGMII PHYs. > No regressions on R-Car E3, H3, M3-W, M3-N, and V4H. This would be interesting to test on a platform if we ever find one which uses MII. The change do match the docs. Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> > --- > drivers/net/ethernet/renesas/ravb.h | 1 + > drivers/net/ethernet/renesas/ravb_main.c | 12 +++++++++++- > 2 files changed, 12 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/ethernet/renesas/ravb.h b/drivers/net/ethernet/renesas/ravb.h > index 6b2444d31fcc3093..9893c91af1050fa1 100644 > --- a/drivers/net/ethernet/renesas/ravb.h > +++ b/drivers/net/ethernet/renesas/ravb.h > @@ -258,6 +258,7 @@ enum APSR_BIT { > APSR_CMSW = 0x00000010, > APSR_RDM = 0x00002000, > APSR_TDM = 0x00004000, > + APSR_MIISELECT = 0x01000000, /* R-Car V4M only */ > }; > > /* RCR */ > diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c > index c1546b916e4ef581..cbe2709e0ace871f 100644 > --- a/drivers/net/ethernet/renesas/ravb_main.c > +++ b/drivers/net/ethernet/renesas/ravb_main.c > @@ -579,6 +579,16 @@ static void ravb_emac_init_rcar(struct net_device *ndev) > ravb_write(ndev, ECSIPR_ICDIP | ECSIPR_MPDIP | ECSIPR_LCHNGIP, ECSIPR); > } > > +static void ravb_emac_init_rcar_apsr(struct net_device *ndev) > +{ > + struct ravb_private *priv = netdev_priv(ndev); > + bool mii = priv->phy_interface == PHY_INTERFACE_MODE_MII; > + > + ravb_modify(ndev, APSR, APSR_MIISELECT, mii ? APSR_MIISELECT : 0); > + > + ravb_emac_init_rcar(ndev); > +} > + > /* E-MAC init function */ > static void ravb_emac_init(struct net_device *ndev) > { > @@ -2657,7 +2667,7 @@ static const struct ravb_hw_info ravb_gen3_hw_info = { > .set_rate = ravb_set_rate_rcar, > .set_feature = ravb_set_features_rcar, > .dmac_init = ravb_dmac_init_rcar, > - .emac_init = ravb_emac_init_rcar, > + .emac_init = ravb_emac_init_rcar_apsr, > .gstrings_stats = ravb_gstrings_stats, > .gstrings_size = sizeof(ravb_gstrings_stats), > .net_hw_features = NETIF_F_RXCSUM, > -- > 2.34.1 >
On 6/14/24 6:25 PM, Geert Uytterhoeven wrote: > All EtherAVB instances on R-Car Gen3/Gen4 SoCs support the RGMII > interface. In addition, the first two EtherAVB instances on R-Car V4M > also support the MII interface, but this is not yet supported by the > driver. > > Add support for MII to the R-Car Gen3/Gen4-specific EMAC initialization > function, by selecting the MII clock instead of the RGMII clock when the But why are you adding such code to the ge3 function? According to the gen3 manual I have, gen3 SoCs don't have MII support... > PHY interface is MII. Note that all implementations of EtherAVB on > R-Car Gen3/Gen4 SoCs have the APSR register, but only MII-capable > instances are documented to have the MIISELECT bit, which has a > documented value of zero when reserved. > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> [...] > diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c > index c1546b916e4ef581..cbe2709e0ace871f 100644 > --- a/drivers/net/ethernet/renesas/ravb_main.c > +++ b/drivers/net/ethernet/renesas/ravb_main.c > @@ -579,6 +579,16 @@ static void ravb_emac_init_rcar(struct net_device *ndev) > ravb_write(ndev, ECSIPR_ICDIP | ECSIPR_MPDIP | ECSIPR_LCHNGIP, ECSIPR); > } > > +static void ravb_emac_init_rcar_apsr(struct net_device *ndev) No, this name doesn't match the currently used naming scheme (which has the SoC type as a last word... I'm suggesting something like ravb_emac_init_rcar_gen4() instead. [...] > @@ -2657,7 +2667,7 @@ static const struct ravb_hw_info ravb_gen3_hw_info = { > .set_rate = ravb_set_rate_rcar, > .set_feature = ravb_set_features_rcar, > .dmac_init = ravb_dmac_init_rcar, > - .emac_init = ravb_emac_init_rcar, > + .emac_init = ravb_emac_init_rcar_apsr, I'm afraid we'll have to add the new ravb_gen4_hw_info variable. We already have the gen4-specific compatible in ravb_match_table[]... [...] MBR, Sergey
Hi Sergey, On Wed, Jun 19, 2024 at 9:01 PM Sergey Shtylyov <s.shtylyov@omp.ru> wrote: > On 6/14/24 6:25 PM, Geert Uytterhoeven wrote: > > All EtherAVB instances on R-Car Gen3/Gen4 SoCs support the RGMII > > interface. In addition, the first two EtherAVB instances on R-Car V4M > > also support the MII interface, but this is not yet supported by the > > driver. > > > > Add support for MII to the R-Car Gen3/Gen4-specific EMAC initialization > > function, by selecting the MII clock instead of the RGMII clock when the > > But why are you adding such code to the ge3 function? According to the gen3 > manual I have, gen3 SoCs don't have MII support... I wanted to limit the number of changes, and avoid the need to add an additional ravb_hw_info structure. The bit is documented to be zero on R-Car Gen3 (but writing one to it seems to stick on some of the later Gen3 variants, so perhaps these do support MII?). > > PHY interface is MII. Note that all implementations of EtherAVB on > > R-Car Gen3/Gen4 SoCs have the APSR register, but only MII-capable > > instances are documented to have the MIISELECT bit, which has a > > documented value of zero when reserved. > > > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> > > [...] > > diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c > > index c1546b916e4ef581..cbe2709e0ace871f 100644 > > --- a/drivers/net/ethernet/renesas/ravb_main.c > > +++ b/drivers/net/ethernet/renesas/ravb_main.c > > @@ -579,6 +579,16 @@ static void ravb_emac_init_rcar(struct net_device *ndev) > > ravb_write(ndev, ECSIPR_ICDIP | ECSIPR_MPDIP | ECSIPR_LCHNGIP, ECSIPR); > > } > > > > +static void ravb_emac_init_rcar_apsr(struct net_device *ndev) > > No, this name doesn't match the currently used naming scheme (which > has the SoC type as a last word... I'm suggesting something like ravb_emac_init_rcar_gen4() instead. > > [...] > > @@ -2657,7 +2667,7 @@ static const struct ravb_hw_info ravb_gen3_hw_info = { > > .set_rate = ravb_set_rate_rcar, > > .set_feature = ravb_set_features_rcar, > > .dmac_init = ravb_dmac_init_rcar, > > - .emac_init = ravb_emac_init_rcar, > > + .emac_init = ravb_emac_init_rcar_apsr, > > I'm afraid we'll have to add the new ravb_gen4_hw_info variable. We already > have the gen4-specific compatible in ravb_match_table[]... If you insist, I will make a v2 doing so... Gr{oetje,eeting}s, Geert
diff --git a/drivers/net/ethernet/renesas/ravb.h b/drivers/net/ethernet/renesas/ravb.h index 6b2444d31fcc3093..9893c91af1050fa1 100644 --- a/drivers/net/ethernet/renesas/ravb.h +++ b/drivers/net/ethernet/renesas/ravb.h @@ -258,6 +258,7 @@ enum APSR_BIT { APSR_CMSW = 0x00000010, APSR_RDM = 0x00002000, APSR_TDM = 0x00004000, + APSR_MIISELECT = 0x01000000, /* R-Car V4M only */ }; /* RCR */ diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c index c1546b916e4ef581..cbe2709e0ace871f 100644 --- a/drivers/net/ethernet/renesas/ravb_main.c +++ b/drivers/net/ethernet/renesas/ravb_main.c @@ -579,6 +579,16 @@ static void ravb_emac_init_rcar(struct net_device *ndev) ravb_write(ndev, ECSIPR_ICDIP | ECSIPR_MPDIP | ECSIPR_LCHNGIP, ECSIPR); } +static void ravb_emac_init_rcar_apsr(struct net_device *ndev) +{ + struct ravb_private *priv = netdev_priv(ndev); + bool mii = priv->phy_interface == PHY_INTERFACE_MODE_MII; + + ravb_modify(ndev, APSR, APSR_MIISELECT, mii ? APSR_MIISELECT : 0); + + ravb_emac_init_rcar(ndev); +} + /* E-MAC init function */ static void ravb_emac_init(struct net_device *ndev) { @@ -2657,7 +2667,7 @@ static const struct ravb_hw_info ravb_gen3_hw_info = { .set_rate = ravb_set_rate_rcar, .set_feature = ravb_set_features_rcar, .dmac_init = ravb_dmac_init_rcar, - .emac_init = ravb_emac_init_rcar, + .emac_init = ravb_emac_init_rcar_apsr, .gstrings_stats = ravb_gstrings_stats, .gstrings_size = sizeof(ravb_gstrings_stats), .net_hw_features = NETIF_F_RXCSUM,
All EtherAVB instances on R-Car Gen3/Gen4 SoCs support the RGMII interface. In addition, the first two EtherAVB instances on R-Car V4M also support the MII interface, but this is not yet supported by the driver. Add support for MII to the R-Car Gen3/Gen4-specific EMAC initialization function, by selecting the MII clock instead of the RGMII clock when the PHY interface is MII. Note that all implementations of EtherAVB on R-Car Gen3/Gen4 SoCs have the APSR register, but only MII-capable instances are documented to have the MIISELECT bit, which has a documented value of zero when reserved. Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> --- Compile-tested only, as all AVB interfaces on the Gray Hawk Single development board are connected to RGMII PHYs. No regressions on R-Car E3, H3, M3-W, M3-N, and V4H. --- drivers/net/ethernet/renesas/ravb.h | 1 + drivers/net/ethernet/renesas/ravb_main.c | 12 +++++++++++- 2 files changed, 12 insertions(+), 1 deletion(-)