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

[Tom Rini]

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.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20161129/1f54b5ef/attachment.sig>