[Resend PATCH v3 2/4] ARM: at91: atmel-ssc: add device tree support

[Resend PATCH v3 2/4] ARM: at91: atmel-ssc: add device tree support

[Bo Shen]

Add atmel-ssc for device tree support

Match "atmel,at91rm9200-ssc" for using pdc for data transfer
Match "atmel,at91sam9g45-ssc" for using dma for data transfer

Signed-off-by: Bo Shen <voice.shen@atmel.com>
Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>
---
Resend
  Fix a error find by Fengguang Yu

  Although there is a NACK from Jean-Christophe PLAGNIOL-VILLARD,
  however the pinctrl for at91 is not in mainline. So, plan to add
  pinctrl support after the pinctrl driver for at91 is merged.

Change since v2
  No change
Change since v1
  change the underscore to dash in atmel-ssc binding document
---
 .../devicetree/bindings/misc/atmel-ssc.txt         |   15 ++++++
 arch/arm/boot/dts/at91sam9260.dtsi                 |    8 ++++
 arch/arm/boot/dts/at91sam9263.dtsi                 |   16 +++++++
 arch/arm/boot/dts/at91sam9g45.dtsi                 |   16 +++++++
 arch/arm/boot/dts/at91sam9x5.dtsi                  |    8 ++++
 arch/arm/mach-at91/at91rm9200.c                    |    3 ++
 arch/arm/mach-at91/at91sam9260.c                   |    1 +
 arch/arm/mach-at91/at91sam9261.c                   |    3 ++
 arch/arm/mach-at91/at91sam9263.c                   |    2 +
 arch/arm/mach-at91/at91sam9g45.c                   |    2 +
 arch/arm/mach-at91/at91sam9rl.c                    |    2 +
 arch/arm/mach-at91/at91sam9x5.c                    |    1 +
 drivers/misc/atmel-ssc.c                           |   49 ++++++++++++++++++--
 13 files changed, 123 insertions(+), 3 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/misc/atmel-ssc.txt

diff --git a/Documentation/devicetree/bindings/misc/atmel-ssc.txt b/Documentation/devicetree/bindings/misc/atmel-ssc.txt
new file mode 100644
index 0000000..38e51ad
--- /dev/null
+++ b/Documentation/devicetree/bindings/misc/atmel-ssc.txt
@@ -0,0 +1,15 @@
+* Atmel SSC driver.
+
+Required properties:
+- compatible: "atmel,at91rm9200-ssc" or "atmel,at91sam9g45-ssc"
+	- atmel,at91rm9200-ssc: support pdc transfer
+	- atmel,at91sam9g45-ssc: support dma transfer
+- reg: Should contain SSC registers location and length
+- interrupts: Should contain SSC interrupt
+
+Example:
+ssc0: ssc@fffbc000 {
+	compatible = "atmel,at91rm9200-ssc";
+	reg = <0xfffbc000 0x4000>;
+	interrupts = <14 4 5>;
+};
diff --git a/arch/arm/boot/dts/at91sam9260.dtsi b/arch/arm/boot/dts/at91sam9260.dtsi
index d410581..aaa42d8 100644
--- a/arch/arm/boot/dts/at91sam9260.dtsi
+++ b/arch/arm/boot/dts/at91sam9260.dtsi
@@ -29,6 +29,7 @@
 		tcb0 = &tcb0;
 		tcb1 = &tcb1;
 		i2c0 = &i2c0;
