[U-Boot] [PATCH] arm: mvebu: switch clearfog to use device-tree i2c and gpio

[Stefan Roese]

Hi Jon,

On 29.05.2018 11:02, Jon Nettleton wrote:
> On Tue, May 29, 2018 at 7:37 AM Stefan Roese <sr at denx.de 
> <mailto:sr at denx.de>> wrote:
> 
>     On 29.05.2018 07:10, Chris Packham wrote:
>      > On Tue, May 29, 2018 at 4:58 PM Baruch Siach <baruch at tkos.co.il
>     <mailto:baruch at tkos.co.il>> wrote:
>      >
>      >> Hi Chris,
>      >
>      >> On Tue, May 29, 2018 at 10:53:38AM +1200, Chris Packham wrote:
>      >>> On Tue, May 29, 2018 at 4:11 AM Baruch Siach <baruch at tkos.co.il
>     <mailto:baruch at tkos.co.il>> wrote:
>      >>>
>      >>>> From: Jon Nettleton <jon at solid-run.com <mailto:jon at solid-run.com>>
>      >>>
>      >>>> This switches the clearfog boards to use DM based gpio and i2c
>      >>>> drivers.  The io expanders are configured via their device-tree
>      >>>> entries.
>      >>>
>      >>>> Signed-off-by: Jon Nettleton <jon at solid-run.com
>     <mailto:jon at solid-run.com>>
>      >>>> [baruch: add DT i2c aliases]
>      >>>> Signed-off-by: Baruch Siach <baruch at tkos.co.il
>     <mailto:baruch at tkos.co.il>>
>      >>>> ---
>      >
>      >> [snip]
>      >
>      >>>> -static struct marvell_io_exp io_exp[] = {
>      >>>> -       { 0x20, 2, 0x40 },      /* Deassert both mini pcie reset
>      > signals
>      >>> */
>      >>>> -       { 0x20, 6, 0xf9 },
>      >>>> -       { 0x20, 2, 0x46 },      /* rst signals and ena USB3
>     current
>      >>> limiter */
>      >>>> -       { 0x20, 6, 0xb9 },
>      >>>> -       { 0x20, 3, 0x00 },      /* Set SFP_TX_DIS to zero */
>      >>>> -       { 0x20, 7, 0xbf },      /* Drive SFP_TX_DIS to zero */
>      >>>> -};
>      >>>> -
>      >>>
>      >>> Will this actually work? As far as I can see u-boot lacks a
>     gpio-hog
>      >>> implementation so despite the fact that these are all
>     enumerated in the
>      > dts
>      >>> they won't have the same effect.
>      >>>
>      >>> I'd be glad to be proven wrong because I'd like to use a
>     gpio-hog for
>      > some
>      >>> other boards.
>      >
>      >> I asked Jon (back on Cc) exactly that before submitting this
>     patch. His
>      >> response:
>      >
>      >>     I think that is fine. They can still be toggled and
>     controlled via the
>      > gpio
>      >>     interface within u-boot. I assume once a proper patch is
>     mainline this
>      > will
>      >>     just start to work.
>      >
>      >> My testing showed now regression because of this patch.
>      >
>      > OK. Did you test with a power cycle? I know some of the pca gpio
>     chips
>      > don't have a reset line so the state may be different depending
>     on if you
>      > power cycled or just used the reset/reboot command. It could also
>     be that
>      > u-boot doesn't access these devices so there's no problem.
>      >
>      > I kinda was hoping for "here's a gpio-hog implementation @
>     patchwork/1234"
>      > :) but I guess I'll have to look into making one myself.
> 
>     I would prefer to not drop these I2C accesses from the ClearFog board
>     C code now, as we don't have a way to do this via the DT right now.
>     Why are they dropped with this patch? Because of the I2C API
>     changes? Please move to the DM I2C API then (dm_i2c_read/write).
> 
>     Thanks,
>     Stefan
> 
> 
> Stefan,
> 
> These do not need to be setup independently any longer.  Moving to 
> DM_GPIO allows the PCI driver to handle the reset lines.  The USB3 reset 
> and current limiter should be handled in board_xhci_enable() which can 
> be done, although I have never seen an issue using USB3 with this 
> configuration since I have implemented it.  Since u-boot doesn't 
> understand the SFP setup at all I would prefer if it left SFP 
> configuration up to Linux.  Just enabling TX on an SFP module without 
> understanding any of the specifics of the configuration is a bit 
> dangerous when dealing with modules that aren't just SGMII 1000base-Tx.

Thanks for the explanation. With this I'm okay with removing the
I2C code from the board file:

Reviewed-by: Stefan Roese <sr at denx.de>

> I will follow up with a patch for board_xhci_enable().

Good.

Thanks,
Stefan