diff mbox series

gpio: Let DM_74X164 be built without CONFIG_SPL_GPIO

Message ID 20200129144115.12789-1-festevam@gmail.com
State Superseded
Headers show
Series gpio: Let DM_74X164 be built without CONFIG_SPL_GPIO | expand

Commit Message

Fabio Estevam Jan. 29, 2020, 2:41 p.m. UTC
Since commit bcee8d6764f9 ("dm: gpio: Allow control of GPIO uclass in SPL")
the CONFIG_DM_74X164 is no longer built for mx7dsabresd_defconfig, as
this target does not use CONFIG_SPL_GPIO.

Remove such dependency and let the the 74X164 GPIO driver be built
again.

This restores Ethernet functionality on the imx7-sdb board as the
Ethernet reset PHY comes from a GPIO driven by a 74LV595PW I/O
expander.

Fixes: bcee8d6764f9 ("dm: gpio: Allow control of GPIO uclass in SPL")
Signed-off-by: Fabio Estevam <festevam at gmail.com>
---
 drivers/gpio/Makefile | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

Comments

Tom Rini Jan. 29, 2020, 2:55 p.m. UTC | #1
On Wed, Jan 29, 2020 at 11:41:15AM -0300, Fabio Estevam wrote:
> Since commit bcee8d6764f9 ("dm: gpio: Allow control of GPIO uclass in SPL")
> the CONFIG_DM_74X164 is no longer built for mx7dsabresd_defconfig, as
> this target does not use CONFIG_SPL_GPIO.
> 
> Remove such dependency and let the the 74X164 GPIO driver be built
> again.
> 
> This restores Ethernet functionality on the imx7-sdb board as the
> Ethernet reset PHY comes from a GPIO driven by a 74LV595PW I/O
> expander.
> 
> Fixes: bcee8d6764f9 ("dm: gpio: Allow control of GPIO uclass in SPL")
> Signed-off-by: Fabio Estevam <festevam at gmail.com>
> ---
>  drivers/gpio/Makefile | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
> index 449046b64c..aa41a24cd0 100644
> --- a/drivers/gpio/Makefile
> +++ b/drivers/gpio/Makefile
> @@ -10,13 +10,11 @@ endif
>  obj-$(CONFIG_$(SPL_TPL_)DM_GPIO) += gpio-uclass.o
>  
>  obj-$(CONFIG_$(SPL_)DM_PCA953X)	+= pca953x_gpio.o
> -ifdef CONFIG_$(SPL_TPL_)GPIO
> -obj-$(CONFIG_DM_74X164)		+= 74x164_gpio.o
> -endif
>  
>  obj-$(CONFIG_AT91_GPIO)	+= at91_gpio.o
>  obj-$(CONFIG_ATMEL_PIO4)	+= atmel_pio4.o
>  obj-$(CONFIG_BCM6345_GPIO)	+= bcm6345_gpio.o
> +obj-$(CONFIG_DM_74X164)		+= 74x164_gpio.o
>  obj-$(CONFIG_INTEL_GPIO)	+= intel_gpio.o
>  obj-$(CONFIG_INTEL_ICH6_GPIO)	+= intel_ich6_gpio.o
>  obj-$(CONFIG_INTEL_BROADWELL_GPIO)	+= intel_broadwell_gpio.o

OK, why is the fix to do what you did here and NOT enable
CONFIG_SPL_GPIO on the platform(s) in question to match the rest of
bcee8d6764f9 and make the obj- line above look like the pca953x_gpio.o
one?  Thanks!
Fabio Estevam Jan. 29, 2020, 2:57 p.m. UTC | #2
On Wed, Jan 29, 2020 at 11:55 AM Tom Rini <trini at konsulko.com> wrote:

> OK, why is the fix to do what you did here and NOT enable
> CONFIG_SPL_GPIO on the platform(s) in question to match the rest of

Because mx7dsabresd_defconfig does not support SPL.
Tom Rini Jan. 29, 2020, 3:35 p.m. UTC | #3
On Wed, Jan 29, 2020 at 11:57:00AM -0300, Fabio Estevam wrote:
> On Wed, Jan 29, 2020 at 11:55 AM Tom Rini <trini at konsulko.com> wrote:
> 
> > OK, why is the fix to do what you did here and NOT enable
> > CONFIG_SPL_GPIO on the platform(s) in question to match the rest of
> 
> Because mx7dsabresd_defconfig does not support SPL.

