From: lee@kernel.org (Lee Jones)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 1/3] ARM: ux500: Move GPIO regulator for SD-card into board DTSs
Date: Tue, 21 Apr 2015 08:33:36 +0100 [thread overview]
Message-ID: <20150421073336.GJ3447@x1> (raw)
In-Reply-To: <CAPDyKFrDOGBkfg48mmsbktCWFTaNV4WnTm_Fui=Kp30u1yH-tQ@mail.gmail.com>
On Tue, 21 Apr 2015, Ulf Hansson wrote:
> On 20 April 2015 at 20:26, Lee Jones <lee@kernel.org> wrote:
> > On Mon, 20 Apr 2015, Ulf Hansson wrote:
> >
> >> The GPIO regulator for the SD-card isn't a ux500 SOC configuration, but
> >> instead it's specific to the board. Move the definition of it, into the
> >> board DTSs.
> >
> > What makes you think that?
>
> Because of how it was structured today.
>
> ste-dbx5x0.dtsi - common for all ux500 boards, thus I considered this
> as the SoC configuration.
ste-dbx5x0.dtsi is common for all ux500 and ux540 boards.
> Then below are board configs which uses the above dtsi:
> ste-href.dtsi - common for href boards (used by ste-hrefprev60.dtsi
> and ste-hrefv60plus.dtsi), have vmmci
> ste-snowball.dts, have vmmci
> ste-ccu8540.dts, don't have vmmci
> ste-ccu9540.dts, don't have vmmci
Ah, got you. In which case it doesn't belong in ste-dbx5x0.dtsi.
> > We normally place the common pieces (of which there are many in this
> > node) in the highest level DTSI file, then add the platform specific
> > ones in the DTS files.
>
> Okay, so maybe it's due to the naming of ste-dbx5x0.dtsi, that I
> thought this was intended to cover the SoC configuration and not any
> of the platform specific stuff.
ste-dbx5x0.dtsi should cover all pieces which are common to all ux500
and ux540 devices. Then the lower level file ste-href-ab8500.dtsi
should cover all pieces which are common to ux500 devices and finally
the DTS files should add board specific information. Duplicate
nodes and properties are frowned upon.
> So what your advise of doing this?
So the file which covers the x500 boards is ste-href-ab8500.dtsi. I
would move the node into there instead. Keep it disabled and enable
the associated nodes in the 2 DTS files.
> >> Fixes: c94a4ab7af3f ("ARM: ux500: Disable the MMCI gpio-regulator by default")
> >> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> >> ---
> >> arch/arm/boot/dts/ste-dbx5x0.dtsi | 17 -----------------
> >> arch/arm/boot/dts/ste-href.dtsi | 17 +++++++++++++++++
> >> arch/arm/boot/dts/ste-snowball.dts | 15 +++++++++++++++
> >> 3 files changed, 32 insertions(+), 17 deletions(-)
> >>
> >> diff --git a/arch/arm/boot/dts/ste-dbx5x0.dtsi b/arch/arm/boot/dts/ste-dbx5x0.dtsi
> >> index bfd3f1c..2201cd5 100644
> >> --- a/arch/arm/boot/dts/ste-dbx5x0.dtsi
> >> +++ b/arch/arm/boot/dts/ste-dbx5x0.dtsi
> >> @@ -1017,23 +1017,6 @@
> >> status = "disabled";
> >> };
> >>
> >> - vmmci: regulator-gpio {
> >> - compatible = "regulator-gpio";
> >> -
> >> - regulator-min-microvolt = <1800000>;
> >> - regulator-max-microvolt = <2900000>;
> >> - regulator-name = "mmci-reg";
> >> - regulator-type = "voltage";
> >> -
> >> - startup-delay-us = <100>;
> >> - enable-active-high;
> >> -
> >> - states = <1800000 0x1
> >> - 2900000 0x0>;
> >> -
> >> - status = "disabled";
> >> - };
> >> -
> >> mcde at a0350000 {
> >> compatible = "stericsson,mcde";
> >> reg = <0xa0350000 0x1000>, /* MCDE */
> >> diff --git a/arch/arm/boot/dts/ste-href.dtsi b/arch/arm/boot/dts/ste-href.dtsi
> >> index bf8f0ed..8cf499a 100644
> >> --- a/arch/arm/boot/dts/ste-href.dtsi
> >> +++ b/arch/arm/boot/dts/ste-href.dtsi
> >> @@ -111,6 +111,23 @@
> >> pinctrl-1 = <&i2c3_sleep_mode>;
> >> };
> >>
> >> + vmmci: regulator-gpio {
> >> + compatible = "regulator-gpio";
> >> +
> >> + regulator-min-microvolt = <1800000>;
> >> + regulator-max-microvolt = <2900000>;
> >> + regulator-name = "mmci-reg";
> >> + regulator-type = "voltage";
> >> +
> >> + startup-delay-us = <100>;
> >> + enable-active-high;
> >> +
> >> + states = <1800000 0x1
> >> + 2900000 0x0>;
> >> +
> >> + status = "disabled";
> >> + };
> >> +
> >> // External Micro SD slot
> >> sdi0_per1 at 80126000 {
> >> arm,primecell-periphid = <0x10480180>;
> >> diff --git a/arch/arm/boot/dts/ste-snowball.dts b/arch/arm/boot/dts/ste-snowball.dts
> >> index 206826a..65a7f63 100644
> >> --- a/arch/arm/boot/dts/ste-snowball.dts
> >> +++ b/arch/arm/boot/dts/ste-snowball.dts
> >> @@ -146,8 +146,23 @@
> >> };
> >>
> >> vmmci: regulator-gpio {
> >> + compatible = "regulator-gpio";
> >> +
> >> gpios = <&gpio7 4 0x4>;
> >> enable-gpio = <&gpio6 25 0x4>;
> >> +
> >> + regulator-min-microvolt = <1800000>;
> >> + regulator-max-microvolt = <2900000>;
> >> + regulator-name = "mmci-reg";
> >> + regulator-type = "voltage";
> >> +
> >> + startup-delay-us = <100>;
> >> + enable-active-high;
> >> +
> >> + states = <1800000 0x1
> >> + 2900000 0x0>;
> >> +
> >> + status = "disabled";
> >> };
> >>
> >> // External Micro SD slot
WARNING: multiple messages have this Message-ID (diff)
From: Lee Jones <lee-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
To: Ulf Hansson <ulf.hansson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Cc: Linus Walleij
<linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>,
"linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org"
<linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org>,
"devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org"
<devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
Bjorn Andersson
<bjorn.andersson-/MT0OVThwyLZJqsBc5GL+g@public.gmane.org>
Subject: Re: [PATCH 1/3] ARM: ux500: Move GPIO regulator for SD-card into board DTSs
Date: Tue, 21 Apr 2015 08:33:36 +0100 [thread overview]
Message-ID: <20150421073336.GJ3447@x1> (raw)
In-Reply-To: <CAPDyKFrDOGBkfg48mmsbktCWFTaNV4WnTm_Fui=Kp30u1yH-tQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Tue, 21 Apr 2015, Ulf Hansson wrote:
> On 20 April 2015 at 20:26, Lee Jones <lee-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> > On Mon, 20 Apr 2015, Ulf Hansson wrote:
> >
> >> The GPIO regulator for the SD-card isn't a ux500 SOC configuration, but
> >> instead it's specific to the board. Move the definition of it, into the
> >> board DTSs.
> >
> > What makes you think that?
>
> Because of how it was structured today.
>
> ste-dbx5x0.dtsi - common for all ux500 boards, thus I considered this
> as the SoC configuration.
ste-dbx5x0.dtsi is common for all ux500 and ux540 boards.
> Then below are board configs which uses the above dtsi:
> ste-href.dtsi - common for href boards (used by ste-hrefprev60.dtsi
> and ste-hrefv60plus.dtsi), have vmmci
> ste-snowball.dts, have vmmci
> ste-ccu8540.dts, don't have vmmci
> ste-ccu9540.dts, don't have vmmci
Ah, got you. In which case it doesn't belong in ste-dbx5x0.dtsi.
> > We normally place the common pieces (of which there are many in this
> > node) in the highest level DTSI file, then add the platform specific
> > ones in the DTS files.
>
> Okay, so maybe it's due to the naming of ste-dbx5x0.dtsi, that I
> thought this was intended to cover the SoC configuration and not any
> of the platform specific stuff.
ste-dbx5x0.dtsi should cover all pieces which are common to all ux500
and ux540 devices. Then the lower level file ste-href-ab8500.dtsi
should cover all pieces which are common to ux500 devices and finally
the DTS files should add board specific information. Duplicate
nodes and properties are frowned upon.
> So what your advise of doing this?
So the file which covers the x500 boards is ste-href-ab8500.dtsi. I
would move the node into there instead. Keep it disabled and enable
the associated nodes in the 2 DTS files.
> >> Fixes: c94a4ab7af3f ("ARM: ux500: Disable the MMCI gpio-regulator by default")
> >> Signed-off-by: Ulf Hansson <ulf.hansson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> >> ---
> >> arch/arm/boot/dts/ste-dbx5x0.dtsi | 17 -----------------
> >> arch/arm/boot/dts/ste-href.dtsi | 17 +++++++++++++++++
> >> arch/arm/boot/dts/ste-snowball.dts | 15 +++++++++++++++
> >> 3 files changed, 32 insertions(+), 17 deletions(-)
> >>
> >> diff --git a/arch/arm/boot/dts/ste-dbx5x0.dtsi b/arch/arm/boot/dts/ste-dbx5x0.dtsi
> >> index bfd3f1c..2201cd5 100644
> >> --- a/arch/arm/boot/dts/ste-dbx5x0.dtsi
> >> +++ b/arch/arm/boot/dts/ste-dbx5x0.dtsi
> >> @@ -1017,23 +1017,6 @@
> >> status = "disabled";
> >> };
> >>
> >> - vmmci: regulator-gpio {
> >> - compatible = "regulator-gpio";
> >> -
> >> - regulator-min-microvolt = <1800000>;
> >> - regulator-max-microvolt = <2900000>;
> >> - regulator-name = "mmci-reg";
> >> - regulator-type = "voltage";
> >> -
> >> - startup-delay-us = <100>;
> >> - enable-active-high;
> >> -
> >> - states = <1800000 0x1
> >> - 2900000 0x0>;
> >> -
> >> - status = "disabled";
> >> - };
> >> -
> >> mcde@a0350000 {
> >> compatible = "stericsson,mcde";
> >> reg = <0xa0350000 0x1000>, /* MCDE */
> >> diff --git a/arch/arm/boot/dts/ste-href.dtsi b/arch/arm/boot/dts/ste-href.dtsi
> >> index bf8f0ed..8cf499a 100644
> >> --- a/arch/arm/boot/dts/ste-href.dtsi
> >> +++ b/arch/arm/boot/dts/ste-href.dtsi
> >> @@ -111,6 +111,23 @@
> >> pinctrl-1 = <&i2c3_sleep_mode>;
> >> };
> >>
> >> + vmmci: regulator-gpio {
> >> + compatible = "regulator-gpio";
> >> +
> >> + regulator-min-microvolt = <1800000>;
> >> + regulator-max-microvolt = <2900000>;
> >> + regulator-name = "mmci-reg";
> >> + regulator-type = "voltage";
> >> +
> >> + startup-delay-us = <100>;
> >> + enable-active-high;
> >> +
> >> + states = <1800000 0x1
> >> + 2900000 0x0>;
> >> +
> >> + status = "disabled";
> >> + };
> >> +
> >> // External Micro SD slot
> >> sdi0_per1@80126000 {
> >> arm,primecell-periphid = <0x10480180>;
> >> diff --git a/arch/arm/boot/dts/ste-snowball.dts b/arch/arm/boot/dts/ste-snowball.dts
> >> index 206826a..65a7f63 100644
> >> --- a/arch/arm/boot/dts/ste-snowball.dts
> >> +++ b/arch/arm/boot/dts/ste-snowball.dts
> >> @@ -146,8 +146,23 @@
> >> };
> >>
> >> vmmci: regulator-gpio {
> >> + compatible = "regulator-gpio";
> >> +
> >> gpios = <&gpio7 4 0x4>;
> >> enable-gpio = <&gpio6 25 0x4>;
> >> +
> >> + regulator-min-microvolt = <1800000>;
> >> + regulator-max-microvolt = <2900000>;
> >> + regulator-name = "mmci-reg";
> >> + regulator-type = "voltage";
> >> +
> >> + startup-delay-us = <100>;
> >> + enable-active-high;
> >> +
> >> + states = <1800000 0x1
> >> + 2900000 0x0>;
> >> +
> >> + status = "disabled";
> >> };
> >>
> >> // External Micro SD slot
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
next prev parent reply other threads:[~2015-04-21 7:33 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-04-20 14:02 [PATCH 0/3] ARM: ux500: Fix SD-card regression by using GPIO regulator Ulf Hansson
2015-04-20 14:02 ` Ulf Hansson
2015-04-20 14:02 ` [PATCH 1/3] ARM: ux500: Move GPIO regulator for SD-card into board DTSs Ulf Hansson
2015-04-20 14:02 ` Ulf Hansson
2015-04-20 18:26 ` Lee Jones
2015-04-20 18:26 ` Lee Jones
2015-04-21 7:15 ` Ulf Hansson
2015-04-21 7:15 ` Ulf Hansson
2015-04-21 7:33 ` Lee Jones [this message]
2015-04-21 7:33 ` Lee Jones
2015-04-21 8:00 ` Ulf Hansson
2015-04-21 8:00 ` Ulf Hansson
2015-04-21 10:34 ` Lee Jones
2015-04-21 10:34 ` Lee Jones
2015-04-20 14:02 ` [PATCH 2/3] ARM: ux500: Enable GPIO regulator for SD-card for HREF boards Ulf Hansson
2015-04-20 14:02 ` Ulf Hansson
2015-04-20 18:27 ` Lee Jones
2015-04-20 18:27 ` Lee Jones
2015-04-20 14:02 ` [PATCH 3/3] ARM: ux500: Enable GPIO regulator for SD-card for snowball Ulf Hansson
2015-04-20 14:02 ` Ulf Hansson
2015-04-20 15:49 ` [PATCH 0/3] ARM: ux500: Fix SD-card regression by using GPIO regulator Bjorn Andersson
2015-04-20 15:49 ` Bjorn Andersson
2015-04-20 18:24 ` Lee Jones
2015-04-20 18:24 ` Lee Jones
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20150421073336.GJ3447@x1 \
--to=lee@kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.