| Message ID | 437a2ed0a95b1d52c5d0a8aed2ab765307e1cb7d.1572774759.git.baruch@tkos.co.il |
|---|---|
| State | Deferred |
| Headers | show |
| Series | [U-Boot,1/2] gpio: fix CONFIG_IS_ENABLED(DM_GPIO) for SPL | expand |
Hi Baruch, On 03.11.19 10:52, Baruch Siach wrote: > There is currently no CONFIG_SPL_DM_GPIO, so CONFIG_IS_ENABLED(DM_GPIO) > is always false. As a result the sdhci driver can not use the DM gpio > API to read the card-detect signal in SPL. This breaks boot from SD card > on the SolidRun Clearfog platform since commit da18c62b6e ("mmc: sdhci: > Implement SDHCI card detect") that added sdhci_get_cd(). > > Add a blind CONFIG_SPL_DM_GPIO symbol that is enabled iff CONFIG_DM_GPIO > is. That makes CONFIG_IS_ENABLED(DM_GPIO) correct for SPL code. > > Signed-off-by: Baruch Siach <baruch@tkos.co.il> > --- > drivers/gpio/Kconfig | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig > index c1ad5d64a35c..9bac341c5ed5 100644 > --- a/drivers/gpio/Kconfig > +++ b/drivers/gpio/Kconfig > @@ -14,6 +14,11 @@ config DM_GPIO > particular GPIOs that they provide. The uclass interface > is defined in include/asm-generic/gpio.h. > > +config SPL_DM_GPIO > + bool > + depends on DM_GPIO > + default y > + > config GPIO_HOG > bool "Enable GPIO hog support" > depends on DM_GPIO > I think its preferred to disable such SPL configurations per default to not increase the code size here - which is pretty crucial on some boards. Better to enable this SPL_DM_GPIO on your target or if it makes sense on your platform (MVEBU perhaps with a select or imply via the MMC driver in Kconfig). What do you think? Thanks, Stefan
Hi Stefan, On Tue, Nov 05, 2019 at 04:14:59PM +0100, Stefan Roese wrote: > On 03.11.19 10:52, Baruch Siach wrote: > > There is currently no CONFIG_SPL_DM_GPIO, so CONFIG_IS_ENABLED(DM_GPIO) > > is always false. As a result the sdhci driver can not use the DM gpio > > API to read the card-detect signal in SPL. This breaks boot from SD card > > on the SolidRun Clearfog platform since commit da18c62b6e ("mmc: sdhci: > > Implement SDHCI card detect") that added sdhci_get_cd(). > > > > Add a blind CONFIG_SPL_DM_GPIO symbol that is enabled iff CONFIG_DM_GPIO > > is. That makes CONFIG_IS_ENABLED(DM_GPIO) correct for SPL code. > > > > Signed-off-by: Baruch Siach <baruch@tkos.co.il> > > --- > > drivers/gpio/Kconfig | 5 +++++ > > 1 file changed, 5 insertions(+) > > > > diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig > > index c1ad5d64a35c..9bac341c5ed5 100644 > > --- a/drivers/gpio/Kconfig > > +++ b/drivers/gpio/Kconfig > > @@ -14,6 +14,11 @@ config DM_GPIO > > particular GPIOs that they provide. The uclass interface > > is defined in include/asm-generic/gpio.h. > > +config SPL_DM_GPIO > > + bool > > + depends on DM_GPIO > > + default y > > + > > config GPIO_HOG > > bool "Enable GPIO hog support" > > depends on DM_GPIO > > I think its preferred to disable such SPL configurations per default > to not increase the code size here - which is pretty crucial on some > boards. Better to enable this SPL_DM_GPIO on your target or if it > makes sense on your platform (MVEBU perhaps with a select or imply > via the MMC driver in Kconfig). > > What do you think? Makes sense to me. Should SPL_DM_GPIO be a blind symbol, or user selectable? Would it make sense to add select SPL_DM_GPIO if (SPL && DM_GPIO) to CONFIG_MMC_SDHCI_MV? baruch
Hi Baruch, On 05.11.19 17:01, Baruch Siach wrote: > Hi Stefan, > > On Tue, Nov 05, 2019 at 04:14:59PM +0100, Stefan Roese wrote: >> On 03.11.19 10:52, Baruch Siach wrote: >>> There is currently no CONFIG_SPL_DM_GPIO, so CONFIG_IS_ENABLED(DM_GPIO) >>> is always false. As a result the sdhci driver can not use the DM gpio >>> API to read the card-detect signal in SPL. This breaks boot from SD card >>> on the SolidRun Clearfog platform since commit da18c62b6e ("mmc: sdhci: >>> Implement SDHCI card detect") that added sdhci_get_cd(). >>> >>> Add a blind CONFIG_SPL_DM_GPIO symbol that is enabled iff CONFIG_DM_GPIO >>> is. That makes CONFIG_IS_ENABLED(DM_GPIO) correct for SPL code. >>> >>> Signed-off-by: Baruch Siach <baruch@tkos.co.il> >>> --- >>> drivers/gpio/Kconfig | 5 +++++ >>> 1 file changed, 5 insertions(+) >>> >>> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig >>> index c1ad5d64a35c..9bac341c5ed5 100644 >>> --- a/drivers/gpio/Kconfig >>> +++ b/drivers/gpio/Kconfig >>> @@ -14,6 +14,11 @@ config DM_GPIO >>> particular GPIOs that they provide. The uclass interface >>> is defined in include/asm-generic/gpio.h. >>> +config SPL_DM_GPIO >>> + bool >>> + depends on DM_GPIO >>> + default y >>> + >>> config GPIO_HOG >>> bool "Enable GPIO hog support" >>> depends on DM_GPIO >> >> I think its preferred to disable such SPL configurations per default >> to not increase the code size here - which is pretty crucial on some >> boards. Better to enable this SPL_DM_GPIO on your target or if it >> makes sense on your platform (MVEBU perhaps with a select or imply >> via the MMC driver in Kconfig). >> >> What do you think? > > Makes sense to me. > > Should SPL_DM_GPIO be a blind symbol, or user selectable? What are the other comparable SPL_DM_foo symbols? My first guess is that they are user selectable. > Would it make sense to add > > select SPL_DM_GPIO if (SPL && DM_GPIO) > > to CONFIG_MMC_SDHCI_MV? Yes, I had something like this on my mind. Please give it a try and submit v2 if it fits. Thanks, Stefan
Hi Baruch, On Sun, Nov 3, 2019 at 5:53 PM Baruch Siach <baruch@tkos.co.il> wrote: > > There is currently no CONFIG_SPL_DM_GPIO, so CONFIG_IS_ENABLED(DM_GPIO) > is always false. As a result the sdhci driver can not use the DM gpio > API to read the card-detect signal in SPL. This breaks boot from SD card > on the SolidRun Clearfog platform since commit da18c62b6e ("mmc: sdhci: > Implement SDHCI card detect") that added sdhci_get_cd(). > > Add a blind CONFIG_SPL_DM_GPIO symbol that is enabled iff CONFIG_DM_GPIO > is. That makes CONFIG_IS_ENABLED(DM_GPIO) correct for SPL code. > > Signed-off-by: Baruch Siach <baruch@tkos.co.il> > --- > drivers/gpio/Kconfig | 5 +++++ > 1 file changed, 5 insertions(+) > Simon has a patch already for this, see http://patchwork.ozlabs.org/patch/1180459/ Regards, Bin
Hi Bin, On 06.11.19 06:47, Bin Meng wrote: > Hi Baruch, > > On Sun, Nov 3, 2019 at 5:53 PM Baruch Siach <baruch@tkos.co.il> wrote: >> >> There is currently no CONFIG_SPL_DM_GPIO, so CONFIG_IS_ENABLED(DM_GPIO) >> is always false. As a result the sdhci driver can not use the DM gpio >> API to read the card-detect signal in SPL. This breaks boot from SD card >> on the SolidRun Clearfog platform since commit da18c62b6e ("mmc: sdhci: >> Implement SDHCI card detect") that added sdhci_get_cd(). >> >> Add a blind CONFIG_SPL_DM_GPIO symbol that is enabled iff CONFIG_DM_GPIO >> is. That makes CONFIG_IS_ENABLED(DM_GPIO) correct for SPL code. >> >> Signed-off-by: Baruch Siach <baruch@tkos.co.il> >> --- >> drivers/gpio/Kconfig | 5 +++++ >> 1 file changed, 5 insertions(+) >> > > Simon has a patch already for this, see > http://patchwork.ozlabs.org/patch/1180459/ Thanks for the link. IIUTC, then the patch from Simon, which also takes care of the TPL GPIO symbol, has the same potential problem that Baruch's patch has. Enabling SPL/TPL_DM_GPIO per default might increase the SPL/TPL image size too much. So it should probably default to 'n' and users / drivers should enable (imply or select via Kconfig) those symbols when required. What do you think? Thanks, Stefan
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig index c1ad5d64a35c..9bac341c5ed5 100644 --- a/drivers/gpio/Kconfig +++ b/drivers/gpio/Kconfig @@ -14,6 +14,11 @@ config DM_GPIO particular GPIOs that they provide. The uclass interface is defined in include/asm-generic/gpio.h. +config SPL_DM_GPIO + bool + depends on DM_GPIO + default y + config GPIO_HOG bool "Enable GPIO hog support" depends on DM_GPIO
There is currently no CONFIG_SPL_DM_GPIO, so CONFIG_IS_ENABLED(DM_GPIO) is always false. As a result the sdhci driver can not use the DM gpio API to read the card-detect signal in SPL. This breaks boot from SD card on the SolidRun Clearfog platform since commit da18c62b6e ("mmc: sdhci: Implement SDHCI card detect") that added sdhci_get_cd(). Add a blind CONFIG_SPL_DM_GPIO symbol that is enabled iff CONFIG_DM_GPIO is. That makes CONFIG_IS_ENABLED(DM_GPIO) correct for SPL code. Signed-off-by: Baruch Siach <baruch@tkos.co.il> --- drivers/gpio/Kconfig | 5 +++++ 1 file changed, 5 insertions(+)