Ah.  But it is DM_GPIO?  I think looking at bcee8d6764f9 there's a few
other i.MX boards that are also broken something needs to be moved to
Kconfig / updated still.

In fact, uh, there's a good bit of confusion around that line in the
Makefile.  The test on 'CONFIG_$(SPL_TPL_)GPIO' will never be true as
there is only 'DM_GPIO' (and SPL/TPL variants) and
'SPL/TPL_GPIO_SUPPORT' and no 'GPIO' itself.  So everyone using
74x164_gpio.o is broken right now.

Do you want to take a pass at trying to unpack and fix this mess or
should I?  Thanks again!
Fabio Estevam Jan. 29, 2020, 3:59 p.m. UTC | #4
On Wed, Jan 29, 2020 at 12:35 PM Tom Rini <trini at konsulko.com> wrote:

> Ah.  But it is DM_GPIO?  I think looking at bcee8d6764f9 there's a few

Yes, CONFIG_DM_GPIO is selected in mx7dsabresd_defconfig.

> other i.MX boards that are also broken something needs to be moved to
> Kconfig / updated still.

I can submit a patch moving CONFIG_DM_74X164 from
include/confiinclude/configs/mx6ul_14x14_evk.h to defconfig.

> In fact, uh, there's a good bit of confusion around that line in the
> Makefile.  The test on 'CONFIG_$(SPL_TPL_)GPIO' will never be true as
> there is only 'DM_GPIO' (and SPL/TPL variants) and
> 'SPL/TPL_GPIO_SUPPORT' and no 'GPIO' itself.  So everyone using
> 74x164_gpio.o is broken right now.

Yes, that's the reason I came up with this patch.

> Do you want to take a pass at trying to unpack and fix this mess or
> should I?  Thanks again!

I am glad to fix this, but what exactly needs to be done?

This patch plus another one that moves CONFIG_DM_74X164 from
include/confiinclude/configs/mx6ul_14x14_evk.h to defconfig is all we
need, no?

Please clarify.
Tom Rini Jan. 29, 2020, 4:10 p.m. UTC | #5
On Wed, Jan 29, 2020 at 12:59:28PM -0300, Fabio Estevam wrote:
> On Wed, Jan 29, 2020 at 12:35 PM Tom Rini <trini at konsulko.com> wrote:
> 
> > Ah.  But it is DM_GPIO?  I think looking at bcee8d6764f9 there's a few
> 
> Yes, CONFIG_DM_GPIO is selected in mx7dsabresd_defconfig.

OK, good.

> 
> > other i.MX boards that are also broken something needs to be moved to
> > Kconfig / updated still.
> 
> I can submit a patch moving CONFIG_DM_74X164 from
> include/confiinclude/configs/mx6ul_14x14_evk.h to defconfig.

OK, please do.

> > In fact, uh, there's a good bit of confusion around that line in the
> > Makefile.  The test on 'CONFIG_$(SPL_TPL_)GPIO' will never be true as
> > there is only 'DM_GPIO' (and SPL/TPL variants) and
> > 'SPL/TPL_GPIO_SUPPORT' and no 'GPIO' itself.  So everyone using
> > 74x164_gpio.o is broken right now.
> 
> Yes, that's the reason I came up with this patch.
> 
> > Do you want to take a pass at trying to unpack and fix this mess or
> > should I?  Thanks again!
> 
> I am glad to fix this, but what exactly needs to be done?
> 
> This patch plus another one that moves CONFIG_DM_74X164 from
> include/confiinclude/configs/mx6ul_14x14_evk.h to defconfig is all we
> need, no?

Lets see.  Yup, talking this all out as gotten me to seeing that yes,
your patch is correct and all we need in the Makefile.  Thanks!

Reviewed-by: Tom Rini <trini at konsulko.com>
Fabio Estevam Jan. 29, 2020, 5 p.m. UTC | #6
Hi Tom,

On Wed, Jan 29, 2020 at 1:10 PM Tom Rini <trini at konsulko.com> wrote:

> Lets see.  Yup, talking this all out as gotten me to seeing that yes,
> your patch is correct and all we need in the Makefile.  Thanks!
>
> Reviewed-by: Tom Rini <trini at konsulko.com>

