* Re: [PATCH/RFC v4] ARM: shmobile: armadillo800eva-reference: add SDHI and MMCIF interfaces
2013-09-23 15:38 [PATCH/RFC v4] ARM: shmobile: armadillo800eva-reference: add SDHI and MMCIF interfaces Guennadi Liakhovetski
@ 2013-09-25 5:36 ` Simon Horman
[not found] ` <20130925053636.GD1916-/R6kz+dDXgpPR4JQBCEnsQ@public.gmane.org>
2013-09-25 9:05 ` Laurent Pinchart
` (11 subsequent siblings)
12 siblings, 1 reply; 18+ messages in thread
From: Simon Horman @ 2013-09-25 5:36 UTC (permalink / raw)
To: linux-sh
On Mon, Sep 23, 2013 at 05:38:47PM +0200, Guennadi Liakhovetski wrote:
> Add SDHI0, SDHI1 and MMCIF interfaces to armadillo800eva-reference with
> regulators and pin configurations.
>
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>
> ---
>
> v4:
>
> 1. now that PFC pinctrl is usable with DT, we can use proper regulators
> and pin configurations on armadillo800eva
>
> 2. corrected SDHI compatibility strings
>
> 3. RFC because I don't know how to enable choosing between CON14 and CON8.
> In .c version this is done by reading GPIO 6. To do the same in DT mode
> we'd probably have to use some run-time DT patching, which isn't possible
> yet, AFAICS.
I agree with your reasoning there, though perhaps Laurent or Magnus
have a more enlightened view of things.
To my way of thinking it would be good to merge this change as is,
which an appropriate disclaimer about functionality in the changelog
above the scissors. But I do not feel strongly about this and I
am happy to wait for fuller functionality.
>
> .../boot/dts/r8a7740-armadillo800eva-reference.dts | 83 ++++++++++++++++++++
> arch/arm/boot/dts/r8a7740.dtsi | 33 ++++++++
> 2 files changed, 116 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/boot/dts/r8a7740-armadillo800eva-reference.dts b/arch/arm/boot/dts/r8a7740-armadillo800eva-reference.dts
> index c638e4a..af15be0 100644
> --- a/arch/arm/boot/dts/r8a7740-armadillo800eva-reference.dts
> +++ b/arch/arm/boot/dts/r8a7740-armadillo800eva-reference.dts
> @@ -34,6 +34,44 @@
> regulator-boot-on;
> };
>
> + vcc_sdhi0: regulator@1 {
> + compatible = "regulator-fixed";
> +
> + regulator-name = "SDHI0 Vcc";
> + regulator-min-microvolt = <3300000>;
> + regulator-max-microvolt = <3300000>;
> +
> + gpio = <&pfc 75 GPIO_ACTIVE_HIGH>;
> + enable-active-high;
> + };
> +
> + vcc_sdhi1: regulator@2 {
> + compatible = "regulator-fixed";
> +
> + regulator-name = "SDHI1 Vcc";
> + regulator-min-microvolt = <3300000>;
> + regulator-max-microvolt = <3300000>;
> +
> + gpio = <&pfc 16 GPIO_ACTIVE_HIGH>;
> + enable-active-high;
> + };
> +
> + vccq_sdhi0: regulator@3 {
> + compatible = "regulator-gpio";
> +
> + regulator-name = "SDHI0 VccQ";
> + regulator-min-microvolt = <1800000>;
> + regulator-max-microvolt = <3300000>;
> + vin-supply = <&vcc_sdhi0>;
> +
> + enable-gpio = <&pfc 74 GPIO_ACTIVE_HIGH>;
> + gpios = <&pfc 17 GPIO_ACTIVE_HIGH>;
> + states = <3300000 0
> + 1800000 1>;
> +
> + enable-active-high;
> + };
> +
> leds {
> compatible = "gpio-leds";
> led1 {
> @@ -76,4 +114,49 @@
> renesas,groups = "intc_irq10";
> renesas,function = "intc";
> };
> +
> + mmc0_pins: mmc0 {
> + renesas,groups = "mmc0_data8_1", "mmc0_ctrl_1";
> + renesas,function = "mmc0";
> + };
> +
> + sdhi0_pins: sdhi0 {
> + renesas,groups = "sdhi0_data4", "sdhi0_ctrl", "sdhi0_wp";
> + renesas,function = "sdhi0";
> + };
> +
> + sdhi1_pins: sdhi1 {
> + renesas,groups = "sdhi1_data4", "sdhi1_ctrl", "sdhi1_cd", "sdhi1_wp";
> + renesas,function = "sdhi1";
> + };
> +};
> +
> +&mmcif0 {
> + pinctrl-0 = <&mmc0_pins>;
> + pinctrl-names = "default";
> +
> + vmmc-supply = <®_3p3v>;
> + bus-width = <8>;
> + non-removable;
> + status = "okay";
> +};
> +
> +&sdhi0 {
> + pinctrl-0 = <&sdhi0_pins>;
> + pinctrl-names = "default";
> +
> + vmmc-supply = <&vcc_sdhi0>;
> + vqmmc-supply = <&vccq_sdhi0>;
> + bus-width = <4>;
> + broken-cd;
> + status = "okay";
> +};
> +
> +&sdhi1 {
> + pinctrl-0 = <&sdhi1_pins>;
> + pinctrl-names = "default";
> +
> + vmmc-supply = <&vcc_sdhi1>;
> + bus-width = <4>;
> + status = "okay";
> };
> diff --git a/arch/arm/boot/dts/r8a7740.dtsi b/arch/arm/boot/dts/r8a7740.dtsi
> index 44d3d52..018c02d 100644
> --- a/arch/arm/boot/dts/r8a7740.dtsi
> +++ b/arch/arm/boot/dts/r8a7740.dtsi
> @@ -159,4 +159,37 @@
> status = "disabled";
> #pwm-cells = <3>;
> };
> +
> + mmcif0: mmcif@e6bd0000 {
> + compatible = "renesas,sh-mmcif";
> + reg = <0xe6bd0000 0x100>;
> + interrupt-parent = <&gic>;
> + interrupts = <0 56 4
> + 0 57 4>;
> + status = "disabled";
> + };
> +
> + sdhi0: sdhi@e6850000 {
> + compatible = "renesas,sdhi-r8a7740";
> + reg = <0xe6850000 0x100>;
> + interrupt-parent = <&gic>;
> + interrupts = <0 117 4
> + 0 118 4
> + 0 119 4>;
> + cap-sd-highspeed;
> + cap-sdio-irq;
> + status = "disabled";
> + };
> +
> + sdhi1: sdhi@e6860000 {
> + compatible = "renesas,sdhi-r8a7740";
> + reg = <0xe6860000 0x100>;
> + interrupt-parent = <&gic>;
> + interrupts = <0 121 4
> + 0 122 4
> + 0 123 4>;
> + cap-sd-highspeed;
> + cap-sdio-irq;
> + status = "disabled";
> + };
> };
> --
> 1.7.2.5
>
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH/RFC v4] ARM: shmobile: armadillo800eva-reference: add SDHI and MMCIF interfaces
2013-09-23 15:38 [PATCH/RFC v4] ARM: shmobile: armadillo800eva-reference: add SDHI and MMCIF interfaces Guennadi Liakhovetski
2013-09-25 5:36 ` Simon Horman
@ 2013-09-25 9:05 ` Laurent Pinchart
2013-09-26 7:46 ` Linus Walleij
` (10 subsequent siblings)
12 siblings, 0 replies; 18+ messages in thread
From: Laurent Pinchart @ 2013-09-25 9:05 UTC (permalink / raw)
To: linux-sh
Hi,
(CC'ing Linus Walleij)
On Wednesday 25 September 2013 14:36:36 Simon Horman wrote:
> On Mon, Sep 23, 2013 at 05:38:47PM +0200, Guennadi Liakhovetski wrote:
> > Add SDHI0, SDHI1 and MMCIF interfaces to armadillo800eva-reference with
> > regulators and pin configurations.
> >
> > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>
> > ---
> >
> > v4:
> >
> > 1. now that PFC pinctrl is usable with DT, we can use proper regulators
> > and pin configurations on armadillo800eva
> >
> > 2. corrected SDHI compatibility strings
> >
> > 3. RFC because I don't know how to enable choosing between CON14 and CON8.
> > In .c version this is done by reading GPIO 6. To do the same in DT mode
> > we'd probably have to use some run-time DT patching, which isn't possible
> > yet, AFAICS.
>
> I agree with your reasoning there, though perhaps Laurent or Magnus
> have a more enlightened view of things.
I'm tempted to say this should be handled by the boot loader, which should
then patch the DT accordingly. This is probably just a poor attempt not to
solve the problem in Linux though :-)
Linus, do you have an opinion on this ? The board has two connectors (MMC/SD 1
and wifi module) that are not usable concurrently. The user can select which
connector to use through a hardware switch that existing board code reads at
init time to determine which platform devices to register and how to configure
pin muxing. Are you aware of a similar problem on other boards that would have
been solved already ?
> To my way of thinking it would be good to merge this change as is,
> which an appropriate disclaimer about functionality in the changelog
> above the scissors. But I do not feel strongly about this and I
> am happy to wait for fuller functionality.
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH/RFC v4] ARM: shmobile: armadillo800eva-reference: add SDHI and MMCIF interfaces
2013-09-23 15:38 [PATCH/RFC v4] ARM: shmobile: armadillo800eva-reference: add SDHI and MMCIF interfaces Guennadi Liakhovetski
2013-09-25 5:36 ` Simon Horman
2013-09-25 9:05 ` Laurent Pinchart
@ 2013-09-26 7:46 ` Linus Walleij
2013-09-26 8:24 ` Guennadi Liakhovetski
` (9 subsequent siblings)
12 siblings, 0 replies; 18+ messages in thread
From: Linus Walleij @ 2013-09-26 7:46 UTC (permalink / raw)
To: linux-sh
On Wed, Sep 25, 2013 at 11:05 AM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>> > 3. RFC because I don't know how to enable choosing between CON14 and CON8.
>> > In .c version this is done by reading GPIO 6. To do the same in DT mode
>> > we'd probably have to use some run-time DT patching, which isn't possible
>> > yet, AFAICS.
>>
>> I agree with your reasoning there, though perhaps Laurent or Magnus
>> have a more enlightened view of things.
>
> I'm tempted to say this should be handled by the boot loader, which should
> then patch the DT accordingly. This is probably just a poor attempt not to
> solve the problem in Linux though :-)
This is not the first time we have come to the conclusion that Linux
need to modify the device tree.
And I clearly remember Grant stating that it is in principle a living
datastructure, it's not like it's read-only.
The actual restrictions seem to be more about things like if you
need to read this GPIO to figure out how to set up the device tree
you already need the base system initialized to access the GPIO
so it becomes a chicken-and-egg problem.
> Linus, do you have an opinion on this ? The board has two connectors (MMC/SD 1
> and wifi module) that are not usable concurrently. The user can select which
> connector to use through a hardware switch that existing board code reads at
> init time to determine which platform devices to register and how to configure
> pin muxing. Are you aware of a similar problem on other boards that would have
> been solved already ?
Just because the pin control tables *can* be initialized from the device
tree doesn't mean all of them *have to*.
No more than all devices in the system have to come from the device
tree. You can still use platform_device_register() and in some cases
it is the most reasonable thing to do.
So use the nice auto-detection scheme you have, and use
pinctrl_register_mappings() from the machine to set up the right
mapping depending on what was detected. Naturally this needs
to happen before that MMC/SD or Wifi module gets probed.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH/RFC v4] ARM: shmobile: armadillo800eva-reference: add SDHI and MMCIF interfaces
2013-09-23 15:38 [PATCH/RFC v4] ARM: shmobile: armadillo800eva-reference: add SDHI and MMCIF interfaces Guennadi Liakhovetski
` (2 preceding siblings ...)
2013-09-26 7:46 ` Linus Walleij
@ 2013-09-26 8:24 ` Guennadi Liakhovetski
2013-09-26 9:41 ` Magnus Damm
` (8 subsequent siblings)
12 siblings, 0 replies; 18+ messages in thread
From: Guennadi Liakhovetski @ 2013-09-26 8:24 UTC (permalink / raw)
To: linux-sh
Hi
On Thu, 26 Sep 2013, Linus Walleij wrote:
> On Wed, Sep 25, 2013 at 11:05 AM, Laurent Pinchart
> <laurent.pinchart@ideasonboard.com> wrote:
>
> >> > 3. RFC because I don't know how to enable choosing between CON14 and CON8.
> >> > In .c version this is done by reading GPIO 6. To do the same in DT mode
> >> > we'd probably have to use some run-time DT patching, which isn't possible
> >> > yet, AFAICS.
> >>
> >> I agree with your reasoning there, though perhaps Laurent or Magnus
> >> have a more enlightened view of things.
> >
> > I'm tempted to say this should be handled by the boot loader, which should
> > then patch the DT accordingly. This is probably just a poor attempt not to
> > solve the problem in Linux though :-)
>
> This is not the first time we have come to the conclusion that Linux
> need to modify the device tree.
In fact, I'm not even sure we have to select. Please correct me if I'm
wrong, but to me it looks like that switch switches the SDHI1 (second
SD-card) interface to either connect to an SD-card connector, or to an
SDIO WLAN module. Do we really have to distinguish between the two? Cannot
we just enable the interface and probe the card? Obviously, the card
detection will not work in the SDIO case. Is this the only difference? If
so, maybe we could make a special card-detection routine, that would
return true, if the switch is in SDIO position, and read the real CD pin,
if the switch activates the slot?
Thanks
Guennadi
> And I clearly remember Grant stating that it is in principle a living
> datastructure, it's not like it's read-only.
>
> The actual restrictions seem to be more about things like if you
> need to read this GPIO to figure out how to set up the device tree
> you already need the base system initialized to access the GPIO
> so it becomes a chicken-and-egg problem.
>
> > Linus, do you have an opinion on this ? The board has two connectors (MMC/SD 1
> > and wifi module) that are not usable concurrently. The user can select which
> > connector to use through a hardware switch that existing board code reads at
> > init time to determine which platform devices to register and how to configure
> > pin muxing. Are you aware of a similar problem on other boards that would have
> > been solved already ?
>
> Just because the pin control tables *can* be initialized from the device
> tree doesn't mean all of them *have to*.
>
> No more than all devices in the system have to come from the device
> tree. You can still use platform_device_register() and in some cases
> it is the most reasonable thing to do.
>
> So use the nice auto-detection scheme you have, and use
> pinctrl_register_mappings() from the machine to set up the right
> mapping depending on what was detected. Naturally this needs
> to happen before that MMC/SD or Wifi module gets probed.
>
> Yours,
> Linus Walleij
>
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH/RFC v4] ARM: shmobile: armadillo800eva-reference: add SDHI and MMCIF interfaces
2013-09-23 15:38 [PATCH/RFC v4] ARM: shmobile: armadillo800eva-reference: add SDHI and MMCIF interfaces Guennadi Liakhovetski
` (3 preceding siblings ...)
2013-09-26 8:24 ` Guennadi Liakhovetski
@ 2013-09-26 9:41 ` Magnus Damm
2013-09-26 9:57 ` Guennadi Liakhovetski
` (7 subsequent siblings)
12 siblings, 0 replies; 18+ messages in thread
From: Magnus Damm @ 2013-09-26 9:41 UTC (permalink / raw)
To: linux-sh
On Mon, Sep 23, 2013 at 8:38 AM, Guennadi Liakhovetski
<g.liakhovetski@gmx.de> wrote:
> Add SDHI0, SDHI1 and MMCIF interfaces to armadillo800eva-reference with
> regulators and pin configurations.
>
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>
> ---
>
> v4:
>
> 1. now that PFC pinctrl is usable with DT, we can use proper regulators
> and pin configurations on armadillo800eva
Good, thanks for your efforts.
> 2. corrected SDHI compatibility strings
Ok, thanks.
> 3. RFC because I don't know how to enable choosing between CON14 and CON8.
> In .c version this is done by reading GPIO 6. To do the same in DT mode
> we'd probably have to use some run-time DT patching, which isn't possible
> yet, AFAICS.
Regarding run time checking, this seems to me that it is something
that cannot be supported by DT at this time. Because of that, would it
be possible to reduce the scope of this patch to only cover the MMC
device that is unrelated to GPIO 6?
I propose that the other two devices should be handled my platform
device code until DT can do runtime switching or when we can use
pinctrl together with bind/unbind to select which MMC device to use.
Thanks,
/ magnus
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH/RFC v4] ARM: shmobile: armadillo800eva-reference: add SDHI and MMCIF interfaces
2013-09-23 15:38 [PATCH/RFC v4] ARM: shmobile: armadillo800eva-reference: add SDHI and MMCIF interfaces Guennadi Liakhovetski
` (4 preceding siblings ...)
2013-09-26 9:41 ` Magnus Damm
@ 2013-09-26 9:57 ` Guennadi Liakhovetski
2013-09-26 10:17 ` Magnus Damm
` (6 subsequent siblings)
12 siblings, 0 replies; 18+ messages in thread
From: Guennadi Liakhovetski @ 2013-09-26 9:57 UTC (permalink / raw)
To: linux-sh
Hi Magnus
On Thu, 26 Sep 2013, Magnus Damm wrote:
> On Mon, Sep 23, 2013 at 8:38 AM, Guennadi Liakhovetski
> <g.liakhovetski@gmx.de> wrote:
> > Add SDHI0, SDHI1 and MMCIF interfaces to armadillo800eva-reference with
> > regulators and pin configurations.
> >
> > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>
> > ---
> >
> > v4:
> >
> > 1. now that PFC pinctrl is usable with DT, we can use proper regulators
> > and pin configurations on armadillo800eva
>
> Good, thanks for your efforts.
>
> > 2. corrected SDHI compatibility strings
>
> Ok, thanks.
>
> > 3. RFC because I don't know how to enable choosing between CON14 and CON8.
> > In .c version this is done by reading GPIO 6. To do the same in DT mode
> > we'd probably have to use some run-time DT patching, which isn't possible
> > yet, AFAICS.
>
> Regarding run time checking, this seems to me that it is something
> that cannot be supported by DT at this time. Because of that, would it
> be possible to reduce the scope of this patch to only cover the MMC
> device that is unrelated to GPIO 6?
>
> I propose that the other two devices should be handled my platform
> device code until DT can do runtime switching or when we can use
> pinctrl together with bind/unbind to select which MMC device to use.
Yes, this is another possibility. But in an earlier email I proposed
another solution, which could work, if my assumption about the use of the
same pin configuration in both cases is correct. I'll try to flip the
switch and test if I can detect the WLAN module (if it is mounted on my
board). I'll report back later.
Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH/RFC v4] ARM: shmobile: armadillo800eva-reference: add SDHI and MMCIF interfaces
2013-09-23 15:38 [PATCH/RFC v4] ARM: shmobile: armadillo800eva-reference: add SDHI and MMCIF interfaces Guennadi Liakhovetski
` (5 preceding siblings ...)
2013-09-26 9:57 ` Guennadi Liakhovetski
@ 2013-09-26 10:17 ` Magnus Damm
2013-09-27 0:55 ` Laurent Pinchart
` (5 subsequent siblings)
12 siblings, 0 replies; 18+ messages in thread
From: Magnus Damm @ 2013-09-26 10:17 UTC (permalink / raw)
To: linux-sh
Hi Guennadi,
On Thu, Sep 26, 2013 at 2:57 AM, Guennadi Liakhovetski
<g.liakhovetski@gmx.de> wrote:
> Hi Magnus
>
> On Thu, 26 Sep 2013, Magnus Damm wrote:
>
>> On Mon, Sep 23, 2013 at 8:38 AM, Guennadi Liakhovetski
>> <g.liakhovetski@gmx.de> wrote:
>> > Add SDHI0, SDHI1 and MMCIF interfaces to armadillo800eva-reference with
>> > regulators and pin configurations.
>> >
>> > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>
>> > ---
>> >
>> > v4:
>> >
>> > 1. now that PFC pinctrl is usable with DT, we can use proper regulators
>> > and pin configurations on armadillo800eva
>>
>> Good, thanks for your efforts.
>>
>> > 2. corrected SDHI compatibility strings
>>
>> Ok, thanks.
>>
>> > 3. RFC because I don't know how to enable choosing between CON14 and CON8.
>> > In .c version this is done by reading GPIO 6. To do the same in DT mode
>> > we'd probably have to use some run-time DT patching, which isn't possible
>> > yet, AFAICS.
>>
>> Regarding run time checking, this seems to me that it is something
>> that cannot be supported by DT at this time. Because of that, would it
>> be possible to reduce the scope of this patch to only cover the MMC
>> device that is unrelated to GPIO 6?
Any thoughts about this? Please break out things that are clear and
send them off before blocking for non-obvious things.
>> I propose that the other two devices should be handled my platform
>> device code until DT can do runtime switching or when we can use
>> pinctrl together with bind/unbind to select which MMC device to use.
>
> Yes, this is another possibility. But in an earlier email I proposed
> another solution, which could work, if my assumption about the use of the
> same pin configuration in both cases is correct. I'll try to flip the
> switch and test if I can detect the WLAN module (if it is mounted on my
> board). I'll report back later.
Ok, thanks.
/ magnus
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH/RFC v4] ARM: shmobile: armadillo800eva-reference: add SDHI and MMCIF interfaces
2013-09-23 15:38 [PATCH/RFC v4] ARM: shmobile: armadillo800eva-reference: add SDHI and MMCIF interfaces Guennadi Liakhovetski
` (6 preceding siblings ...)
2013-09-26 10:17 ` Magnus Damm
@ 2013-09-27 0:55 ` Laurent Pinchart
2013-09-27 14:08 ` Linus Walleij
` (4 subsequent siblings)
12 siblings, 0 replies; 18+ messages in thread
From: Laurent Pinchart @ 2013-09-27 0:55 UTC (permalink / raw)
To: linux-sh
Hi Linus,
On Thursday 26 September 2013 09:46:24 Linus Walleij wrote:
> On Wed, Sep 25, 2013 at 11:05 AM, Laurent Pinchart wrote:
> >> > 3. RFC because I don't know how to enable choosing between CON14 and
> >> > CON8.
> >> > In .c version this is done by reading GPIO 6. To do the same in DT mode
> >> > we'd probably have to use some run-time DT patching, which isn't
> >> > possible yet, AFAICS.
> >>
> >> I agree with your reasoning there, though perhaps Laurent or Magnus
> >> have a more enlightened view of things.
> >
> > I'm tempted to say this should be handled by the boot loader, which should
> > then patch the DT accordingly. This is probably just a poor attempt not to
> > solve the problem in Linux though :-)
>
> This is not the first time we have come to the conclusion that Linux
> need to modify the device tree.
>
> And I clearly remember Grant stating that it is in principle a living
> datastructure, it's not like it's read-only.
>
> The actual restrictions seem to be more about things like if you
> need to read this GPIO to figure out how to set up the device tree
> you already need the base system initialized to access the GPIO
> so it becomes a chicken-and-egg problem.
Exactly.
> > Linus, do you have an opinion on this ? The board has two connectors
> > (MMC/SD 1 and wifi module) that are not usable concurrently. The user can
> > select which connector to use through a hardware switch that existing
> > board code reads at init time to determine which platform devices to
> > register and how to configure pin muxing. Are you aware of a similar
> > problem on other boards that would have been solved already ?
>
> Just because the pin control tables *can* be initialized from the device
> tree doesn't mean all of them *have to*.
>
> No more than all devices in the system have to come from the device
> tree. You can still use platform_device_register() and in some cases
> it is the most reasonable thing to do.
>
> So use the nice auto-detection scheme you have, and use
> pinctrl_register_mappings() from the machine to set up the right
> mapping depending on what was detected. Naturally this needs
> to happen before that MMC/SD or Wifi module gets probed.
I'd like to avoid moving the MMC/SD controller registration from the soc.dtsi
file to board code because one board does something weird. SoC devices should
really be declared in soc.dtsi, albeit in a disabled state.
Is there a way to patch the device tree in the board code init function to
turn "disabled" into "okay" based on the state of a GPIO ? I suppose not if
the GPIO controller is instantiated from DT. Is there any board function we
could hook this in ?
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH/RFC v4] ARM: shmobile: armadillo800eva-reference: add SDHI and MMCIF interfaces
2013-09-23 15:38 [PATCH/RFC v4] ARM: shmobile: armadillo800eva-reference: add SDHI and MMCIF interfaces Guennadi Liakhovetski
` (7 preceding siblings ...)
2013-09-27 0:55 ` Laurent Pinchart
@ 2013-09-27 14:08 ` Linus Walleij
2013-09-29 6:49 ` Laurent Pinchart
` (3 subsequent siblings)
12 siblings, 0 replies; 18+ messages in thread
From: Linus Walleij @ 2013-09-27 14:08 UTC (permalink / raw)
To: linux-sh
On Fri, Sep 27, 2013 at 2:55 AM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> Is there a way to patch the device tree in the board code init function to
> turn "disabled" into "okay" based on the state of a GPIO ?
Can't you just locate the node using of_find_node_by_path()
or whatever and then use this:
int of_update_property(struct device_node *np, struct property *newprop)?
It's not like I'm a master of DT but... I'd very much like to know as well,
because it'd be very useful :-)
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH/RFC v4] ARM: shmobile: armadillo800eva-reference: add SDHI and MMCIF interfaces
2013-09-23 15:38 [PATCH/RFC v4] ARM: shmobile: armadillo800eva-reference: add SDHI and MMCIF interfaces Guennadi Liakhovetski
` (8 preceding siblings ...)
2013-09-27 14:08 ` Linus Walleij
@ 2013-09-29 6:49 ` Laurent Pinchart
2013-09-29 23:20 ` Linus Walleij
` (2 subsequent siblings)
12 siblings, 0 replies; 18+ messages in thread
From: Laurent Pinchart @ 2013-09-29 6:49 UTC (permalink / raw)
To: linux-sh
Hi Linus,
On Friday 27 September 2013 16:08:31 Linus Walleij wrote:
> On Fri, Sep 27, 2013 at 2:55 AM, Laurent Pinchart wrote:
> > Is there a way to patch the device tree in the board code init function to
> > turn "disabled" into "okay" based on the state of a GPIO ?
>
> Can't you just locate the node using of_find_node_by_path()
> or whatever and then use this:
> int of_update_property(struct device_node *np, struct property *newprop)?
Where can I do that ? I need the PFC/GPIO devices to have been probed, so
board init isn't an option, but I also need the device to be enabled not to
have been probed yet.
One option would be to use a bus notifier to find out when the PFC/GPIO
devices are available, and extend of_update_property() with a notification
mechanism (either generic, or specific to the enabled propery) to detect when
a device gets enabled and probe it.
Before implementing that, I'd like to know if this approach has a chance to be
accepted.
> It's not like I'm a master of DT but... I'd very much like to know as well,
> because it'd be very useful :-)
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH/RFC v4] ARM: shmobile: armadillo800eva-reference: add SDHI and MMCIF interfaces
2013-09-23 15:38 [PATCH/RFC v4] ARM: shmobile: armadillo800eva-reference: add SDHI and MMCIF interfaces Guennadi Liakhovetski
` (9 preceding siblings ...)
2013-09-29 6:49 ` Laurent Pinchart
@ 2013-09-29 23:20 ` Linus Walleij
2013-09-30 11:10 ` Laurent Pinchart
2013-09-30 13:15 ` Grant Likely
2013-09-30 14:38 ` Laurent Pinchart
12 siblings, 1 reply; 18+ messages in thread
From: Linus Walleij @ 2013-09-29 23:20 UTC (permalink / raw)
To: linux-sh
On Sun, Sep 29, 2013 at 8:49 AM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> On Friday 27 September 2013 16:08:31 Linus Walleij wrote:
>> Can't you just locate the node using of_find_node_by_path()
>> or whatever and then use this:
>> int of_update_property(struct device_node *np, struct property *newprop)?
>
> Where can I do that ? I need the PFC/GPIO devices to have been probed, so
> board init isn't an option, but I also need the device to be enabled not to
> have been probed yet.
>
> One option would be to use a bus notifier to find out when the PFC/GPIO
> devices are available, and extend of_update_property() with a notification
> mechanism (either generic, or specific to the enabled propery) to detect when
> a device gets enabled and probe it.
Argh that sounds awfully complicated...
I would consider doing this right inside the PCF/GPIO driver
right at the end of its probe function. As it is related to pins
anyway... or is that too ugly?
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH/RFC v4] ARM: shmobile: armadillo800eva-reference: add SDHI and MMCIF interfaces
2013-09-29 23:20 ` Linus Walleij
@ 2013-09-30 11:10 ` Laurent Pinchart
2013-10-08 11:19 ` Linus Walleij
0 siblings, 1 reply; 18+ messages in thread
From: Laurent Pinchart @ 2013-09-30 11:10 UTC (permalink / raw)
To: Linus Walleij
Cc: Rob Herring, Grant Likely, Simon Horman, Guennadi Liakhovetski,
linux-sh, devicetree, Magnus Damm
Hi Linus,
[Updated the devicetree mailing list e-mail address]
On Monday 30 September 2013 01:20:46 Linus Walleij wrote:
> On Sun, Sep 29, 2013 at 8:49 AM, Laurent Pinchart wrote:
> > On Friday 27 September 2013 16:08:31 Linus Walleij wrote:
> >> Can't you just locate the node using of_find_node_by_path()
> >> or whatever and then use this:
> >> int of_update_property(struct device_node *np, struct property *newprop)?
> >
> > Where can I do that ? I need the PFC/GPIO devices to have been probed, so
> > board init isn't an option, but I also need the device to be enabled not
> > to have been probed yet.
> >
> > One option would be to use a bus notifier to find out when the PFC/GPIO
> > devices are available, and extend of_update_property() with a notification
> > mechanism (either generic, or specific to the enabled propery) to detect
> > when a device gets enabled and probe it.
>
> Argh that sounds awfully complicated...
>
> I would consider doing this right inside the PCF/GPIO driver right at the
> end of its probe function. As it is related to pins anyway... or is that too
> ugly?
As a reminder, we need to decide whether to register an SDHI (SD controller)
device or a WiFi module device, and thus configure pinmuxing appropriately,
based on the state of a GPIO connected to a user-accessible switch. This
behaviour is board-specific, not SoC-specific. That's why I believe the code
should be in board code.
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH/RFC v4] ARM: shmobile: armadillo800eva-reference: add SDHI and MMCIF interfaces
2013-09-30 11:10 ` Laurent Pinchart
@ 2013-10-08 11:19 ` Linus Walleij
2013-10-08 14:32 ` Laurent Pinchart
0 siblings, 1 reply; 18+ messages in thread
From: Linus Walleij @ 2013-10-08 11:19 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Rob Herring, Grant Likely, Simon Horman, Guennadi Liakhovetski,
linux-sh@vger.kernel.org, devicetree@vger.kernel.org, Magnus Damm
On Mon, Sep 30, 2013 at 1:10 PM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> Hi Linus,
>
> [Updated the devicetree mailing list e-mail address]
>
> On Monday 30 September 2013 01:20:46 Linus Walleij wrote:
>> On Sun, Sep 29, 2013 at 8:49 AM, Laurent Pinchart wrote:
>> > On Friday 27 September 2013 16:08:31 Linus Walleij wrote:
>> >> Can't you just locate the node using of_find_node_by_path()
>> >> or whatever and then use this:
>> >> int of_update_property(struct device_node *np, struct property *newprop)?
>> >
>> > Where can I do that ? I need the PFC/GPIO devices to have been probed, so
>> > board init isn't an option, but I also need the device to be enabled not
>> > to have been probed yet.
>> >
>> > One option would be to use a bus notifier to find out when the PFC/GPIO
>> > devices are available, and extend of_update_property() with a notification
>> > mechanism (either generic, or specific to the enabled propery) to detect
>> > when a device gets enabled and probe it.
>>
>> Argh that sounds awfully complicated...
>>
>> I would consider doing this right inside the PCF/GPIO driver right at the
>> end of its probe function. As it is related to pins anyway... or is that too
>> ugly?
>
> As a reminder, we need to decide whether to register an SDHI (SD controller)
> device or a WiFi module device, and thus configure pinmuxing appropriately,
> based on the state of a GPIO connected to a user-accessible switch. This
> behaviour is board-specific, not SoC-specific. That's why I believe the code
> should be in board code.
Hm, I might have misunderstood this earlier, so if the board code is
going to register *either* SDHI *or* WIFI, both as some kind of
struct device then as you say:
> The board has two connectors (MMC/SD 1
> and wifi module) that are not usable concurrently. The user can select which
> connector to use through a hardware switch that existing board code reads at
> init time (...)
So depending on what you read there you're going to register a
struct device named mmc0 or wifi0 or something, then you just register
the pin control tables for both cases and the apropriate default state
will be selected by the device core right before the device driver
gets probed. (grep for pinctrl_bind_pins). It is perfectly fine to have
unused pin states in the table.
Then the problem is unrelated to pin control and more about how
to register these devices, nothing to do with pin control.
Such as if they should both be marked as "disabled" in the DT and
activated by the kernel, or registered by adding a platform device
or something...
But I guess I got it wrong?
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH/RFC v4] ARM: shmobile: armadillo800eva-reference: add SDHI and MMCIF interfaces
2013-10-08 11:19 ` Linus Walleij
@ 2013-10-08 14:32 ` Laurent Pinchart
0 siblings, 0 replies; 18+ messages in thread
From: Laurent Pinchart @ 2013-10-08 14:32 UTC (permalink / raw)
To: Linus Walleij
Cc: Rob Herring, Grant Likely, Simon Horman, Guennadi Liakhovetski,
linux-sh@vger.kernel.org, devicetree@vger.kernel.org, Magnus Damm
Hi Linus (and Guennadi and Grant, there are questions for your below),
On Tuesday 08 October 2013 13:19:04 Linus Walleij wrote:
> On Mon, Sep 30, 2013 at 1:10 PM, Laurent Pinchart wrote:
> > Hi Linus,
> >
> > [Updated the devicetree mailing list e-mail address]
> >
> > On Monday 30 September 2013 01:20:46 Linus Walleij wrote:
> >> On Sun, Sep 29, 2013 at 8:49 AM, Laurent Pinchart wrote:
> >> > On Friday 27 September 2013 16:08:31 Linus Walleij wrote:
> >> >> Can't you just locate the node using of_find_node_by_path()
> >> >> or whatever and then use this:
> >> >> int of_update_property(struct device_node *np, struct property
> >> >> *newprop)?
> >> >
> >> > Where can I do that ? I need the PFC/GPIO devices to have been probed,
> >> > so board init isn't an option, but I also need the device to be enabled
> >> > not to have been probed yet.
> >> >
> >> > One option would be to use a bus notifier to find out when the PFC/GPIO
> >> > devices are available, and extend of_update_property() with a
> >> > notification mechanism (either generic, or specific to the enabled
> >> > propery) to detect when a device gets enabled and probe it.
> >>
> >> Argh that sounds awfully complicated...
> >>
> >> I would consider doing this right inside the PCF/GPIO driver right at the
> >> end of its probe function. As it is related to pins anyway... or is that
> >> too ugly?
> >
> > As a reminder, we need to decide whether to register an SDHI (SD
> > controller) device or a WiFi module device, and thus configure pinmuxing
> > appropriately, based on the state of a GPIO connected to a
> > user-accessible switch. This behaviour is board-specific, not
> > SoC-specific. That's why I believe the code should be in board code.
>
> Hm, I might have misunderstood this earlier, so if the board code is
> going to register *either* SDHI *or* WIFI, both as some kind of
> struct device then as you say:
>
> > The board has two connectors (MMC/SD 1
> > and wifi module) that are not usable concurrently. The user can select
> > which connector to use through a hardware switch that existing board code
> > reads at init time (...)
>
> So depending on what you read there you're going to register a struct device
> named mmc0 or wifi0 or something, then you just register the pin control
> tables for both cases and the apropriate default state will be selected by
> the device core right before the device driver gets probed. (grep for
> pinctrl_bind_pins). It is perfectly fine to have unused pin states in the
> table.
>
> Then the problem is unrelated to pin control and more about how to register
> these devices, nothing to do with pin control.
Correct.
> Such as if they should both be marked as "disabled" in the DT and activated
> by the kernel, or registered by adding a platform device or something...
I believe a way to mark a disabled device as enabled from board code would be
interesting. The function could be called from a platform bus notifier that
reacts on the GPIO device being probed. It's a bit of a corner case, but it
doesn't sound too hackish to me.
Grant, would you be fine with such a mechanism ? The devices would be marked
with status = "disabled" in DT and a new function would be added to enable
them. That function would modify the DT status to "okay" and trigger device
registration.
> But I guess I got it wrong?
I believe you got it right. At least your understanding matches mine :-)
Guennadi, could you please confirm ?
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH/RFC v4] ARM: shmobile: armadillo800eva-reference: add SDHI and MMCIF interfaces
2013-09-23 15:38 [PATCH/RFC v4] ARM: shmobile: armadillo800eva-reference: add SDHI and MMCIF interfaces Guennadi Liakhovetski
` (10 preceding siblings ...)
2013-09-29 23:20 ` Linus Walleij
@ 2013-09-30 13:15 ` Grant Likely
2013-09-30 14:38 ` Laurent Pinchart
12 siblings, 0 replies; 18+ messages in thread
From: Grant Likely @ 2013-09-30 13:15 UTC (permalink / raw)
To: linux-sh
On Thu, Sep 26, 2013 at 8:46 AM, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Wed, Sep 25, 2013 at 11:05 AM, Laurent Pinchart
> <laurent.pinchart@ideasonboard.com> wrote:
>
>>> > 3. RFC because I don't know how to enable choosing between CON14 and CON8.
>>> > In .c version this is done by reading GPIO 6. To do the same in DT mode
>>> > we'd probably have to use some run-time DT patching, which isn't possible
>>> > yet, AFAICS.
>>>
>>> I agree with your reasoning there, though perhaps Laurent or Magnus
>>> have a more enlightened view of things.
>>
>> I'm tempted to say this should be handled by the boot loader, which should
>> then patch the DT accordingly. This is probably just a poor attempt not to
>> solve the problem in Linux though :-)
>
> This is not the first time we have come to the conclusion that Linux
> need to modify the device tree.
>
> And I clearly remember Grant stating that it is in principle a living
> datastructure, it's not like it's read-only.
You still don't want the kernel modifying the DT. The DT is primarily
a communication mechanism from the firmware/platform to the kernel. If
the platform is responsible for describing the correct configuration
then it belongs in the DT. If however the kernel needs to do the work
of figuring out which configuration to use at runtime, then it would
be better for the DT to describe the possible configurations and let
the kernel choose the appropriate one.
> The actual restrictions seem to be more about things like if you
> need to read this GPIO to figure out how to set up the device tree
> you already need the base system initialized to access the GPIO
> so it becomes a chicken-and-egg problem.
>> Linus, do you have an opinion on this ? The board has two connectors (MMC/SD 1
>> and wifi module) that are not usable concurrently. The user can select which
>> connector to use through a hardware switch that existing board code reads at
>> init time to determine which platform devices to register and how to configure
>> pin muxing. Are you aware of a similar problem on other boards that would have
>> been solved already ?
>
> Just because the pin control tables *can* be initialized from the device
> tree doesn't mean all of them *have to*.
Right. Also, there are patches floating about that allow additional
device tree fragments to be loaded after the rest of the system is
booted. It was specifically designed for the beaglebone expansion
connectors. That might be the way to solve your problem here. (I
really want to get that feature merged, but I sheepishly admit that I
haven't been able to spend any time on it) :-(
g.
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH/RFC v4] ARM: shmobile: armadillo800eva-reference: add SDHI and MMCIF interfaces
2013-09-23 15:38 [PATCH/RFC v4] ARM: shmobile: armadillo800eva-reference: add SDHI and MMCIF interfaces Guennadi Liakhovetski
` (11 preceding siblings ...)
2013-09-30 13:15 ` Grant Likely
@ 2013-09-30 14:38 ` Laurent Pinchart
12 siblings, 0 replies; 18+ messages in thread
From: Laurent Pinchart @ 2013-09-30 14:38 UTC (permalink / raw)
To: linux-sh
Hi Grant,
On Monday 30 September 2013 14:15:15 Grant Likely wrote:
> On Thu, Sep 26, 2013 at 8:46 AM, Linus Walleij wrote:
> > On Wed, Sep 25, 2013 at 11:05 AM, Laurent Pinchart wrote:
> >>> > 3. RFC because I don't know how to enable choosing between CON14 and
> >>> > CON8. In .c version this is done by reading GPIO 6. To do the same in
> >>> > DT mode we'd probably have to use some run-time DT patching, which
> >>> > isn't possible yet, AFAICS.
> >>>
> >>> I agree with your reasoning there, though perhaps Laurent or Magnus
> >>> have a more enlightened view of things.
> >>
> >> I'm tempted to say this should be handled by the boot loader, which
> >> should then patch the DT accordingly. This is probably just a poor
> >> attempt not to solve the problem in Linux though :-)
> >
> > This is not the first time we have come to the conclusion that Linux
> > need to modify the device tree.
> >
> > And I clearly remember Grant stating that it is in principle a living
> > datastructure, it's not like it's read-only.
>
> You still don't want the kernel modifying the DT. The DT is primarily
> a communication mechanism from the firmware/platform to the kernel. If
> the platform is responsible for describing the correct configuration
> then it belongs in the DT. If however the kernel needs to do the work
> of figuring out which configuration to use at runtime, then it would
> be better for the DT to describe the possible configurations and let
> the kernel choose the appropriate one.
Could you please elaborate a bit on how you envision this being implemented ?
> > The actual restrictions seem to be more about things like if you
> > need to read this GPIO to figure out how to set up the device tree
> > you already need the base system initialized to access the GPIO
> > so it becomes a chicken-and-egg problem.
> >
> >> Linus, do you have an opinion on this ? The board has two connectors
> >> (MMC/SD 1 and wifi module) that are not usable concurrently. The user
> >> can select which connector to use through a hardware switch that
> >> existing board code reads at init time to determine which platform
> >> devices to register and how to configure pin muxing. Are you aware of a
> >> similar problem on other boards that would have been solved already ?
> >
> > Just because the pin control tables *can* be initialized from the device
> > tree doesn't mean all of them *have to*.
>
> Right. Also, there are patches floating about that allow additional device
> tree fragments to be loaded after the rest of the system is booted. It was
> specifically designed for the beaglebone expansion connectors. That might be
> the way to solve your problem here. (I really want to get that feature
> merged, but I sheepishly admit that I haven't been able to spend any time on
> it) :-(
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 18+ messages in thread