[PATCH 00/19] First HREF Device Tree enablement patch-set

[PATCH 00/19] First HREF Device Tree enablement patch-set

[Lee Jones]

Based on v3.6-rc4

This patch consists of Device Tree related enablement for
ST-Ericsson's development HREF hardware reference board.
Most devices are very similar, if not the same to the
previously enabled Snowball low-cost development board.
These are referenced from the relevant .dtsi file and
merely enabled in the HREF specific Device Tree. There 
are also one or two minor Device Tree related fixes which
were stumbled upon along the way.

 arch/arm/boot/dts/hrefv60plus.dts      |  140 ++++++++++++++++++++++++++++++++
 arch/arm/boot/dts/snowball.dts         |    1 -
 arch/arm/mach-ux500/board-mop500-sdi.c |    4 +-
 arch/arm/mach-ux500/board-mop500.c     |   12 +--
 arch/arm/mach-ux500/board-mop500.h     |    2 +
 drivers/gpio/gpio-tc3589x.c            |  120 +++++++++++++++++++--------
 drivers/i2c/busses/i2c-nomadik.c       |    4 +
 drivers/mfd/mfd-core.c                 |   10 ++-
 drivers/mfd/tc3589x.c                  |  112 +++++++++++++++++++------
 include/linux/mfd/tc3589x.h            |    1 +
 10 files changed, 327 insertions(+), 79 deletions(-)

[PATCH 01/19] ARM: ux500: Add skeleton Device Tree for the HREF reference board

[Lee Jones]

First commit applying the new Device Tree for ST-Ericsson's u8500
based hardware reference board.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 arch/arm/boot/dts/hrefv60plus.dts |   26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)
 create mode 100644 arch/arm/boot/dts/hrefv60plus.dts

diff --git a/arch/arm/boot/dts/hrefv60plus.dts b/arch/arm/boot/dts/hrefv60plus.dts
new file mode 100644
index 0000000..96bf8a3
--- /dev/null
+++ b/arch/arm/boot/dts/hrefv60plus.dts
@@ -0,0 +1,26 @@
+/*
+ * Copyright 2012 ST-Ericsson AB
+ *
+ * The code contained herein is licensed under the GNU General Public
+ * License. You may obtain a copy of the GNU General Public License
+ * Version 2 or later at the following locations:
+ *
+ * http://www.opensource.org/licenses/gpl-license.html
+ * http://www.gnu.org/copyleft/gpl.html
+ */
+
+/dts-v1/;
+/include/ "dbx5x0.dtsi"
+
+/ {
+	model = "ST-Ericsson HREF platform with Device Tree";
+	compatible = "st-ericsson,hrefv60+";
+
+	memory {
+		reg = <0x00000000 0x20000000>;
+	};
+
+	soc-u9500 {
+
+	};
+};
-- 
1.7.9.5

Re: [PATCH 01/19] ARM: ux500: Add skeleton Device Tree for the HREF reference board

[Linus Walleij]

On Fri, Sep 7, 2012 at 1:14 PM, Lee Jones <lee.jones@linaro.org> wrote:

> First commit applying the new Device Tree for ST-Ericsson's u8500
> based hardware reference board.
>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>

> +/dts-v1/;
> +/include/ "dbx5x0.dtsi"

This include is not even in linux-next, is this patch series dependent
on some other series I have not seen, or that I have forgot that
I saw (and even possibly ACKed...)

Apart from that it looks good.

Yours,
Linus Walleij

Re: [PATCH 01/19] ARM: ux500: Add skeleton Device Tree for the HREF reference board

[Lee Jones]

On Mon, Sep 10, 2012 at 01:47:09AM -0700, Linus Walleij wrote:
> On Fri, Sep 7, 2012 at 1:14 PM, Lee Jones <lee.jones@linaro.org> wrote:
> 
> > First commit applying the new Device Tree for ST-Ericsson's u8500
> > based hardware reference board.
> >
> > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> 
> > +/dts-v1/;
> > +/include/ "dbx5x0.dtsi"
> 
> This include is not even in linux-next, is this patch series dependent
> on some other series I have not seen, or that I have forgot that
> I saw (and even possibly ACKed...)

Yes, all of the above.

I have them all here locally. Will test push something today hopefully.

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH 01/19] ARM: ux500: Add skeleton Device Tree for the HREF reference board

[Linus Walleij]

On Fri, Sep 14, 2012 at 11:37 AM, Lee Jones <lee.jones@linaro.org> wrote:
> On Mon, Sep 10, 2012 at 01:47:09AM -0700, Linus Walleij wrote:
>> On Fri, Sep 7, 2012 at 1:14 PM, Lee Jones <lee.jones@linaro.org> wrote:
>>
>> > First commit applying the new Device Tree for ST-Ericsson's u8500
>> > based hardware reference board.
>> >
>> > Signed-off-by: Lee Jones <lee.jones@linaro.org>
>>
>> > +/dts-v1/;
>> > +/include/ "dbx5x0.dtsi"
>>
>> This include is not even in linux-next, is this patch series dependent
>> on some other series I have not seen, or that I have forgot that
>> I saw (and even possibly ACKed...)
>
> Yes, all of the above.
>
> I have them all here locally. Will test push something today hopefully.

OK then, when I see the complete set I will probably get the context.
Acked-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

[PATCH 02/19] ARM: ux500: Add UART support to the HREF Device Tree

[Lee Jones]

Here we add three nodes enabling UART support on the HREF hardware
reference board.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 arch/arm/boot/dts/hrefv60plus.dts |   10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/arm/boot/dts/hrefv60plus.dts b/arch/arm/boot/dts/hrefv60plus.dts
index 96bf8a3..32cd7a0 100644
--- a/arch/arm/boot/dts/hrefv60plus.dts
+++ b/arch/arm/boot/dts/hrefv60plus.dts
@@ -21,6 +21,16 @@
 	};
 
 	soc-u9500 {
+		uart@80120000 {
+			status = "okay";
+		};
 
+		uart@80121000 {
+			status = "okay";
+		};
+
+		uart@80007000 {
+			status = "okay";
+		};
 	};
 };
-- 
1.7.9.5

Re: [PATCH 02/19] ARM: ux500: Add UART support to the HREF Device Tree

[Linus Walleij]

On Fri, Sep 7, 2012 at 1:14 PM, Lee Jones <lee.jones@linaro.org> wrote:

> Here we add three nodes enabling UART support on the HREF hardware
> reference board.
>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>

Acked-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

[PATCH 03/19] ARM: ux500: Pass SDI DMA information though AUX_DATA to MMCI

[Lee Jones]

There are four SDIs on the HREF, as opposed to the original two
enabled on Snowball. To get them working we have to pass their DMA
information in the same was as we need with the other two.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 arch/arm/mach-ux500/board-mop500-sdi.c |    4 ++--
 arch/arm/mach-ux500/board-mop500.c     |    2 ++
 arch/arm/mach-ux500/board-mop500.h     |    2 ++
 3 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-ux500/board-mop500-sdi.c b/arch/arm/mach-ux500/board-mop500-sdi.c
index 18ff781..9c8e4a9 100644
--- a/arch/arm/mach-ux500/board-mop500-sdi.c
+++ b/arch/arm/mach-ux500/board-mop500-sdi.c
@@ -152,7 +152,7 @@ static struct stedma40_chan_cfg sdi1_dma_cfg_tx = {
 };
 #endif
 
-static struct mmci_platform_data mop500_sdi1_data = {
+struct mmci_platform_data mop500_sdi1_data = {
 	.ocr_mask	= MMC_VDD_29_30,
 	.f_max		= 50000000,
 	.capabilities	= MMC_CAP_4_BIT_DATA,
@@ -189,7 +189,7 @@ static struct stedma40_chan_cfg mop500_sdi2_dma_cfg_tx = {
 };
 #endif
 
-static struct mmci_platform_data mop500_sdi2_data = {
+struct mmci_platform_data mop500_sdi2_data = {
 	.ocr_mask	= MMC_VDD_165_195,
 	.f_max		= 50000000,
 	.capabilities	= MMC_CAP_4_BIT_DATA | MMC_CAP_8_BIT_DATA |
diff --git a/arch/arm/mach-ux500/board-mop500.c b/arch/arm/mach-ux500/board-mop500.c
index 9d2d08e..9575150 100644
--- a/arch/arm/mach-ux500/board-mop500.c
+++ b/arch/arm/mach-ux500/board-mop500.c
@@ -754,6 +754,8 @@ struct of_dev_auxdata u8500_auxdata_lookup[] __initdata = {
 	/* Requires DMA bindings. */
 	OF_DEV_AUXDATA("arm,pl022", 0x80002000, "ssp0",  &ssp0_plat),
 	OF_DEV_AUXDATA("arm,pl18x", 0x80126000, "sdi0",  &mop500_sdi0_data),
+	OF_DEV_AUXDATA("arm,pl18x", 0x80118000, "sdi1",  &mop500_sdi1_data),
+	OF_DEV_AUXDATA("arm,pl18x", 0x80005000, "sdi2",  &mop500_sdi2_data),
 	OF_DEV_AUXDATA("arm,pl18x", 0x80114000, "sdi4",  &mop500_sdi4_data),
 	/* Requires clock name bindings. */
 	OF_DEV_AUXDATA("st,nomadik-gpio", 0x8012e000, "gpio.0", NULL),
diff --git a/arch/arm/mach-ux500/board-mop500.h b/arch/arm/mach-ux500/board-mop500.h
index a0f920d..1f86387 100644
--- a/arch/arm/mach-ux500/board-mop500.h
+++ b/arch/arm/mach-ux500/board-mop500.h
@@ -81,6 +81,8 @@
 struct device;
 struct i2c_board_info;
 extern struct mmci_platform_data mop500_sdi0_data;
+extern struct mmci_platform_data mop500_sdi1_data;
+extern struct mmci_platform_data mop500_sdi2_data;
 extern struct mmci_platform_data mop500_sdi4_data;
 extern struct msp_i2s_platform_data msp0_platform_data;
 extern struct msp_i2s_platform_data msp1_platform_data;
-- 
1.7.9.5

Re: [PATCH 03/19] ARM: ux500: Pass SDI DMA information though AUX_DATA to MMCI

[Linus Walleij]

On Fri, Sep 7, 2012 at 1:14 PM, Lee Jones <lee.jones@linaro.org> wrote:

> There are four SDIs on the HREF, as opposed to the original two
> enabled on Snowball. To get them working we have to pass their DMA
> information in the same was as we need with the other two.
>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>

Yeah this will all go away once we get our precious DMA DT
bindings.

Acked-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

[PATCH 04/19] ARM: ux500: Add SDI (MMC) support to the HREF Device Tree

[Lee Jones]

Here we add the Device Tree nodes which are required to successfully
probe the MMCI driver which will enable the four cards available on
ST-Ericsson's HREF hardware development platform.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 arch/arm/boot/dts/hrefv60plus.dts |   45 +++++++++++++++++++++++++++++++++++++
 1 file changed, 45 insertions(+)

diff --git a/arch/arm/boot/dts/hrefv60plus.dts b/arch/arm/boot/dts/hrefv60plus.dts
index 32cd7a0..62b2ac1 100644
--- a/arch/arm/boot/dts/hrefv60plus.dts
+++ b/arch/arm/boot/dts/hrefv60plus.dts
@@ -32,5 +32,50 @@
 		uart@80007000 {
 			status = "okay";
 		};
+
+		// External Micro SD slot
+		sdi@80126000 {
+			arm,primecell-periphid = <0x10480180>;
+			max-frequency = <50000000>;
+			bus-width = <8>;
+			mmc-cap-sd-highspeed;
+			mmc-cap-mmc-highspeed;
+			vmmc-supply = <&ab8500_ldo_aux3_reg>;
+
+			#gpio-cells = <1>;
+			cd-gpios  = <&gpio2 31 0x4>; // 95
+
+			status = "okay";
+		};
+
+		// WLAN SDIO channel
+		sdi@80118000 {
+			arm,primecell-periphid = <0x10480180>;
+			max-frequency = <50000000>;
+			bus-width = <4>;
+
+			status = "okay";
+		};
+
+		// PoP:ed eMMC
+		sdi@80005000 {
+			arm,primecell-periphid = <0x10480180>;
+			max-frequency = <50000000>;
+			bus-width = <8>;
+			mmc-cap-mmc-highspeed;
+
+			status = "okay";
+		};
+
+		// On-board eMMC
+		sdi@80114000 {
+			arm,primecell-periphid = <0x10480180>;
+		        max-frequency = <50000000>;
+			bus-width = <8>;
+			mmc-cap-mmc-highspeed;
+			vmmc-supply = <&ab8500_ldo_aux2_reg>;
+
+			status = "okay";
+		};
 	};
 };
-- 
1.7.9.5

Re: [PATCH 04/19] ARM: ux500: Add SDI (MMC) support to the HREF Device Tree

[Arnd Bergmann]

On Friday 07 September 2012, Lee Jones wrote:
> +               // External Micro SD slot
> +               sdi@80126000 {
> +                       arm,primecell-periphid = <0x10480180>;
> +                       max-frequency = <50000000>;
> +                       bus-width = <8>;
> +                       mmc-cap-sd-highspeed;
> +                       mmc-cap-mmc-highspeed;
> +                       vmmc-supply = <&ab8500_ldo_aux3_reg>;
> +
> +                       #gpio-cells = <1>;
> +                       cd-gpios  = <&gpio2 31 0x4>; // 95
> +
> +                       status = "okay";
> +               };
> +

The #gpio-cells property seems misplaced here.

	Arnd

Re: [PATCH 04/19] ARM: ux500: Add SDI (MMC) support to the HREF Device Tree

[Lee Jones]

On Fri, Sep 07, 2012 at 12:29:31PM +0000, Arnd Bergmann wrote:
> On Friday 07 September 2012, Lee Jones wrote:
> > +               // External Micro SD slot
> > +               sdi@80126000 {
> > +                       arm,primecell-periphid = <0x10480180>;
> > +                       max-frequency = <50000000>;
> > +                       bus-width = <8>;
> > +                       mmc-cap-sd-highspeed;
> > +                       mmc-cap-mmc-highspeed;
> > +                       vmmc-supply = <&ab8500_ldo_aux3_reg>;
> > +
> > +                       #gpio-cells = <1>;
> > +                       cd-gpios  = <&gpio2 31 0x4>; // 95
> > +
> > +                       status = "okay";
> > +               };
> > +
> 
> The #gpio-cells property seems misplaced here.

Are you reading my patches in reverse order? ;)

If not, good spot. :)

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH 04/19] ARM: ux500: Add SDI (MMC) support to the HREF Device Tree

[Linus Walleij]

On Fri, Sep 7, 2012 at 1:14 PM, Lee Jones <lee.jones@linaro.org> wrote:

I really want Ulf, Per or Johan to have a look at this, so Cc:ing them..

> +               // External Micro SD slot

Are C99 comments OK in DTS files?
Else /* comment like that */
(See Documentation/CodingStyle)
Ah well, nitpick, I don't care really.

> +               sdi@80126000 {

Not just sdi@ please, these have names in the current
board code that comes from the DB8500 reference manual.

In this case 80120000 is the per1 (peripheral group one),
and offset 6000 in that group is SDI0.

So the quick fix is to name it like:

sdi0_per1@80126000

But basically we have a deeper problem here. I think the
ux500 Device Tree should be arranged after peripheral
block so it actually reflects the hardware (yeah, a big
fat sorry for not realizing that before....)

So it should *actually* be like:

per_group_1 {
    sdi0@80126000 {
    };
};

per_group_2 {
    sdi4@80114000 {
    };
    sdi1@80118000 {
    };
    sdi3@80119000 {
    };
};

Then I do not know if it's also possible to use the
peripheral group offsets for the registers? Can you do
this thing and have it working?

per_group_1 {
    reg = <0x80110000 0x10000>;
    sdi0@80126000 {
        reg = <0x6000 0x10000>;
    };
};

I.e. so getting the regs for sdi0 would give 0x80116000?

Any of the above proper solutions require a heavy patch
on the SoC .dtsi pushing all peripherals to their
peripheral group to go in first of course.

> +                       arm,primecell-periphid = <0x10480180>;
> +                       max-frequency = <50000000>;
> +                       bus-width = <8>;

Isn't *THIS* (sdi0) the one which is 4 bits? (SD cards only have 4
lines...)