+		ssc0 = &ssc0;
 	};
 	cpus {
 		cpu@0 {
@@ -212,6 +213,13 @@
 				status = "disabled";
 			};
 
+			ssc0: ssc@fffbc000 {
+				compatible = "atmel,at91rm9200-ssc";
+				reg = <0xfffbc000 0x4000>;
+				interrupts = <14 4 5>;
+				status = "disable";
+			};
+
 			adc0: adc@fffe0000 {
 				compatible = "atmel,at91sam9260-adc";
 				reg = <0xfffe0000 0x100>;
diff --git a/arch/arm/boot/dts/at91sam9263.dtsi b/arch/arm/boot/dts/at91sam9263.dtsi
index 3e6e5c1..3b721ee 100644
--- a/arch/arm/boot/dts/at91sam9263.dtsi
+++ b/arch/arm/boot/dts/at91sam9263.dtsi
@@ -25,6 +25,8 @@
 		gpio4 = &pioE;
 		tcb0 = &tcb0;
 		i2c0 = &i2c0;
+		ssc0 = &ssc0;
+		ssc1 = &ssc1;
 	};
 	cpus {
 		cpu@0 {
@@ -173,6 +175,20 @@
 				status = "disabled";
 			};
 
+			ssc0: ssc@fff98000 {
+				compatible = "atmel,at91rm9200-ssc";
+				reg = <0xfff98000 0x4000>;
+				interrupts = <16 4 5>;
+				status = "disable";
+			};
+
+			ssc1: ssc@fff9c000 {
+				compatible = "atmel,at91rm9200-ssc";
+				reg = <0xfff9c000 0x4000>;
+				interrupts = <17 4 5>;
+				status = "disable";
+			};
+
 			macb0: ethernet@fffbc000 {
 				compatible = "cdns,at32ap7000-macb", "cdns,macb";
 				reg = <0xfffbc000 0x100>;
diff --git a/arch/arm/boot/dts/at91sam9g45.dtsi b/arch/arm/boot/dts/at91sam9g45.dtsi
index 3add030..acfa207 100644
--- a/arch/arm/boot/dts/at91sam9g45.dtsi
+++ b/arch/arm/boot/dts/at91sam9g45.dtsi
@@ -31,6 +31,8 @@
 		tcb1 = &tcb1;
 		i2c0 = &i2c0;
 		i2c1 = &i2c1;
+		ssc0 = &ssc0;
+		ssc1 = &ssc1;
 	};
 	cpus {
 		cpu@0 {
@@ -226,6 +228,20 @@
 				status = "disabled";
 			};
 
+			ssc0: ssc@fff9c000 {
+				compatible = "atmel,at91sam9g45-ssc";
+				reg = <0xfff9c000 0x4000>;
+				interrupts = <16 4 5>;
+				status = "disable";
+			};
+
+			ssc1: ssc@fffa0000 {
+				compatible = "atmel,at91sam9g45-ssc";
+				reg = <0xfffa0000 0x4000>;
+				interrupts = <17 4 5>;
+				status = "disable";
+			};
+
 			adc0: adc@fffb0000 {
 				compatible = "atmel,at91sam9260-adc";
 				reg = <0xfffb0000 0x100>;
diff --git a/arch/arm/boot/dts/at91sam9x5.dtsi b/arch/arm/boot/dts/at91sam9x5.dtsi
index 03fc136..69667d0 100644
--- a/arch/arm/boot/dts/at91sam9x5.dtsi
+++ b/arch/arm/boot/dts/at91sam9x5.dtsi
@@ -30,6 +30,7 @@
 		i2c0 = &i2c0;
 		i2c1 = &i2c1;
 		i2c2 = &i2c2;
+		ssc0 = &ssc0;
 	};
 	cpus {
 		cpu@0 {
@@ -87,6 +88,13 @@
 				interrupts = <1 4 7>;
 			};
 
+			ssc0: ssc@f0010000 {
+				compatible = "atmel,at91sam9g45-ssc";
+				reg = <0xf0010000 0x4000>;
+				interrupts = <28 4 5>;
+				status = "disable";
+			};
+
 			tcb0: timer@f8008000 {
 				compatible = "atmel,at91sam9x5-tcb";
 				reg = <0xf8008000 0x100>;
diff --git a/arch/arm/mach-at91/at91rm9200.c b/arch/arm/mach-at91/at91rm9200.c
index 400d1a3..af47c75 100644
--- a/arch/arm/mach-at91/at91rm9200.c
+++ b/arch/arm/mach-at91/at91rm9200.c
@@ -187,6 +187,9 @@ static struct clk_lookup periph_clocks_lookups[] = {
 	CLKDEV_CON_DEV_ID("pclk", "at91rm9200_ssc.0", &ssc0_clk),
 	CLKDEV_CON_DEV_ID("pclk", "at91rm9200_ssc.1", &ssc1_clk),
 	CLKDEV_CON_DEV_ID("pclk", "at91rm9200_ssc.2", &ssc2_clk),
+	CLKDEV_CON_DEV_ID("pclk", "fffd0000.ssc", &ssc0_clk),
+	CLKDEV_CON_DEV_ID("pclk", "fffd4000.ssc", &ssc1_clk),
+	CLKDEV_CON_DEV_ID("pclk", "fffd8000.ssc", &ssc2_clk),
 	CLKDEV_CON_DEV_ID(NULL, "i2c-at91rm9200.0", &twi_clk),
 	/* fake hclk clock */
 	CLKDEV_CON_DEV_ID("hclk", "at91_ohci", &ohci_clk),
diff --git a/arch/arm/mach-at91/at91sam9260.c b/arch/arm/mach-at91/at91sam9260.c
index d14ab6a..a41eb3d 100644
--- a/arch/arm/mach-at91/at91sam9260.c
+++ b/arch/arm/mach-at91/at91sam9260.c
@@ -211,6 +211,7 @@ static struct clk_lookup periph_clocks_lookups[] = {
 	CLKDEV_CON_DEV_ID("t1_clk", "atmel_tcb.1", &tc4_clk),
 	CLKDEV_CON_DEV_ID("t2_clk", "atmel_tcb.1", &tc5_clk),
 	CLKDEV_CON_DEV_ID("pclk", "at91rm9200_ssc.0", &ssc_clk),
+	CLKDEV_CON_DEV_ID("pclk", "fffbc000.ssc", &ssc_clk),
 	CLKDEV_CON_DEV_ID(NULL, "i2c-at91sam9260.0", &twi_clk),
 	CLKDEV_CON_DEV_ID(NULL, "i2c-at91sam9g20.0", &twi_clk),
 	/* more usart lookup table for DT entries */
diff --git a/arch/arm/mach-at91/at91sam9261.c b/arch/arm/mach-at91/at91sam9261.c
index c7b6057..7fcbe05 100644
--- a/arch/arm/mach-at91/at91sam9261.c
+++ b/arch/arm/mach-at91/at91sam9261.c
@@ -177,6 +177,9 @@ static struct clk_lookup periph_clocks_lookups[] = {
 	CLKDEV_CON_DEV_ID("pclk", "at91rm9200_ssc.0", &ssc0_clk),
 	CLKDEV_CON_DEV_ID("pclk", "at91rm9200_ssc.1", &ssc1_clk),
 	CLKDEV_CON_DEV_ID("pclk", "at91rm9200_ssc.2", &ssc2_clk),
+	CLKDEV_CON_DEV_ID("pclk", "fffbc000.ssc", &ssc0_clk),
+	CLKDEV_CON_DEV_ID("pclk", "fffc0000.ssc", &ssc1_clk),
+	CLKDEV_CON_DEV_ID("pclk", "fffc4000.ssc", &ssc2_clk),
 	CLKDEV_CON_DEV_ID("hclk", "at91_ohci", &hck0),
 	CLKDEV_CON_DEV_ID(NULL, "i2c-at91sam9261.0", &twi_clk),
 	CLKDEV_CON_DEV_ID(NULL, "i2c-at91sam9g10.0", &twi_clk),
diff --git a/arch/arm/mach-at91/at91sam9263.c b/arch/arm/mach-at91/at91sam9263.c
index b87be18..c0f4c8c 100644
--- a/arch/arm/mach-at91/at91sam9263.c
+++ b/arch/arm/mach-at91/at91sam9263.c
@@ -188,6 +188,8 @@ static struct clk_lookup periph_clocks_lookups[] = {
 	CLKDEV_CON_ID("hclk", &macb_clk),
 	CLKDEV_CON_DEV_ID("pclk", "at91rm9200_ssc.0", &ssc0_clk),
 	CLKDEV_CON_DEV_ID("pclk", "at91rm9200_ssc.1", &ssc1_clk),
+	CLKDEV_CON_DEV_ID("pclk", "fff98000.ssc", &ssc0_clk),
+	CLKDEV_CON_DEV_ID("pclk", "fff9c000.ssc", &ssc1_clk),
 	CLKDEV_CON_DEV_ID("mci_clk", "atmel_mci.0", &mmc0_clk),
 	CLKDEV_CON_DEV_ID("mci_clk", "atmel_mci.1", &mmc1_clk),
 	CLKDEV_CON_DEV_ID("spi_clk", "atmel_spi.0", &spi0_clk),
diff --git a/arch/arm/mach-at91/at91sam9g45.c b/arch/arm/mach-at91/at91sam9g45.c
index f4f96a6..a4282d3 100644
--- a/arch/arm/mach-at91/at91sam9g45.c
+++ b/arch/arm/mach-at91/at91sam9g45.c
@@ -241,6 +241,8 @@ static struct clk_lookup periph_clocks_lookups[] = {
 	CLKDEV_CON_DEV_ID(NULL, "i2c-at91sam9g10.1", &twi1_clk),
 	CLKDEV_CON_DEV_ID("pclk", "at91sam9g45_ssc.0", &ssc0_clk),
 	CLKDEV_CON_DEV_ID("pclk", "at91sam9g45_ssc.1", &ssc1_clk),
+	CLKDEV_CON_DEV_ID("pclk", "fff9c000.ssc", &ssc0_clk),
+	CLKDEV_CON_DEV_ID("pclk", "fffa0000.ssc", &ssc1_clk),
 	CLKDEV_CON_DEV_ID(NULL, "atmel-trng", &trng_clk),
 	CLKDEV_CON_DEV_ID(NULL, "atmel_sha", &aestdessha_clk),
 	CLKDEV_CON_DEV_ID(NULL, "atmel_tdes", &aestdessha_clk),
diff --git a/arch/arm/mach-at91/at91sam9rl.c b/arch/arm/mach-at91/at91sam9rl.c
index 4110b54..b683fdc 100644
--- a/arch/arm/mach-at91/at91sam9rl.c
+++ b/arch/arm/mach-at91/at91sam9rl.c
@@ -186,6 +186,8 @@ static struct clk_lookup periph_clocks_lookups[] = {
 	CLKDEV_CON_DEV_ID("t2_clk", "atmel_tcb.0", &tc2_clk),
 	CLKDEV_CON_DEV_ID("pclk", "at91rm9200_ssc.0", &ssc0_clk),
 	CLKDEV_CON_DEV_ID("pclk", "at91rm9200_ssc.1", &ssc1_clk),
+	CLKDEV_CON_DEV_ID("pclk", "fffc0000.ssc", &ssc0_clk),
+	CLKDEV_CON_DEV_ID("pclk", "fffc4000.ssc", &ssc1_clk),
 	CLKDEV_CON_DEV_ID(NULL, "i2c-at91sam9g20.0", &twi0_clk),
 	CLKDEV_CON_DEV_ID(NULL, "i2c-at91sam9g20.1", &twi1_clk),
 	CLKDEV_CON_ID("pioA", &pioA_clk),
diff --git a/arch/arm/mach-at91/at91sam9x5.c b/arch/arm/mach-at91/at91sam9x5.c
index e503538..18fbbb2 100644
--- a/arch/arm/mach-at91/at91sam9x5.c
+++ b/arch/arm/mach-at91/at91sam9x5.c
@@ -231,6 +231,7 @@ static struct clk_lookup periph_clocks_lookups[] = {
 	CLKDEV_CON_DEV_ID("t0_clk", "f800c000.timer", &tcb0_clk),
 	CLKDEV_CON_DEV_ID("dma_clk", "ffffec00.dma-controller", &dma0_clk),
 	CLKDEV_CON_DEV_ID("dma_clk", "ffffee00.dma-controller", &dma1_clk),
+	CLKDEV_CON_DEV_ID("pclk", "f0010000.ssc", &ssc_clk),
 	CLKDEV_CON_DEV_ID(NULL, "f8010000.i2c", &twi0_clk),
 	CLKDEV_CON_DEV_ID(NULL, "f8014000.i2c", &twi1_clk),
 	CLKDEV_CON_DEV_ID(NULL, "f8018000.i2c", &twi2_clk),
diff --git a/drivers/misc/atmel-ssc.c b/drivers/misc/atmel-ssc.c
index f40abd8..a769719 100644
--- a/drivers/misc/atmel-ssc.c
+++ b/drivers/misc/atmel-ssc.c
@@ -18,6 +18,8 @@
 #include <linux/slab.h>
 #include <linux/module.h>
 
+#include <linux/of.h>
+
 /* Serialize access to ssc_list and user count */
 static DEFINE_SPINLOCK(user_lock);
 static LIST_HEAD(ssc_list);
@@ -29,7 +31,13 @@ struct ssc_device *ssc_request(unsigned int ssc_num)
 
 	spin_lock(&user_lock);
 	list_for_each_entry(ssc, &ssc_list, list) {
-		if (ssc->pdev->id == ssc_num) {
+		if (ssc->pdev->dev.of_node) {
+			if (of_alias_get_id(ssc->pdev->dev.of_node, "ssc")
+				== ssc_num) {
+				ssc_valid = 1;
+				break;
+			}
+		} else if (ssc->pdev->id == ssc_num) {
 			ssc_valid = 1;
 			break;
 		}
@@ -88,10 +96,41 @@ static const struct platform_device_id atmel_ssc_devtypes[] = {
 	}
 };
 
+#ifdef CONFIG_OF
+static const struct of_device_id atmel_ssc_dt_ids[] = {
+	{
+		.compatible = "atmel,at91rm9200-ssc",
+		.data = &at91rm9200_config,
+	}, {
+		.compatible = "atmel,at91sam9g45-ssc",
+		.data = &at91sam9g45_config,
+	}, {
+		/* sentinel */
+	}
+};
+MODULE_DEVICE_TABLE(of, atmel_ssc_dt_ids);
+#endif
+
+static inline const struct atmel_ssc_platform_data * __init
+	atmel_ssc_get_driver_data(struct platform_device *pdev)
+{
+	if (pdev->dev.of_node) {
+		const struct of_device_id *match;
+		match = of_match_node(atmel_ssc_dt_ids, pdev->dev.of_node);
+		if (match == NULL)
+			return NULL;
+		return match->data;
+	}
+
+	return (struct atmel_ssc_platform_data *)
+		platform_get_device_id(pdev)->driver_data;
+}
+
 static int ssc_probe(struct platform_device *pdev)
 {
 	struct resource *regs;
 	struct ssc_device *ssc;
+	const struct atmel_ssc_platform_data *plat_dat;
 
 	ssc = devm_kzalloc(&pdev->dev, sizeof(struct ssc_device), GFP_KERNEL);
 	if (!ssc) {
@@ -100,8 +139,11 @@ static int ssc_probe(struct platform_device *pdev)
 	}
 
 	ssc->pdev = pdev;
-	ssc->pdata = (struct atmel_ssc_platform_data *)
-			platform_get_device_id(pdev)->driver_data;
+
+	plat_dat = atmel_ssc_get_driver_data(pdev);
+	if (!plat_dat)
+		return -ENODEV;
+	ssc->pdata = (struct atmel_ssc_platform_data *)plat_dat;
 
 	regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (!regs) {
@@ -160,6 +202,7 @@ static struct platform_driver ssc_driver = {
 	.driver		= {
 		.name		= "ssc",
 		.owner		= THIS_MODULE,
+		.of_match_table	= of_match_ptr(atmel_ssc_dt_ids),
 	},
 	.id_table	= atmel_ssc_devtypes,
 	.probe		= ssc_probe,
-- 
1.7.9.5

Re: [Resend PATCH v3 2/4] ARM: at91: atmel-ssc: add device tree support

[Mark Brown]

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

On Wed, Nov 07, 2012 at 11:41:41AM +0800, Bo Shen wrote:
> Add atmel-ssc for device tree support
> 
> Match "atmel,at91rm9200-ssc" for using pdc for data transfer
> Match "atmel,at91sam9g45-ssc" for using dma for data transfer

Applied, thanks.

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

Re: [Resend PATCH v3 2/4] ARM: at91: atmel-ssc: add device tree support

[Jean-Christophe PLAGNIOL-VILLARD]

On 09:24 Wed 07 Nov     , Mark Brown wrote:
> On Wed, Nov 07, 2012 at 11:41:41AM +0800, Bo Shen wrote:
> > Add atmel-ssc for device tree support
> > 
> > Match "atmel,at91rm9200-ssc" for using pdc for data transfer
> > Match "atmel,at91sam9g45-ssc" for using dma for data transfer
> 
> Applied, thanks.

Mark I nack the patch series this patch series does not run on any DT kernel

as no picntrl is done

do not apply until this patfh handle the pinctrl

Best Regards,
J.

Re: [Resend PATCH v3 2/4] ARM: at91: atmel-ssc: add device tree support

[Mark Brown]

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

On Wed, Nov 07, 2012 at 09:47:33AM +0100, Jean-Christophe PLAGNIOL-VILLARD wrote:

> Mark I nack the patch series this patch series does not run on any DT kernel

> as no picntrl is done

> do not apply until this patfh handle the pinctrl

Nicholas seemed to be disagreeing you on that one and his observation
that it won't be an issue until v3.8 anyway seemed totally reasonable.
In any case it doesn't seems like it'll be an issue to get it resolved
before it actually gets merged down to become an issue.

Of course there's also some ongoing discussion on the question of having
to add mindless pinctrl_get_set_default() to all the drivers at all, the
fact that we're having to do that at all seems like a failure to me at
least.

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

Re: [Resend PATCH v3 2/4] ARM: at91: atmel-ssc: add device tree support

[Jean-Christophe PLAGNIOL-VILLARD]

On 11:41 Wed 07 Nov     , Bo Shen wrote:
> Add atmel-ssc for device tree support
> 
> Match "atmel,at91rm9200-ssc" for using pdc for data transfer
> Match "atmel,at91sam9g45-ssc" for using dma for data transfer
> 
> Signed-off-by: Bo Shen <voice.shen@atmel.com>
> Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>
> ---
> Resend
>   Fix a error find by Fengguang Yu
> 
>   Although there is a NACK from Jean-Christophe PLAGNIOL-VILLARD,
>   however the pinctrl for at91 is not in mainline. So, plan to add
>   pinctrl support after the pinctrl driver for at91 is merged.
yes the pinctrl is mainline in -next via pinctrl tree

s code does not work on the DT kernel untill it's done

NACK stand I refuse this touch the at91 dtsi untiol it's done correctly

Best Regards,
J.

Re: [Resend PATCH v3 2/4] ARM: at91: atmel-ssc: add device tree support

[Mark Brown]

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

On Wed, Nov 07, 2012 at 09:49:32AM +0100, Jean-Christophe PLAGNIOL-VILLARD wrote:

> yes the pinctrl is mainline in -next via pinctrl tree

> s code does not work on the DT kernel untill it's done

> NACK stand I refuse this touch the at91 dtsi untiol it's done correctly

...and here's another mail (plus a further private mail on the same
subject) by the time I managed to reply to the first one, the timestamps
say in about ten minutes :/  What I'm seeing here is two of the Atmel
maintainers disagreeing about a patch so I've got to pick one view and
right now applying the patch looks like it moves us further forwards.

Like I say it's not an issue right now and it seems like it should be
trivial to fix.  Is anything needed here other than adding the
pinctrl_get_set_default() call, in which case would it not be less
effort all round to just send the patch?  If there is more involved then
what is needed?

Thinking about it a little more I'm actually wondering why whatever
pinctrl change made this mandatory didn't go through and update all the
drivers.

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

Re: [Resend PATCH v3 2/4] ARM: at91: atmel-ssc: add device tree support

[Jean-Christophe PLAGNIOL-VILLARD]

On 10:21 Wed 07 Nov     , Mark Brown wrote:
> On Wed, Nov 07, 2012 at 09:49:32AM +0100, Jean-Christophe PLAGNIOL-VILLARD wrote:
> 
> > yes the pinctrl is mainline in -next via pinctrl tree
> 
> > s code does not work on the DT kernel untill it's done
> 
> > NACK stand I refuse this touch the at91 dtsi untiol it's done correctly
> 
> ...and here's another mail (plus a further private mail on the same
> subject) by the time I managed to reply to the first one, the timestamps
> say in about ten minutes :/  What I'm seeing here is two of the Atmel
> maintainers disagreeing about a patch so I've got to pick one view and
> right now applying the patch looks like it moves us further forwards.
> 
> Like I say it's not an issue right now and it seems like it should be
> trivial to fix.  Is anything needed here other than adding the
> pinctrl_get_set_default() call, in which case would it not be less
> effort all round to just send the patch?  If there is more involved then
> what is needed?
for the driver point of view I agreee with you for the dtsi I disagree I miss
100 lignes of pinctrl description

so today if no pinctrl the pin are input gpio and the drivers will not work

if this code was tested this work MUST have been done otherwise we will apply
non tested work

so dor the dtsi part I do NACK it and the pinctrl code is arropund for 2 or 3
months so no excuse even Joachim who add the rm9200 support to DT handled the
pinctrl so no exeption on at91 when switching to DT the pinctrl is mandatory

If you want to apply the part of the patch that touch only the drivers I'm
fine but for the dtsi no


Best Regards,
J.
> 
> Thinking about it a little more I'm actually wondering why whatever
> pinctrl change made this mandatory didn't go through and update all the
> drivers.

Re: [Resend PATCH v3 2/4] ARM: at91: atmel-ssc: add device tree support

[Mark Brown]

On Wed, Nov 07, 2012 at 11:41:21AM +0100, Jean-Christophe PLAGNIOL-VILLARD wrote:
> On 10:21 Wed 07 Nov     , Mark Brown wrote:

> > Like I say it's not an issue right now and it seems like it should be
> > trivial to fix.  Is anything needed here other than adding the
> > pinctrl_get_set_default() call, in which case would it not be less
> > effort all round to just send the patch?  If there is more involved then
> > what is needed?

> for the driver point of view I agreee with you for the dtsi I disagree I miss
> 100 lignes of pinctrl description

> so today if no pinctrl the pin are input gpio and the drivers will not work

> if this code was tested this work MUST have been done otherwise we will apply
> non tested work

> so dor the dtsi part I do NACK it and the pinctrl code is arropund for 2 or 3
> months so no excuse even Joachim who add the rm9200 support to DT handled the
> pinctrl so no exeption on at91 when switching to DT the pinctrl is mandatory

> If you want to apply the part of the patch that touch only the drivers I'm
> fine but for the dtsi no

So what the above is saying is that we should have a followup patch to
add the pinctrl setup to the DT (probably one that goes along with the
pinctrl changes and adds complete pinctrl setup for the affected boards
rather than just doing the audio piecemeal) and do the mindless call in
the driver.  That doesn't seem impossible.

So long as we get those things in place prior to the merge window we
should be fine.  Even if we don't manage that it still seems like having
the patch is better than not having it, from the sounds of it the boards
are going to be totally broken if they boot with DT but without the
pinctrl setup and the ASoC bindings are pretty much an orthogonal issue
to that.  Having the bindings for the SSC (which depend on this AFAICT)
will allow other machines which are doing a better job with pinctrl to
work.

Does that seem reasonable?

Re: [Resend PATCH v3 2/4] ARM: at91: atmel-ssc: add device tree support

[Jean-Christophe PLAGNIOL-VILLARD]

On 14:05 Wed 07 Nov     , Mark Brown wrote:
> On Wed, Nov 07, 2012 at 11:41:21AM +0100, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > On 10:21 Wed 07 Nov     , Mark Brown wrote:
> 
> > > Like I say it's not an issue right now and it seems like it should be
> > > trivial to fix.  Is anything needed here other than adding the
> > > pinctrl_get_set_default() call, in which case would it not be less
> > > effort all round to just send the patch?  If there is more involved then
> > > what is needed?
> 
> > for the driver point of view I agreee with you for the dtsi I disagree I miss
> > 100 lignes of pinctrl description
> 
> > so today if no pinctrl the pin are input gpio and the drivers will not work
> 
> > if this code was tested this work MUST have been done otherwise we will apply
> > non tested work
> 
> > so dor the dtsi part I do NACK it and the pinctrl code is arropund for 2 or 3
> > months so no excuse even Joachim who add the rm9200 support to DT handled the
> > pinctrl so no exeption on at91 when switching to DT the pinctrl is mandatory
> 
> > If you want to apply the part of the patch that touch only the drivers I'm
> > fine but for the dtsi no
> 
> So what the above is saying is that we should have a followup patch to
> add the pinctrl setup to the DT (probably one that goes along with the
> pinctrl changes and adds complete pinctrl setup for the affected boards
> rather than just doing the audio piecemeal) and do the mindless call in
> the driver.  That doesn't seem impossible.
> 
> So long as we get those things in place prior to the merge window we
> should be fine.  Even if we don't manage that it still seems like having
> the patch is better than not having it, from the sounds of it the boards
> are going to be totally broken if they boot with DT but without the
> pinctrl setup and the ASoC bindings are pretty much an orthogonal issue
> to that.  Having the bindings for the SSC (which depend on this AFAICT)
> will allow other machines which are doing a better job with pinctrl to
> work.
so for me Bo re do his patch series by splitting in 2 patch seires

the one via ASoC for the driver with the pinctrl for now but MUST add it later

the arch/arm related via AT91 tree where the pinctrl is mandatory in the dtsi

I'm no going to spend hours to do the pinctrl to test Bo patch that he is
supposed to have deon to test this code

As basically today on DT none of the board will work if BO does no provide the
pinctrl so for me he MUST put in the dtsi

is it ok with you?

Best Regards,
J.

Re: [Resend PATCH v3 2/4] ARM: at91: atmel-ssc: add device tree support

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 1192 bytes --]

On Wed, Nov 07, 2012 at 03:30:46PM +0100, Jean-Christophe PLAGNIOL-VILLARD wrote:

> the one via ASoC for the driver with the pinctrl for now but MUST add it later

> the arch/arm related via AT91 tree where the pinctrl is mandatory in the dtsi

> I'm no going to spend hours to do the pinctrl to test Bo patch that he is
> supposed to have deon to test this code

> As basically today on DT none of the board will work if BO does no provide the
> pinctrl so for me he MUST put in the dtsi

> is it ok with you?

Or add the pinmux bindings for other IPs over in the pinmux branch and
the pinmux binding for SSC here (which will help with bisection as the
DT bindings for the audio will then always exist in conjunction with the
.dtsi changes for them).  Either way is fine with me; I'll leave things
as they are until I see some new patches.

Though I have to say now that I look in -next it's a bit surprising that
there are so many IPs in the file that have no pinmix configuration,
based on what you're saying it seems like the entire board will be
falling over with the pinmux code anyway - in the case of the sam9g20ek
we'd at least need I2C pinmux settings.  I may be misunderstanding.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 192 bytes --]

_______________________________________________
devicetree-discuss mailing list
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [Resend PATCH v3 2/4] ARM: at91: atmel-ssc: add device tree support

[Jean-Christophe PLAGNIOL-VILLARD]

On 15:44 Wed 07 Nov     , Mark Brown wrote:
> On Wed, Nov 07, 2012 at 03:30:46PM +0100, Jean-Christophe PLAGNIOL-VILLARD wrote:
> 
> > the one via ASoC for the driver with the pinctrl for now but MUST add it later
> 
> > the arch/arm related via AT91 tree where the pinctrl is mandatory in the dtsi
> 
> > I'm no going to spend hours to do the pinctrl to test Bo patch that he is
> > supposed to have deon to test this code
> 
> > As basically today on DT none of the board will work if BO does no provide the
> > pinctrl so for me he MUST put in the dtsi
> 
> > is it ok with you?
> 
> Or add the pinmux bindings for other IPs over in the pinmux branch and
> the pinmux binding for SSC here (which will help with bisection as the
> DT bindings for the audio will then always exist in conjunction with the
> .dtsi changes for them).  Either way is fine with me; I'll leave things
> as they are until I see some new patches.
> 
> Though I have to say now that I look in -next it's a bit surprising that
> there are so many IPs in the file that have no pinmix configuration,
> based on what you're saying it seems like the entire board will be
> falling over with the pinmux code anyway - in the case of the sam9g20ek
> we'd at least need I2C pinmux settings.  I may be misunderstanding.

yes I've a new patch series that finish to add them as they was in the Dt
before the add of the pinctrl (macb/usb device /host) i2c after as on 9g20ek
we use gpio i2c so no need to do any pinctrl for now

but now new switch to DT must handle the pinctrl

Best Regards,
J.

Re: [Resend PATCH v3 2/4] ARM: at91: atmel-ssc: add device tree support

[Jean-Christophe PLAGNIOL-VILLARD]

On 15:44 Wed 07 Nov     , Mark Brown wrote:
> On Wed, Nov 07, 2012 at 03:30:46PM +0100, Jean-Christophe PLAGNIOL-VILLARD wrote:
> 
> > the one via ASoC for the driver with the pinctrl for now but MUST add it later
> 
> > the arch/arm related via AT91 tree where the pinctrl is mandatory in the dtsi
> 
> > I'm no going to spend hours to do the pinctrl to test Bo patch that he is
> > supposed to have deon to test this code
> 
> > As basically today on DT none of the board will work if BO does no provide the
> > pinctrl so for me he MUST put in the dtsi
> 
> > is it ok with you?
> 
> Or add the pinmux bindings for other IPs over in the pinmux branch and
> the pinmux binding for SSC here (which will help with bisection as the
> DT bindings for the audio will then always exist in conjunction with the
> .dtsi changes for them).  Either way is fine with me; I'll leave things
> as they are until I see some new patches.
for where it's merge honestly I do not care much but the pinctrl must be
handled correctly

Best Regards,
J.

Re: [Resend PATCH v3 2/4] ARM: at91: atmel-ssc: add device tree support

[Bo Shen]

Hi J,

On 11/7/2012 23:16, Jean-Christophe PLAGNIOL-VILLARD wrote:
> On 15:44 Wed 07 Nov     , Mark Brown wrote:
>> On Wed, Nov 07, 2012 at 03:30:46PM +0100, Jean-Christophe PLAGNIOL-VILLARD wrote:
>>
>>> the one via ASoC for the driver with the pinctrl for now but MUST add it later
>>
>>> the arch/arm related via AT91 tree where the pinctrl is mandatory in the dtsi
>>
>>> I'm no going to spend hours to do the pinctrl to test Bo patch that he is
>>> supposed to have deon to test this code
>>
>>> As basically today on DT none of the board will work if BO does no provide the
>>> pinctrl so for me he MUST put in the dtsi
>>
>>> is it ok with you?
>>
>> Or add the pinmux bindings for other IPs over in the pinmux branch and
>> the pinmux binding for SSC here (which will help with bisection as the
>> DT bindings for the audio will then always exist in conjunction with the
>> .dtsi changes for them).  Either way is fine with me; I'll leave things
>> as they are until I see some new patches.
> for where it's merge honestly I do not care much but the pinctrl must be
> handled correctly

I will send patches to add pinctrl support.
For atmel ssc driver, it is simple, however for dtsi, which point should 
I based on? which git and which branch? Because, I am not sure which 
branch on git://github.com/at91linux/linux-at91.git is the right point.

Thanks.

Best Regards,
Bo Shen

> Best Regards,
> J.
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>