| Message ID | 20230315145248.1639364-2-linux@roeck-us.net |
|---|---|
| State | New |
| Headers | show |
| Series | Support both Ethernet interfaces on i.MX6UL and i.MX7 | expand |
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
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
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;
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(-)