> +                       mmc-cap-sd-highspeed;
> +                       mmc-cap-mmc-highspeed;
> +                       vmmc-supply = <&ab8500_ldo_aux3_reg>;
> +
> +                       #gpio-cells = <1>;

Arnd already complained about this...

> +                       cd-gpios  = <&gpio2 31 0x4>; // 95

95? 95 what? Lightning McQueen?
http://en.wikipedia.org/wiki/Lightning_McQueen

Elaborate or drop ;-)

> +
> +                       status = "okay";
> +               };
> +
> +               // WLAN SDIO channel
> +               sdi@80118000 {
> +                       arm,primecell-periphid = <0x10480180>;
> +                       max-frequency = <50000000>;
> +                       bus-width = <4>;

Isn't this 8 bit capable?

> +
> +                       status = "okay";
> +               };
> +
> +               // PoP:ed eMMC
> +               sdi@80005000 {
> +                       arm,primecell-periphid = <0x10480180>;
> +                       max-frequency = <50000000>;
> +                       bus-width = <8>;
> +                       mmc-cap-mmc-highspeed;
> +
> +                       status = "okay";
> +               };
> +
> +               // On-board eMMC
> +               sdi@80114000 {
> +                       arm,primecell-periphid = <0x10480180>;
> +                       max-frequency = <50000000>;
> +                       bus-width = <8>;
> +                       mmc-cap-mmc-highspeed;
> +                       vmmc-supply = <&ab8500_ldo_aux2_reg>;
> +
> +                       status = "okay";
> +               };
>         };
>  };

Inspecting board-mop500-sdi.c I see missing pieces that gets me
worried about whether this device tree even works, and which
I am sure has regressions in highly demanding usecases because
these things are there for a reason:

- .sigdir is defined with some flags for the MMCI platform data
  for sdi0 (SD card), and no DT bindings are available for it.
  I think Ulf had a patch for pushing this into the driver that will
  need to go in first.

- SDI0 has a callback .ios_handler that I guess must be passed
  in using platform data. If you don't have this, level-shifting
  cards will not be run at optimal voltage, symptom: you cannot
  gear up signals level and will get bad blocks at some high
  speeds on some cards.

(The .ocr_mask is surplus though!)

I really think the above needs to be resolved (preferrably
by pusing some of Ulfs MMCI patches) before we can move
on with MMC DT.

Yours,
Linus Walleij

[PATCH 05/19] ARM: ux500: Stop registering HREF's SDI devices from platform data

[Lee Jones]

The HREF Device Tree now has support for its four SDIs. So, if we
continue registering them from platform code when Device Tree is
enabled we'll cause a double probe of the same devices. To prevent
that we'll just remove registration.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 arch/arm/mach-ux500/board-mop500.c |    1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/arm/mach-ux500/board-mop500.c b/arch/arm/mach-ux500/board-mop500.c
index 9575150..a42418b 100644
--- a/arch/arm/mach-ux500/board-mop500.c
+++ b/arch/arm/mach-ux500/board-mop500.c
@@ -840,7 +840,6 @@ static void __init u8500_init_machine(void)
 		platform_add_devices(mop500_platform_devs,
 				ARRAY_SIZE(mop500_platform_devs));
 
-		hrefv60_sdi_init(parent);
 		mop500_audio_init(parent);
 
 		i2c0_devs = ARRAY_SIZE(mop500_i2c0_devices);
-- 
1.7.9.5

Re: [PATCH 05/19] ARM: ux500: Stop registering HREF's SDI devices from platform data

[Arnd Bergmann]

On Friday 07 September 2012, Lee Jones wrote:
> 
> The HREF Device Tree now has support for its four SDIs. So, if we
> continue registering them from platform code when Device Tree is
> enabled we'll cause a double probe of the same devices. To prevent
> that we'll just remove registration.
> 
> Signed-off-by: Lee Jones <lee.jones@linaro.org>
> ---
>  arch/arm/mach-ux500/board-mop500.c |    1 -
>  1 file changed, 1 deletion(-)

The change is ok, but I think it should go into the same patch
as the one before, so you can atomically switch from one to the other.

	Arnd

[PATCH 06/19] ARM: ux500: Add nodes for the MSP into the HREF Device Tree

[Lee Jones]

This patch enables the two important MSP devices for ST-Ericsson's
hardware reference development board.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 arch/arm/boot/dts/hrefv60plus.dts |    8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/arm/boot/dts/hrefv60plus.dts b/arch/arm/boot/dts/hrefv60plus.dts
index 62b2ac1..4f0aee8 100644
--- a/arch/arm/boot/dts/hrefv60plus.dts
+++ b/arch/arm/boot/dts/hrefv60plus.dts
@@ -77,5 +77,13 @@
 
 			status = "okay";
 		};
+
+		msp1: msp@80124000 {
+			status = "okay";
+		};
+
+		msp3: msp@80125000 {
+			status = "okay";
+		};
 	};
 };
-- 
1.7.9.5

Re: [PATCH 06/19] ARM: ux500: Add nodes for the MSP into the HREF Device Tree

[Linus Walleij]

On Fri, Sep 7, 2012 at 1:14 PM, Lee Jones <lee.jones@linaro.org> wrote:

> This patch enables the two important MSP devices for ST-Ericsson's
> hardware reference development board.
>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>
> ---
>  arch/arm/boot/dts/hrefv60plus.dts |    8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/arch/arm/boot/dts/hrefv60plus.dts b/arch/arm/boot/dts/hrefv60plus.dts
> index 62b2ac1..4f0aee8 100644
> --- a/arch/arm/boot/dts/hrefv60plus.dts
> +++ b/arch/arm/boot/dts/hrefv60plus.dts
> @@ -77,5 +77,13 @@
>
>                         status = "okay";
>                 };
> +
> +               msp1: msp@80124000 {
> +                       status = "okay";
> +               };
> +
> +               msp3: msp@80125000 {
> +                       status = "okay";
> +               };
>         };
>  };

Does this really "enable" the MSPs as the title claims?

Aren's there any kind of bindings needed, not even a
register base?

Yours,
Linus Walleij

Re: [PATCH 06/19] ARM: ux500: Add nodes for the MSP into the HREF Device Tree

[Lee Jones]

On Mon, Sep 10, 2012 at 02:53:45AM -0700, Linus Walleij wrote:
> On Fri, Sep 7, 2012 at 1:14 PM, Lee Jones <lee.jones@linaro.org> wrote:
> 
> > This patch enables the two important MSP devices for ST-Ericsson's
> > hardware reference development board.
> >
> > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> > ---
> >  arch/arm/boot/dts/hrefv60plus.dts |    8 ++++++++
> >  1 file changed, 8 insertions(+)
> >
> > diff --git a/arch/arm/boot/dts/hrefv60plus.dts b/arch/arm/boot/dts/hrefv60plus.dts
> > index 62b2ac1..4f0aee8 100644
> > --- a/arch/arm/boot/dts/hrefv60plus.dts
> > +++ b/arch/arm/boot/dts/hrefv60plus.dts
> > @@ -77,5 +77,13 @@
> >
> >                         status = "okay";
> >                 };
> > +
> > +               msp1: msp@80124000 {
> > +                       status = "okay";
> > +               };
> > +
> > +               msp3: msp@80125000 {
> > +                       status = "okay";
> > +               };
> >         };
> >  };
> 
> Does this really "enable" the MSPs as the title claims?

Yes they do.

> Aren's there any kind of bindings needed, not even a
> register base?

All that information is in the "disabled" node contained in .dtsi file.

+               msp0: msp@80123000 {
+                       compatible = "stericsson,ux500-msp-i2s";
+                       reg = <0x80123000 0x1000>;
+                       interrupts = <0 31 0x4>;
+                       v-ape-supply = <&db8500_vape_reg>;
+                       status = "disabled";

Then this patch "enable"s it (as the title suggests). :)

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH 06/19] ARM: ux500: Add nodes for the MSP into the HREF Device Tree

[Linus Walleij]

On Fri, Sep 14, 2012 at 11:23 AM, Lee Jones <lee.jones@linaro.org> wrote:
> On Mon, Sep 10, 2012 at 02:53:45AM -0700, Linus Walleij wrote:
>> On Fri, Sep 7, 2012 at 1:14 PM, Lee Jones <lee.jones@linaro.org> wrote:
>>
>> > +               msp3: msp@80125000 {
>> > +                       status = "okay";
>> > +               };
>> >         };
>> >  };
>>
>> Does this really "enable" the MSPs as the title claims?
>
> Yes they do.
>
>> Aren's there any kind of bindings needed, not even a
>> register base?
>
> All that information is in the "disabled" node contained in .dtsi file.

Argh it's hard to read DT patches without parsing through all the
.dtsi:s in your head first ...

Acked-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

[PATCH 07/19] ARM: ux500: Add all encompassing sound node to the HREF Device Tree

[Lee Jones]

This is where we link together all of the SoC sound components for
a given platform. This all encompassing sound node is only found
in the very lowest hierarchical DTS file, since the component mix may
well change from board variant to board variant.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 arch/arm/boot/dts/hrefv60plus.dts |    7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm/boot/dts/hrefv60plus.dts b/arch/arm/boot/dts/hrefv60plus.dts
index 4f0aee8..d8ed29e 100644
--- a/arch/arm/boot/dts/hrefv60plus.dts
+++ b/arch/arm/boot/dts/hrefv60plus.dts
@@ -78,6 +78,13 @@
 			status = "okay";
 		};
 
+		sound {
+			compatible = "stericsson,snd-soc-mop500";
+
+			stericsson,cpu-dai = <&msp1 &msp3>;
+			stericsson,audio-codec = <&codec>;
+		};
+
 		msp1: msp@80124000 {
 			status = "okay";
 		};
-- 
1.7.9.5

Re: [PATCH 07/19] ARM: ux500: Add all encompassing sound node to the HREF Device Tree

[Linus Walleij]

On Fri, Sep 7, 2012 at 1:14 PM, Lee Jones <lee.jones@linaro.org> wrote:

> This is where we link together all of the SoC sound components for
> a given platform. This all encompassing sound node is only found
> in the very lowest hierarchical DTS file, since the component mix may
> well change from board variant to board variant.
>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>
> ---
>  arch/arm/boot/dts/hrefv60plus.dts |    7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/arch/arm/boot/dts/hrefv60plus.dts b/arch/arm/boot/dts/hrefv60plus.dts
> index 4f0aee8..d8ed29e 100644
> --- a/arch/arm/boot/dts/hrefv60plus.dts
> +++ b/arch/arm/boot/dts/hrefv60plus.dts
> @@ -78,6 +78,13 @@
>                         status = "okay";
>                 };
>
> +               sound {
> +                       compatible = "stericsson,snd-soc-mop500";
> +
> +                       stericsson,cpu-dai = <&msp1 &msp3>;
> +                       stericsson,audio-codec = <&codec>;

Where is this codec coming from?

It's certainly not in the linux-next tree...

This is another of these patches that I prefer if Ola
or Roger or even Mark could comment on, I really don't know
too much about ASoC DT bindings...

Yours,
Linus Walleij

Re: [PATCH 07/19] ARM: ux500: Add all encompassing sound node to the HREF Device Tree

[Lee Jones]

On Mon, Sep 10, 2012 at 02:56:20AM -0700, Linus Walleij wrote:
> On Fri, Sep 7, 2012 at 1:14 PM, Lee Jones <lee.jones@linaro.org> wrote:
> 
> > This is where we link together all of the SoC sound components for
> > a given platform. This all encompassing sound node is only found
> > in the very lowest hierarchical DTS file, since the component mix may
> > well change from board variant to board variant.
> >
> > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> > ---
> >  arch/arm/boot/dts/hrefv60plus.dts |    7 +++++++
> >  1 file changed, 7 insertions(+)
> >
> > diff --git a/arch/arm/boot/dts/hrefv60plus.dts b/arch/arm/boot/dts/hrefv60plus.dts
> > index 4f0aee8..d8ed29e 100644
> > --- a/arch/arm/boot/dts/hrefv60plus.dts
> > +++ b/arch/arm/boot/dts/hrefv60plus.dts
> > @@ -78,6 +78,13 @@
> >                         status = "okay";
> >                 };
> >
> > +               sound {
> > +                       compatible = "stericsson,snd-soc-mop500";
> > +
> > +                       stericsson,cpu-dai = <&msp1 &msp3>;
> > +                       stericsson,audio-codec = <&codec>;
> 
> Where is this codec coming from?
> 
> It's certainly not in the linux-next tree...

No, I'm holding it locally.

My Audio patch-set is still awaiting Ola's Acks (6 patches)

... unless they're further down my mailbox.

/me keeps reading.

> This is another of these patches that I prefer if Ola
> or Roger or even Mark could comment on, I really don't know
> too much about ASoC DT bindings...



-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH 08/19] ARM: ux500: Stop registering Audio devices for HREF when DT is enabled

[Lee Jones]

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 arch/arm/mach-ux500/board-mop500.c |    2 --
 1 file changed, 2 deletions(-)

diff --git a/arch/arm/mach-ux500/board-mop500.c b/arch/arm/mach-ux500/board-mop500.c
index a42418b..0219c276 100644
--- a/arch/arm/mach-ux500/board-mop500.c
+++ b/arch/arm/mach-ux500/board-mop500.c
@@ -840,8 +840,6 @@ static void __init u8500_init_machine(void)
 		platform_add_devices(mop500_platform_devs,
 				ARRAY_SIZE(mop500_platform_devs));
 
-		mop500_audio_init(parent);
-
 		i2c0_devs = ARRAY_SIZE(mop500_i2c0_devices);
 		i2c0_devs -= NUM_PRE_V60_I2C0_DEVICES;
 
-- 
1.7.9.5

Re: [PATCH 08/19] ARM: ux500: Stop registering Audio devices for HREF when DT is enabled

[Linus Walleij]

On Fri, Sep 7, 2012 at 1:14 PM, Lee Jones <lee.jones@linaro.org> wrote:

> Signed-off-by: Lee Jones <lee.jones@linaro.org>

Acked-by: Linus Walleij <linus.walleij@linaro.org>

For this patch alone (because I see what it does) but I think there
are deep deps on earlier patches, some not even in this patch
set, that need to be ACKed by the audio people.

Yours,
Linus Walleij

[PATCH 09/19] ARM: ux500: Enable SSP (SPI) for HREF when booting Device Tree

[Lee Jones]

Here we add a new SSP node to the HREF's Device Tree file which
activates the generic one found in the .dtsi file. This will
allow probing of the SSP driver when Device Tree is enabled.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 arch/arm/boot/dts/hrefv60plus.dts |    4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/boot/dts/hrefv60plus.dts b/arch/arm/boot/dts/hrefv60plus.dts
index d8ed29e..49bdabd 100644
--- a/arch/arm/boot/dts/hrefv60plus.dts
+++ b/arch/arm/boot/dts/hrefv60plus.dts
@@ -92,5 +92,9 @@
 		msp3: msp@80125000 {
 			status = "okay";
 		};
+
+		ssp@80002000 {
+			status = "okay";
+		};
 	};
 };
-- 
1.7.9.5

Re: [PATCH 09/19] ARM: ux500: Enable SSP (SPI) for HREF when booting Device Tree

[Linus Walleij]

On Fri, Sep 7, 2012 at 1:14 PM, Lee Jones <lee.jones@linaro.org> wrote:

> Here we add a new SSP node to the HREF's Device Tree file which
> activates the generic one found in the .dtsi file. This will
> allow probing of the SSP driver when Device Tree is enabled.
>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>

We have to look closer at this.

There are merged PL022 DT bindings (by Roland Stigge)
nowadays, see commit 6d3952a7dfa80919842bbe01ac7f693d40a1eb84
in linux-next:

 Optional properties:
+- num-cs : total number of chipselects
 - cs-gpios : should specify GPIOs used for chipselects.
   The gpios will be referred to as reg = <index> in the SPI child nodes.
   If unspecified, a single SPI device without a chip select can be used.

+SPI slave nodes must be children of the SPI master node and can
+contain the following properties.
+
+- pl022,interface : interface type:
+       0: SPI
+       1: Texas Instruments Synchronous Serial Frame Format
+       2: Microwire (Half Duplex)
+- pl022,com-mode : polling, interrupt or dma
+- pl022,rx-level-trig : Rx FIFO watermark level
+- pl022,tx-level-trig : Tx FIFO watermark level
+- pl022,ctrl-len : Microwire interface: Control length
+- pl022,wait-state : Microwire interface: Wait state
+- pl022,duplex : Microwire interface: Full/Half duplex

