[PATCH 0/5] Support both Ethernet interfaces on i.MX6UL and i.MX7

[PATCH 0/5] Support both Ethernet interfaces on i.MX6UL and i.MX7

[Guenter Roeck]

The SOC on i.MX6UL and i.MX7 has 2 Ethernet interfaces. The PHY on each may
be connected to separate MDIO busses, or both may be connected on the same
MDIO bus using different PHY addresses. Commit 461c51ad4275 ("Add a phy-num
property to the i.MX FEC emulator") added support for specifying PHY
addresses, but it did not provide support for linking the second PHY on
a given MDIO bus to the other Ethernet interface.

To be able to support two PHY instances on a single MDIO bus, two properties
are needed: First, there needs to be a flag indicating if the MDIO bus on
a given Ethernet interface is connected. If not, attempts to read from this
bus must always return 0xffff. Implement this property as phy-connected.
Second, if the MDIO bus on an interface is active, it needs a link to the
consumer interface to be able to provide PHY access for it. Implement this
property as phy-consumer.

The first patch of the series implements support in hw/net/imx_fec.c.
Patches 2..5 set the necessary properties in i.MX6UL and i.MX7 emulations.
With this series in place, both Ethernet interfaces on affected emulations
are functional.

----------------------------------------------------------------
Guenter Roeck (5):
      hw/net/imx_fec: Support two Ethernet interfaces connected to single MDIO bus
      fsl-imx6ul: Add fec[12]-phy-connected properties
      arm/mcimx6ul-evk: Set fec1-phy-connected property to false
      fsl-imx7: Add fec[12]-phy-connected properties
      arm/mcimx7d-sabre: Set fec2-phy-connected property to false

 hw/arm/fsl-imx6ul.c         | 20 ++++++++++++++++++++
 hw/arm/fsl-imx7.c           | 20 ++++++++++++++++++++
 hw/arm/mcimx6ul-evk.c       |  2 ++
 hw/arm/mcimx7d-sabre.c      |  2 ++
 hw/net/imx_fec.c            | 27 +++++++++++++++++++++++----
 include/hw/arm/fsl-imx6ul.h |  1 +
 include/hw/arm/fsl-imx7.h   |  1 +
 include/hw/net/imx_fec.h    |  2 ++
 8 files changed, 71 insertions(+), 4 deletions(-)

[PATCH 1/5] hw/net/imx_fec: Support two Ethernet interfaces connected to single MDIO bus

[Guenter Roeck]

The SOC on i.MX6UL and i.MX7 has 2 Ethernet interfaces. The PHY on each may
be connected to separate MDIO busses, or both may be connected on the same
MDIO bus using different PHY addresses. Commit 461c51ad4275 ("Add a phy-num
property to the i.MX FEC emulator") added support for specifying PHY
addresses, but it did not provide support for linking the second PHY on
a given MDIO bus to the other Ethernet interface.

To be able to support two PHY instances on a single MDIO bus, two properties
are needed: First, there needs to be a flag indicating if the MDIO bus on
a given Ethernet interface is connected. If not, attempts to read from this
bus must always return 0xffff. Implement this property as phy-connected.
Second, if the MDIO bus on an interface is active, it needs a link to the
consumer interface to be able to provide PHY access for it. Implement this
property as phy-consumer.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 hw/net/imx_fec.c         | 27 +++++++++++++++++++++++----
 include/hw/net/imx_fec.h |  2 ++
 2 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/hw/net/imx_fec.c b/hw/net/imx_fec.c
index c862d96593..5d1f1f104c 100644
--- a/hw/net/imx_fec.c
+++ b/hw/net/imx_fec.c
@@ -282,11 +282,19 @@ static uint32_t imx_phy_read(IMXFECState *s, int reg)
     uint32_t val;
     uint32_t phy = reg / 32;
 
-    if (phy != s->phy_num) {
-        trace_imx_phy_read_num(phy, s->phy_num);
+    if (!s->phy_connected) {
         return 0xffff;
     }
 
+    if (phy != s->phy_num) {
+        if (s->phy_consumer && phy == s->phy_consumer->phy_num) {
+            s = s->phy_consumer;
+        } else {
+            trace_imx_phy_read_num(phy, s->phy_num);
+            return 0xffff;
+        }
+    }
+
     reg %= 32;
 
     switch (reg) {
@@ -343,11 +351,19 @@ static void imx_phy_write(IMXFECState *s, int reg, uint32_t val)
 {
     uint32_t phy = reg / 32;
 
-    if (phy != s->phy_num) {
-        trace_imx_phy_write_num(phy, s->phy_num);
+    if (!s->phy_connected) {
         return;
     }
 
+    if (phy != s->phy_num) {
+        if (s->phy_consumer && phy == s->phy_consumer->phy_num) {
+            s = s->phy_consumer;
+        } else {
+            trace_imx_phy_write_num(phy, s->phy_num);
+            return;
+        }
+    }
+
     reg %= 32;
 
     trace_imx_phy_write(val, phy, reg);
@@ -1327,6 +1343,9 @@ static Property imx_eth_properties[] = {
     DEFINE_NIC_PROPERTIES(IMXFECState, conf),
     DEFINE_PROP_UINT32("tx-ring-num", IMXFECState, tx_ring_num, 1),
     DEFINE_PROP_UINT32("phy-num", IMXFECState, phy_num, 0),
+    DEFINE_PROP_BOOL("phy-connected", IMXFECState, phy_connected, true),
+    DEFINE_PROP_LINK("phy-consumer", IMXFECState, phy_consumer, TYPE_IMX_FEC,
+                     IMXFECState *),
     DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/include/hw/net/imx_fec.h b/include/hw/net/imx_fec.h
index e3a8755db9..2d13290c78 100644
--- a/include/hw/net/imx_fec.h
+++ b/include/hw/net/imx_fec.h
@@ -270,6 +270,8 @@ struct IMXFECState {
     uint32_t phy_int;
     uint32_t phy_int_mask;
     uint32_t phy_num;
+    bool phy_connected;
+    struct IMXFECState *phy_consumer;
 
     bool is_fec;
 
-- 
2.39.2

[PATCH 2/5] fsl-imx6ul: Add fec[12]-phy-connected properties

[Guenter Roeck]

Add fec[12]-phy-connected properties and use it to set phy-connected
and phy-consumer properties for imx_fec.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 hw/arm/fsl-imx6ul.c         | 20 ++++++++++++++++++++
 include/hw/arm/fsl-imx6ul.h |  1 +
 2 files changed, 21 insertions(+)

diff --git a/hw/arm/fsl-imx6ul.c b/hw/arm/fsl-imx6ul.c
index d88d6cc1c5..2189dcbb72 100644
--- a/hw/arm/fsl-imx6ul.c
+++ b/hw/arm/fsl-imx6ul.c
@@ -407,7 +407,23 @@ static void fsl_imx6ul_realize(DeviceState *dev, Error **errp)
 
     /*
      * Ethernet
+     *
+     * We must use two loops since phy_connected affects the other interface
+     * and we have to set all properties before calling sysbus_realize().
      */
+    for (i = 0; i < FSL_IMX6UL_NUM_ETHS; i++) {
+        object_property_set_bool(OBJECT(&s->eth[i]), "phy-connected",
+                                 s->phy_connected[i], &error_abort);
+        /*
+         * If the MDIO bus on this controller is not connected, assume the
+         * other controller provides support for it.
+         */
+        if (!s->phy_connected[i]) {
+            object_property_set_link(OBJECT(&s->eth[1 - i]), "phy-consumer",
+                                     OBJECT(&s->eth[i]), &error_abort);
+        }
+    }
+
     for (i = 0; i < FSL_IMX6UL_NUM_ETHS; i++) {
         static const hwaddr FSL_IMX6UL_ENETn_ADDR[FSL_IMX6UL_NUM_ETHS] = {
             FSL_IMX6UL_ENET1_ADDR,
@@ -620,6 +636,10 @@ static void fsl_imx6ul_realize(DeviceState *dev, Error **errp)
 static Property fsl_imx6ul_properties[] = {
     DEFINE_PROP_UINT32("fec1-phy-num", FslIMX6ULState, phy_num[0], 0),
     DEFINE_PROP_UINT32("fec2-phy-num", FslIMX6ULState, phy_num[1], 1),
+    DEFINE_PROP_BOOL("fec1-phy-connected", FslIMX6ULState, phy_connected[0],
+                     true),
+    DEFINE_PROP_BOOL("fec2-phy-connected", FslIMX6ULState, phy_connected[1],
+                     true),
     DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/include/hw/arm/fsl-imx6ul.h b/include/hw/arm/fsl-imx6ul.h
index 1952cb984d..9ee15ae38d 100644
--- a/include/hw/arm/fsl-imx6ul.h
+++ b/include/hw/arm/fsl-imx6ul.h
@@ -89,6 +89,7 @@ struct FslIMX6ULState {
     MemoryRegion       ocram_alias;
 
     uint32_t           phy_num[FSL_IMX6UL_NUM_ETHS];
+    bool               phy_connected[FSL_IMX6UL_NUM_ETHS];
 };
 
 enum FslIMX6ULMemoryMap {
-- 
2.39.2

[PATCH 3/5] arm/mcimx6ul-evk: Set fec1-phy-connected property to false

[Guenter Roeck]

On mcimx6ul-evk, the MDIO bus is connected to the second Ethernet
interface. Set fec1-phy-connected to false to reflect this.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 hw/arm/mcimx6ul-evk.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/arm/mcimx6ul-evk.c b/hw/arm/mcimx6ul-evk.c
index d83c3c380e..3ac1e2ea9b 100644
--- a/hw/arm/mcimx6ul-evk.c
+++ b/hw/arm/mcimx6ul-evk.c
@@ -41,6 +41,8 @@ static void mcimx6ul_evk_init(MachineState *machine)
     object_property_add_child(OBJECT(machine), "soc", OBJECT(s));
     object_property_set_uint(OBJECT(s), "fec1-phy-num", 2, &error_fatal);
     object_property_set_uint(OBJECT(s), "fec2-phy-num", 1, &error_fatal);
+    object_property_set_bool(OBJECT(s), "fec1-phy-connected", false,
+                             &error_fatal);
     qdev_realize(DEVICE(s), NULL, &error_fatal);
 
     memory_region_add_subregion(get_system_memory(), FSL_IMX6UL_MMDC_ADDR,
-- 
2.39.2

[PATCH 4/5] fsl-imx7: Add fec[12]-phy-connected properties

[Guenter Roeck]

Add fec[12]-phy-connected properties and use it to set phy-connected
and phy-consumer properties for imx_fec.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 hw/arm/fsl-imx7.c         | 20 ++++++++++++++++++++
 include/hw/arm/fsl-imx7.h |  1 +
 2 files changed, 21 insertions(+)

diff --git a/hw/arm/fsl-imx7.c b/hw/arm/fsl-imx7.c
index afc7480799..9e41d4b677 100644
--- a/hw/arm/fsl-imx7.c
+++ b/hw/arm/fsl-imx7.c
@@ -395,7 +395,23 @@ static void fsl_imx7_realize(DeviceState *dev, Error **errp)
 
     /*
      * Ethernet
+     *
+     * We must use two loops since phy_connected affects the other interface
+     * and we have to set all properties before calling sysbus_realize().
      */
+    for (i = 0; i < FSL_IMX7_NUM_ETHS; i++) {
+        object_property_set_bool(OBJECT(&s->eth[i]), "phy-connected",
+                                 s->phy_connected[i], &error_abort);
+        /*
+         * If the MDIO bus on this controller is not connected, assume the
+         * other controller provides support for it.
+         */
+        if (!s->phy_connected[i]) {
+            object_property_set_link(OBJECT(&s->eth[1 - i]), "phy-consumer",
+                                     OBJECT(&s->eth[i]), &error_abort);
+        }
+    }
+
     for (i = 0; i < FSL_IMX7_NUM_ETHS; i++) {
         static const hwaddr FSL_IMX7_ENETn_ADDR[FSL_IMX7_NUM_ETHS] = {
             FSL_IMX7_ENET1_ADDR,
@@ -601,6 +617,10 @@ static void fsl_imx7_realize(DeviceState *dev, Error **errp)
 static Property fsl_imx7_properties[] = {
     DEFINE_PROP_UINT32("fec1-phy-num", FslIMX7State, phy_num[0], 0),
     DEFINE_PROP_UINT32("fec2-phy-num", FslIMX7State, phy_num[1], 1),
+    DEFINE_PROP_BOOL("fec1-phy-connected", FslIMX7State, phy_connected[0],
+                     true),
+    DEFINE_PROP_BOOL("fec2-phy-connected", FslIMX7State, phy_connected[1],
+                     true),
     DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/include/hw/arm/fsl-imx7.h b/include/hw/arm/fsl-imx7.h
index 355bd8ea83..54ea2f0890 100644
--- a/include/hw/arm/fsl-imx7.h
+++ b/include/hw/arm/fsl-imx7.h
@@ -82,6 +82,7 @@ struct FslIMX7State {
     ChipideaState      usb[FSL_IMX7_NUM_USBS];
     DesignwarePCIEHost pcie;
     uint32_t           phy_num[FSL_IMX7_NUM_ETHS];
+    bool               phy_connected[FSL_IMX7_NUM_ETHS];
 };
 
 enum FslIMX7MemoryMap {
-- 
2.39.2

[PATCH 5/5] arm/mcimx7d-sabre: Set fec2-phy-connected property to false

[Guenter Roeck]

On mcimx7d-sabre, the MDIO bus is connected to the first Ethernet
interface. Set fec2-phy-connected to false to reflect this.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 hw/arm/mcimx7d-sabre.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/arm/mcimx7d-sabre.c b/hw/arm/mcimx7d-sabre.c
index 6182b15f19..d1778122b6 100644
--- a/hw/arm/mcimx7d-sabre.c
+++ b/hw/arm/mcimx7d-sabre.c
@@ -41,6 +41,8 @@ static void mcimx7d_sabre_init(MachineState *machine)
 
     s = FSL_IMX7(object_new(TYPE_FSL_IMX7));
     object_property_add_child(OBJECT(machine), "soc", OBJECT(s));
+    object_property_set_bool(OBJECT(s), "fec2-phy-connected", false,
+                             &error_fatal);
     qdev_realize(DEVICE(s), NULL, &error_fatal);
 
     memory_region_add_subregion(get_system_memory(), FSL_IMX7_MMDC_ADDR,
-- 
2.39.2

Re: [PATCH 1/5] hw/net/imx_fec: Support two Ethernet interfaces connected to single MDIO bus

[Peter Maydell]

On Wed, 15 Mar 2023 at 14:52, Guenter Roeck <linux@roeck-us.net> wrote:
>
> The SOC on i.MX6UL and i.MX7 has 2 Ethernet interfaces. The PHY on each may
> be connected to separate MDIO busses, or both may be connected on the same
> MDIO bus using different PHY addresses. Commit 461c51ad4275 ("Add a phy-num
> property to the i.MX FEC emulator") added support for specifying PHY
> addresses, but it did not provide support for linking the second PHY on
> a given MDIO bus to the other Ethernet interface.
>
> To be able to support two PHY instances on a single MDIO bus, two properties
> are needed: First, there needs to be a flag indicating if the MDIO bus on
> a given Ethernet interface is connected. If not, attempts to read from this
> bus must always return 0xffff. Implement this property as phy-connected.
> Second, if the MDIO bus on an interface is active, it needs a link to the
> consumer interface to be able to provide PHY access for it. Implement this
> property as phy-consumer.
>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>

> @@ -282,11 +282,19 @@ static uint32_t imx_phy_read(IMXFECState *s, int reg)
>      uint32_t val;
>      uint32_t phy = reg / 32;
>
> -    if (phy != s->phy_num) {
> -        trace_imx_phy_read_num(phy, s->phy_num);
> +    if (!s->phy_connected) {
>          return 0xffff;
>      }
>
> +    if (phy != s->phy_num) {
> +        if (s->phy_consumer && phy == s->phy_consumer->phy_num) {
> +            s = s->phy_consumer;

This does work, but it leaves me wondering if we should really
be modelling the phy as a separate device object, so that we
can use link properties to connect the right phy to the right
IMXFECState rather than having this odd "actually use the pointer
to this other instance of the device"... A quick glance through
the code suggests that the phy and the ethernet controller
proper don't really care about each others' internals.
(imx_phy_update_irq() does call imx_eth_update() but AFAICT
that is unnecessary because imx_eth_update() doesn't care about
any of the phy state...)

thanks
-- PMM

Re: [PATCH 1/5] hw/net/imx_fec: Support two Ethernet interfaces connected to single MDIO bus

[Guenter Roeck]

On Thu, Mar 30, 2023 at 05:31:13PM +0100, Peter Maydell wrote:
> On Wed, 15 Mar 2023 at 14:52, Guenter Roeck <linux@roeck-us.net> wrote:
> >
> > The SOC on i.MX6UL and i.MX7 has 2 Ethernet interfaces. The PHY on each may
> > be connected to separate MDIO busses, or both may be connected on the same
> > MDIO bus using different PHY addresses. Commit 461c51ad4275 ("Add a phy-num
> > property to the i.MX FEC emulator") added support for specifying PHY
> > addresses, but it did not provide support for linking the second PHY on
> > a given MDIO bus to the other Ethernet interface.
> >
> > To be able to support two PHY instances on a single MDIO bus, two properties
> > are needed: First, there needs to be a flag indicating if the MDIO bus on
> > a given Ethernet interface is connected. If not, attempts to read from this
> > bus must always return 0xffff. Implement this property as phy-connected.
> > Second, if the MDIO bus on an interface is active, it needs a link to the
> > consumer interface to be able to provide PHY access for it. Implement this
> > property as phy-consumer.
> >
> > Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> 
> > @@ -282,11 +282,19 @@ static uint32_t imx_phy_read(IMXFECState *s, int reg)
> >      uint32_t val;
> >      uint32_t phy = reg / 32;
> >
> > -    if (phy != s->phy_num) {
> > -        trace_imx_phy_read_num(phy, s->phy_num);
> > +    if (!s->phy_connected) {
> >          return 0xffff;
> >      }
> >
> > +    if (phy != s->phy_num) {
> > +        if (s->phy_consumer && phy == s->phy_consumer->phy_num) {
> > +            s = s->phy_consumer;
> 
> This does work, but it leaves me wondering if we should really
> be modelling the phy as a separate device object, so that we
> can use link properties to connect the right phy to the right
> IMXFECState rather than having this odd "actually use the pointer
> to this other instance of the device"... A quick glance through

Possibly, but I don't understand well enough how this would work
to be able to implement it. I'll be happy to test patches from others,
of course.

Thanks,
Guenter

> the code suggests that the phy and the ethernet controller
> proper don't really care about each others' internals.
> (imx_phy_update_irq() does call imx_eth_update() but AFAICT
> that is unnecessary because imx_eth_update() doesn't care about
> any of the phy state...)
> 
> thanks
> -- PMM

Re: [PATCH 0/5] Support both Ethernet interfaces on i.MX6UL and i.MX7

[Peter Maydell]

On Wed, 15 Mar 2023 at 14:52, Guenter Roeck <linux@roeck-us.net> wrote:
>
> The SOC on i.MX6UL and i.MX7 has 2 Ethernet interfaces. The PHY on each may
> be connected to separate MDIO busses, or both may be connected on the same
> MDIO bus using different PHY addresses. Commit 461c51ad4275 ("Add a phy-num
> property to the i.MX FEC emulator") added support for specifying PHY
> addresses, but it did not provide support for linking the second PHY on
> a given MDIO bus to the other Ethernet interface.
>
> To be able to support two PHY instances on a single MDIO bus, two properties
> are needed: First, there needs to be a flag indicating if the MDIO bus on
> a given Ethernet interface is connected. If not, attempts to read from this
> bus must always return 0xffff. Implement this property as phy-connected.
> Second, if the MDIO bus on an interface is active, it needs a link to the
> consumer interface to be able to provide PHY access for it. Implement this
> property as phy-consumer.

So I was having a look at this to see if it was reasonably easy to
split out the PHY into its own device object, and I'm a bit confused.
I know basically 0 about MDIO, but wikipedia says that MDIO buses
have one master (the ethernet MAC) and potentially multiple PHYs.
However it looks like this patchset has configurations where
multiple MACs talk to the same MDIO bus. Am I confused about the
patchset, about the hardware, or about what MDIO supports?

thanks
-- PMM

Re: [PATCH 0/5] Support both Ethernet interfaces on i.MX6UL and i.MX7

[Guenter Roeck]

On 4/18/23 05:10, Peter Maydell wrote:
> On Wed, 15 Mar 2023 at 14:52, Guenter Roeck <linux@roeck-us.net> wrote:
>>
>> The SOC on i.MX6UL and i.MX7 has 2 Ethernet interfaces. The PHY on each may
>> be connected to separate MDIO busses, or both may be connected on the same
>> MDIO bus using different PHY addresses. Commit 461c51ad4275 ("Add a phy-num
>> property to the i.MX FEC emulator") added support for specifying PHY
>> addresses, but it did not provide support for linking the second PHY on
>> a given MDIO bus to the other Ethernet interface.
>>
>> To be able to support two PHY instances on a single MDIO bus, two properties
>> are needed: First, there needs to be a flag indicating if the MDIO bus on
>> a given Ethernet interface is connected. If not, attempts to read from this
>> bus must always return 0xffff. Implement this property as phy-connected.
>> Second, if the MDIO bus on an interface is active, it needs a link to the
>> consumer interface to be able to provide PHY access for it. Implement this
>> property as phy-consumer.
> 
> So I was having a look at this to see if it was reasonably easy to
> split out the PHY into its own device object, and I'm a bit confused.
> I know basically 0 about MDIO, but wikipedia says that MDIO buses
> have one master (the ethernet MAC) and potentially multiple PHYs.
> However it looks like this patchset has configurations where
> multiple MACs talk to the same MDIO bus. Am I confused about the
> patchset, about the hardware, or about what MDIO supports?
> 

It is quite similar to I2C, a serial interface with one master/controller
and a number of devices (PHYs) connected to it. There is a nice graphic
example at https://prodigytechno.com/mdio-management-data-input-output/.
Not sure I understand what is confusing about it. Can you explain ?

Thanks,
Guenter

Re: [PATCH 0/5] Support both Ethernet interfaces on i.MX6UL and i.MX7

[Peter Maydell]

On Tue, 18 Apr 2023 at 15:42, Guenter Roeck <linux@roeck-us.net> wrote:
>
> On 4/18/23 05:10, Peter Maydell wrote:
> > On Wed, 15 Mar 2023 at 14:52, Guenter Roeck <linux@roeck-us.net> wrote:
> > So I was having a look at this to see if it was reasonably easy to
> > split out the PHY into its own device object, and I'm a bit confused.
> > I know basically 0 about MDIO, but wikipedia says that MDIO buses
> > have one master (the ethernet MAC) and potentially multiple PHYs.
> > However it looks like this patchset has configurations where
> > multiple MACs talk to the same MDIO bus. Am I confused about the
> > patchset, about the hardware, or about what MDIO supports?
> >
>
> It is quite similar to I2C, a serial interface with one master/controller
> and a number of devices (PHYs) connected to it. There is a nice graphic
> example at https://prodigytechno.com/mdio-management-data-input-output/.
> Not sure I understand what is confusing about it. Can you explain ?

I guess I don't understand what the topology is for these specific
SoCs, then. If there's only one master that might be connected
to multiple PHYs, why does one ethernet device in QEMU need to
know about the other one? Are the PHYs connected to just that
first ethernet device, or to both? This bit in your cover letter
makes it sound like "both ethernet interfaces connect to the same
MDIO bus which has both PHYs on it":

>> The SOC on i.MX6UL and i.MX7 has 2 Ethernet interfaces. The PHY on each may
>> be connected to separate MDIO busses, or both may be connected on the same
>> MDIO bus using different PHY addresses.

but maybe I'm misreading it.

thanks
-- PMM

Re: [PATCH 0/5] Support both Ethernet interfaces on i.MX6UL and i.MX7

[Guenter Roeck]

On 4/18/23 07:46, Peter Maydell wrote:
> On Tue, 18 Apr 2023 at 15:42, Guenter Roeck <linux@roeck-us.net> wrote:
>>
>> On 4/18/23 05:10, Peter Maydell wrote:
>>> On Wed, 15 Mar 2023 at 14:52, Guenter Roeck <linux@roeck-us.net> wrote:
>>> So I was having a look at this to see if it was reasonably easy to
>>> split out the PHY into its own device object, and I'm a bit confused.
>>> I know basically 0 about MDIO, but wikipedia says that MDIO buses
>>> have one master (the ethernet MAC) and potentially multiple PHYs.
>>> However it looks like this patchset has configurations where
>>> multiple MACs talk to the same MDIO bus. Am I confused about the
>>> patchset, about the hardware, or about what MDIO supports?
>>>
>>
>> It is quite similar to I2C, a serial interface with one master/controller
>> and a number of devices (PHYs) connected to it. There is a nice graphic
>> example at https://prodigytechno.com/mdio-management-data-input-output/.
>> Not sure I understand what is confusing about it. Can you explain ?
> 
> I guess I don't understand what the topology is for these specific
> SoCs, then. If there's only one master that might be connected
> to multiple PHYs, why does one ethernet device in QEMU need to
> know about the other one? Are the PHYs connected to just that
> first ethernet device, or to both? This bit in your cover letter
> makes it sound like "both ethernet interfaces connect to the same
> MDIO bus which has both PHYs on it":
> 

Yes, that is exactly how it is, similar to the configuration in the picture
at prodigytechno.com. I don't recall what I wrote in the cover letter, but
"Both Ethernet PHYs connect to the same MDIO bus which is connected to one
of the Ethernet MACs" would be the most accurate description I can think of.

>>> The SOC on i.MX6UL and i.MX7 has 2 Ethernet interfaces. The PHY on each may
>>> be connected to separate MDIO busses, or both may be connected on the same
>>> MDIO bus using different PHY addresses.
> 

Each MAC (Ethernet interface, instance of TYPE_IMX_FEC in qemu) has its own
MDIO bus. Currently QEMU assumes that each PHY is connected to the MDIO bus
on its associated MAC interface. That is not the case on the emulated boards,
where all PHYs are connected to a single MDIO bus.

Userspace, when talking to the Ethernet controllers, knows that the PHY
of the second Ethernet controller is connected to the MDIO bus on the first
Ethernet controller. QEMU has to be told about that and otherwise misses that
MDIO commands sent to the second PHY (on the first Ethernet controller)
influence the second MAC interface.

 From this exchange I can only assume that my implementation is unacceptable.
All I can say is that it works.

Thanks,
Guenter

Re: [PATCH 0/5] Support both Ethernet interfaces on i.MX6UL and i.MX7

[Peter Maydell]

On Tue, 18 Apr 2023 at 16:18, Guenter Roeck <linux@roeck-us.net> wrote:
> On 4/18/23 07:46, Peter Maydell wrote:
> > I guess I don't understand what the topology is for these specific
> > SoCs, then. If there's only one master that might be connected
> > to multiple PHYs, why does one ethernet device in QEMU need to
> > know about the other one? Are the PHYs connected to just that
> > first ethernet device, or to both? This bit in your cover letter
> > makes it sound like "both ethernet interfaces connect to the same
> > MDIO bus which has both PHYs on it":
> >
>
> Yes, that is exactly how it is, similar to the configuration in the picture
> at prodigytechno.com. I don't recall what I wrote in the cover letter, but
> "Both Ethernet PHYs connect to the same MDIO bus which is connected to one
> of the Ethernet MACs" would be the most accurate description I can think of.

> Each MAC (Ethernet interface, instance of TYPE_IMX_FEC in qemu) has its own
> MDIO bus. Currently QEMU assumes that each PHY is connected to the MDIO bus
> on its associated MAC interface. That is not the case on the emulated boards,
> where all PHYs are connected to a single MDIO bus.

So looking again at that diagram on that website, I think I understand
now: for data transfer to/from the outside world, MAC1 talks only through
PHY1 and MAC2 only through PHY2 (over the links marked "MII/GMII/XGMII"),
but the "control" connection is via MDIO, and on these boards you have to
configure PHY2 by doing the MDIO reads and writes via MAC1, even though
MAC1 has nothing otherwise to do with PHY2 ? (And MAC2 has no devices on
its MDIO bus at all.)

> Userspace, when talking to the Ethernet controllers, knows that the PHY
> of the second Ethernet controller is connected to the MDIO bus on the first
> Ethernet controller. QEMU has to be told about that and otherwise misses that
> MDIO commands sent to the second PHY (on the first Ethernet controller)
> influence the second MAC interface.
>
>  From this exchange I can only assume that my implementation is unacceptable.

Not at all -- I'm just trying to understand what the hardware we're
modelling is doing, so I can figure out what we "ought" in theory
to be doing and whether that's too much pain to do right now...

thanks
-- PMM

Re: [PATCH 0/5] Support both Ethernet interfaces on i.MX6UL and i.MX7

[Guenter Roeck]

On 4/18/23 08:32, Peter Maydell wrote:
> On Tue, 18 Apr 2023 at 16:18, Guenter Roeck <linux@roeck-us.net> wrote:
>> On 4/18/23 07:46, Peter Maydell wrote:
>>> I guess I don't understand what the topology is for these specific
>>> SoCs, then. If there's only one master that might be connected
>>> to multiple PHYs, why does one ethernet device in QEMU need to
>>> know about the other one? Are the PHYs connected to just that
>>> first ethernet device, or to both? This bit in your cover letter
>>> makes it sound like "both ethernet interfaces connect to the same
>>> MDIO bus which has both PHYs on it":
>>>
>>
>> Yes, that is exactly how it is, similar to the configuration in the picture
>> at prodigytechno.com. I don't recall what I wrote in the cover letter, but
>> "Both Ethernet PHYs connect to the same MDIO bus which is connected to one
>> of the Ethernet MACs" would be the most accurate description I can think of.
> 
>> Each MAC (Ethernet interface, instance of TYPE_IMX_FEC in qemu) has its own
>> MDIO bus. Currently QEMU assumes that each PHY is connected to the MDIO bus
>> on its associated MAC interface. That is not the case on the emulated boards,
>> where all PHYs are connected to a single MDIO bus.
> 
> So looking again at that diagram on that website, I think I understand
> now: for data transfer to/from the outside world, MAC1 talks only through
> PHY1 and MAC2 only through PHY2 (over the links marked "MII/GMII/XGMII"),
> but the "control" connection is via MDIO, and on these boards you have to
> configure PHY2 by doing the MDIO reads and writes via MAC1, even though
> MAC1 has nothing otherwise to do with PHY2 ? (And MAC2 has no devices on
> its MDIO bus at all.)
> 

Correct.

Thanks,
Guenter

Re: [PATCH 0/5] Support both Ethernet interfaces on i.MX6UL and i.MX7

[Peter Maydell]

On Tue, 18 Apr 2023 at 17:55, Guenter Roeck <linux@roeck-us.net> wrote:
> On 4/18/23 08:32, Peter Maydell wrote:
> > So looking again at that diagram on that website, I think I understand
> > now: for data transfer to/from the outside world, MAC1 talks only through
> > PHY1 and MAC2 only through PHY2 (over the links marked "MII/GMII/XGMII"),
> > but the "control" connection is via MDIO, and on these boards you have to
> > configure PHY2 by doing the MDIO reads and writes via MAC1, even though
> > MAC1 has nothing otherwise to do with PHY2 ? (And MAC2 has no devices on
> > its MDIO bus at all.)
> >
>
> Correct.

Thanks. Now that I understand how the hardware is put together,
I had a think about what the theoretically 'correct' design
would be. We could do it, but it's a bit of a pain -- among
other things, QEMU's architecture doesn't really consider
the idea of devices that are on more than one bus (as the
PHY is on both the MDIO and the MII), so you have to pick
one as the 'real' bus and then use property links or similar
for the other. And we get into having to look at all the
other uses of PHYs in QEMU so we can design the interface
to be general enough to be usable elsewhere. All of which is
more effort than seems worthwhile for just these imx devices.

So I'm going to take this series into target-arm.next;
if anybody in future wants to properly model the MAC-PHY
interface we can feed this use-case in as one of the
requirement then...

-- PMM