[U-Boot] [PATCH] sunxi: move CONFIG_SATAPWR to Kconfig option

[Maxime Ripard]

On Mon, Dec 05, 2016 at 09:43:45PM +0100, Jelle van der Waa wrote:
> On 11/29/16 at 05:25pm, Tom Rini wrote:
> > On Tue, Nov 29, 2016 at 10:17:42PM +0100, Maxime Ripard wrote:
> > > Hi Tom,
> > > 
> > > On Tue, Nov 29, 2016 at 04:11:38PM -0500, Tom Rini wrote:
> > > > On Tue, Nov 29, 2016 at 09:39:32PM +0100, Maxime Ripard wrote:
> > > > > On Mon, Nov 28, 2016 at 02:20:10PM +0100, Hans de Goede wrote:
> > > > > > HI,
> > > > > > 
> > > > > > On 28-11-16 13:42, Maxime Ripard wrote:
> > > > > > > Hi Hans,
> > > > > > > 
> > > > > > > On Fri, Nov 25, 2016 at 09:12:30AM +0100, Hans de Goede wrote:
> > > > > > > > Hi,
> > > > > > > > 
> > > > > > > > On 24-11-16 22:22, Maxime Ripard wrote:
> > > > > > > > > On Wed, Nov 23, 2016 at 07:28:16PM +0100, Jelle van der Waa wrote:
> > > > > > > > > > Introduce a new CONFIG_SATAPWR Kconfig option to replace the
> > > > > > > > > > option in CONFIG_SYS_EXTRA_OPTIONS.
> > > > > > > > > > 
> > > > > > > > > > Signed-off-by: Jelle van der Waa <jelle at vdwaa.nl>
> > > > > > > > > > ---
> > > > > > > > > >  board/sunxi/Kconfig                        |  7 +++++++
> > > > > > > > > >  board/sunxi/board.c                        | 11 ++++++-----
> > > > > > > > > >  configs/A10-OLinuXino-Lime_defconfig       |  3 ++-
> > > > > > > > > >  configs/A20-OLinuXino-Lime2_defconfig      |  3 ++-
> > > > > > > > > >  configs/A20-OLinuXino-Lime_defconfig       |  3 ++-
> > > > > > > > > >  configs/A20-OLinuXino_MICRO_defconfig      |  3 ++-
> > > > > > > > > >  configs/A20-Olimex-SOM-EVB_defconfig       |  3 ++-
> > > > > > > > > >  configs/Cubieboard2_defconfig              |  3 ++-
> > > > > > > > > >  configs/Cubieboard_defconfig               |  3 ++-
> > > > > > > > > >  configs/Cubietruck_defconfig               |  3 ++-
> > > > > > > > > >  configs/Itead_Ibox_A20_defconfig           |  3 ++-
> > > > > > > > > >  configs/Lamobo_R1_defconfig                |  3 ++-
> > > > > > > > > >  configs/Linksprite_pcDuino3_Nano_defconfig |  3 ++-
> > > > > > > > > >  configs/Linksprite_pcDuino3_defconfig      |  3 ++-
> > > > > > > > > >  configs/Sinovoip_BPI_M3_defconfig          |  2 +-
> > > > > > > > > >  configs/orangepi_plus_defconfig            |  3 ++-
> > > > > > > > > >  16 files changed, 40 insertions(+), 19 deletions(-)
> > > > > > > > > > 
> > > > > > > > > > diff --git a/board/sunxi/Kconfig b/board/sunxi/Kconfig
> > > > > > > > > > index ae2fba1..fe2f7b4 100644
> > > > > > > > > > --- a/board/sunxi/Kconfig
> > > > > > > > > > +++ b/board/sunxi/Kconfig
> > > > > > > > > > @@ -667,6 +667,13 @@ config GMAC_TX_DELAY
> > > > > > > > > >  	---help---
> > > > > > > > > >  	Set the GMAC Transmit Clock Delay Chain value.
> > > > > > > > > > 
> > > > > > > > > > +config SATAPWR
> > > > > > > > > > +	string "power pin for SATA"
> > > > > > > > > > +	default ""
> > > > > > > > > > +	---help---
> > > > > > > > > > +	Set the power pin for SATA. This takes a string in the format
> > > > > > > > > > +	understood by sunxi_name_to_gpio, e.g. PH1 for pin 1 of port H.
> > > > > > > > > > +
> > > > > > > > > 
> > > > > > > > > This looks like a rather generic option. Can't this be in
> > > > > > > > > drivers/block instead?
> > > > > > > > 
> > > > > > > > The proper solution would be to get the info from devicetree,
> > > > > > > > which requires regulator support, which we don't have yet
> > > > > > > > for sunxi. In the mean time getting rid of the need for
> > > > > > > > CONFIG_SYS_EXTRA_OPTIONS is a worthwhile goal in itself
> > > > > > > > IMHO.
> > > > > > > 
> > > > > > > Yes, but a GPIO to enable the SATA 5V rail seems like a rather common
> > > > > > > thing, and definitely not Allwinner specific.
> > > > > > > 
> > > > > > > Moving that option to drivers/block would make more sense I guess.
> > > > > > 
> > > > > > Hmm, but in the end this should be removed, as everything should'
> > > > > > be using devicetree, so I'm not convinced it is a good idea
> > > > > > to introduce a generic option for this.
> > > > > > 
> > > > > > Anyways either way is fine with me.
> > > > > 
> > > > > I don't know, in the end, not all platforms will be converted to
> > > > > device tree, so it still might be something worth adding.
> > > > 
> > > > We also don't want to get hung up on fixing sub-optional solutions while
> > > > also converting to Kconfig.  We can move the option now and see about
> > > > re-naming or getting the information elsewhere afterwards.  Thanks!
> > > 
> > > I'm not sure I got what you wanted. Do you want it to be merged in
> > > drivers/block as a generic option, or in board/sunxi as a
> > > platform-specific one?
> > 
> > I'm fine with either place.  Today it's used just on sunxi as a config
> > option with that name, so board/sunxi/Kconfig and make a note to
> > investigate how all of the other platforms that have this issue handle
> > it.  For example, my gut (based on some other imx6 stuff I've been
> > doing) is that on imx6 they just fiddle the GPIO and not have it in
> > Kconfig because rather than 1 board.c and N defconfig files they have
> > (roughly) N board.c files and N defconfigs.
> 
> Maxime, what do you prefer?
> 
> I later also want to move the following options from
> CONFIG_SYS_EXTRA_OPTIONS. (haven't done so since those patches will
> conflict)
> 
> * SUNXI_GMAC
> 
> Should go to board/sunxi/Kconfig, I assume. The drivers is there too.
> 
> * RGMII
> 
> This is an interesting one, also used in drivers/net/macb.c and it seems
> selected for sama5d3xek by a #define. Not sure what to do here
> 
> * MACPWR=SUNXI_GPH(23)
> 
> Same issue as above, this patch :)

We can do it as Tom suggested for now. Put it in board/sunxi with a
comment that we should look into merging that with other
architectures>

Maxime


-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20161209/e524a7f0/attachment.sig>