Currently this is defined as auxdata in board-mop500.c
but notanly cs-gpios (num chipselects) should come from
device tree instead.

It appears Roland has written his bindings such that DT
data augments platform data (yes, I am also getting crazy
about this prioritization, mea culpa for ACKing this without
proper discussion) so it appears that you could actually
use AUXDATA and some stuff in the DT at the same
time.

Or you could keep it like this and only have AUXDATA
as it stands, I just want a discussion about what is the right
thing to do before we commit to anything.

Yours,
Linus Walleij

Re: [PATCH 09/19] ARM: ux500: Enable SSP (SPI) for HREF when booting Device Tree

[Lee Jones]

On Mon, Sep 10, 2012 at 04:11:10AM -0700, Linus Walleij wrote:
> On Fri, Sep 7, 2012 at 1:14 PM, Lee Jones <lee.jones@linaro.org> wrote:
> 
> > Here we add a new SSP node to the HREF's Device Tree file which
> > activates the generic one found in the .dtsi file. This will
> > allow probing of the SSP driver when Device Tree is enabled.
> >
> > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> 
> We have to look closer at this.
> 
> There are merged PL022 DT bindings (by Roland Stigge)
> nowadays, see commit 6d3952a7dfa80919842bbe01ac7f693d40a1eb84
> in linux-next:
> 
>  Optional properties:
> +- num-cs : total number of chipselects
>  - cs-gpios : should specify GPIOs used for chipselects.
>    The gpios will be referred to as reg = <index> in the SPI child nodes.
>    If unspecified, a single SPI device without a chip select can be used.
> 
> +SPI slave nodes must be children of the SPI master node and can
> +contain the following properties.
> +
> +- pl022,interface : interface type:
> +       0: SPI
> +       1: Texas Instruments Synchronous Serial Frame Format
> +       2: Microwire (Half Duplex)
> +- pl022,com-mode : polling, interrupt or dma
> +- pl022,rx-level-trig : Rx FIFO watermark level
> +- pl022,tx-level-trig : Tx FIFO watermark level
> +- pl022,ctrl-len : Microwire interface: Control length
> +- pl022,wait-state : Microwire interface: Wait state
> +- pl022,duplex : Microwire interface: Full/Half duplex
> 
> Currently this is defined as auxdata in board-mop500.c
> but notanly cs-gpios (num chipselects) should come from
> device tree instead.
> 
> It appears Roland has written his bindings such that DT
> data augments platform data (yes, I am also getting crazy
> about this prioritization, mea culpa for ACKing this without
> proper discussion) so it appears that you could actually
> use AUXDATA and some stuff in the DT at the same
> time.
> 
> Or you could keep it like this and only have AUXDATA
> as it stands, I just want a discussion about what is the right
> thing to do before we commit to anything.

Sounds fine.

I'll pull this for now.

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH 09/19] ARM: ux500: Enable SSP (SPI) for HREF when booting Device Tree

[Roland Stigge]

Hi!

On 09/10/2012 01:11 PM, Linus Walleij wrote:
> +SPI slave nodes must be children of the SPI master node and can
> +contain the following properties.
> +
> +- pl022,interface : interface type:
> +       0: SPI
> +       1: Texas Instruments Synchronous Serial Frame Format
> +       2: Microwire (Half Duplex)
> +- pl022,com-mode : polling, interrupt or dma
> +- pl022,rx-level-trig : Rx FIFO watermark level
> +- pl022,tx-level-trig : Tx FIFO watermark level
> +- pl022,ctrl-len : Microwire interface: Control length
> +- pl022,wait-state : Microwire interface: Wait state
> +- pl022,duplex : Microwire interface: Full/Half duplex
> 
> Currently this is defined as auxdata in board-mop500.c
> but notanly cs-gpios (num chipselects) should come from
> device tree instead.
> 
> It appears Roland has written his bindings such that DT
> data augments platform data (yes, I am also getting crazy
> about this prioritization, mea culpa for ACKing this without
> proper discussion) so it appears that you could actually
> use AUXDATA and some stuff in the DT at the same
> time.

Sorry for the incompleteness of the devicetree conversion. I'm sending a
patch (separately) that makes it possible to specify everything via
devicetree, so you can choose between dt and platform data.

Except in case of callback specification (dma_filter()), you need to
provide platform data.

Interestingly, when I removed the actual platform data from the board
file, I noticed that I still needed to specify a device name (like
dev:ssp0) to make it work. But this seems to be expected according to
the documentation of OF_DEV_AUXDATA(). Are there any plans or ideas how
to fix this?

When we have sorted out this driver change (please check the new pl022
specific dt property names!), I will provide patches for arm-soc to
actually use this new interface via dts files.

Thanks in advance,

Roland

Re: [PATCH 09/19] ARM: ux500: Enable SSP (SPI) for HREF when booting Device Tree

[Linus Walleij]

On Mon, Sep 17, 2012 at 7:03 PM, Roland Stigge <stigge@antcom.de> wrote:
> On 09/10/2012 01:11 PM, Linus Walleij wrote:
>>
>> It appears Roland has written his bindings such that DT
>> data augments platform data (yes, I am also getting crazy
>> about this prioritization, mea culpa for ACKing this without
>> proper discussion) so it appears that you could actually
>> use AUXDATA and some stuff in the DT at the same
>> time.
>
> Sorry for the incompleteness of the devicetree conversion. I'm sending a
> patch (separately) that makes it possible to specify everything via
> devicetree, so you can choose between dt and platform data.

OK it was not such a big deal, but many many thanks for fixing this
up! :-)

> Except in case of callback specification (dma_filter()), you need to
> provide platform data.

OK.

> Interestingly, when I removed the actual platform data from the board
> file, I noticed that I still needed to specify a device name (like
> dev:ssp0) to make it work. But this seems to be expected according to
> the documentation of OF_DEV_AUXDATA(). Are there any plans or ideas how
> to fix this?

This is very likely because the clock tree has a name like "dev:ssp0"
encoded for this device, and if you don't nail it down like that the
device name will be the same as the node in your device tree and
then clock lookup will fail.

The "real fix" is to convert the clock drivers to use device tree so
the drivers can just refer to the phandles to figure out what clock
node they need.

Along with the DMA channel mapping this is one of the major
roadblocks to finalizing the device tree adoptions.

> When we have sorted out this driver change (please check the new pl022
> specific dt property names!), I will provide patches for arm-soc to
> actually use this new interface via dts files.

OK cool I guess you will do this for the LPC32xx? Or are you testing
this on some other platforms?

Yours,
Linus Walleij

Re: [PATCH 09/19] ARM: ux500: Enable SSP (SPI) for HREF when booting Device Tree

[Roland Stigge]

Hi,

On 09/18/2012 02:08 PM, Linus Walleij wrote:
>> When we have sorted out this driver change (please check the new pl022
>> specific dt property names!), I will provide patches for arm-soc to
>> actually use this new interface via dts files.
> 
> OK cool I guess you will do this for the LPC32xx? Or are you testing
> this on some other platforms?

LPC32xx for now. The new code is backwards compatible, so I'd prefer for
others to adopt and test it on their platforms individually before
patching everything at once without testing.

Roland

[PATCH 10/19] ARM: ux500: Remove redundant #gpio-cell properties from HREF and Snowball DT

[Lee Jones]

These properties have no place here as the populated nodes are
not related to GPIO Controllers.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 arch/arm/boot/dts/hrefv60plus.dts |    1 -
 arch/arm/boot/dts/snowball.dts    |    1 -
 2 files changed, 2 deletions(-)

diff --git a/arch/arm/boot/dts/hrefv60plus.dts b/arch/arm/boot/dts/hrefv60plus.dts
index 49bdabd..7f0dddb 100644
--- a/arch/arm/boot/dts/hrefv60plus.dts
+++ b/arch/arm/boot/dts/hrefv60plus.dts
@@ -42,7 +42,6 @@
 			mmc-cap-mmc-highspeed;
 			vmmc-supply = <&ab8500_ldo_aux3_reg>;
 
-			#gpio-cells = <1>;
 			cd-gpios  = <&gpio2 31 0x4>; // 95
 
 			status = "okay";
diff --git a/arch/arm/boot/dts/snowball.dts b/arch/arm/boot/dts/snowball.dts
index 2969997..702c0ba 100644
--- a/arch/arm/boot/dts/snowball.dts
+++ b/arch/arm/boot/dts/snowball.dts
@@ -127,7 +127,6 @@
 			mmc-cap-mmc-highspeed;
 			vmmc-supply = <&ab8500_ldo_aux3_reg>;
 
-			#gpio-cells = <1>;
 			cd-gpios  = <&gpio6 26 0x4>; // 218
 			cd-inverted;
 
-- 
1.7.9.5

Re: [PATCH 10/19] ARM: ux500: Remove redundant #gpio-cell properties from HREF and Snowball DT

[Linus Walleij]

On Fri, Sep 7, 2012 at 1:14 PM, Lee Jones <lee.jones@linaro.org> wrote:

> These properties have no place here as the populated nodes are
> not related to GPIO Controllers.
>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>

Squash this into the previous patch adding them...
I think Arnd already noted.

Yours,
Linus Walleij

Re: [PATCH 10/19] ARM: ux500: Remove redundant #gpio-cell properties from HREF and Snowball DT

[Lee Jones]

On Mon, Sep 10, 2012 at 04:12:23AM -0700, Linus Walleij wrote:
> On Fri, Sep 7, 2012 at 1:14 PM, Lee Jones <lee.jones@linaro.org> wrote:
> 
> > These properties have no place here as the populated nodes are
> > not related to GPIO Controllers.
> >
> > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> 
> Squash this into the previous patch adding them...
> I think Arnd already noted.

Well he noted that I'd duplicated the error from the snowball.dts
file, then rectified in this patch, which is fair enough. However,
I don't think the snowball.dts fix should be squashed into an HREF
enablement patch, and I can't find a suitable patch in the patch-
set where it would really fit. Should I leave the Snowball fix
separate and just fixup the HREF .dts file to never duplicate the
anomaly?

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH 10/19] ARM: ux500: Remove redundant #gpio-cell properties from HREF and Snowball DT

[Linus Walleij]

On Fri, Sep 14, 2012 at 11:10 AM, Lee Jones <lee.jones@linaro.org> wrote:
> On Mon, Sep 10, 2012 at 04:12:23AM -0700, Linus Walleij wrote:
>> On Fri, Sep 7, 2012 at 1:14 PM, Lee Jones <lee.jones@linaro.org> wrote:
>>
>> > These properties have no place here as the populated nodes are
>> > not related to GPIO Controllers.
>> >
>> > Signed-off-by: Lee Jones <lee.jones@linaro.org>
>>
>> Squash this into the previous patch adding them...
>> I think Arnd already noted.
>
> Well he noted that I'd duplicated the error from the snowball.dts
> file, then rectified in this patch, which is fair enough. However,
> I don't think the snowball.dts fix should be squashed into an HREF
> enablement patch, and I can't find a suitable patch in the patch-
> set where it would really fit. Should I leave the Snowball fix
> separate and just fixup the HREF .dts file to never duplicate the
> anomaly?

No you're right, this should be separate.

I was confused that it was in the same patch set, I would
suggest just sending this one on it's own as a snowball fix
fair and square, and have ARM SoC push it to the -rcs as a
regression fix, but your choice.
Acked-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

[PATCH 11/19] ARM: ux500: Add all known I2C sub-device nodes to the HREF DT

[Lee Jones]

Here we add the skeleton nodes for each of the known I2C sub-devices
currently registered on ST-Ericsson's HREF Development Platform. We
will fill these nodes in turn as the drivers are enabled for Device
Tree.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 arch/arm/boot/dts/hrefv60plus.dts |   26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/arch/arm/boot/dts/hrefv60plus.dts b/arch/arm/boot/dts/hrefv60plus.dts
index 7f0dddb..ec9ec4f 100644
--- a/arch/arm/boot/dts/hrefv60plus.dts
+++ b/arch/arm/boot/dts/hrefv60plus.dts
@@ -95,5 +95,31 @@
 		ssp@80002000 {
 			status = "okay";
 		};
+
+		i2c@80004000 {
+			tc3589x@42 {
+				compatible = "tc3589x";
+				reg = <0x42>;
+			};
+			tps61052@33 {
+				compatible = "tps61052";
+				reg = <0x33>;
+			};
+		};
+
+		i2c@80128000 {
+			lp5521@0x33 {
+				compatible = "lp5521";
+				reg = <0x33>;
+			};
+			lp5521@0x34 {
+				compatible = "lp5521";
+				reg = <0x34>;
+			};
+			bh1780@0x29 {
+				compatible = "rohm,bh1780gli";
+				reg = <0x33>;
+			};
+		};
 	};
 };
-- 
1.7.9.5

Re: [PATCH 11/19] ARM: ux500: Add all known I2C sub-device nodes to the HREF DT

[Linus Walleij]

On Fri, Sep 7, 2012 at 1:14 PM, Lee Jones <lee.jones@linaro.org> wrote:

> Here we add the skeleton nodes for each of the known I2C sub-devices
> currently registered on ST-Ericsson's HREF Development Platform. We
> will fill these nodes in turn as the drivers are enabled for Device
> Tree.
>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>

Can't we have it the other way around (discussing patch ordering
with Arnd simultaneously), so you first add the DT support to the
subdevices, then add the more complete nodes to the device tree?

And if you put in a device tree like this, will these devices
not collide with the ones that are currently supplied from
the boardfiles? I'm a bit confused here, maybe I'm not smart
enough to understand this...

Yours,
Linus Walleij

Re: [PATCH 11/19] ARM: ux500: Add all known I2C sub-device nodes to the HREF DT

[Lee Jones]

On Mon, Sep 10, 2012 at 04:34:15AM -0700, Linus Walleij wrote:
> On Fri, Sep 7, 2012 at 1:14 PM, Lee Jones <lee.jones@linaro.org> wrote:
> 
> > Here we add the skeleton nodes for each of the known I2C sub-devices
> > currently registered on ST-Ericsson's HREF Development Platform. We
> > will fill these nodes in turn as the drivers are enabled for Device
> > Tree.
> >
> > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> 
> Can't we have it the other way around (discussing patch ordering
> with Arnd simultaneously), so you first add the DT support to the
> subdevices, then add the more complete nodes to the device tree?
> 
> And if you put in a device tree like this, will these devices
> not collide with the ones that are currently supplied from
> the boardfiles? I'm a bit confused here, maybe I'm not smart
> enough to understand this...

Okay, I've reordered the patch-set to address these possible issues.

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH 11/19] ARM: ux500: Add all known I2C sub-device nodes to the HREF DT

[Linus Walleij]

On Fri, Sep 14, 2012 at 10:47 AM, Lee Jones <lee.jones@linaro.org> wrote:
> On Mon, Sep 10, 2012 at 04:34:15AM -0700, Linus Walleij wrote:
>> On Fri, Sep 7, 2012 at 1:14 PM, Lee Jones <lee.jones@linaro.org> wrote:
>>
>> > Here we add the skeleton nodes for each of the known I2C sub-devices
>> > currently registered on ST-Ericsson's HREF Development Platform. We
>> > will fill these nodes in turn as the drivers are enabled for Device
>> > Tree.
>> >
>> > Signed-off-by: Lee Jones <lee.jones@linaro.org>
>>
>> Can't we have it the other way around (discussing patch ordering
>> with Arnd simultaneously), so you first add the DT support to the
>> subdevices, then add the more complete nodes to the device tree?
>>
>> And if you put in a device tree like this, will these devices
>> not collide with the ones that are currently supplied from
>> the boardfiles? I'm a bit confused here, maybe I'm not smart
>> enough to understand this...
>
> Okay, I've reordered the patch-set to address these possible issues.

OK sorry for delay, I've been patch-swamped...

Acked-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

Re: [PATCH 11/19] ARM: ux500: Add all known I2C sub-device nodes to the HREF DT

[Lee Jones]

