diff mbox series

[v4,12/14] arm: dts: include gpio nodes for card detect

Message ID 20200625041017.26204-13-walter.lozano@collabora.com
State Accepted
Commit 407009a426f2f31d3af81d02e0109437b1155995
Headers show
Series improve OF_PLATDATA support | expand

Commit Message

Walter Lozano June 25, 2020, 4:10 a.m. UTC 
Several MMC drivers use GPIO for card detection with cd-gpios property in
the MMC node pointing to a GPIO node. However, as U-Boot tries to save
space by keeping only required nodes using u-boot* properties, several
devices tree result in having only in the MMC node but not the GPIO node
associated to cd-gpios.

This patch, fixes several ocurrence of this issue.

Signed-off-by: Walter Lozano <walter.lozano at collabora.com>
---

 arch/arm/dts/da850-evm-u-boot.dtsi            |  4 ++++
 arch/arm/dts/da850-lcdk-u-boot.dtsi           |  4 ++++
 arch/arm/dts/rk3288-u-boot.dtsi               |  4 ++++
 arch/arm/dts/rk3288-veyron-speedy-u-boot.dtsi |  2 +-
 arch/arm/dts/rk3288-veyron-u-boot.dtsi        | 11 +++++++++++
 5 files changed, 24 insertions(+), 1 deletion(-)
 create mode 100644 arch/arm/dts/rk3288-veyron-u-boot.dtsi

Comments

Simon Glass June 26, 2020, 1:43 a.m. UTC | #1 
On Wed, 24 Jun 2020 at 22:11, Walter Lozano <walter.lozano at collabora.com> wrote:
>
> Several MMC drivers use GPIO for card detection with cd-gpios property in
> the MMC node pointing to a GPIO node. However, as U-Boot tries to save
> space by keeping only required nodes using u-boot* properties, several
> devices tree result in having only in the MMC node but not the GPIO node
> associated to cd-gpios.
>
> This patch, fixes several ocurrence of this issue.
>
> Signed-off-by: Walter Lozano <walter.lozano at collabora.com>
> ---
>
>  arch/arm/dts/da850-evm-u-boot.dtsi            |  4 ++++
>  arch/arm/dts/da850-lcdk-u-boot.dtsi           |  4 ++++
>  arch/arm/dts/rk3288-u-boot.dtsi               |  4 ++++
>  arch/arm/dts/rk3288-veyron-speedy-u-boot.dtsi |  2 +-
>  arch/arm/dts/rk3288-veyron-u-boot.dtsi        | 11 +++++++++++
>  5 files changed, 24 insertions(+), 1 deletion(-)
>  create mode 100644 arch/arm/dts/rk3288-veyron-u-boot.dtsi

Reviewed-by: Simon Glass <sjg at chromium.org>
Adam Ford June 26, 2020, 4:37 a.m. UTC | #2 
On Wed, Jun 24, 2020 at 11:11 PM Walter Lozano
<walter.lozano at collabora.com> wrote:
>
> Several MMC drivers use GPIO for card detection with cd-gpios property in
> the MMC node pointing to a GPIO node. However, as U-Boot tries to save
> space by keeping only required nodes using u-boot* properties, several
> devices tree result in having only in the MMC node but not the GPIO node
> associated to cd-gpios.
>
> This patch, fixes several ocurrence of this issue.
>
> Signed-off-by: Walter Lozano <walter.lozano at collabora.com>
> ---
>
>  arch/arm/dts/da850-evm-u-boot.dtsi            |  4 ++++
>  arch/arm/dts/da850-lcdk-u-boot.dtsi           |  4 ++++
>  arch/arm/dts/rk3288-u-boot.dtsi               |  4 ++++
>  arch/arm/dts/rk3288-veyron-speedy-u-boot.dtsi |  2 +-
>  arch/arm/dts/rk3288-veyron-u-boot.dtsi        | 11 +++++++++++
>  5 files changed, 24 insertions(+), 1 deletion(-)
>  create mode 100644 arch/arm/dts/rk3288-veyron-u-boot.dtsi
>
> diff --git a/arch/arm/dts/da850-evm-u-boot.dtsi b/arch/arm/dts/da850-evm-u-boot.dtsi
> index d9afc5edf4..d588628641 100644
> --- a/arch/arm/dts/da850-evm-u-boot.dtsi
> +++ b/arch/arm/dts/da850-evm-u-boot.dtsi
> @@ -39,3 +39,7 @@
>  &spi1 {
>         u-boot,dm-spl;
>  };
> +
> +&gpio {
> +       u-boot,dm-spl;
> +};

