[U-Boot] [PATCH 1/2] drivers: net: driver for MDIO muxes controlled over I2C

[Alex Marginean]

Hi Bin, Simon,

On 7/16/2019 11:21 AM, Bin Meng wrote:
> +Simon,
> 
> Hi Alex,
> 
> On Mon, Jul 15, 2019 at 7:45 PM Alexandru Marginean
> <alexandru.marginean at nxp.com> wrote:
>>
>> On 7/13/2019 7:02 AM, Bin Meng wrote:
>>> Hi Alex,
>>>
>>> On Fri, Jul 12, 2019 at 10:21 PM Alex Marginean
>>> <alexandru.marginean at nxp.com> wrote:
>>>>
>>>> This driver is used for MDIO muxes driven over I2C.  This is currently
>>>> used on Freescale LS1028A QDS board, on which the physical MDIO MUX is
>>>> controlled by an on-board FPGA which in turn is configured through I2C.
>>>>
>>>> Signed-off-by: Alex Marginean <alexm.osslist at gmail.com>
>>>> ---
>>>>
>>>> Depends on https://patchwork.ozlabs.org/project/uboot/list/?series=119124
>>>>
>>>>    drivers/net/Kconfig            |   8 +++
>>>>    drivers/net/Makefile           |   1 +
>>>>    drivers/net/mdio_mux_i2c_reg.c | 108 +++++++++++++++++++++++++++++++++
>>>>    3 files changed, 117 insertions(+)
>>>>    create mode 100644 drivers/net/mdio_mux_i2c_reg.c
>>>>
>>>> diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
>>>> index 4d85fb1716..93535f7acb 100644
>>>> --- a/drivers/net/Kconfig
>>>> +++ b/drivers/net/Kconfig
>>>> @@ -595,4 +595,12 @@ config FSL_ENETC
>>>>             This driver supports the NXP ENETC Ethernet controller found on some
>>>>             of the NXP SoCs.
>>>>
>>>> +config MDIO_MUX_I2C_REG
>>>
>>> I don't see current Linux upstream has any I2CREG support. I believe
>>> you will upstream the Linux support too?
>>
>> Linux support is somewhat different so we won't have the same driver and
>> binding there.  The main difference is that Linux has the concept of a
>> mux controller, independent of the type of bus it muxes.  I didn't add
>> this to U-Boot, not yet at least.
>> This specific MDIO mux would be driven in Linux  by a pair of devices:
>> - a mux controller with bindings defined by reg-mux.txt [1], this one
>> implements the select function,
>> - MDIO mux as a client of the controller, defined by
>> mdio-mux-multiplexer.txt [2], which deals with the MDIO specific side of
>> things.
>>
> 
> Thanks for the detailed explanations! I thought people wanted to have
> exact the same DT bindings as Linux so that we can sync-up upstream
> kernel DT changes to U-Boot. That's why at first I checked whether
> kernel has such bindings in place.
> 
>> In U-Boot I picked up the relevant properties from the two bindings, the
>> MDIO mux device in U-Boot is supposed to implement select like a generic
>> Linux MUX, while the uclass code deals with MDIO.
>> I noticed U-Boot already has an I2C MUX class, along the lines of the
>> MDIO one, I'm not sure if it's useful enough to introduce a new class of
>> MUX controllers and then make all other kind of muxes depend on it.  For
>> now at least the U-Boot mux drivers are simple enough and the extra
>> class wouldn't help much.
>>
>> The other difference is that in Linux the mux controller is driven by
>> the 'MMIO register bitfield-controlled multiplexer driver' [3], which is
>> not I2C specific.  It is built on top of regmap which I also think is a
>> bit too much for U-Boot.  For the U-Boot driver I just called I2C API
>> directly and in that context it made sense to add I2C to the driver
>> name, but there won't be an equivalent I2C based driver for Linux.
>>
> 
> For your current implementation I believe that's OK, at least it is
> verified on LS1028. But there must be a reason that Linux kernel chose
> a DT that is different. I suspect the usage of kernel DT makes us stay
> future-proof.

Having a generic mux controller that's independent of the muxed bus has
its merit, of course.  Right now the API that a MDIO MUX must implement
in U-Boot has just a _select function, which, by itself, has not direct
relation to MDIO and could be used for other buses.  So mux controllers
could work in U-Boot, at least to keep U-Boot in sync with Linux.

I'm not sure about reg/mmio-mux.  Those work in Linux because of regmap
and layers that abstract the underlying bus which I think are a bit too
much for U-Boot.  Of course without that abstraction we can't use
"reg-mux" or "mdio-mux-multiplexer" compatibility strings for the more
specific devices and drivers in U-Boot.

> 
>> I'd really appreciate some feedback on this.  I think the fairly simple
>> approach in U-Boot is good enough, but I'm open to suggestions.
> 
> I am open on this topic too. Agreed that for U-Boot we always want a
> simple implementation for device drivers. I am adding Simon for his
> opinion on this topic as well.

Thank you, let's see what Simon thinks about this.

Alex

> 
>>
>>
>> [1]
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/mux/reg-mux.txt
>> [2]
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/net/mdio-mux-multiplexer.txt
>> [3]
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/mux/mmio.c
>>
> 
> Regards,
> Bin
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot
>