On Mon, Sep 10, 2012 at 04:34:15AM -0700, Linus Walleij wrote:
> On Fri, Sep 7, 2012 at 1:14 PM, Lee Jones <lee.jones@linaro.org> wrote:
> 
> > Here we add the skeleton nodes for each of the known I2C sub-devices
> > currently registered on ST-Ericsson's HREF Development Platform. We
> > will fill these nodes in turn as the drivers are enabled for Device
> > Tree.
> >
> > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> 
> Can't we have it the other way around (discussing patch ordering
> with Arnd simultaneously), so you first add the DT support to the
> subdevices, then add the more complete nodes to the device tree?
> 
> And if you put in a device tree like this, will these devices
> not collide with the ones that are currently supplied from
> the boardfiles? I'm a bit confused here, maybe I'm not smart
> enough to understand this...

We finished this conversation about future ordering didn't we?

Would you mind reviewing the code in the patch please, as it's
still missing your Ack, so can't send.

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH 12/19] i2c-nomadik: Register sub-devices when passed via Device Tree

[Lee Jones]

Each I2C device can be correctly probed already using Device Tree,
but the sub-devices still have to be registered by calls to
i2c_register_board_info(). After this patch, each sub-device can
be registered directly from Device Tree instead, removing the
requirement for the aforementioned calls from platform code.

CC: linux-i2c@vger.kernel.org
CC: Wolfram Sang <w.sang@pengutronix.de>
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/i2c/busses/i2c-nomadik.c |    4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/i2c/busses/i2c-nomadik.c b/drivers/i2c/busses/i2c-nomadik.c
index 5d1a970..01231c2 100644
--- a/drivers/i2c/busses/i2c-nomadik.c
+++ b/drivers/i2c/busses/i2c-nomadik.c
@@ -26,6 +26,7 @@
 #include <linux/pm_runtime.h>
 #include <linux/platform_data/i2c-nomadik.h>
 #include <linux/of.h>
+#include <linux/of_i2c.h>
 
 #define DRIVER_NAME "nmk-i2c"
 
@@ -998,6 +999,7 @@ static int nmk_i2c_probe(struct amba_device *adev, const struct amba_id *id)
 	}
 
 	adap = &dev->adap;
+	adap->dev.of_node = np;
 	adap->dev.parent = &adev->dev;
 	adap->owner	= THIS_MODULE;
 	adap->class	= I2C_CLASS_HWMON | I2C_CLASS_SPD;
@@ -1027,6 +1029,8 @@ static int nmk_i2c_probe(struct amba_device *adev, const struct amba_id *id)
 		goto err_add_adap;
 	}
 
+	of_i2c_register_devices(adap);
+
 	pm_runtime_put(&adev->dev);
 
 	return 0;
-- 
1.7.9.5

Re: [PATCH 12/19] i2c-nomadik: Register sub-devices when passed via Device Tree

[Linus Walleij]

On Fri, Sep 7, 2012 at 1:14 PM, Lee Jones <lee.jones@linaro.org> wrote:

> Each I2C device can be correctly probed already using Device Tree,
> but the sub-devices still have to be registered by calls to
> i2c_register_board_info(). After this patch, each sub-device can
> be registered directly from Device Tree instead, removing the
> requirement for the aforementioned calls from platform code.
>
> CC: linux-i2c@vger.kernel.org
> CC: Wolfram Sang <w.sang@pengutronix.de>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>

The patch as such is fine.
Acked-by: Linus Walleij <linus.walleij@linaro.org>

...but there will be merge issues (of the type I'm
discussing with Arnd in another thread).

> ---
>  drivers/i2c/busses/i2c-nomadik.c |    4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/drivers/i2c/busses/i2c-nomadik.c b/drivers/i2c/busses/i2c-nomadik.c
> index 5d1a970..01231c2 100644
> --- a/drivers/i2c/busses/i2c-nomadik.c
> +++ b/drivers/i2c/busses/i2c-nomadik.c
> @@ -26,6 +26,7 @@
>  #include <linux/pm_runtime.h>
>  #include <linux/platform_data/i2c-nomadik.h>
>  #include <linux/of.h>
> +#include <linux/of_i2c.h>

This is clearly dependent on the other patches to I2C to
be merged first (the include above is not yet there in
linux-next even), so this patch should go into Wolfram's
tree right? Else we have to rip out all the patches to the
I2C driver from his tree and funnel it all through ARM SoC.

With the changes to the device tree itself being dependent
on this, they should thus also go through the I2C tree.

But they may be heavily dependent on the other DT
changes you've done, so they cannot be applied to
Wolfram's tree...

And thus you may need to hold this change off
until the I2C changes are upstream, or start creating
cross-tree dependencies.

Maybe I shouldn't worry about these things and just
ACK stuff, but I have such problems with cross-tree
merges myself so just trying to be helpful :-/

Yours,
Linus Walleij

Re: [PATCH 12/19] i2c-nomadik: Register sub-devices when passed via Device Tree

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 2370 bytes --]

On Mon, Sep 10, 2012 at 04:42:36AM -0700, Linus Walleij wrote:
> On Fri, Sep 7, 2012 at 1:14 PM, Lee Jones <lee.jones@linaro.org> wrote:
> 
> > Each I2C device can be correctly probed already using Device Tree,
> > but the sub-devices still have to be registered by calls to
> > i2c_register_board_info(). After this patch, each sub-device can
> > be registered directly from Device Tree instead, removing the
> > requirement for the aforementioned calls from platform code.
> >
> > CC: linux-i2c@vger.kernel.org
> > CC: Wolfram Sang <w.sang@pengutronix.de>
> > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> 
> The patch as such is fine.
> Acked-by: Linus Walleij <linus.walleij@linaro.org>
> 
> ...but there will be merge issues (of the type I'm
> discussing with Arnd in another thread).
> 
> > ---
> >  drivers/i2c/busses/i2c-nomadik.c |    4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/drivers/i2c/busses/i2c-nomadik.c b/drivers/i2c/busses/i2c-nomadik.c
> > index 5d1a970..01231c2 100644
> > --- a/drivers/i2c/busses/i2c-nomadik.c
> > +++ b/drivers/i2c/busses/i2c-nomadik.c
> > @@ -26,6 +26,7 @@
> >  #include <linux/pm_runtime.h>
> >  #include <linux/platform_data/i2c-nomadik.h>
> >  #include <linux/of.h>
> > +#include <linux/of_i2c.h>
> 
> This is clearly dependent on the other patches to I2C to
> be merged first (the include above is not yet there in
> linux-next even), so this patch should go into Wolfram's
> tree right? Else we have to rip out all the patches to the
> I2C driver from his tree and funnel it all through ARM SoC.

First, I'd like to have this patch squashed with "i2c: nomadik: Add
Device Tree support to the Nomadik I2C driver". I wanted to do this on
my own, but the patches do not apply to 3.6-rc5 (with or without
regulator removal patch from Linus)?

I can also take the I2C related changes to the devicetrees via my tree.
This is not uncommon. Some people prefer to do this via their soc-trees,
though. I don't care much since this is not really a hard dependency
causing build failures or merge conflicts, but just needs a little extra
time until the patches are all there...

Thanks,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH 12/19] i2c-nomadik: Register sub-devices when passed via Device Tree

[Lee Jones]

On Wed, Sep 12, 2012 at 12:52:02PM +0200, Wolfram Sang wrote:
> On Mon, Sep 10, 2012 at 04:42:36AM -0700, Linus Walleij wrote:
> > On Fri, Sep 7, 2012 at 1:14 PM, Lee Jones <lee.jones@linaro.org> wrote:
> > 
> > > Each I2C device can be correctly probed already using Device Tree,
> > > but the sub-devices still have to be registered by calls to
> > > i2c_register_board_info(). After this patch, each sub-device can
> > > be registered directly from Device Tree instead, removing the
> > > requirement for the aforementioned calls from platform code.
> > >
> > > CC: linux-i2c@vger.kernel.org
> > > CC: Wolfram Sang <w.sang@pengutronix.de>
> > > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> > 
> > The patch as such is fine.
> > Acked-by: Linus Walleij <linus.walleij@linaro.org>
> > 
> > ...but there will be merge issues (of the type I'm
> > discussing with Arnd in another thread).
> > 
> > > ---
> > >  drivers/i2c/busses/i2c-nomadik.c |    4 ++++
> > >  1 file changed, 4 insertions(+)
> > >
> > > diff --git a/drivers/i2c/busses/i2c-nomadik.c b/drivers/i2c/busses/i2c-nomadik.c
> > > index 5d1a970..01231c2 100644
> > > --- a/drivers/i2c/busses/i2c-nomadik.c
> > > +++ b/drivers/i2c/busses/i2c-nomadik.c
> > > @@ -26,6 +26,7 @@
> > >  #include <linux/pm_runtime.h>
> > >  #include <linux/platform_data/i2c-nomadik.h>
> > >  #include <linux/of.h>
> > > +#include <linux/of_i2c.h>
> > 
> > This is clearly dependent on the other patches to I2C to
> > be merged first (the include above is not yet there in
> > linux-next even), so this patch should go into Wolfram's
> > tree right? Else we have to rip out all the patches to the
> > I2C driver from his tree and funnel it all through ARM SoC.
> 
> First, I'd like to have this patch squashed with "i2c: nomadik: Add
> Device Tree support to the Nomadik I2C driver". I wanted to do this on
> my own, but the patches do not apply to 3.6-rc5 (with or without
> regulator removal patch from Linus)?

I'm really not keen on squashing all my patches together. They are
clearly have very different purposes. If you think they are closely
related, then pull them in sequentially, but please don't squash
all my work into a single patch for no other reason than convenience.

> I can also take the I2C related changes to the devicetrees via my tree.
> This is not uncommon. Some people prefer to do this via their soc-trees,
> though. I don't care much since this is not really a hard dependency
> causing build failures or merge conflicts, but just needs a little extra
> time until the patches are all there...

It would be better for all the Device Tree changes go in as a single
patch-set. Again, I don't care where they go, so long as they go in
together. arm-soc seems like the most generic place for them to be
pulled into though.

Kind regards,
Lee

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH 12/19] i2c-nomadik: Register sub-devices when passed via Device Tree

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 1913 bytes --]

Hi,

> > First, I'd like to have this patch squashed with "i2c: nomadik: Add
> > Device Tree support to the Nomadik I2C driver". I wanted to do this on
> > my own, but the patches do not apply to 3.6-rc5 (with or without
> > regulator removal patch from Linus)?
> 
> I'm really not keen on squashing all my patches together. They are
> clearly have very different purposes. If you think they are closely
> related, then pull them in sequentially, but please don't squash
> all my work into a single patch for no other reason than convenience.

I can't follow this reasoning. I never asked you to squash all patches,
only those two needed to get proper device tree support. Why would you
want to let the device being detected via DT and not scan the child
nodes immediately?

> > I can also take the I2C related changes to the devicetrees via my tree.
> > This is not uncommon. Some people prefer to do this via their soc-trees,
> > though. I don't care much since this is not really a hard dependency
> > causing build failures or merge conflicts, but just needs a little extra
> > time until the patches are all there...
> 
> It would be better for all the Device Tree changes go in as a single
> patch-set. Again, I don't care where they go, so long as they go in
> together. arm-soc seems like the most generic place for them to be
> pulled into though.

This reasoning I can follow, but how should I know you aimed for that? I
only saw a patch [3/3] in one series making the driver probable via DT
and a patch [12/19] in another series to scan the child nodes. That's
all the infos I got. Some more context would have been helpful. Is there
a branch somewhere with all the things collected?

Regards,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH 12/19] i2c-nomadik: Register sub-devices when passed via Device Tree

[Lee Jones]

> > > First, I'd like to have this patch squashed with "i2c: nomadik: Add
> > > Device Tree support to the Nomadik I2C driver". I wanted to do this on
> > > my own, but the patches do not apply to 3.6-rc5 (with or without
> > > regulator removal patch from Linus)?
> > 
> > I'm really not keen on squashing all my patches together. They are
> > clearly have very different purposes. If you think they are closely
> > related, then pull them in sequentially, but please don't squash
> > all my work into a single patch for no other reason than convenience.
> 
> I can't follow this reasoning. I never asked you to squash all patches,
> only those two needed to get proper device tree support. Why would you
> want to let the device being detected via DT and not scan the child
> nodes immediately?

Ah, sorry. That's my fault for rushing though my ridiculously bloated post-
vacation inbox. I *stupidly* thought you wanted me to squash two different
patches, rather than these two. As such I unreservedly retract my previous
statement. Yes, please squash.

> > > I can also take the I2C related changes to the devicetrees via my tree.
> > > This is not uncommon. Some people prefer to do this via their soc-trees,
> > > though. I don't care much since this is not really a hard dependency
> > > causing build failures or merge conflicts, but just needs a little extra
> > > time until the patches are all there...
> > 
> > It would be better for all the Device Tree changes go in as a single
> > patch-set. Again, I don't care where they go, so long as they go in
> > together. arm-soc seems like the most generic place for them to be
> > pulled into though.
> 
> This reasoning I can follow, but how should I know you aimed for that? I
> only saw a patch [3/3] in one series making the driver probable via DT
> and a patch [12/19] in another series to scan the child nodes. That's
> all the infos I got. Some more context would have been helpful. Is there
> a branch somewhere with all the things collected?

There will be. I'm currently just Ack collecting. 

In fact wait ...

<3 mins pass>

Now there is:
  git://git.linaro.org/people/ljones/linux-3.0-ux500.git preview-for-next

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH 12/19] i2c-nomadik: Register sub-devices when passed via Device Tree

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 1227 bytes --]


> Ah, sorry. That's my fault for rushing though my ridiculously bloated post-
> vacation inbox.

:)

> > all the infos I got. Some more context would have been helpful. Is there
> > a branch somewhere with all the things collected?
> 
> There will be. I'm currently just Ack collecting. 
> 
> In fact wait ...
> 
> <3 mins pass>
> 
> Now there is:
>   git://git.linaro.org/people/ljones/linux-3.0-ux500.git preview-for-next

OK from having a glimpse, I'd prefer to squash (or have squashed)

	i2c: nomadik: Add Device Tree support to the Nomadik I2C driver
	Documentation: Device Tree binding information for i2c-nomadik driver
		(Documentation should go along with the code)
	i2c-nomadik: Register sub-devices when passed via Device Tree

into one patch and take that via the I2C tree. This will allow me to
resolve the merge conflict in my tree on my own. I don't see a strong
reason to have those go via arm-soc (like prevented build-failures or
easier merges), if I missed such a reason, please let me know.

Regards,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH 12/19] i2c-nomadik: Register sub-devices when passed via Device Tree

[Lee Jones]

> > Now there is:
> >   git://git.linaro.org/people/ljones/linux-3.0-ux500.git preview-for-next
> 
> OK from having a glimpse, I'd prefer to squash (or have squashed)
> 
> 	i2c: nomadik: Add Device Tree support to the Nomadik I2C driver
> 	i2c-nomadik: Register sub-devices when passed via Device Tree

Sure, no problem

> 	Documentation: Device Tree binding information for i2c-nomadik driver
> 		(Documentation should go along with the code)

I'm still not too sure about how this benefits anyone. You can ensure
the documentation is placed in an adjacent patch, so it's just as easy
to make reference to.

> into one patch and take that via the I2C tree. This will allow me to
> resolve the merge conflict in my tree on my own. I don't see a strong
> reason to have those go via arm-soc (like prevented build-failures or
> easier merges), if I missed such a reason, please let me know.

I don't have any good reasons for you not to take the patch(es) though
your tree. Please go ahead.

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH 12/19] i2c-nomadik: Register sub-devices when passed via Device Tree

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 693 bytes --]


> > 	Documentation: Device Tree binding information for i2c-nomadik driver
> > 		(Documentation should go along with the code)
> 
> I'm still not too sure about how this benefits anyone. You can ensure
> the documentation is placed in an adjacent patch, so it's just as easy
> to make reference to.

It makes reviewing a lot easier if the documentation what is supposed to
be coded and the actual code are in one place. It also prevents what
happened in this case where the documentation never showed up on the
i2c-list.

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH 12/19] i2c-nomadik: Register sub-devices when passed via Device Tree

[Lee Jones]

Hi Wolfram,

Are you taking these soon? I've rebased on -next, but they're
still in my local delta.

Kind regards,
Lee

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH 12/19] i2c-nomadik: Register sub-devices when passed via Device Tree

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 306 bytes --]


> Are you taking these soon? I've rebased on -next, but they're
> still in my local delta.

Applied to -next now, will push out later today.

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH 12/19] i2c-nomadik: Register sub-devices when passed via Device Tree

[Lee Jones]