I don't know that this is needed for the da850-evm since it doesn't
boot from sd/mmc.  It can boot from SPI, NAND or NOR Flash depending
on the config option selected, but none of them need the gpio during
SPL.  The gpio is loaded during normal U-Boot.  I will try to run some
tests to make sure it still boots in the next few days.  I know space
is getting tight in SPL.

> diff --git a/arch/arm/dts/da850-lcdk-u-boot.dtsi b/arch/arm/dts/da850-lcdk-u-boot.dtsi
> index b372d06ca9..d50775c173 100644
> --- a/arch/arm/dts/da850-lcdk-u-boot.dtsi
> +++ b/arch/arm/dts/da850-lcdk-u-boot.dtsi
> @@ -28,3 +28,7 @@
>  &serial2 {
>         u-boot,dm-spl;
>  };
> +
> +&gpio {
> +       u-boot,dm-spl;
> +};
> diff --git a/arch/arm/dts/rk3288-u-boot.dtsi b/arch/arm/dts/rk3288-u-boot.dtsi
> index 6d31735362..51b6e018bd 100644
> --- a/arch/arm/dts/rk3288-u-boot.dtsi
> +++ b/arch/arm/dts/rk3288-u-boot.dtsi
> @@ -43,3 +43,7 @@
>  &noc {
>         u-boot,dm-pre-reloc;
>  };
> +
> +&gpio7 {
> +       u-boot,dm-pre-reloc;
> +};
> diff --git a/arch/arm/dts/rk3288-veyron-speedy-u-boot.dtsi b/arch/arm/dts/rk3288-veyron-speedy-u-boot.dtsi
> index eccc069368..251fbdee71 100644
> --- a/arch/arm/dts/rk3288-veyron-speedy-u-boot.dtsi
> +++ b/arch/arm/dts/rk3288-veyron-speedy-u-boot.dtsi
> @@ -3,7 +3,7 @@
>   * Copyright 2015 Google, Inc
>   */
>
> -#include "rk3288-u-boot.dtsi"
> +#include "rk3288-veyron-u-boot.dtsi"
>
>  &dmc {
>         rockchip,pctl-timing = <0x215 0xc8 0x0 0x35 0x26 0x2 0x70 0x2000d
> diff --git a/arch/arm/dts/rk3288-veyron-u-boot.dtsi b/arch/arm/dts/rk3288-veyron-u-boot.dtsi
> new file mode 100644
> index 0000000000..899fe6e7a0
> --- /dev/null
> +++ b/arch/arm/dts/rk3288-veyron-u-boot.dtsi
> @@ -0,0 +1,11 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Copyright 2015 Google, Inc
> + */
> +
> +#include "rk3288-u-boot.dtsi"
> +
> +&gpio7 {
> +       u-boot,dm-pre-reloc;
> +};
> +
> --
> 2.20.1
>
Adam Ford June 26, 2020, 12:26 p.m. UTC | #3 
On Thu, Jun 25, 2020 at 11:37 PM Adam Ford <aford173 at gmail.com> wrote:
>
> On Wed, Jun 24, 2020 at 11:11 PM Walter Lozano
> <walter.lozano at collabora.com> wrote:
> >
> > Several MMC drivers use GPIO for card detection with cd-gpios property in
> > the MMC node pointing to a GPIO node. However, as U-Boot tries to save
> > space by keeping only required nodes using u-boot* properties, several
> > devices tree result in having only in the MMC node but not the GPIO node
> > associated to cd-gpios.
> >
> > This patch, fixes several ocurrence of this issue.
> >
> > Signed-off-by: Walter Lozano <walter.lozano at collabora.com>
> > ---
> >
> >  arch/arm/dts/da850-evm-u-boot.dtsi            |  4 ++++
> >  arch/arm/dts/da850-lcdk-u-boot.dtsi           |  4 ++++
> >  arch/arm/dts/rk3288-u-boot.dtsi               |  4 ++++
> >  arch/arm/dts/rk3288-veyron-speedy-u-boot.dtsi |  2 +-
> >  arch/arm/dts/rk3288-veyron-u-boot.dtsi        | 11 +++++++++++
> >  5 files changed, 24 insertions(+), 1 deletion(-)
> >  create mode 100644 arch/arm/dts/rk3288-veyron-u-boot.dtsi
> >
> > diff --git a/arch/arm/dts/da850-evm-u-boot.dtsi b/arch/arm/dts/da850-evm-u-boot.dtsi
> > index d9afc5edf4..d588628641 100644
> > --- a/arch/arm/dts/da850-evm-u-boot.dtsi
> > +++ b/arch/arm/dts/da850-evm-u-boot.dtsi
> > @@ -39,3 +39,7 @@
> >  &spi1 {
> >         u-boot,dm-spl;
> >  };
> > +
> > +&gpio {
> > +       u-boot,dm-spl;
> > +};
>
> I don't know that this is needed for the da850-evm since it doesn't
> boot from sd/mmc.  It can boot from SPI, NAND or NOR Flash depending
> on the config option selected, but none of them need the gpio during
> SPL.  The gpio is loaded during normal U-Boot.  I will try to run some
> tests to make sure it still boots in the next few days.  I know space
> is getting tight in SPL.

I applied your patches and built.
FYI, "git am" didn't let them apply nicely, but there could be some
missing dependent patches I was missing.  I was able to patch with
"patch"
The board booted as expected.

I examined the generated dtb for SPL since this board doesn't use
OF_PLATDATA.  It looks like we could remove both the GPIO and the MMC
modes from SPL, but I'm not going to worry about it unless we can't
boot any more.  If/when that happens, I'll spend more time trying to
free up space in SPL.
For now,, for the series...

Tested-by: Adam Ford <aford173 at gmail.com> #da850-evm

adam
>
> > diff --git a/arch/arm/dts/da850-lcdk-u-boot.dtsi b/arch/arm/dts/da850-lcdk-u-boot.dtsi
> > index b372d06ca9..d50775c173 100644
> > --- a/arch/arm/dts/da850-lcdk-u-boot.dtsi
> > +++ b/arch/arm/dts/da850-lcdk-u-boot.dtsi
> > @@ -28,3 +28,7 @@
> >  &serial2 {
> >         u-boot,dm-spl;
> >  };
> > +
> > +&gpio {
> > +       u-boot,dm-spl;
> > +};
> > diff --git a/arch/arm/dts/rk3288-u-boot.dtsi b/arch/arm/dts/rk3288-u-boot.dtsi
> > index 6d31735362..51b6e018bd 100644
> > --- a/arch/arm/dts/rk3288-u-boot.dtsi
> > +++ b/arch/arm/dts/rk3288-u-boot.dtsi
> > @@ -43,3 +43,7 @@
> >  &noc {
> >         u-boot,dm-pre-reloc;
> >  };
> > +
> > +&gpio7 {
> > +       u-boot,dm-pre-reloc;
> > +};
> > diff --git a/arch/arm/dts/rk3288-veyron-speedy-u-boot.dtsi b/arch/arm/dts/rk3288-veyron-speedy-u-boot.dtsi
> > index eccc069368..251fbdee71 100644
> > --- a/arch/arm/dts/rk3288-veyron-speedy-u-boot.dtsi
> > +++ b/arch/arm/dts/rk3288-veyron-speedy-u-boot.dtsi
> > @@ -3,7 +3,7 @@
> >   * Copyright 2015 Google, Inc
> >   */
> >
> > -#include "rk3288-u-boot.dtsi"
> > +#include "rk3288-veyron-u-boot.dtsi"
> >
> >  &dmc {
> >         rockchip,pctl-timing = <0x215 0xc8 0x0 0x35 0x26 0x2 0x70 0x2000d
> > diff --git a/arch/arm/dts/rk3288-veyron-u-boot.dtsi b/arch/arm/dts/rk3288-veyron-u-boot.dtsi
> > new file mode 100644
> > index 0000000000..899fe6e7a0
> > --- /dev/null
> > +++ b/arch/arm/dts/rk3288-veyron-u-boot.dtsi
> > @@ -0,0 +1,11 @@
> > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> > +/*
> > + * Copyright 2015 Google, Inc
> > + */
> > +
> > +#include "rk3288-u-boot.dtsi"
> > +
> > +&gpio7 {
> > +       u-boot,dm-pre-reloc;
> > +};
> > +
> > --
> > 2.20.1
> >
Walter Lozano June 26, 2020, 1:11 p.m. UTC | #4 
Hi Adam,

On 26/6/20 09:26, Adam Ford wrote:
> On Thu, Jun 25, 2020 at 11:37 PM Adam Ford <aford173 at gmail.com> wrote:
>> On Wed, Jun 24, 2020 at 11:11 PM Walter Lozano
>> <walter.lozano at collabora.com> wrote:
>>> Several MMC drivers use GPIO for card detection with cd-gpios property in
>>> the MMC node pointing to a GPIO node. However, as U-Boot tries to save
>>> space by keeping only required nodes using u-boot* properties, several
>>> devices tree result in having only in the MMC node but not the GPIO node
>>> associated to cd-gpios.
>>>
>>> This patch, fixes several ocurrence of this issue.
>>>
>>> Signed-off-by: Walter Lozano <walter.lozano at collabora.com>
>>> ---
>>>
>>>   arch/arm/dts/da850-evm-u-boot.dtsi            |  4 ++++
>>>   arch/arm/dts/da850-lcdk-u-boot.dtsi           |  4 ++++
>>>   arch/arm/dts/rk3288-u-boot.dtsi               |  4 ++++
>>>   arch/arm/dts/rk3288-veyron-speedy-u-boot.dtsi |  2 +-
>>>   arch/arm/dts/rk3288-veyron-u-boot.dtsi        | 11 +++++++++++
>>>   5 files changed, 24 insertions(+), 1 deletion(-)
>>>   create mode 100644 arch/arm/dts/rk3288-veyron-u-boot.dtsi
>>>
>>> diff --git a/arch/arm/dts/da850-evm-u-boot.dtsi b/arch/arm/dts/da850-evm-u-boot.dtsi
>>> index d9afc5edf4..d588628641 100644
>>> --- a/arch/arm/dts/da850-evm-u-boot.dtsi
>>> +++ b/arch/arm/dts/da850-evm-u-boot.dtsi
>>> @@ -39,3 +39,7 @@
>>>   &spi1 {
>>>          u-boot,dm-spl;
>>>   };
>>> +
>>> +&gpio {
>>> +       u-boot,dm-spl;
>>> +};
>> I don't know that this is needed for the da850-evm since it doesn't
>> boot from sd/mmc.  It can boot from SPI, NAND or NOR Flash depending
>> on the config option selected, but none of them need the gpio during
>> SPL.  The gpio is loaded during normal U-Boot.  I will try to run some
>> tests to make sure it still boots in the next few days.  I know space
>> is getting tight in SPL.
> I applied your patches and built.
> FYI, "git am" didn't let them apply nicely, but there could be some
> missing dependent patches I was missing.  I was able to patch with
> "patch"
> The board booted as expected.


Thanks for your feedback and testing.


> I examined the generated dtb for SPL since this board doesn't use
> OF_PLATDATA.  It looks like we could remove both the GPIO and the MMC
> modes from SPL, but I'm not going to worry about it unless we can't
> boot any more.  If/when that happens, I'll spend more time trying to
> free up space in SPL.
> For now,, for the series...
>
> Tested-by: Adam Ford <aford173 at gmail.com> #da850-evm
>   

I found this possible issue while testing different configurations, but 
it is more related to omapl138_lcdk_defconfig (da850-lcdk) which seems 
to use OF_PLATDATA. In the case of da850-evm, if there are issue with 
space in SPL, maybe we can consider removing both MMC and GPIO node and 
also enable OF_PLATDATA, hopefully if omapl138_lcdk_defconfig works as 
expected the effort would be little.

Regards,

Walter

>>> diff --git a/arch/arm/dts/da850-lcdk-u-boot.dtsi b/arch/arm/dts/da850-lcdk-u-boot.dtsi
>>> index b372d06ca9..d50775c173 100644
>>> --- a/arch/arm/dts/da850-lcdk-u-boot.dtsi
>>> +++ b/arch/arm/dts/da850-lcdk-u-boot.dtsi
>>> @@ -28,3 +28,7 @@
>>>   &serial2 {
>>>          u-boot,dm-spl;
>>>   };
>>> +
>>> +&gpio {
>>> +       u-boot,dm-spl;
>>> +};
>>> diff --git a/arch/arm/dts/rk3288-u-boot.dtsi b/arch/arm/dts/rk3288-u-boot.dtsi
>>> index 6d31735362..51b6e018bd 100644
>>> --- a/arch/arm/dts/rk3288-u-boot.dtsi
>>> +++ b/arch/arm/dts/rk3288-u-boot.dtsi
>>> @@ -43,3 +43,7 @@
>>>   &noc {
>>>          u-boot,dm-pre-reloc;
>>>   };
>>> +
>>> +&gpio7 {
>>> +       u-boot,dm-pre-reloc;
>>> +};
>>> diff --git a/arch/arm/dts/rk3288-veyron-speedy-u-boot.dtsi b/arch/arm/dts/rk3288-veyron-speedy-u-boot.dtsi
>>> index eccc069368..251fbdee71 100644
>>> --- a/arch/arm/dts/rk3288-veyron-speedy-u-boot.dtsi
>>> +++ b/arch/arm/dts/rk3288-veyron-speedy-u-boot.dtsi
>>> @@ -3,7 +3,7 @@
>>>    * Copyright 2015 Google, Inc
>>>    */
>>>
>>> -#include "rk3288-u-boot.dtsi"
>>> +#include "rk3288-veyron-u-boot.dtsi"
>>>
>>>   &dmc {
>>>          rockchip,pctl-timing = <0x215 0xc8 0x0 0x35 0x26 0x2 0x70 0x2000d
>>> diff --git a/arch/arm/dts/rk3288-veyron-u-boot.dtsi b/arch/arm/dts/rk3288-veyron-u-boot.dtsi
>>> new file mode 100644
>>> index 0000000000..899fe6e7a0
>>> --- /dev/null
>>> +++ b/arch/arm/dts/rk3288-veyron-u-boot.dtsi
>>> @@ -0,0 +1,11 @@
>>> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
>>> +/*
>>> + * Copyright 2015 Google, Inc
>>> + */
>>> +
>>> +#include "rk3288-u-boot.dtsi"
>>> +
>>> +&gpio7 {
>>> +       u-boot,dm-pre-reloc;
>>> +};
>>> +
>>> --
>>> 2.20.1
>>>
diff mbox series

Patch

diff --git a/arch/arm/dts/da850-evm-u-boot.dtsi b/arch/arm/dts/da850-evm-u-boot.dtsi
index d9afc5edf4..d588628641 100644
--- a/arch/arm/dts/da850-evm-u-boot.dtsi
+++ b/arch/arm/dts/da850-evm-u-boot.dtsi
@@ -39,3 +39,7 @@ 
 &spi1 {
 	u-boot,dm-spl;
 };
+
+&gpio {
+	u-boot,dm-spl;
+};
diff --git a/arch/arm/dts/da850-lcdk-u-boot.dtsi b/arch/arm/dts/da850-lcdk-u-boot.dtsi
index b372d06ca9..d50775c173 100644
--- a/arch/arm/dts/da850-lcdk-u-boot.dtsi
+++ b/arch/arm/dts/da850-lcdk-u-boot.dtsi
@@ -28,3 +28,7 @@ 
 &serial2 {
 	u-boot,dm-spl;
 };
+
+&gpio {
+	u-boot,dm-spl;
+};
diff --git a/arch/arm/dts/rk3288-u-boot.dtsi b/arch/arm/dts/rk3288-u-boot.dtsi
index 6d31735362..51b6e018bd 100644
--- a/arch/arm/dts/rk3288-u-boot.dtsi
+++ b/arch/arm/dts/rk3288-u-boot.dtsi
@@ -43,3 +43,7 @@ 
 &noc {
 	u-boot,dm-pre-reloc;
 };
+
+&gpio7 {
+	u-boot,dm-pre-reloc;
+};
diff --git a/arch/arm/dts/rk3288-veyron-speedy-u-boot.dtsi b/arch/arm/dts/rk3288-veyron-speedy-u-boot.dtsi
index eccc069368..251fbdee71 100644
--- a/arch/arm/dts/rk3288-veyron-speedy-u-boot.dtsi
+++ b/arch/arm/dts/rk3288-veyron-speedy-u-boot.dtsi
@@ -3,7 +3,7 @@ 
  * Copyright 2015 Google, Inc
  */
 
-#include "rk3288-u-boot.dtsi"
+#include "rk3288-veyron-u-boot.dtsi"
 
 &dmc {
 	rockchip,pctl-timing = <0x215 0xc8 0x0 0x35 0x26 0x2 0x70 0x2000d
diff --git a/arch/arm/dts/rk3288-veyron-u-boot.dtsi b/arch/arm/dts/rk3288-veyron-u-boot.dtsi
new file mode 100644
index 0000000000..899fe6e7a0
--- /dev/null
+++ b/arch/arm/dts/rk3288-veyron-u-boot.dtsi
@@ -0,0 +1,11 @@ 
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Copyright 2015 Google, Inc
+ */
+
+#include "rk3288-u-boot.dtsi"
+
+&gpio7 {
+	u-boot,dm-pre-reloc;
+};
+