I have just sent v2 that places it inside the ifndef CONFIG_SPL_BUILD block.

This way we don't have issues on SPL targets, such as mx6ul_14x14_evk_defconfig.

Thanks
Joris Offouga Jan. 29, 2020, 7:21 p.m. UTC | #7
Hi Fabio,

Le 29/01/2020 à 17:10, Tom Rini a écrit :
> On Wed, Jan 29, 2020 at 12:59:28PM -0300, Fabio Estevam wrote:
>> On Wed, Jan 29, 2020 at 12:35 PM Tom Rini <trini at konsulko.com> wrote:
>>
>>> Ah.  But it is DM_GPIO?  I think looking at bcee8d6764f9 there's a few
>> Yes, CONFIG_DM_GPIO is selected in mx7dsabresd_defconfig.
> OK, good.
>
>>> other i.MX boards that are also broken something needs to be moved to
>>> Kconfig / updated still.
>> I can submit a patch moving CONFIG_DM_74X164 from
>> include/confiinclude/configs/mx6ul_14x14_evk.h to defconfig.
> OK, please do.
>
>>> In fact, uh, there's a good bit of confusion around that line in the
>>> Makefile.  The test on 'CONFIG_$(SPL_TPL_)GPIO' will never be true as
>>> there is only 'DM_GPIO' (and SPL/TPL variants) and
>>> 'SPL/TPL_GPIO_SUPPORT' and no 'GPIO' itself.  So everyone using
>>> 74x164_gpio.o is broken right now.
>> Yes, that's the reason I came up with this patch.
>>
>>> Do you want to take a pass at trying to unpack and fix this mess or
>>> should I?  Thanks again!
>> I am glad to fix this, but what exactly needs to be done?
>>
>> This patch plus another one that moves CONFIG_DM_74X164 from
>> include/confiinclude/configs/mx6ul_14x14_evk.h to defconfig is all we
>> need, no?
> Lets see.  Yup, talking this all out as gotten me to seeing that yes,
> your patch is correct and all we need in the Makefile.  Thanks!
>
> Reviewed-by: Tom Rini <trini at konsulko.com>
>
It works on my side :

U-Boot 2020.04-rc1-00849-ge7ab1cb3f0-dirty (Jan 29 2020 - 20:15:47 +0100)

CPU:   Freescale i.MX7D rev1.2 1000 MHz (running at 792 MHz)
CPU:   Commercial temperature grade (0C to 95C) at 32C
Reset cause: POR
Model: Freescale i.MX7 SabreSD Board
Board: i.MX7D SABRESD in secure mode
DRAM:  1 GiB
PMIC: PFUZE3000 DEV_ID=0x30 REV_ID=0x11
MMC:   FSL_SDHC: 0, FSL_SDHC: 1, FSL_SDHC: 2
Loading Environment from MMC... OK
Video: 480x272x24
In:    serial
Out:   serial
Err:   serial
Net:
Warning: ethernet at 30be0000 using MAC address from ROM
eth0: ethernet at 30be0000
Hit any key to stop autoboot:  0

thanks

Tested-by: Joris Offouga <offougajoris at gmail.com>

Best regards,

Joris
diff mbox series

Patch

diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 449046b64c..aa41a24cd0 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -10,13 +10,11 @@  endif
 obj-$(CONFIG_$(SPL_TPL_)DM_GPIO) += gpio-uclass.o
 
 obj-$(CONFIG_$(SPL_)DM_PCA953X)	+= pca953x_gpio.o
-ifdef CONFIG_$(SPL_TPL_)GPIO
-obj-$(CONFIG_DM_74X164)		+= 74x164_gpio.o
-endif
 
 obj-$(CONFIG_AT91_GPIO)	+= at91_gpio.o
 obj-$(CONFIG_ATMEL_PIO4)	+= atmel_pio4.o
 obj-$(CONFIG_BCM6345_GPIO)	+= bcm6345_gpio.o
+obj-$(CONFIG_DM_74X164)		+= 74x164_gpio.o
 obj-$(CONFIG_INTEL_GPIO)	+= intel_gpio.o
 obj-$(CONFIG_INTEL_ICH6_GPIO)	+= intel_ich6_gpio.o
 obj-$(CONFIG_INTEL_BROADWELL_GPIO)	+= intel_broadwell_gpio.o