On Mon, Sep 10, 2012 at 04:42:36AM -0700, Linus Walleij wrote:
> On Fri, Sep 7, 2012 at 1:14 PM, Lee Jones <lee.jones@linaro.org> wrote:
> 
> > Each I2C device can be correctly probed already using Device Tree,
> > but the sub-devices still have to be registered by calls to
> > i2c_register_board_info(). After this patch, each sub-device can
> > be registered directly from Device Tree instead, removing the
> > requirement for the aforementioned calls from platform code.
> >
> > CC: linux-i2c@vger.kernel.org
> > CC: Wolfram Sang <w.sang@pengutronix.de>
> > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> 
> The patch as such is fine.
> Acked-by: Linus Walleij <linus.walleij@linaro.org>
> 
> ...but there will be merge issues (of the type I'm
> discussing with Arnd in another thread).
> 
> > ---
> >  drivers/i2c/busses/i2c-nomadik.c |    4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/drivers/i2c/busses/i2c-nomadik.c b/drivers/i2c/busses/i2c-nomadik.c
> > index 5d1a970..01231c2 100644
> > --- a/drivers/i2c/busses/i2c-nomadik.c
> > +++ b/drivers/i2c/busses/i2c-nomadik.c
> > @@ -26,6 +26,7 @@
> >  #include <linux/pm_runtime.h>
> >  #include <linux/platform_data/i2c-nomadik.h>
> >  #include <linux/of.h>
> > +#include <linux/of_i2c.h>
> 
> This is clearly dependent on the other patches to I2C to
> be merged first (the include above is not yet there in
> linux-next even), so this patch should go into Wolfram's
> tree right? Else we have to rip out all the patches to the
> I2C driver from his tree and funnel it all through ARM SoC.
> 
> With the changes to the device tree itself being dependent
> on this, they should thus also go through the I2C tree.
> 
> But they may be heavily dependent on the other DT
> changes you've done, so they cannot be applied to
> Wolfram's tree...
> 
> And thus you may need to hold this change off
> until the I2C changes are upstream, or start creating
> cross-tree dependencies.
> 
> Maybe I shouldn't worry about these things and just
> ACK stuff, but I have such problems with cross-tree
> merges myself so just trying to be helpful :-/

If I send all my stuff though arm-soc, then I have no
dependencies at all. All my patches are based on the latest
-rc. It seems a little silly to split them all up and push
them through lots of different trees. That way we're bound
to increase the chances of merge conflicts surely?

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH 13/19] ARM: ux500: Stop registering I2C sub-devices for HREF when DT is enabled

[Lee Jones]

Here we remove all calls to register I2C sub-devices from platform
code when Device Tree is enabled. Instead the I2C driver will parse
the Device Tree for them.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 arch/arm/mach-ux500/board-mop500.c |    7 -------
 1 file changed, 7 deletions(-)

diff --git a/arch/arm/mach-ux500/board-mop500.c b/arch/arm/mach-ux500/board-mop500.c
index 0219c276..181ed25 100644
--- a/arch/arm/mach-ux500/board-mop500.c
+++ b/arch/arm/mach-ux500/board-mop500.c
@@ -840,13 +840,6 @@ static void __init u8500_init_machine(void)
 		platform_add_devices(mop500_platform_devs,
 				ARRAY_SIZE(mop500_platform_devs));
 
-		i2c0_devs = ARRAY_SIZE(mop500_i2c0_devices);
-		i2c0_devs -= NUM_PRE_V60_I2C0_DEVICES;
-
-		i2c_register_board_info(0, mop500_i2c0_devices, i2c0_devs);
-		i2c_register_board_info(2, mop500_i2c2_devices,
-					ARRAY_SIZE(mop500_i2c2_devices));
-
 		mop500_uib_init();
 	}
 
-- 
1.7.9.5

Re: [PATCH 13/19] ARM: ux500: Stop registering I2C sub-devices for HREF when DT is enabled

[Linus Walleij]

On Fri, Sep 7, 2012 at 1:14 PM, Lee Jones <lee.jones@linaro.org> wrote:

> Here we remove all calls to register I2C sub-devices from platform
> code when Device Tree is enabled. Instead the I2C driver will parse
> the Device Tree for them.
>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>

Acked-by: Linus Walleij <linus.walleij@linaro.org>
(But you need the deps in place first, of course.)

Yours,
Linus Walleij

[PATCH 14/19] ARM: ux500: Apply tc3589x's GPIO/IRQ properties to HREF's DT

[Lee Jones]

This patch ensures the tc3589x and tc3598x-gpio devices are
rightfully given interrupt controller status. We also describe
the tc3598x-gpio GPIO expander in full and specify it as a GPIO
controller. Finally we reference that the external MMC slot's
Card Detection GPIO on the HREF is located on the tc3598x-gpio
expander.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 arch/arm/boot/dts/hrefv60plus.dts |   17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/hrefv60plus.dts b/arch/arm/boot/dts/hrefv60plus.dts
index ec9ec4f..b3290c7 100644
--- a/arch/arm/boot/dts/hrefv60plus.dts
+++ b/arch/arm/boot/dts/hrefv60plus.dts
@@ -42,7 +42,7 @@
 			mmc-cap-mmc-highspeed;
 			vmmc-supply = <&ab8500_ldo_aux3_reg>;
 
-			cd-gpios  = <&gpio2 31 0x4>; // 95
+			cd-gpios  = <&tc3589x_gpio 3 0x4>;
 
 			status = "okay";
 		};
@@ -100,6 +100,21 @@
 			tc3589x@42 {
 				compatible = "tc3589x";
 				reg = <0x42>;
+				interrupt-parent = <&gpio6>;
+				interrupts = <25 0x1>;
+
+				interrupt-controller;
+				#interrupt-cells = <2>;
+
+				tc3589x_gpio: tc3589x_gpio {
+					compatible = "tc3589x-gpio";
+					interrupts = <0 0x1>;
+
+					interrupt-controller;
+					#interrupt-cells = <2>;
+					gpio-controller;
+					#gpio-cells = <2>;
+				};
 			};
 			tps61052@33 {
 				compatible = "tps61052";
-- 
1.7.9.5

Re: [PATCH 14/19] ARM: ux500: Apply tc3589x's GPIO/IRQ properties to HREF's DT

[Linus Walleij]

On Fri, Sep 7, 2012 at 1:14 PM, Lee Jones <lee.jones@linaro.org> wrote:

> This patch ensures the tc3589x and tc3598x-gpio devices are
> rightfully given interrupt controller status. We also describe
> the tc3598x-gpio GPIO expander in full and specify it as a GPIO
> controller. Finally we reference that the external MMC slot's
> Card Detection GPIO on the HREF is located on the tc3598x-gpio
> expander.
>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>

This should come after the enablement patches instead of
before them, right? (Or be merged into them even as
we've discussed.)

Apart from that it looks good,
Acked-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

Re: [PATCH 14/19] ARM: ux500: Apply tc3589x's GPIO/IRQ properties to HREF's DT

[Lee Jones]

On Mon, Sep 10, 2012 at 05:58:41AM -0700, Linus Walleij wrote:
> On Fri, Sep 7, 2012 at 1:14 PM, Lee Jones <lee.jones@linaro.org> wrote:
> 
> > This patch ensures the tc3589x and tc3598x-gpio devices are
> > rightfully given interrupt controller status. We also describe
> > the tc3598x-gpio GPIO expander in full and specify it as a GPIO
> > controller. Finally we reference that the external MMC slot's
> > Card Detection GPIO on the HREF is located on the tc3598x-gpio
> > expander.
> >
> > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> 
> This should come after the enablement patches instead of
> before them, right? (Or be merged into them even as
> we've discussed.)

No, it needs to come before, or else the legacy values will
be used which aren't correct.

> Apart from that it looks good,
> Acked-by: Linus Walleij <linus.walleij@linaro.org>
> 
> Yours,
> Linus Walleij

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH 15/19] mfd: Don't convert just one IRQ using irqdomain if a range is provided

[Lee Jones]

MFD core code attempts to convert specified hardware (local) IRQ
numbers to virtual-IRQs, which something Linux can understand. This
works great when only one IRQ is specified. However, converting
entire ranges is currently unsupported. If this occurs we issue a
kernel warning to inform the user of this, but we continue to
convert the first specified IRQ anyway and replace the range. This
is not the correct behaviour. This patch ensures that if a range
is specified, it is left untouched.

CC: Samuel Ortiz <sameo@linux.intel.com>
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/mfd/mfd-core.c |   10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/mfd/mfd-core.c b/drivers/mfd/mfd-core.c
index f8b7771..f07cf69 100644
--- a/drivers/mfd/mfd-core.c
+++ b/drivers/mfd/mfd-core.c
@@ -126,10 +126,12 @@ static int mfd_add_device(struct device *parent, int id,
 		} else if (cell->resources[r].flags & IORESOURCE_IRQ) {
 			if (domain) {
 				/* Unable to create mappings for IRQ ranges. */
-				WARN_ON(cell->resources[r].start !=
-					cell->resources[r].end);
-				res[r].start = res[r].end = irq_create_mapping(
-					domain, cell->resources[r].start);
+				if (!WARN_ON(cell->resources[r].start !=
+						cell->resources[r].end))
+					res[r].start = res[r].end =
+						irq_create_mapping(
+							domain,
+							cell->resources[r].start);
 			} else {
 				res[r].start = irq_base +
 					cell->resources[r].start;
-- 
1.7.9.5

Re: [PATCH 15/19] mfd: Don't convert just one IRQ using irqdomain if a range is provided

[Arnd Bergmann]

On Friday 07 September 2012, Lee Jones wrote:
> MFD core code attempts to convert specified hardware (local) IRQ
> numbers to virtual-IRQs, which something Linux can understand. This
> works great when only one IRQ is specified. However, converting
> entire ranges is currently unsupported. If this occurs we issue a
> kernel warning to inform the user of this, but we continue to
> convert the first specified IRQ anyway and replace the range. This
> is not the correct behaviour. This patch ensures that if a range
> is specified, it is left untouched.
> 
> CC: Samuel Ortiz <sameo@linux.intel.com>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>

I don't see the advantage of the change. The warning already tells
us that the input to mfd_add_device was incorrect, so nothing the
function does can reliably fix it. Leaving the resource empty
is just as wrong as listing only the first interrupt.

	Arnd

Re: [PATCH 15/19] mfd: Don't convert just one IRQ using irqdomain if a range is provided

[Lee Jones]

On Fri, Sep 07, 2012 at 12:35:41PM +0000, Arnd Bergmann wrote:
> On Friday 07 September 2012, Lee Jones wrote:
> > MFD core code attempts to convert specified hardware (local) IRQ
> > numbers to virtual-IRQs, which something Linux can understand. This
> > works great when only one IRQ is specified. However, converting
> > entire ranges is currently unsupported. If this occurs we issue a
> > kernel warning to inform the user of this, but we continue to
> > convert the first specified IRQ anyway and replace the range. This
> > is not the correct behaviour. This patch ensures that if a range
> > is specified, it is left untouched.
> > 
> > CC: Samuel Ortiz <sameo@linux.intel.com>
> > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> 
> I don't see the advantage of the change. The warning already tells
> us that the input to mfd_add_device was incorrect, so nothing the
> function does can reliably fix it. Leaving the resource empty
> is just as wrong as listing only the first interrupt.

My thinking was to leave them in a range, then have the target driver
convert each entry in the range manually. But what you're saying is
that no registration should be attempted using ranges at all? What if
the range was vast? Surely a resource array a few hundred lines long
isn't correct either? My immediate example would be "GPIO_INT6", which
has 31 lines. Do you want them all splitting out individually?

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH 15/19] mfd: Don't convert just one IRQ using irqdomain if a range is provided

[Arnd Bergmann]

On Friday 07 September 2012, Lee Jones wrote:
> On Fri, Sep 07, 2012 at 12:35:41PM +0000, Arnd Bergmann wrote:
> > On Friday 07 September 2012, Lee Jones wrote:
> > > MFD core code attempts to convert specified hardware (local) IRQ
> > > numbers to virtual-IRQs, which something Linux can understand. This
> > > works great when only one IRQ is specified. However, converting
> > > entire ranges is currently unsupported. If this occurs we issue a
> > > kernel warning to inform the user of this, but we continue to
> > > convert the first specified IRQ anyway and replace the range. This
> > > is not the correct behaviour. This patch ensures that if a range
> > > is specified, it is left untouched.
> > > 
> > > CC: Samuel Ortiz <sameo@linux.intel.com>
> > > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> > 
> > I don't see the advantage of the change. The warning already tells
> > us that the input to mfd_add_device was incorrect, so nothing the
> > function does can reliably fix it. Leaving the resource empty
> > is just as wrong as listing only the first interrupt.
> 
> My thinking was to leave them in a range, then have the target driver
> convert each entry in the range manually. But what you're saying is
> that no registration should be attempted using ranges at all? What if
> the range was vast? Surely a resource array a few hundred lines long
> isn't correct either? My immediate example would be "GPIO_INT6", which
> has 31 lines. Do you want them all splitting out individually?

The examples I had seen before were all just ranges of two interrupts,
and in those cases it was clear that splitting them would be best.

In the exampled of the ab8500-gpio driver, it looks like the resource is
not actually being used, and the gpio driver implements its own irq_chip,
so maybe we can get away with not solving this problem for now.

	Arnd

Re: [PATCH 15/19] mfd: Don't convert just one IRQ using irqdomain if a range is provided

[Lee Jones]

On Fri, Sep 07, 2012 at 01:37:26PM +0000, Arnd Bergmann wrote:
> On Friday 07 September 2012, Lee Jones wrote:
> > On Fri, Sep 07, 2012 at 12:35:41PM +0000, Arnd Bergmann wrote:
> > > On Friday 07 September 2012, Lee Jones wrote:
> > > > MFD core code attempts to convert specified hardware (local) IRQ
> > > > numbers to virtual-IRQs, which something Linux can understand. This
> > > > works great when only one IRQ is specified. However, converting
> > > > entire ranges is currently unsupported. If this occurs we issue a
> > > > kernel warning to inform the user of this, but we continue to
> > > > convert the first specified IRQ anyway and replace the range. This
> > > > is not the correct behaviour. This patch ensures that if a range
> > > > is specified, it is left untouched.
> > > > 
> > > > CC: Samuel Ortiz <sameo@linux.intel.com>
> > > > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> > > 
> > > I don't see the advantage of the change. The warning already tells
> > > us that the input to mfd_add_device was incorrect, so nothing the
> > > function does can reliably fix it. Leaving the resource empty
> > > is just as wrong as listing only the first interrupt.
> > 
> > My thinking was to leave them in a range, then have the target driver
> > convert each entry in the range manually. But what you're saying is
> > that no registration should be attempted using ranges at all? What if
> > the range was vast? Surely a resource array a few hundred lines long
> > isn't correct either? My immediate example would be "GPIO_INT6", which
> > has 31 lines. Do you want them all splitting out individually?
> 
> The examples I had seen before were all just ranges of two interrupts,
> and in those cases it was clear that splitting them would be best.
> 
> In the exampled of the ab8500-gpio driver, it looks like the resource is
> not actually being used, and the gpio driver implements its own irq_chip,
> so maybe we can get away with not solving this problem for now.

Understood. I'd still feel more comfortable if we didn't trash the
range. I think it would be best to show the warning, and leave the
range for its target driver to take care - hence the patch.

... but it's your call.

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH 15/19] mfd: Don't convert just one IRQ using irqdomain if a range is provided

[Arnd Bergmann]

On Friday 07 September 2012, Lee Jones wrote:
> On Fri, Sep 07, 2012 at 01:37:26PM +0000, Arnd Bergmann wrote:
> > On Friday 07 September 2012, Lee Jones wrote:
> > > On Fri, Sep 07, 2012 at 12:35:41PM +0000, Arnd Bergmann wrote:
> >
> > The examples I had seen before were all just ranges of two interrupts,
> > and in those cases it was clear that splitting them would be best.
> > 
> > In the exampled of the ab8500-gpio driver, it looks like the resource is
> > not actually being used, and the gpio driver implements its own irq_chip,
> > so maybe we can get away with not solving this problem for now.
> 
> Understood. I'd still feel more comfortable if we didn't trash the
> range. I think it would be best to show the warning, and leave the
> range for its target driver to take care - hence the patch.
> 
> ... but it's your call.

I'm fine with whatever Samuel sees fit here.  My personal opinion is
that leaving the range alone for the child driver to do the conversion
would be too inconsistent and only lead to confusion with driver authors.

	Arnd

Re: [PATCH 15/19] mfd: Don't convert just one IRQ using irqdomain if a range is provided

[Samuel Ortiz]

Hi Lee, Arnd,

On Fri, Sep 07, 2012 at 01:57:27PM +0000, Arnd Bergmann wrote:
> On Friday 07 September 2012, Lee Jones wrote:
> > On Fri, Sep 07, 2012 at 01:37:26PM +0000, Arnd Bergmann wrote:
> > > On Friday 07 September 2012, Lee Jones wrote:
> > > > On Fri, Sep 07, 2012 at 12:35:41PM +0000, Arnd Bergmann wrote:
> > >
> > > The examples I had seen before were all just ranges of two interrupts,
> > > and in those cases it was clear that splitting them would be best.
> > > 
> > > In the exampled of the ab8500-gpio driver, it looks like the resource is
> > > not actually being used, and the gpio driver implements its own irq_chip,
> > > so maybe we can get away with not solving this problem for now.
> > 
> > Understood. I'd still feel more comfortable if we didn't trash the
> > range. I think it would be best to show the warning, and leave the
> > range for its target driver to take care - hence the patch.
> > 
> > ... but it's your call.
> 
> I'm fine with whatever Samuel sees fit here.  My personal opinion is
> that leaving the range alone for the child driver to do the conversion
> would be too inconsistent and only lead to confusion with driver authors.
Although I agree modifying the range is not very nice from the MFD core, I
also think that the actual mapping should always be handled by MFD and not
depend on wether the range is a singleton or not. Moreover the semantics of
leaving the range untouched meaning that we haven't done the mappings is
obscure.
So I'm not taking this patch, sorry Lee.

Cheers,
Samuel.

-- 
Intel Open Source Technology Centre
http://oss.intel.com/

Re: [PATCH 15/19] mfd: Don't convert just one IRQ using irqdomain if a range is provided

[Lee Jones]

On Mon, Sep 17, 2012 at 03:45:50PM +0200, Samuel Ortiz wrote:
> Hi Lee, Arnd,
> 
> On Fri, Sep 07, 2012 at 01:57:27PM +0000, Arnd Bergmann wrote:
> > On Friday 07 September 2012, Lee Jones wrote:
> > > On Fri, Sep 07, 2012 at 01:37:26PM +0000, Arnd Bergmann wrote:
> > > > On Friday 07 September 2012, Lee Jones wrote:
> > > > > On Fri, Sep 07, 2012 at 12:35:41PM +0000, Arnd Bergmann wrote:
> > > >
> > > > The examples I had seen before were all just ranges of two interrupts,
> > > > and in those cases it was clear that splitting them would be best.
> > > > 
> > > > In the exampled of the ab8500-gpio driver, it looks like the resource is
> > > > not actually being used, and the gpio driver implements its own irq_chip,
> > > > so maybe we can get away with not solving this problem for now.
> > > 
> > > Understood. I'd still feel more comfortable if we didn't trash the
> > > range. I think it would be best to show the warning, and leave the
> > > range for its target driver to take care - hence the patch.
> > > 
> > > ... but it's your call.
> > 
> > I'm fine with whatever Samuel sees fit here.  My personal opinion is
> > that leaving the range alone for the child driver to do the conversion
> > would be too inconsistent and only lead to confusion with driver authors.
> Although I agree modifying the range is not very nice from the MFD core, I
> also think that the actual mapping should always be handled by MFD and not
> depend on wether the range is a singleton or not. Moreover the semantics of
> leaving the range untouched meaning that we haven't done the mappings is
> obscure.
> So I'm not taking this patch, sorry Lee.

No problem.

Would it be better if we _did_ support ranges, and map all of the
IRQs in the range instead?

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH 15/19] mfd: Don't convert just one IRQ using irqdomain if a range is provided

[Samuel Ortiz]

Hi Lee,

On Mon, Sep 17, 2012 at 03:11:07PM +0100, Lee Jones wrote:
> On Mon, Sep 17, 2012 at 03:45:50PM +0200, Samuel Ortiz wrote:
> > Hi Lee, Arnd,
> > 
> > On Fri, Sep 07, 2012 at 01:57:27PM +0000, Arnd Bergmann wrote:
> > > On Friday 07 September 2012, Lee Jones wrote:
> > > > On Fri, Sep 07, 2012 at 01:37:26PM +0000, Arnd Bergmann wrote:
> > > > > On Friday 07 September 2012, Lee Jones wrote:
> > > > > > On Fri, Sep 07, 2012 at 12:35:41PM +0000, Arnd Bergmann wrote:
> > > > >
> > > > > The examples I had seen before were all just ranges of two interrupts,
> > > > > and in those cases it was clear that splitting them would be best.
> > > > > 
> > > > > In the exampled of the ab8500-gpio driver, it looks like the resource is
> > > > > not actually being used, and the gpio driver implements its own irq_chip,
> > > > > so maybe we can get away with not solving this problem for now.
> > > > 
> > > > Understood. I'd still feel more comfortable if we didn't trash the
> > > > range. I think it would be best to show the warning, and leave the
> > > > range for its target driver to take care - hence the patch.
> > > > 
> > > > ... but it's your call.
> > > 
> > > I'm fine with whatever Samuel sees fit here.  My personal opinion is
> > > that leaving the range alone for the child driver to do the conversion
> > > would be too inconsistent and only lead to confusion with driver authors.
> > Although I agree modifying the range is not very nice from the MFD core, I
> > also think that the actual mapping should always be handled by MFD and not
> > depend on wether the range is a singleton or not. Moreover the semantics of
> > leaving the range untouched meaning that we haven't done the mappings is
> > obscure.
> > So I'm not taking this patch, sorry Lee.
> 
> No problem.
> 
> Would it be better if we _did_ support ranges, and map all of the
> IRQs in the range instead?
I think that would be a reasonable solution, unless Arnd or Mark see a serious
problem with that.

Cheers,
Samuel.

-- 
Intel Open Source Technology Centre
http://oss.intel.com/

[PATCH 16/19] mfd: Provide the tc3589x with its own IRQ domain

[Lee Jones]

In preparation for Device Tree enablement all IRQ controllers
should control their own IRQ domain. This patch provides just
that.

CC: Samuel Ortiz <sameo@linux.intel.com>
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/mfd/tc3589x.c       |   73 ++++++++++++++++++++++++++++---------------
 include/linux/mfd/tc3589x.h |    1 +
 2 files changed, 48 insertions(+), 26 deletions(-)

diff --git a/drivers/mfd/tc3589x.c b/drivers/mfd/tc3589x.c
index b56ba6b..2df44ac 100644
--- a/drivers/mfd/tc3589x.c
+++ b/drivers/mfd/tc3589x.c
@@ -9,6 +9,7 @@
 #include <linux/module.h>
 #include <linux/interrupt.h>
 #include <linux/irq.h>
+#include <linux/irqdomain.h>
 #include <linux/slab.h>
 #include <linux/i2c.h>
 #include <linux/mfd/core.h>
@@ -168,8 +169,9 @@ again:
 
 	while (status) {
 		int bit = __ffs(status);
+		int virq = irq_create_mapping(tc3589x->domain, bit);
 
-		handle_nested_irq(tc3589x->irq_base + bit);
+		handle_nested_irq(virq);
 		status &= ~(1 << bit);
 	}
 
@@ -186,38 +188,60 @@ again:
 	return IRQ_HANDLED;
 }
 
