* [PATCH net-next] net: mvpp2: add support for mii
@ 2023-12-06 16:01 Stefan Eichenberger
2023-12-06 17:27 ` Maxime Chevallier
0 siblings, 1 reply; 4+ messages in thread
From: Stefan Eichenberger @ 2023-12-06 16:01 UTC (permalink / raw)
To: mw, linux, davem, edumazet, kuba, pabeni; +Cc: netdev, linux-kernel
Currently, mvpp2 only supports RGMII. This commit adds support for MII.
The description in Marvell's functional specification seems to be wrong.
To enable MII, we need to set GENCONF_CTRL0_PORT3_RGMII, while for RGMII
we need to clear it. This is also how U-Boot handles it.
Signed-off-by: Stefan Eichenberger <eichest@gmail.com>
---
.../net/ethernet/marvell/mvpp2/mvpp2_main.c | 24 ++++++++++++++++---
1 file changed, 21 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
index 93137606869e..6f136f42e2bf 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
@@ -1513,10 +1513,21 @@ static void mvpp22_gop_init_rgmii(struct mvpp2_port *port)
regmap_write(priv->sysctrl_base, GENCONF_PORT_CTRL0, val);
regmap_read(priv->sysctrl_base, GENCONF_CTRL0, &val);
- if (port->gop_id == 2)
+ if (port->gop_id == 2) {
val |= GENCONF_CTRL0_PORT2_RGMII;
- else if (port->gop_id == 3)
+ } else if (port->gop_id == 3) {
val |= GENCONF_CTRL0_PORT3_RGMII_MII;
+
+ /* According to the specification, GENCONF_CTRL0_PORT3_RGMII
+ * should be set to 1 for RGMII and 0 for MII. However, tests
+ * show that it is the other way around. This is also what
+ * U-Boot does for mvpp2, so it is assumed to be correct.
+ */
+ if (port->phy_interface == PHY_INTERFACE_MODE_MII)
+ val |= GENCONF_CTRL0_PORT3_RGMII;
+ else
+ val &= ~GENCONF_CTRL0_PORT3_RGMII;
+ }
regmap_write(priv->sysctrl_base, GENCONF_CTRL0, val);
}
@@ -1615,6 +1626,7 @@ static int mvpp22_gop_init(struct mvpp2_port *port, phy_interface_t interface)
return 0;
switch (interface) {
+ case PHY_INTERFACE_MODE_MII:
case PHY_INTERFACE_MODE_RGMII:
case PHY_INTERFACE_MODE_RGMII_ID:
case PHY_INTERFACE_MODE_RGMII_RXID:
@@ -6948,8 +6960,11 @@ static int mvpp2_port_probe(struct platform_device *pdev,
MAC_10000FD;
}
- if (mvpp2_port_supports_rgmii(port))
+ if (mvpp2_port_supports_rgmii(port)) {
phy_interface_set_rgmii(port->phylink_config.supported_interfaces);
+ __set_bit(PHY_INTERFACE_MODE_MII,
+ port->phylink_config.supported_interfaces);
+ }
if (comphy) {
/* If a COMPHY is present, we can support any of the
@@ -6973,6 +6988,9 @@ static int mvpp2_port_probe(struct platform_device *pdev,
port->phylink_config.supported_interfaces);
__set_bit(PHY_INTERFACE_MODE_SGMII,
port->phylink_config.supported_interfaces);
+ } else if (phy_mode == PHY_INTERFACE_MODE_MII) {
+ __set_bit(PHY_INTERFACE_MODE_100BASEX,
+ port->phylink_config.supported_interfaces);
}
phylink = phylink_create(&port->phylink_config, port_fwnode,
--
2.40.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH net-next] net: mvpp2: add support for mii
2023-12-06 16:01 [PATCH net-next] net: mvpp2: add support for mii Stefan Eichenberger
@ 2023-12-06 17:27 ` Maxime Chevallier
2023-12-07 9:01 ` Stefan Eichenberger
0 siblings, 1 reply; 4+ messages in thread
From: Maxime Chevallier @ 2023-12-06 17:27 UTC (permalink / raw)
To: Stefan Eichenberger
Cc: mw, linux, davem, edumazet, kuba, pabeni, netdev, linux-kernel
Hello Stefan,
On Wed, 6 Dec 2023 17:01:25 +0100
Stefan Eichenberger <eichest@gmail.com> wrote:
> Currently, mvpp2 only supports RGMII. This commit adds support for MII.
> The description in Marvell's functional specification seems to be wrong.
> To enable MII, we need to set GENCONF_CTRL0_PORT3_RGMII, while for RGMII
> we need to clear it. This is also how U-Boot handles it.
>
> Signed-off-by: Stefan Eichenberger <eichest@gmail.com>
> ---
> .../net/ethernet/marvell/mvpp2/mvpp2_main.c | 24 ++++++++++++++++---
> 1 file changed, 21 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> index 93137606869e..6f136f42e2bf 100644
> --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> @@ -1513,10 +1513,21 @@ static void mvpp22_gop_init_rgmii(struct mvpp2_port *port)
> regmap_write(priv->sysctrl_base, GENCONF_PORT_CTRL0, val);
>
> regmap_read(priv->sysctrl_base, GENCONF_CTRL0, &val);
> - if (port->gop_id == 2)
> + if (port->gop_id == 2) {
> val |= GENCONF_CTRL0_PORT2_RGMII;
> - else if (port->gop_id == 3)
> + } else if (port->gop_id == 3) {
> val |= GENCONF_CTRL0_PORT3_RGMII_MII;
> +
> + /* According to the specification, GENCONF_CTRL0_PORT3_RGMII
> + * should be set to 1 for RGMII and 0 for MII. However, tests
> + * show that it is the other way around. This is also what
> + * U-Boot does for mvpp2, so it is assumed to be correct.
> + */
> + if (port->phy_interface == PHY_INTERFACE_MODE_MII)
> + val |= GENCONF_CTRL0_PORT3_RGMII;
> + else
> + val &= ~GENCONF_CTRL0_PORT3_RGMII;
> + }
> regmap_write(priv->sysctrl_base, GENCONF_CTRL0, val);
> }
>
> @@ -1615,6 +1626,7 @@ static int mvpp22_gop_init(struct mvpp2_port *port, phy_interface_t interface)
> return 0;
>
> switch (interface) {
> + case PHY_INTERFACE_MODE_MII:
> case PHY_INTERFACE_MODE_RGMII:
> case PHY_INTERFACE_MODE_RGMII_ID:
> case PHY_INTERFACE_MODE_RGMII_RXID:
> @@ -6948,8 +6960,11 @@ static int mvpp2_port_probe(struct platform_device *pdev,
> MAC_10000FD;
> }
>
> - if (mvpp2_port_supports_rgmii(port))
> + if (mvpp2_port_supports_rgmii(port)) {
> phy_interface_set_rgmii(port->phylink_config.supported_interfaces);
> + __set_bit(PHY_INTERFACE_MODE_MII,
> + port->phylink_config.supported_interfaces);
> + }
>
> if (comphy) {
> /* If a COMPHY is present, we can support any of the
> @@ -6973,6 +6988,9 @@ static int mvpp2_port_probe(struct platform_device *pdev,
> port->phylink_config.supported_interfaces);
> __set_bit(PHY_INTERFACE_MODE_SGMII,
> port->phylink_config.supported_interfaces);
> + } else if (phy_mode == PHY_INTERFACE_MODE_MII) {
> + __set_bit(PHY_INTERFACE_MODE_100BASEX,
> + port->phylink_config.supported_interfaces);
Can you explain that part ? I don't understand why 100BaseX is being
reported as a supported mode here. This whole section of the function
is about detecting what can be reported based on the presence or not of
a comphy driver / hardcoded comphy config. I don't think the comphy
here has anything to do with MII / 100BaseX
If 100BaseX can be carried on MII (which I don't know), shouldn't it be
reported no matter what ?
Thanks,
Maxime
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH net-next] net: mvpp2: add support for mii
2023-12-06 17:27 ` Maxime Chevallier
@ 2023-12-07 9:01 ` Stefan Eichenberger
2023-12-07 11:27 ` Maxime Chevallier
0 siblings, 1 reply; 4+ messages in thread
From: Stefan Eichenberger @ 2023-12-07 9:01 UTC (permalink / raw)
To: Maxime Chevallier
Cc: mw, linux, davem, edumazet, kuba, pabeni, netdev, linux-kernel
Hi Maxime,
On Wed, Dec 06, 2023 at 06:27:05PM +0100, Maxime Chevallier wrote:
> > @@ -6973,6 +6988,9 @@ static int mvpp2_port_probe(struct platform_device *pdev,
> > port->phylink_config.supported_interfaces);
> > __set_bit(PHY_INTERFACE_MODE_SGMII,
> > port->phylink_config.supported_interfaces);
> > + } else if (phy_mode == PHY_INTERFACE_MODE_MII) {
> > + __set_bit(PHY_INTERFACE_MODE_100BASEX,
> > + port->phylink_config.supported_interfaces);
>
> Can you explain that part ? I don't understand why 100BaseX is being
> reported as a supported mode here. This whole section of the function
> is about detecting what can be reported based on the presence or not of
> a comphy driver / hardcoded comphy config. I don't think the comphy
> here has anything to do with MII / 100BaseX
>
> If 100BaseX can be carried on MII (which I don't know), shouldn't it be
> reported no matter what ?
I missunderstood that part, I thought it is a translation from interface
type to speed but it is obviously not. I already verfied that everything
works without this part and will remove it in version 2 of the patch.
Thanks a lot for the review!
Regards,
Stefan
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH net-next] net: mvpp2: add support for mii
2023-12-07 9:01 ` Stefan Eichenberger
@ 2023-12-07 11:27 ` Maxime Chevallier
0 siblings, 0 replies; 4+ messages in thread
From: Maxime Chevallier @ 2023-12-07 11:27 UTC (permalink / raw)
To: Stefan Eichenberger
Cc: mw, linux, davem, edumazet, kuba, pabeni, netdev, linux-kernel
Hello Stefan,
On Thu, 7 Dec 2023 10:01:08 +0100
Stefan Eichenberger <eichest@gmail.com> wrote:
> Hi Maxime,
>
> On Wed, Dec 06, 2023 at 06:27:05PM +0100, Maxime Chevallier wrote:
> > > @@ -6973,6 +6988,9 @@ static int mvpp2_port_probe(struct platform_device *pdev,
> > > port->phylink_config.supported_interfaces);
> > > __set_bit(PHY_INTERFACE_MODE_SGMII,
> > > port->phylink_config.supported_interfaces);
> > > + } else if (phy_mode == PHY_INTERFACE_MODE_MII) {
> > > + __set_bit(PHY_INTERFACE_MODE_100BASEX,
> > > + port->phylink_config.supported_interfaces);
> >
> > Can you explain that part ? I don't understand why 100BaseX is being
> > reported as a supported mode here. This whole section of the function
> > is about detecting what can be reported based on the presence or not of
> > a comphy driver / hardcoded comphy config. I don't think the comphy
> > here has anything to do with MII / 100BaseX
> >
> > If 100BaseX can be carried on MII (which I don't know), shouldn't it be
> > reported no matter what ?
>
> I missunderstood that part, I thought it is a translation from interface
> type to speed but it is obviously not. I already verfied that everything
> works without this part and will remove it in version 2 of the patch.
> Thanks a lot for the review!
No problem, thanks for the patch :)
Maxime
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-12-07 11:27 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-12-06 16:01 [PATCH net-next] net: mvpp2: add support for mii Stefan Eichenberger
2023-12-06 17:27 ` Maxime Chevallier
2023-12-07 9:01 ` Stefan Eichenberger
2023-12-07 11:27 ` Maxime Chevallier
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.