[PATCH] sunxi: don't define MACPWR and SATAPWR to empty strings

[PATCH] sunxi: don't define MACPWR and SATAPWR to empty strings

[Peter Robinson]

There's checks in board/sunxi/board.c if either MACPWR or SATAPWR are
defined and they are defined by default to a empty string which means
on vast majority of AllWinner boards when they're not required the
code is still run and with SATAPWR we even get an unnecessary 500ms
delay booting. So let's not define a default for these options so the
code is only run for boards that need it.

Signed-off-by: Peter Robinson <pbrobinson@gmail.com>
---
 arch/arm/mach-sunxi/Kconfig | 2 --
 1 file changed, 2 deletions(-)

diff --git a/arch/arm/mach-sunxi/Kconfig b/arch/arm/mach-sunxi/Kconfig
index 49ef217f08..a566e10315 100644
--- a/arch/arm/mach-sunxi/Kconfig
+++ b/arch/arm/mach-sunxi/Kconfig
@@ -578,7 +578,6 @@ config OLD_SUNXI_KERNEL_COMPAT
 
 config MACPWR
 	string "MAC power pin"
-	default ""
 	help
 	  Set the pin used to power the MAC. This takes a string in the format
 	  understood by sunxi_name_to_gpio, e.g. PH1 for pin 1 of port H.
@@ -969,7 +968,6 @@ endchoice
 
 config SATAPWR
 	string "SATA power pin"
-	default ""
 	help
 	  Set the pins used to power the SATA. This takes a string in the
 	  format understood by sunxi_name_to_gpio, e.g. PH1 for pin 1 of
-- 
2.29.2

[PATCH] sunxi: don't define MACPWR and SATAPWR to empty strings

[André Przywara]

On 18/01/2021 00:18, Peter Robinson wrote:
> There's checks in board/sunxi/board.c if either MACPWR or SATAPWR are
> defined and they are defined by default to a empty string which means
> on vast majority of AllWinner boards when they're not required the
> code is still run and with SATAPWR we even get an unnecessary 500ms
> delay booting. So let's not define a default for these options so the
> code is only run for boards that need it.

Ouch, that's a good one!
Looks good to me, I will do some checks and testing to verify.

Actually it looks even worse, since we don't check the return value of
sunxi_name_to_gpio(), and gpio_direction_output() doesn't either. I
wonder how this didn't crash before ...

Plus I wonder if we can move those two pins to be handled by DT.

Anyway, first things first...
Thanks for the patch!

Cheers,
Andre

> 
> Signed-off-by: Peter Robinson <pbrobinson@gmail.com>
> ---
>  arch/arm/mach-sunxi/Kconfig | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/arch/arm/mach-sunxi/Kconfig b/arch/arm/mach-sunxi/Kconfig
> index 49ef217f08..a566e10315 100644
> --- a/arch/arm/mach-sunxi/Kconfig
> +++ b/arch/arm/mach-sunxi/Kconfig
> @@ -578,7 +578,6 @@ config OLD_SUNXI_KERNEL_COMPAT
>  
>  config MACPWR
>  	string "MAC power pin"
> -	default ""
>  	help
>  	  Set the pin used to power the MAC. This takes a string in the format
>  	  understood by sunxi_name_to_gpio, e.g. PH1 for pin 1 of port H.
> @@ -969,7 +968,6 @@ endchoice
>  
>  config SATAPWR
>  	string "SATA power pin"
> -	default ""
>  	help
>  	  Set the pins used to power the SATA. This takes a string in the
>  	  format understood by sunxi_name_to_gpio, e.g. PH1 for pin 1 of
> 

[PATCH] sunxi: don't define MACPWR and SATAPWR to empty strings

[Andre Przywara]

On 18/01/2021 00:18, Peter Robinson wrote:

Hi Peter,