-static int tc3589x_irq_init(struct tc3589x *tc3589x)
+static int tc3589x_irq_map(struct irq_domain *d, unsigned int virq,
+				irq_hw_number_t hwirq)
 {
-	int base = tc3589x->irq_base;
-	int irq;
+	struct tc3589x *tc3589x = d->host_data;
 
-	for (irq = base; irq < base + TC3589x_NR_INTERNAL_IRQS; irq++) {
-		irq_set_chip_data(irq, tc3589x);
-		irq_set_chip_and_handler(irq, &dummy_irq_chip,
-					 handle_edge_irq);
-		irq_set_nested_thread(irq, 1);
+	irq_set_chip_data(virq, tc3589x);
+	irq_set_chip_and_handler(virq, &dummy_irq_chip,
+				handle_edge_irq);
+	irq_set_nested_thread(virq, 1);
 #ifdef CONFIG_ARM
-		set_irq_flags(irq, IRQF_VALID);
+	set_irq_flags(virq, IRQF_VALID);
 #else
-		irq_set_noprobe(irq);
+	irq_set_noprobe(virq);
 #endif
-	}
 
 	return 0;
 }
 
-static void tc3589x_irq_remove(struct tc3589x *tc3589x)
+static void tc3589x_irq_unmap(struct irq_domain *d, unsigned int virq)
 {
-	int base = tc3589x->irq_base;
-	int irq;
-
-	for (irq = base; irq < base + TC3589x_NR_INTERNAL_IRQS; irq++) {
 #ifdef CONFIG_ARM
-		set_irq_flags(irq, 0);
+	set_irq_flags(virq, 0);
 #endif
-		irq_set_chip_and_handler(irq, NULL, NULL);
-		irq_set_chip_data(irq, NULL);
+	irq_set_chip_and_handler(virq, NULL, NULL);
+	irq_set_chip_data(virq, NULL);
+}
+
+static struct irq_domain_ops tc3589x_irq_ops = {
+        .map    = tc3589x_irq_map,
+	.unmap  = tc3589x_irq_unmap,
+        .xlate  = irq_domain_xlate_twocell,
+};
+
+static int tc3589x_irq_init(struct tc3589x *tc3589x)
+{
+	int base = tc3589x->irq_base;
+
+	if (base) {
+		tc3589x->domain = irq_domain_add_legacy(
+			NULL, TC3589x_NR_INTERNAL_IRQS, base,
+			0, &tc3589x_irq_ops, tc3589x);
 	}
+	else {
+		tc3589x->domain = irq_domain_add_linear(
+			NULL, TC3589x_NR_INTERNAL_IRQS,
+			&tc3589x_irq_ops, tc3589x);
+	}
+
+	if (!tc3589x->domain) {
+		dev_err(tc3589x->dev, "Failed to create irqdomain\n");
+		return -ENOSYS;
+	}
+
+	return 0;
 }
 
 static int tc3589x_chip_init(struct tc3589x *tc3589x)
@@ -263,7 +287,7 @@ static int __devinit tc3589x_device_init(struct tc3589x *tc3589x)
 	if (blocks & TC3589x_BLOCK_GPIO) {
 		ret = mfd_add_devices(tc3589x->dev, -1, tc3589x_dev_gpio,
 				      ARRAY_SIZE(tc3589x_dev_gpio), NULL,
-				      tc3589x->irq_base, NULL);
+				      tc3589x->irq_base, tc3589x->domain);
 		if (ret) {
 			dev_err(tc3589x->dev, "failed to add gpio child\n");
 			return ret;
@@ -274,7 +298,7 @@ static int __devinit tc3589x_device_init(struct tc3589x *tc3589x)
 	if (blocks & TC3589x_BLOCK_KEYPAD) {
 		ret = mfd_add_devices(tc3589x->dev, -1, tc3589x_dev_keypad,
 				      ARRAY_SIZE(tc3589x_dev_keypad), NULL,
-				      tc3589x->irq_base, NULL);
+				      tc3589x->irq_base, tc3589x->domain);
 		if (ret) {
 			dev_err(tc3589x->dev, "failed to keypad child\n");
 			return ret;
@@ -323,7 +347,7 @@ static int __devinit tc3589x_probe(struct i2c_client *i2c,
 				   "tc3589x", tc3589x);
 	if (ret) {
 		dev_err(tc3589x->dev, "failed to request IRQ: %d\n", ret);
-		goto out_removeirq;
+		goto out_free;
 	}
 
 	ret = tc3589x_device_init(tc3589x);
@@ -336,8 +360,6 @@ static int __devinit tc3589x_probe(struct i2c_client *i2c,
 
 out_freeirq:
 	free_irq(tc3589x->i2c->irq, tc3589x);
-out_removeirq:
-	tc3589x_irq_remove(tc3589x);
 out_free:
 	kfree(tc3589x);
 	return ret;
@@ -350,7 +372,6 @@ static int __devexit tc3589x_remove(struct i2c_client *client)
 	mfd_remove_devices(tc3589x->dev);
 
 	free_irq(tc3589x->i2c->irq, tc3589x);
-	tc3589x_irq_remove(tc3589x);
 
 	kfree(tc3589x);
 
diff --git a/include/linux/mfd/tc3589x.h b/include/linux/mfd/tc3589x.h
index 3acb3a8..6b8e1ff 100644
--- a/include/linux/mfd/tc3589x.h
+++ b/include/linux/mfd/tc3589x.h
@@ -117,6 +117,7 @@ struct tc3589x {
 	struct mutex lock;
 	struct device *dev;
 	struct i2c_client *i2c;
+	struct irq_domain *domain;
 
 	int irq_base;
 	int num_gpio;
-- 
1.7.9.5

Re: [PATCH 16/19] mfd: Provide the tc3589x with its own IRQ domain

[Linus Walleij]

On Fri, Sep 7, 2012 at 1:14 PM, Lee Jones <lee.jones@linaro.org> wrote:

> In preparation for Device Tree enablement all IRQ controllers
> should control their own IRQ domain. This patch provides just
> that.
>
> CC: Samuel Ortiz <sameo@linux.intel.com>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>

It's always nice to have irqdomains added, excellent patch!

> +       if (base) {
> +               tc3589x->domain = irq_domain_add_legacy(
> +                       NULL, TC3589x_NR_INTERNAL_IRQS, base,
> +                       0, &tc3589x_irq_ops, tc3589x);
>         }
> +       else {
> +               tc3589x->domain = irq_domain_add_linear(
> +                       NULL, TC3589x_NR_INTERNAL_IRQS,
> +                       &tc3589x_irq_ops, tc3589x);
> +       }

This is especially elegant, well done.
Not my subsystem, but FWIW:
Acked-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

Re: [PATCH 16/19] mfd: Provide the tc3589x with its own IRQ domain

[Samuel Ortiz]

Hi Lee,

On Fri, Sep 07, 2012 at 12:14:56PM +0100, Lee Jones wrote:
> In preparation for Device Tree enablement all IRQ controllers
> should control their own IRQ domain. This patch provides just
> that.
> 
> CC: Samuel Ortiz <sameo@linux.intel.com>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>
> ---
>  drivers/mfd/tc3589x.c       |   73 ++++++++++++++++++++++++++++---------------
>  include/linux/mfd/tc3589x.h |    1 +
>  2 files changed, 48 insertions(+), 26 deletions(-)
Applied, thanks.

Cheers,
Samuel.

-- 
Intel Open Source Technology Centre
http://oss.intel.com/

[PATCH 17/19] mfd: Enable the tc3589x for Device Tree

[Lee Jones]

Here we provide a means to probe and extract vital information
from Device Tree when booting with it enabled. Without this
patch sub-devices wouldn't be able to reference the tc3589x
from Device Tree.

CC: Samuel Ortiz <sameo@linux.intel.com>
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/mfd/tc3589x.c |   43 ++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 40 insertions(+), 3 deletions(-)

diff --git a/drivers/mfd/tc3589x.c b/drivers/mfd/tc3589x.c
index 2df44ac..8f4c853 100644
--- a/drivers/mfd/tc3589x.c
+++ b/drivers/mfd/tc3589x.c
@@ -12,6 +12,7 @@
 #include <linux/irqdomain.h>
 #include <linux/slab.h>
 #include <linux/i2c.h>
+#include <linux/of.h>
 #include <linux/mfd/core.h>
 #include <linux/mfd/tc3589x.h>
 
@@ -146,6 +147,7 @@ static struct mfd_cell tc3589x_dev_gpio[] = {
 		.name		= "tc3589x-gpio",
 		.num_resources	= ARRAY_SIZE(gpio_resources),
 		.resources	= &gpio_resources[0],
+		.of_compatible	= "tc3589x-gpio",
 	},
 };
 
@@ -154,6 +156,7 @@ static struct mfd_cell tc3589x_dev_keypad[] = {
 		.name           = "tc3589x-keypad",
 		.num_resources  = ARRAY_SIZE(keypad_resources),
 		.resources      = &keypad_resources[0],
+		.of_compatible	= "tc3589x-keypad",
 	},
 };
 
@@ -221,7 +224,7 @@ static struct irq_domain_ops tc3589x_irq_ops = {
         .xlate  = irq_domain_xlate_twocell,
 };
 
-static int tc3589x_irq_init(struct tc3589x *tc3589x)
+static int tc3589x_irq_init(struct tc3589x *tc3589x, struct device_node *np)
 {
 	int base = tc3589x->irq_base;
 
@@ -232,7 +235,7 @@ static int tc3589x_irq_init(struct tc3589x *tc3589x)
 	}
 	else {
 		tc3589x->domain = irq_domain_add_linear(
-			NULL, TC3589x_NR_INTERNAL_IRQS,
+			np, TC3589x_NR_INTERNAL_IRQS,
 			&tc3589x_irq_ops, tc3589x);
 	}
 
@@ -309,13 +312,47 @@ static int __devinit tc3589x_device_init(struct tc3589x *tc3589x)
 	return ret;
 }
 
+static int tc3589x_of_probe(struct device_node *np,
+			struct tc3589x_platform_data *pdata)
+{
+	struct device_node *child;
+
+	for_each_child_of_node(np, child) {
+		if (!strcmp(child->name, "tc3589x_gpio")) {
+			pdata->block |= TC3589x_BLOCK_GPIO;
+		}
+		if (!strcmp(child->name, "tc3589x_keypad")) {
+			pdata->block |= TC3589x_BLOCK_KEYPAD;
+		}
+	}
+
+	return 0;
+}
+
 static int __devinit tc3589x_probe(struct i2c_client *i2c,
 				   const struct i2c_device_id *id)
 {
 	struct tc3589x_platform_data *pdata = i2c->dev.platform_data;
+	struct device_node *np = i2c->dev.of_node;
 	struct tc3589x *tc3589x;
 	int ret;
 
+	if (!pdata) {
+		if (np) {
+			pdata = devm_kzalloc(&i2c->dev, sizeof(*pdata), GFP_KERNEL);
+			if (!pdata)
+				return -ENOMEM;
+
+			ret = tc3589x_of_probe(np, pdata);
+			if (ret)
+				return ret;
+		}
+		else {
+			dev_err(&i2c->dev, "No platform data or DT found\n");
+			return -EINVAL;
+		}
+	}
+
 	if (!i2c_check_functionality(i2c->adapter, I2C_FUNC_SMBUS_BYTE_DATA
 				     | I2C_FUNC_SMBUS_I2C_BLOCK))
 		return -EIO;
@@ -338,7 +375,7 @@ static int __devinit tc3589x_probe(struct i2c_client *i2c,
 	if (ret)
 		goto out_free;
 
-	ret = tc3589x_irq_init(tc3589x);
+	ret = tc3589x_irq_init(tc3589x, np);
 	if (ret)
 		goto out_free;
 
-- 
1.7.9.5

Re: [PATCH 17/19] mfd: Enable the tc3589x for Device Tree

[Linus Walleij]

On Fri, Sep 7, 2012 at 1:14 PM, Lee Jones <lee.jones@linaro.org> wrote:

> Here we provide a means to probe and extract vital information
> from Device Tree when booting with it enabled. Without this
> patch sub-devices wouldn't be able to reference the tc3589x
> from Device Tree.
>
> CC: Samuel Ortiz <sameo@linux.intel.com>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>

Looks like a good start (I guess all subdevices need to be
augmented too...) FWIW:
Acked-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

Re: [PATCH 17/19] mfd: Enable the tc3589x for Device Tree

[Samuel Ortiz]

Hi Lee,

On Fri, Sep 07, 2012 at 12:14:57PM +0100, Lee Jones wrote:
> Here we provide a means to probe and extract vital information
> from Device Tree when booting with it enabled. Without this
> patch sub-devices wouldn't be able to reference the tc3589x
> from Device Tree.
> 
> CC: Samuel Ortiz <sameo@linux.intel.com>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>
> ---
>  drivers/mfd/tc3589x.c |   43 ++++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 40 insertions(+), 3 deletions(-)
Applied as well, thanks.

Cheers,
Samuel.

-- 
Intel Open Source Technology Centre
http://oss.intel.com/

[PATCH 18/19] gpio: Provide the tc3589x GPIO expander driver with an IRQ domain

[Lee Jones]

In preparation for Device Tree enablement all IRQ controllers
should control their own IRQ domain. This patch provides just
that for the tc3589x GPIO expander.

CC: Grant Likely <grant.likely@secretlab.ca>
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/gpio/gpio-tc3589x.c |   98 ++++++++++++++++++++++++++++++-------------
 1 file changed, 70 insertions(+), 28 deletions(-)

diff --git a/drivers/gpio/gpio-tc3589x.c b/drivers/gpio/gpio-tc3589x.c
index 2a82e89..6e89009 100644
--- a/drivers/gpio/gpio-tc3589x.c
+++ b/drivers/gpio/gpio-tc3589x.c
@@ -12,6 +12,7 @@
 #include <linux/slab.h>
 #include <linux/gpio.h>
 #include <linux/irq.h>
+#include <linux/irqdomain.h>
 #include <linux/interrupt.h>
 #include <linux/mfd/tc3589x.h>
 
@@ -29,6 +30,7 @@ struct tc3589x_gpio {
 	struct tc3589x *tc3589x;
 	struct device *dev;
 	struct mutex irq_lock;
+	struct irq_domain *domain;
 
 	int irq_base;
 
@@ -92,11 +94,28 @@ static int tc3589x_gpio_direction_input(struct gpio_chip *chip,
 	return tc3589x_set_bits(tc3589x, reg, 1 << pos, 0);
 }
 
+/**
+ * tc3589x_gpio_irq_get_virq(): Map an interrupt on a chip to a virtual IRQ
+ *
+ * @tc3589x_gpio: tc3589x_gpio_irq controller to operate on.
+ * @irq: index of the interrupt requested in the chip IRQs
+ *
+ * Useful for drivers to request their own IRQs.
+ */
+static int tc3589x_gpio_irq_get_virq(struct tc3589x_gpio *tc3589x_gpio,
+				     int irq)
+{
+	if (!tc3589x_gpio)
+		return -EINVAL;
+
+	return irq_create_mapping(tc3589x_gpio->domain, irq);
+}
+
 static int tc3589x_gpio_to_irq(struct gpio_chip *chip, unsigned offset)
 {
 	struct tc3589x_gpio *tc3589x_gpio = to_tc3589x_gpio(chip);
 
-	return tc3589x_gpio->irq_base + offset;
+	return tc3589x_gpio_irq_get_virq(tc3589x_gpio, offset);
 }
 
 static struct gpio_chip template_chip = {
@@ -113,7 +132,7 @@ static struct gpio_chip template_chip = {
 static int tc3589x_gpio_irq_set_type(struct irq_data *d, unsigned int type)
 {
 	struct tc3589x_gpio *tc3589x_gpio = irq_data_get_irq_chip_data(d);
-	int offset = d->irq - tc3589x_gpio->irq_base;
+	int offset = d->hwirq;
 	int regoffset = offset / 8;
 	int mask = 1 << (offset % 8);
 
@@ -175,7 +194,7 @@ static void tc3589x_gpio_irq_sync_unlock(struct irq_data *d)
 static void tc3589x_gpio_irq_mask(struct irq_data *d)
 {
 	struct tc3589x_gpio *tc3589x_gpio = irq_data_get_irq_chip_data(d);
-	int offset = d->irq - tc3589x_gpio->irq_base;
+	int offset = d->hwirq;
 	int regoffset = offset / 8;
 	int mask = 1 << (offset % 8);
 
@@ -185,7 +204,7 @@ static void tc3589x_gpio_irq_mask(struct irq_data *d)
 static void tc3589x_gpio_irq_unmask(struct irq_data *d)
 {
 	struct tc3589x_gpio *tc3589x_gpio = irq_data_get_irq_chip_data(d);
-	int offset = d->irq - tc3589x_gpio->irq_base;
+	int offset = d->hwirq;
 	int regoffset = offset / 8;
 	int mask = 1 << (offset % 8);
 
@@ -222,8 +241,9 @@ static irqreturn_t tc3589x_gpio_irq(int irq, void *dev)
 		while (stat) {
 			int bit = __ffs(stat);
 			int line = i * 8 + bit;
+			int virq = tc3589x_gpio_irq_get_virq(tc3589x_gpio, line);
 
-			handle_nested_irq(tc3589x_gpio->irq_base + line);
+			handle_nested_irq(virq);
 			stat &= ~(1 << bit);
 		}
 
@@ -233,38 +253,60 @@ static irqreturn_t tc3589x_gpio_irq(int irq, void *dev)
 	return IRQ_HANDLED;
 }
 
-static int tc3589x_gpio_irq_init(struct tc3589x_gpio *tc3589x_gpio)
+static int tc3589x_gpio_irq_map(struct irq_domain *d, unsigned int virq,
+				irq_hw_number_t hwirq)
 {
-	int base = tc3589x_gpio->irq_base;
-	int irq;
+	struct tc3589x *tc3589x_gpio = d->host_data;
 
-	for (irq = base; irq < base + tc3589x_gpio->chip.ngpio; irq++) {
-		irq_set_chip_data(irq, tc3589x_gpio);
-		irq_set_chip_and_handler(irq, &tc3589x_gpio_irq_chip,
-					 handle_simple_irq);
-		irq_set_nested_thread(irq, 1);
+	irq_set_chip_data(virq, tc3589x_gpio);
+	irq_set_chip_and_handler(virq, &tc3589x_gpio_irq_chip,
+				handle_simple_irq);
+	irq_set_nested_thread(virq, 1);
 #ifdef CONFIG_ARM
-		set_irq_flags(irq, IRQF_VALID);
+	set_irq_flags(virq, IRQF_VALID);
 #else
-		irq_set_noprobe(irq);
+	irq_set_noprobe(virq);
 #endif
-	}
 
 	return 0;
 }
 
-static void tc3589x_gpio_irq_remove(struct tc3589x_gpio *tc3589x_gpio)
+static void tc3589x_gpio_irq_unmap(struct irq_domain *d, unsigned int virq)
 {
-	int base = tc3589x_gpio->irq_base;
-	int irq;
-
-	for (irq = base; irq < base + tc3589x_gpio->chip.ngpio; irq++) {
 #ifdef CONFIG_ARM
-		set_irq_flags(irq, 0);
+	set_irq_flags(virq, 0);
 #endif
-		irq_set_chip_and_handler(irq, NULL, NULL);
-		irq_set_chip_data(irq, NULL);
+	irq_set_chip_and_handler(virq, NULL, NULL);
+	irq_set_chip_data(virq, NULL);
+}
+
+static struct irq_domain_ops tc3589x_irq_ops = {
+        .map    = tc3589x_gpio_irq_map,
+        .unmap  = tc3589x_gpio_irq_unmap,
+        .xlate  = irq_domain_xlate_twocell,
+};
+
+static int tc3589x_gpio_irq_init(struct tc3589x_gpio *tc3589x_gpio)
+{
+	int base = tc3589x_gpio->irq_base;
+
+	if (base) {
+		tc3589x_gpio->domain = irq_domain_add_legacy(
+			NULL, tc3589x_gpio->chip.ngpio, base,
+			0, &tc3589x_irq_ops, tc3589x_gpio);
 	}
+	else {
+		tc3589x_gpio->domain = irq_domain_add_linear(
+			NULL, tc3589x_gpio->chip.ngpio,
+			&tc3589x_irq_ops, tc3589x_gpio);
+	}
+
+	if (!tc3589x_gpio->domain) {
+		dev_err(tc3589x_gpio->dev, "Failed to create irqdomain\n");
+		return -ENOSYS;
+	}
+
+	return 0;
 }
 
 static int __devinit tc3589x_gpio_probe(struct platform_device *pdev)
@@ -299,6 +341,9 @@ static int __devinit tc3589x_gpio_probe(struct platform_device *pdev)
 
 	tc3589x_gpio->irq_base = tc3589x->irq_base + TC3589x_INT_GPIO(0);
 
+	tc3589x_gpio->irq_base = tc3589x->irq_base ?
+		tc3589x->irq_base + TC3589x_INT_GPIO(0) : 0;
+
 	/* Bring the GPIO module out of reset */
 	ret = tc3589x_set_bits(tc3589x, TC3589x_RSTCTRL,
 			       TC3589x_RSTCTRL_GPIRST, 0);
@@ -313,7 +358,7 @@ static int __devinit tc3589x_gpio_probe(struct platform_device *pdev)
 				   "tc3589x-gpio", tc3589x_gpio);
 	if (ret) {
 		dev_err(&pdev->dev, "unable to get irq: %d\n", ret);
-		goto out_removeirq;
+		goto out_free;
 	}
 
 	ret = gpiochip_add(&tc3589x_gpio->chip);
@@ -331,8 +376,6 @@ static int __devinit tc3589x_gpio_probe(struct platform_device *pdev)
 
 out_freeirq:
 	free_irq(irq, tc3589x_gpio);
-out_removeirq:
-	tc3589x_gpio_irq_remove(tc3589x_gpio);
 out_free:
 	kfree(tc3589x_gpio);
 	return ret;
@@ -357,7 +400,6 @@ static int __devexit tc3589x_gpio_remove(struct platform_device *pdev)
 	}
 
 	free_irq(irq, tc3589x_gpio);
-	tc3589x_gpio_irq_remove(tc3589x_gpio);
 
 	platform_set_drvdata(pdev, NULL);
 	kfree(tc3589x_gpio);
-- 
1.7.9.5

Re: [PATCH 18/19] gpio: Provide the tc3589x GPIO expander driver with an IRQ domain

[Linus Walleij]

On Fri, Sep 7, 2012 at 1:14 PM, Lee Jones <lee.jones@linaro.org> wrote:

> In preparation for Device Tree enablement all IRQ controllers
> should control their own IRQ domain. This patch provides just
> that for the tc3589x GPIO expander.
>
> CC: Grant Likely <grant.likely@secretlab.ca>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>

Looks very good again,
Acked-by: Linus Walleij <linus.walleij@linaro.org>

I presume you will want to push the whole shebang through
ARM SoC, but if that doesn't work out I am very tempted to
apply this patch to the GPIO tree immediately so it lands
in v3.7.

Yours,
Linus Walleij

Re: [PATCH 18/19] gpio: Provide the tc3589x GPIO expander driver with an IRQ domain

[Linus Walleij]

On Mon, Sep 10, 2012 at 3:10 PM, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Fri, Sep 7, 2012 at 1:14 PM, Lee Jones <lee.jones@linaro.org> wrote:
>
>> In preparation for Device Tree enablement all IRQ controllers
>> should control their own IRQ domain. This patch provides just
>> that for the tc3589x GPIO expander.
>>
>> CC: Grant Likely <grant.likely@secretlab.ca>
>> Signed-off-by: Lee Jones <lee.jones@linaro.org>
>
> Looks very good again,
> Acked-by: Linus Walleij <linus.walleij@linaro.org>
>
> I presume you will want to push the whole shebang through
> ARM SoC, but if that doesn't work out I am very tempted to
> apply this patch to the GPIO tree immediately so it lands
> in v3.7.

If I hear nothing I will probably apply this to the GPIO tree
tonight since it's a real nice patch and we really need irqdomain
conversion to happen all over the place... this driver has
many users so I want some rotation in linux-next for it.

Yours,
Linus Walleij

Re: [PATCH 18/19] gpio: Provide the tc3589x GPIO expander driver with an IRQ domain

[Lee Jones]

On Mon, Sep 10, 2012 at 06:10:59AM -0700, Linus Walleij wrote:
> On Fri, Sep 7, 2012 at 1:14 PM, Lee Jones <lee.jones@linaro.org> wrote:
> 
> > In preparation for Device Tree enablement all IRQ controllers
> > should control their own IRQ domain. This patch provides just
> > that for the tc3589x GPIO expander.
> >
> > CC: Grant Likely <grant.likely@secretlab.ca>
> > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> 
> Looks very good again,
> Acked-by: Linus Walleij <linus.walleij@linaro.org>
> 
> I presume you will want to push the whole shebang through
> ARM SoC, but if that doesn't work out I am very tempted to
> apply this patch to the GPIO tree immediately so it lands
> in v3.7.

I'd like to push my patch-set for v3.7 too.

I just need to collect all my Acks before I can send a pull-request.

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH 18/19] gpio: Provide the tc3589x GPIO expander driver with an IRQ domain

[Linus Walleij]

On Fri, Sep 7, 2012 at 1:14 PM, Lee Jones <lee.jones@linaro.org> wrote:

> In preparation for Device Tree enablement all IRQ controllers
> should control their own IRQ domain. This patch provides just
> that for the tc3589x GPIO expander.
>
> CC: Grant Likely <grant.likely@secretlab.ca>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>

I've applied this to my GPIO tree, I couldn't resist. It's just such
a nice patch nicing up the code and I wanted it tested so it can
go into the next merge window.

If you need to send patches to the ARM SoC tree, just state
the devel branch from my tree as dependency.

Yours,
Linus Walleij

[PATCH 19/19] gpio: Enable the tc3298x GPIO expander driver for Device Tree

[Lee Jones]

Here we provide a means to probe and extract vital information
from Device Tree when booting with it enabled. Without this
patch sub-devices wouldn't be able to reference the tc3589x-gpio
expander from Device Tree.

CC: Grant Likely <grant.likely@secretlab.ca>
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/gpio/gpio-tc3589x.c |   26 +++++++++++++++++---------
 1 file changed, 17 insertions(+), 9 deletions(-)

diff --git a/drivers/gpio/gpio-tc3589x.c b/drivers/gpio/gpio-tc3589x.c
index 6e89009..1e48317 100644
--- a/drivers/gpio/gpio-tc3589x.c
+++ b/drivers/gpio/gpio-tc3589x.c
@@ -11,6 +11,7 @@
 #include <linux/platform_device.h>
 #include <linux/slab.h>
 #include <linux/gpio.h>
+#include <linux/of.h>
 #include <linux/irq.h>
 #include <linux/irqdomain.h>
 #include <linux/interrupt.h>
@@ -286,7 +287,8 @@ static struct irq_domain_ops tc3589x_irq_ops = {
         .xlate  = irq_domain_xlate_twocell,
 };
 
-static int tc3589x_gpio_irq_init(struct tc3589x_gpio *tc3589x_gpio)
+static int tc3589x_gpio_irq_init(struct tc3589x_gpio *tc3589x_gpio,
+				struct device_node *np)
 {
 	int base = tc3589x_gpio->irq_base;
 
@@ -297,7 +299,7 @@ static int tc3589x_gpio_irq_init(struct tc3589x_gpio *tc3589x_gpio)
 	}
 	else {
 		tc3589x_gpio->domain = irq_domain_add_linear(
-			NULL, tc3589x_gpio->chip.ngpio,
+			np, tc3589x_gpio->chip.ngpio,
 			&tc3589x_irq_ops, tc3589x_gpio);
 	}
 
@@ -313,13 +315,17 @@ static int __devinit tc3589x_gpio_probe(struct platform_device *pdev)
 {
 	struct tc3589x *tc3589x = dev_get_drvdata(pdev->dev.parent);
 	struct tc3589x_gpio_platform_data *pdata;
+	struct device_node *np = pdev->dev.of_node;
 	struct tc3589x_gpio *tc3589x_gpio;
 	int ret;
 	int irq;
 
 	pdata = tc3589x->pdata->gpio;
-	if (!pdata)
-		return -ENODEV;
+
+	if (!(pdata || np)) {
+		dev_err(&pdev->dev, "No platform data or Device Tree found\n");
+		return -EINVAL;
+	}
 
 	irq = platform_get_irq(pdev, 0);
 	if (irq < 0)
@@ -337,9 +343,11 @@ static int __devinit tc3589x_gpio_probe(struct platform_device *pdev)
 	tc3589x_gpio->chip = template_chip;
 	tc3589x_gpio->chip.ngpio = tc3589x->num_gpio;
 	tc3589x_gpio->chip.dev = &pdev->dev;
-	tc3589x_gpio->chip.base = pdata->gpio_base;
+	tc3589x_gpio->chip.base = (pdata) ? pdata->gpio_base : -1;
 
-	tc3589x_gpio->irq_base = tc3589x->irq_base + TC3589x_INT_GPIO(0);
+#ifdef CONFIG_OF_GPIO
+        tc3589x_gpio->chip.of_node = np;
+#endif
 
 	tc3589x_gpio->irq_base = tc3589x->irq_base ?
 		tc3589x->irq_base + TC3589x_INT_GPIO(0) : 0;
@@ -350,7 +358,7 @@ static int __devinit tc3589x_gpio_probe(struct platform_device *pdev)
 	if (ret < 0)
 		goto out_free;
 
-	ret = tc3589x_gpio_irq_init(tc3589x_gpio);
+	ret = tc3589x_gpio_irq_init(tc3589x_gpio, np);
 	if (ret)
 		goto out_free;
 
@@ -367,7 +375,7 @@ static int __devinit tc3589x_gpio_probe(struct platform_device *pdev)
 		goto out_freeirq;
 	}
 
-	if (pdata->setup)
+	if (pdata && pdata->setup)
 		pdata->setup(tc3589x, tc3589x_gpio->chip.base);
 
 	platform_set_drvdata(pdev, tc3589x_gpio);
@@ -389,7 +397,7 @@ static int __devexit tc3589x_gpio_remove(struct platform_device *pdev)
 	int irq = platform_get_irq(pdev, 0);
 	int ret;
 
-	if (pdata->remove)
+	if (pdata && pdata->remove)
 		pdata->remove(tc3589x, tc3589x_gpio->chip.base);
 
 	ret = gpiochip_remove(&tc3589x_gpio->chip);
-- 
1.7.9.5

Re: [PATCH 19/19] gpio: Enable the tc3298x GPIO expander driver for Device Tree

[Linus Walleij]

On Fri, Sep 7, 2012 at 1:14 PM, Lee Jones <lee.jones@linaro.org> wrote:

> Here we provide a means to probe and extract vital information
> from Device Tree when booting with it enabled. Without this
> patch sub-devices wouldn't be able to reference the tc3589x-gpio
> expander from Device Tree.
>
> CC: Grant Likely <grant.likely@secretlab.ca>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>

Basically this looks very good, but:

> @@ -337,9 +343,11 @@ static int __devinit tc3589x_gpio_probe(struct platform_device *pdev)
>         tc3589x_gpio->chip = template_chip;
>         tc3589x_gpio->chip.ngpio = tc3589x->num_gpio;
>         tc3589x_gpio->chip.dev = &pdev->dev;
> -       tc3589x_gpio->chip.base = pdata->gpio_base;
> +       tc3589x_gpio->chip.base = (pdata) ? pdata->gpio_base : -1;
>
> -       tc3589x_gpio->irq_base = tc3589x->irq_base + TC3589x_INT_GPIO(0);
> +#ifdef CONFIG_OF_GPIO
> +        tc3589x_gpio->chip.of_node = np;
> +#endif

Isn't the newstyle way of doing this something like:

if (IS_ENABLED(CONFIG_OF_GPIO))
  tc3589x_gpio->chip.of_node = np;

I'm a bit uncertain about this since I never quite used
it myself, test it with OF disabled and see if it works.

This construct also accepts modules using OF, see
include/linux/kconfig.h, somebody might be using this as
a module.

Yours,
Linus Walleij

Re: [PATCH 19/19] gpio: Enable the tc3298x GPIO expander driver for Device Tree

[Linus Walleij]

On Fri, Sep 7, 2012 at 1:14 PM, Lee Jones <lee.jones@linaro.org> wrote:

> Here we provide a means to probe and extract vital information
> from Device Tree when booting with it enabled. Without this
> patch sub-devices wouldn't be able to reference the tc3589x-gpio
> expander from Device Tree.
>
> CC: Grant Likely <grant.likely@secretlab.ca>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>

Applied as well.

Yours,
Linus Walleij

Re: [PATCH 00/19] First HREF Device Tree enablement patch-set

[Arnd Bergmann]

On Friday 07 September 2012, Lee Jones wrote:
> Based on v3.6-rc4
> 
> This patch consists of Device Tree related enablement for
> ST-Ericsson's development HREF hardware reference board.
> Most devices are very similar, if not the same to the
> previously enabled Snowball low-cost development board.
> These are referenced from the relevant .dtsi file and
> merely enabled in the HREF specific Device Tree. There 
> are also one or two minor Device Tree related fixes which
> were stumbled upon along the way.

The patches basically look good to me (except patch 15, see
comment there), but they are not ordered in a way that allows
bisection. I can see two ways to do this better:

a) first do all the code changes, one by one, then add the
fully populated device tree in the last patch.

b) as commented on patch 5, atomically change over each
device in both the DT source and the board code.
Also do the driver changes (i2c, tc3589x) before changing
them in the device tree.

	Arnd

Re: [PATCH 00/19] First HREF Device Tree enablement patch-set

[Lee Jones]

On Fri, Sep 07, 2012 at 12:41:16PM +0000, Arnd Bergmann wrote:
> On Friday 07 September 2012, Lee Jones wrote:
> > Based on v3.6-rc4
> > 
> > This patch consists of Device Tree related enablement for
> > ST-Ericsson's development HREF hardware reference board.
> > Most devices are very similar, if not the same to the
> > previously enabled Snowball low-cost development board.
> > These are referenced from the relevant .dtsi file and
> > merely enabled in the HREF specific Device Tree. There 
> > are also one or two minor Device Tree related fixes which
> > were stumbled upon along the way.
> 
> The patches basically look good to me (except patch 15, see
> comment there), but they are not ordered in a way that allows
> bisection. I can see two ways to do this better:
> 
> a) first do all the code changes, one by one, then add the
> fully populated device tree in the last patch.
> 
> b) as commented on patch 5, atomically change over each
> device in both the DT source and the board code.
> Also do the driver changes (i2c, tc3589x) before changing
> them in the device tree.

Just running this by you, as there is method in the madness.

Linus wanted to keep changes to the Device Tree and changes
in platform code separate, which is my reason for submitting
all of my changes to date that way.

What I do (not sure if I've achieved that here yet, I'll need
to take another look) is; make changes to the driver which
enable it for Device Tree use. Then apply the DT node and remove 
platform registration in two subsequent patches respectively. 
Then when we come to bisect and land in between them we still
have a perfectly working driver, only the second probe fails 
which the only side-effect is some warnings in the boot log.

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH 00/19] First HREF Device Tree enablement patch-set

[Arnd Bergmann]

On Friday 07 September 2012, Lee Jones wrote:
> Just running this by you, as there is method in the madness.
> 
> Linus wanted to keep changes to the Device Tree and changes
> in platform code separate, which is my reason for submitting
> all of my changes to date that way.
> 
> What I do (not sure if I've achieved that here yet, I'll need
> to take another look) is; make changes to the driver which
> enable it for Device Tree use. Then apply the DT node and remove 
> platform registration in two subsequent patches respectively. 
> Then when we come to bisect and land in between them we still
> have a perfectly working driver, only the second probe fails 
> which the only side-effect is some warnings in the boot log.

I'm sorry that you are getting conflicting directions from Linus
and me. We can use the approach you suggest here this time,
but I'd prefer if we can all agree on how to do this in the
future.

Linus: What is the reason you want to see the commits split up
like this? We normally try to split up changes into small
atomic improvements, but splitting them even further seems
counterproductive, and loses the context information in the
changeset description.

	Arnd

Re: [PATCH 00/19] First HREF Device Tree enablement patch-set

[Lee Jones]

On Fri, Sep 07, 2012 at 01:58:38PM +0000, Arnd Bergmann wrote:
> On Friday 07 September 2012, Lee Jones wrote:
> > Just running this by you, as there is method in the madness.
> > 
> > Linus wanted to keep changes to the Device Tree and changes
> > in platform code separate, which is my reason for submitting
> > all of my changes to date that way.
> > 
> > What I do (not sure if I've achieved that here yet, I'll need
> > to take another look) is; make changes to the driver which
> > enable it for Device Tree use. Then apply the DT node and remove 
> > platform registration in two subsequent patches respectively. 
> > Then when we come to bisect and land in between them we still
> > have a perfectly working driver, only the second probe fails 
> > which the only side-effect is some warnings in the boot log.
> 
> I'm sorry that you are getting conflicting directions from Linus
> and me. We can use the approach you suggest here this time,
> but I'd prefer if we can all agree on how to do this in the
> future.

I don't have an issue with that.

It would be good to have some guidelines to adhere to.

I'll double check that these ones are in the correct order, but
otherwise leave them as they are for this submission.

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH 00/19] First HREF Device Tree enablement patch-set

[Linus Walleij]

On Fri, Sep 7, 2012 at 3:58 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Friday 07 September 2012, Lee Jones wrote:
>> (...)
>> Linus wanted to keep changes to the Device Tree and changes
>> in platform code separate, which is my reason for submitting
>> all of my changes to date that way.
> (...)
> Linus: What is the reason you want to see the commits split up
> like this?

Basically that was at a point when we were changing a lot of
subsystem trees with DT patches that were merged in out-of-order
fashion.

Then it's better to have the DT changes to be pushed
separately at the end of the merge window after all subtrees
with dependent changes are merged. (Or even in the next merge
window if there is no hurry, but we always seem to be in a hurry...)

If all the changes are in the same subtree (like ARM SoC with
ACKs from all over the place where needed) you can do it
this way instead, and I agree it looks better.

So basically two different merge strategies get you to split
patches differently.

Any way is fine with me, do as Arnd suggests if all is going
in through ARM SoC.

Yours,
Linus Walleij

Re: [PATCH 00/19] First HREF Device Tree enablement patch-set

[Arnd Bergmann]

On Monday 10 September 2012, Linus Walleij wrote:
> Basically that was at a point when we were changing a lot of
> subsystem trees with DT patches that were merged in out-of-order
> fashion.
> 
> Then it's better to have the DT changes to be pushed
> separately at the end of the merge window after all subtrees
> with dependent changes are merged. (Or even in the next merge
> window if there is no hurry, but we always seem to be in a hurry...)
> 
> If all the changes are in the same subtree (like ARM SoC with
> ACKs from all over the place where needed) you can do it
> this way instead, and I agree it looks better.

I think we're talking about different things:

I think it's fine to do the device driver conversion separately
from the platform changes, but doing that properly means that the
platform changes should only go in after the drivers have changed.

Inside of the platform code, I absolutely think we should change the
board file and the dts file simultaneously. When the driver offers
two ways of probing a device, this lets us change from one way to
the other without going through a phase where we use neither of them
or (worse) both at the same time.

	Arnd

Re: [PATCH 00/19] First HREF Device Tree enablement patch-set

[Linus Walleij]

On Mon, Sep 10, 2012 at 3:13 AM, Arnd Bergmann <arnd@arndb.de> wrote:

> I think it's fine to do the device driver conversion separately
> from the platform changes, but doing that properly means that the
> platform changes should only go in after the drivers have changed.

OK, in the diffstat we're discussing we have this:

 drivers/gpio/gpio-tc3589x.c            |  120 +++++++++++++++++++--------
 drivers/i2c/busses/i2c-nomadik.c       |    4 +
 drivers/mfd/mfd-core.c                 |   10 ++-
 drivers/mfd/tc3589x.c                  |  112 +++++++++++++++++++------
 include/linux/mfd/tc3589x.h            |    1 +

> Inside of the platform code, I absolutely think we should change the
> board file and the dts file simultaneously.

Sure, no problem. But this patch series might have a title
problem, as it is not isolated to the platform code at all.

> When the driver offers
> two ways of probing a device, this lets us change from one way to
> the other without going through a phase where we use neither of them
> or (worse) both at the same time.

Maybe unrelated, but as usual it works as long as there are no
DMA channels or callback functions :-/

The MMC, PL022 patches in this patch set: both need
AUXDATA and DT data simultaneously due to DMA channels
and callbacks passed in as AUXDATA.

Yours,
Linus Walleij

Re: [PATCH 00/19] First HREF Device Tree enablement patch-set

[Arnd Bergmann]

On Monday 10 September 2012, Linus Walleij wrote:
> > When the driver offers
> > two ways of probing a device, this lets us change from one way to
> > the other without going through a phase where we use neither of them
> > or (worse) both at the same time.
> 
> Maybe unrelated, but as usual it works as long as there are no
> DMA channels or callback functions :-/
> 
> The MMC, PL022 patches in this patch set: both need
> AUXDATA and DT data simultaneously due to DMA channels
> and callbacks passed in as AUXDATA.

Yes, that's another discussion. When this happens, we obviously
keep the platform data (or add a new version of it), but I'd still
use one patch to add the node to the device tree and simultaneously
add the auxdata and remove the static device registration.

	Arnd