On Wed, Mar 21, 2018 at 1:29 PM, Jagan Teki jagannadh.teki@gmail.com wrote:
On Wed, Mar 21, 2018 at 6:54 PM, Peng Fan peng.fan@nxp.com wrote:
> -----Original Message----- > From: Jagan Teki [mailto:jagannadh.teki@gmail.com] > Sent: 2018年3月21日 17:19 > To: Peng Fan peng.fan@nxp.com > Cc: Joe Hershberger joe.hershberger@ni.com; Fabio Estevam > fabio.estevam@nxp.com; U-Boot Mailing List > u-boot@lists.denx.de > Subject: Re: [U-Boot] [PATCH V2 4/5] net: fec: sharing MDIO for > two enet controllers > > On Wed, Mar 21, 2018 at 2:31 PM, Peng Fan peng.fan@nxp.com
wrote:
> > On i.MX6SX, 6UL and 7D, there are two enet controllers each has > > a MDIO port. But Some boards share one MDIO port for the two enets. > > So introduce a configuration CONFIG_FEC_MXC_MDIO_BASE to > > indicate the MDIO port for sharing. > > > > To i.MX28, adapt to use the new config > > > > Signed-off-by: Peng Fan peng.fan@nxp.com > > Acked-by: Joe Hershberger joe.hershberger@ni.com > > Cc: Fabio Estevam fabio.estevam@nxp.com > > --- > > > > V2: > > adapt mx28 to use FEC_MXC_MDIO_BASE > > > > drivers/net/Kconfig | 9 ++++++++- > > drivers/net/fec_mxc.c | 8 ++++++-- > > include/configs/mx28evk.h | 1 + > > 3 files changed, 15 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig index > > de1947ccc1..c5fd6f648a 100644 > > --- a/drivers/net/Kconfig > > +++ b/drivers/net/Kconfig > > @@ -147,9 +147,16 @@ config ETHOC > > help > > This MAC is present in OpenRISC and Xtensa XTFPGA
boards.
> > > > +config FEC_MXC_MDIO_BASE > > + hex "MDIO base address for the FEC controller" > > + depends on FEC_MXC > > + help > > + This specifies the MDIO registers base address. It is used
when
> > + two FEC controllers share MDIO bus. > > How about retrieving this from dt? we have board that support dt > along with dm supporting.
It's ok to retrieve the info from dt, but we are not only support dt, we also need to support non-dt case. Define FEC_MXC_MDIO_BASE is the simplest method to achieve that I think.
But patch adds the same for DM case which usually retrieve the info from dt, point here is to get rid of ifdef and new CONFIG_ ie where DM and dts play smart atleast.
The uboot fec_mxc driver or net driver is not that sync with linux code, Using dt here, need to parse phy-handle = <ðphy0>;, then parse ethphy0 parent to get reg, I just think this like hack.
No, I think we can do this with adding new DM MDIO similar to DM PHY which recently done. May be some sort of efforts but it is permanent.
We do not have that driver now, so could we first have this patch? When DM MDIO ready, this piece code could be removed then?
ie. up to Joe. Honestly this macro become removed in future, my point here is why we need to maintain dead macro instead of adding proper maintainable stuff. I'm pretty sure adding DM_MDIO is straight forward and as of now just add what we need and rest will implement future. You may become victim to others to move DM_ETH as soon as possible :)
It would be ideal if you wanted to implement DM MDIO, but I can also appreciate that this is not already there for you to use. As I commented when I acked this, the I'm OK with this approach at this time due to the state of the DM support in eth.
As a side note, maybe moving other boards that use this NIC to DM_ETH and removing non-DM support would be a better cleanup to start with.
Cheers, -Joe