> There's checks in board/sunxi/board.c if either MACPWR or SATAPWR are
> defined and they are defined by default to a empty string which means
> on vast majority of AllWinner boards when they're not required the
> code is still run and with SATAPWR we even get an unnecessary 500ms
> delay booting. So let's not define a default for these options so the
> code is only run for boards that need it.
> 
> Signed-off-by: Peter Robinson <pbrobinson@gmail.com>
> ---
>  arch/arm/mach-sunxi/Kconfig | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/arch/arm/mach-sunxi/Kconfig b/arch/arm/mach-sunxi/Kconfig
> index 49ef217f08..a566e10315 100644
> --- a/arch/arm/mach-sunxi/Kconfig
> +++ b/arch/arm/mach-sunxi/Kconfig
> @@ -578,7 +578,6 @@ config OLD_SUNXI_KERNEL_COMPAT
>  
>  config MACPWR
>  	string "MAC power pin"
> -	default ""

But that doesn't really help, does it? The kernel Kconfig documentation
doesn't mention this explicitly, but the implicit default for strings is
actually "", so I still see CONFIG_MACPWR="" in my .config, even after
your patch. And consequently I see all the code still being executed.

I made a patch the does the check in the board.c, I would be grateful if
you could test this.

Cheers,
Andre

>  	help
>  	  Set the pin used to power the MAC. This takes a string in
> the format understood by sunxi_name_to_gpio, e.g. PH1 for pin 1 of
> port H. @@ -969,7 +968,6 @@ endchoice
>  
>  config SATAPWR
>  	string "SATA power pin"
> -	default ""
>  	help
>  	  Set the pins used to power the SATA. This takes a string
> in the format understood by sunxi_name_to_gpio, e.g. PH1 for pin 1 of
> 

[PATCH] sunxi: don't define MACPWR and SATAPWR to empty strings

[Peter Robinson]

On Tue, Jan 19, 2021 at 1:05 AM Andre Przywara <andre.przywara@arm.com> wrote:
>
> On 18/01/2021 00:18, Peter Robinson wrote:
>
> Hi Peter,
>
> > There's checks in board/sunxi/board.c if either MACPWR or SATAPWR are
> > defined and they are defined by default to a empty string which means
> > on vast majority of AllWinner boards when they're not required the
> > code is still run and with SATAPWR we even get an unnecessary 500ms
> > delay booting. So let's not define a default for these options so the
> > code is only run for boards that need it.
> >
> > Signed-off-by: Peter Robinson <pbrobinson@gmail.com>
> > ---
> >  arch/arm/mach-sunxi/Kconfig | 2 --
> >  1 file changed, 2 deletions(-)
> >
> > diff --git a/arch/arm/mach-sunxi/Kconfig b/arch/arm/mach-sunxi/Kconfig
> > index 49ef217f08..a566e10315 100644
> > --- a/arch/arm/mach-sunxi/Kconfig
> > +++ b/arch/arm/mach-sunxi/Kconfig
> > @@ -578,7 +578,6 @@ config OLD_SUNXI_KERNEL_COMPAT
> >
> >  config MACPWR
> >       string "MAC power pin"
> > -     default ""
>
> But that doesn't really help, does it? The kernel Kconfig documentation
> doesn't mention this explicitly, but the implicit default for strings is
> actually "", so I still see CONFIG_MACPWR="" in my .config, even after
> your patch. And consequently I see all the code still being executed.

I had a couple of different versions of this patch from last year so
maybe I missed something when I went back around and reviewed it. The
new patch also seems fine.

> I made a patch the does the check in the board.c, I would be grateful if
> you could test this.

Seems fine to me in testing across a couple of devices.

> Cheers,
> Andre
>
> >       help
> >         Set the pin used to power the MAC. This takes a string in
> > the format understood by sunxi_name_to_gpio, e.g. PH1 for pin 1 of
> > port H. @@ -969,7 +968,6 @@ endchoice
> >
> >  config SATAPWR
> >       string "SATA power pin"
> > -     default ""
> >       help
> >         Set the pins used to power the SATA. This takes a string
> > in the format understood by sunxi_name_to_gpio, e.g. PH1 for pin 1 of
> >
>