[PATCH] pwm: renesas-tpu: Add DT support

[PATCH] pwm: renesas-tpu: Add DT support

[Laurent Pinchart]

Specify DT bindings for the TPU PWM controller and add OF support to the
driver.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 .../devicetree/bindings/pwm/renesas,tpu-pwm.txt    | 28 +++++++++++++++
 drivers/pwm/pwm-renesas-tpu.c                      | 41 ++++++++++++++++++----
 2 files changed, 62 insertions(+), 7 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/pwm/renesas,tpu-pwm.txt

This patch depends on the "[PATCH v2 0/4] Add PWM polarity flag macro for DT"
series that is scheduled for merge in the PWM tree, and should thus go in via
the same tree.

The code has been tested on an Armadillo800EVA with additional platform patches
that I'm going to submit next.

diff --git a/Documentation/devicetree/bindings/pwm/renesas,tpu-pwm.txt b/Documentation/devicetree/bindings/pwm/renesas,tpu-pwm.txt
new file mode 100644
index 0000000..b067e84
--- /dev/null
+++ b/Documentation/devicetree/bindings/pwm/renesas,tpu-pwm.txt
@@ -0,0 +1,28 @@
+* Renesas R-Car Timer Pulse Unit PWM Controller
+
+Required Properties:
+
+  - compatible: should be one of the following.
+    - "renesas,tpu-r8a73a4": for R8A77A4 (R-Mobile APE6) compatible PWM controller.
+    - "renesas,tpu-r8a7740": for R8A7740 (R-Mobile A1) compatible PWM controller.
+    - "renesas,tpu-r8a7790": for R8A7790 (R-Car H2) compatible PWM controller.
+    - "renesas,tpu-sh7372": for SH7372 (SH-Mobile AP4) compatible PWM controller.
+    - "renesas,tpu": for generic R-Car TPU PWM controller.
+
+  - reg: Base address and length of each memory resource used by the PWM
+    controller hardware module.
+
+  - #pwm-cells: should be 3. See pwm.txt in this directory for a description of
+    the cells format. The only third cell flag supported by this binding is
+    PWM_POLARITY_INVERTED.
+
+Please refer to pwm.txt in this directory for details of the common PWM bindings
+used by client devices.
+
+Example: R8A7740 (R-Car A1) TPU controller node
+
+	tpu: pwm@e6600000 {
+		compatible = "renesas,tpu-r8a7740", "renesas,tpu";
+		reg = <0xe6600000 0x100>;
+		#pwm-cells = <3>;
+	};
diff --git a/drivers/pwm/pwm-renesas-tpu.c b/drivers/pwm/pwm-renesas-tpu.c
index 2600892..3eeffff 100644
--- a/drivers/pwm/pwm-renesas-tpu.c
+++ b/drivers/pwm/pwm-renesas-tpu.c
@@ -20,6 +20,7 @@
 #include <linux/ioport.h>
 #include <linux/module.h>
 #include <linux/mutex.h>
+#include <linux/of.h>
 #include <linux/platform_data/pwm-renesas-tpu.h>
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
@@ -86,7 +87,7 @@ struct tpu_pwm_device {
 
 struct tpu_device {
 	struct platform_device *pdev;
-	struct tpu_pwm_platform_data *pdata;
+	enum pwm_polarity polarities[TPU_CHANNEL_MAX];
 	struct pwm_chip chip;
 	spinlock_t lock;
 
@@ -228,8 +229,7 @@ static int tpu_pwm_request(struct pwm_chip *chip, struct pwm_device *_pwm)
 
 	pwm->tpu = tpu;
 	pwm->channel = _pwm->hwpwm;
-	pwm->polarity = tpu->pdata ? tpu->pdata->channels[pwm->channel].polarity
-		      : PWM_POLARITY_NORMAL;
+	pwm->polarity = tpu->polarities[pwm->channel];
 	pwm->prescaler = 0;
 	pwm->period = 0;
 	pwm->duty = 0;
@@ -388,6 +388,16 @@ static const struct pwm_ops tpu_pwm_ops = {
  * Probe and remove
  */
 
+static void tpu_parse_pdata(struct tpu_device *tpu)
+{
+	struct tpu_pwm_platform_data *pdata = tpu->pdev->dev.platform_data;
+	unsigned int i;
+
+	for (i = 0; i < ARRAY_SIZE(tpu->polarities); ++i)
+		tpu->polarities[i] = pdata ? pdata->channels[i].polarity
+				   : PWM_POLARITY_NORMAL;
+}
+
 static int tpu_probe(struct platform_device *pdev)
 {
 	struct tpu_device *tpu;
@@ -400,7 +410,11 @@ static int tpu_probe(struct platform_device *pdev)
 		return -ENOMEM;
 	}
 
-	tpu->pdata = pdev->dev.platform_data;
+	spin_lock_init(&tpu->lock);
+	tpu->pdev = pdev;
+
+	/* Initialize device configuration from platform data. */
+	tpu_parse_pdata(tpu);
 
 	/* Map memory, get clock and pin control. */
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
@@ -422,11 +436,10 @@ static int tpu_probe(struct platform_device *pdev)
 	/* Initialize and register the device. */
 	platform_set_drvdata(pdev, tpu);
 
-	spin_lock_init(&tpu->lock);
-	tpu->pdev = pdev;
-
 	tpu->chip.dev = &pdev->dev;
 	tpu->chip.ops = &tpu_pwm_ops;
+	tpu->chip.of_xlate = of_pwm_xlate_with_flags;
+	tpu->chip.of_pwm_n_cells = 3;
 	tpu->chip.base = -1;
 	tpu->chip.npwm = TPU_CHANNEL_MAX;
 
@@ -457,12 +470,26 @@ static int tpu_remove(struct platform_device *pdev)
 	return 0;
 }
 
+#ifdef CONFIG_OF
+static const struct of_device_id tpu_of_table[] = {
+	{ .compatible = "renesas,tpu-r8a73a4", },
+	{ .compatible = "renesas,tpu-r8a7740", },
+	{ .compatible = "renesas,tpu-r8a7790", },
+	{ .compatible = "renesas,tpu-sh7372", },
+	{ .compatible = "renesas,tpu", },
+	{ },
+};
+
+MODULE_DEVICE_TABLE(of, tpu_of_table);
+#endif
+
 static struct platform_driver tpu_driver = {
 	.probe		= tpu_probe,
 	.remove		= tpu_remove,
 	.driver		= {
 		.name	= "renesas-tpu-pwm",
 		.owner	= THIS_MODULE,
+		.of_match_table = of_match_ptr(tpu_of_table),
 	}
 };
 
-- 
Regards,

Laurent Pinchart

Re: [PATCH] pwm: renesas-tpu: Add DT support

[Laurent Pinchart]

Hi Thierry,

I'd like to get this patch in v3.12, could you please take it in your tree ?

On Friday 26 July 2013 00:27:41 Laurent Pinchart wrote:
> Specify DT bindings for the TPU PWM controller and add OF support to the
> driver.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
>  .../devicetree/bindings/pwm/renesas,tpu-pwm.txt    | 28 +++++++++++++++
>  drivers/pwm/pwm-renesas-tpu.c                      | 41 +++++++++++++++----
>  2 files changed, 62 insertions(+), 7 deletions(-)
>  create mode 100644
> Documentation/devicetree/bindings/pwm/renesas,tpu-pwm.txt
> 
> This patch depends on the "[PATCH v2 0/4] Add PWM polarity flag macro for
> DT" series that is scheduled for merge in the PWM tree, and should thus go
> in via the same tree.
> 
> The code has been tested on an Armadillo800EVA with additional platform
> patches that I'm going to submit next.
> 
> diff --git a/Documentation/devicetree/bindings/pwm/renesas,tpu-pwm.txt
> b/Documentation/devicetree/bindings/pwm/renesas,tpu-pwm.txt new file mode
> 100644
> index 0000000..b067e84
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pwm/renesas,tpu-pwm.txt
> @@ -0,0 +1,28 @@
> +* Renesas R-Car Timer Pulse Unit PWM Controller
> +
> +Required Properties:
> +
> +  - compatible: should be one of the following.
> +    - "renesas,tpu-r8a73a4": for R8A77A4 (R-Mobile APE6) compatible PWM
> controller. +    - "renesas,tpu-r8a7740": for R8A7740 (R-Mobile A1)
> compatible PWM controller. +    - "renesas,tpu-r8a7790": for R8A7790 (R-Car
> H2) compatible PWM controller. +    - "renesas,tpu-sh7372": for SH7372
> (SH-Mobile AP4) compatible PWM controller. +    - "renesas,tpu": for
> generic R-Car TPU PWM controller.
> +
> +  - reg: Base address and length of each memory resource used by the PWM
> +    controller hardware module.
> +
> +  - #pwm-cells: should be 3. See pwm.txt in this directory for a
> description of +    the cells format. The only third cell flag supported by
> this binding is +    PWM_POLARITY_INVERTED.
> +
> +Please refer to pwm.txt in this directory for details of the common PWM
> bindings +used by client devices.
> +
> +Example: R8A7740 (R-Car A1) TPU controller node
> +
> +	tpu: pwm@e6600000 {
> +		compatible = "renesas,tpu-r8a7740", "renesas,tpu";
> +		reg = <0xe6600000 0x100>;
> +		#pwm-cells = <3>;
> +	};
> diff --git a/drivers/pwm/pwm-renesas-tpu.c b/drivers/pwm/pwm-renesas-tpu.c
> index 2600892..3eeffff 100644
> --- a/drivers/pwm/pwm-renesas-tpu.c
> +++ b/drivers/pwm/pwm-renesas-tpu.c
> @@ -20,6 +20,7 @@
>  #include <linux/ioport.h>
>  #include <linux/module.h>
>  #include <linux/mutex.h>
> +#include <linux/of.h>
>  #include <linux/platform_data/pwm-renesas-tpu.h>
>  #include <linux/platform_device.h>
>  #include <linux/pm_runtime.h>
> @@ -86,7 +87,7 @@ struct tpu_pwm_device {
> 
>  struct tpu_device {
>  	struct platform_device *pdev;
> -	struct tpu_pwm_platform_data *pdata;
> +	enum pwm_polarity polarities[TPU_CHANNEL_MAX];
>  	struct pwm_chip chip;
>  	spinlock_t lock;
> 
> @@ -228,8 +229,7 @@ static int tpu_pwm_request(struct pwm_chip *chip, struct
> pwm_device *_pwm)
> 
>  	pwm->tpu = tpu;
>  	pwm->channel = _pwm->hwpwm;
> -	pwm->polarity = tpu->pdata ? tpu->pdata->channels[pwm->channel].polarity
> -		      : PWM_POLARITY_NORMAL;
> +	pwm->polarity = tpu->polarities[pwm->channel];
>  	pwm->prescaler = 0;
>  	pwm->period = 0;
>  	pwm->duty = 0;
> @@ -388,6 +388,16 @@ static const struct pwm_ops tpu_pwm_ops = {
>   * Probe and remove
>   */
> 
> +static void tpu_parse_pdata(struct tpu_device *tpu)
> +{
> +	struct tpu_pwm_platform_data *pdata = tpu->pdev->dev.platform_data;
> +	unsigned int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(tpu->polarities); ++i)
> +		tpu->polarities[i] = pdata ? pdata->channels[i].polarity
> +				   : PWM_POLARITY_NORMAL;
> +}
> +
>  static int tpu_probe(struct platform_device *pdev)
>  {
>  	struct tpu_device *tpu;
> @@ -400,7 +410,11 @@ static int tpu_probe(struct platform_device *pdev)
>  		return -ENOMEM;
>  	}
> 
> -	tpu->pdata = pdev->dev.platform_data;
> +	spin_lock_init(&tpu->lock);
> +	tpu->pdev = pdev;
> +
> +	/* Initialize device configuration from platform data. */
> +	tpu_parse_pdata(tpu);
> 
>  	/* Map memory, get clock and pin control. */
>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> @@ -422,11 +436,10 @@ static int tpu_probe(struct platform_device *pdev)
>  	/* Initialize and register the device. */
>  	platform_set_drvdata(pdev, tpu);
> 
> -	spin_lock_init(&tpu->lock);
> -	tpu->pdev = pdev;
> -
>  	tpu->chip.dev = &pdev->dev;
>  	tpu->chip.ops = &tpu_pwm_ops;
> +	tpu->chip.of_xlate = of_pwm_xlate_with_flags;
> +	tpu->chip.of_pwm_n_cells = 3;
>  	tpu->chip.base = -1;
>  	tpu->chip.npwm = TPU_CHANNEL_MAX;
> 
> @@ -457,12 +470,26 @@ static int tpu_remove(struct platform_device *pdev)
>  	return 0;
>  }
> 
> +#ifdef CONFIG_OF
> +static const struct of_device_id tpu_of_table[] = {
> +	{ .compatible = "renesas,tpu-r8a73a4", },
> +	{ .compatible = "renesas,tpu-r8a7740", },
> +	{ .compatible = "renesas,tpu-r8a7790", },
> +	{ .compatible = "renesas,tpu-sh7372", },
> +	{ .compatible = "renesas,tpu", },
> +	{ },
> +};
> +
> +MODULE_DEVICE_TABLE(of, tpu_of_table);
> +#endif
> +
>  static struct platform_driver tpu_driver = {
>  	.probe		= tpu_probe,
>  	.remove		= tpu_remove,
>  	.driver		= {
>  		.name	= "renesas-tpu-pwm",
>  		.owner	= THIS_MODULE,
> +		.of_match_table = of_match_ptr(tpu_of_table),
>  	}
>  };
-- 
Regards,

Laurent Pinchart

Re: [PATCH] pwm: renesas-tpu: Add DT support

[Thierry Reding]

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

On Thu, Aug 08, 2013 at 12:44:42PM +0200, Laurent Pinchart wrote:
> Hi Thierry,
> 
> I'd like to get this patch in v3.12, could you please take it in your tree ?

I'm Cc'ing the device tree bindings maintainers. Given that this uses
only the standard PWM bindings in the first place I suppose it would be
okay to take this in, but I'd like to run it past them to make sure.

> On Friday 26 July 2013 00:27:41 Laurent Pinchart wrote:
> > Specify DT bindings for the TPU PWM controller and add OF support to the
> > driver.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> > ---
> >  .../devicetree/bindings/pwm/renesas,tpu-pwm.txt    | 28 +++++++++++++++
> >  drivers/pwm/pwm-renesas-tpu.c                      | 41 +++++++++++++++----
> >  2 files changed, 62 insertions(+), 7 deletions(-)
> >  create mode 100644
> > Documentation/devicetree/bindings/pwm/renesas,tpu-pwm.txt
> > 
> > This patch depends on the "[PATCH v2 0/4] Add PWM polarity flag macro for
> > DT" series that is scheduled for merge in the PWM tree, and should thus go
> > in via the same tree.
> > 
> > The code has been tested on an Armadillo800EVA with additional platform
> > patches that I'm going to submit next.
> > 
> > diff --git a/Documentation/devicetree/bindings/pwm/renesas,tpu-pwm.txt
> > b/Documentation/devicetree/bindings/pwm/renesas,tpu-pwm.txt new file mode
> > 100644
> > index 0000000..b067e84
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/pwm/renesas,tpu-pwm.txt
> > @@ -0,0 +1,28 @@
> > +* Renesas R-Car Timer Pulse Unit PWM Controller
> > +
> > +Required Properties:
> > +
> > +  - compatible: should be one of the following.
> > +    - "renesas,tpu-r8a73a4": for R8A77A4 (R-Mobile APE6) compatible PWM
> > controller. +    - "renesas,tpu-r8a7740": for R8A7740 (R-Mobile A1)
> > compatible PWM controller. +    - "renesas,tpu-r8a7790": for R8A7790 (R-Car
> > H2) compatible PWM controller. +    - "renesas,tpu-sh7372": for SH7372
> > (SH-Mobile AP4) compatible PWM controller. +    - "renesas,tpu": for
> > generic R-Car TPU PWM controller.
> > +
> > +  - reg: Base address and length of each memory resource used by the PWM
> > +    controller hardware module.
> > +
> > +  - #pwm-cells: should be 3. See pwm.txt in this directory for a
> > description of +    the cells format. The only third cell flag supported by
> > this binding is +    PWM_POLARITY_INVERTED.
> > +
> > +Please refer to pwm.txt in this directory for details of the common PWM
> > bindings +used by client devices.
> > +
> > +Example: R8A7740 (R-Car A1) TPU controller node
> > +
> > +	tpu: pwm@e6600000 {
> > +		compatible = "renesas,tpu-r8a7740", "renesas,tpu";
> > +		reg = <0xe6600000 0x100>;
> > +		#pwm-cells = <3>;
> > +	};
> > diff --git a/drivers/pwm/pwm-renesas-tpu.c b/drivers/pwm/pwm-renesas-tpu.c
> > index 2600892..3eeffff 100644
> > --- a/drivers/pwm/pwm-renesas-tpu.c
> > +++ b/drivers/pwm/pwm-renesas-tpu.c
> > @@ -20,6 +20,7 @@
> >  #include <linux/ioport.h>
> >  #include <linux/module.h>
> >  #include <linux/mutex.h>
> > +#include <linux/of.h>
> >  #include <linux/platform_data/pwm-renesas-tpu.h>
> >  #include <linux/platform_device.h>
> >  #include <linux/pm_runtime.h>
> > @@ -86,7 +87,7 @@ struct tpu_pwm_device {
> > 
> >  struct tpu_device {
> >  	struct platform_device *pdev;
> > -	struct tpu_pwm_platform_data *pdata;
> > +	enum pwm_polarity polarities[TPU_CHANNEL_MAX];
> >  	struct pwm_chip chip;
> >  	spinlock_t lock;
> > 
> > @@ -228,8 +229,7 @@ static int tpu_pwm_request(struct pwm_chip *chip, struct
> > pwm_device *_pwm)
> > 
> >  	pwm->tpu = tpu;
> >  	pwm->channel = _pwm->hwpwm;
> > -	pwm->polarity = tpu->pdata ? tpu->pdata->channels[pwm->channel].polarity
> > -		      : PWM_POLARITY_NORMAL;
> > +	pwm->polarity = tpu->polarities[pwm->channel];
> >  	pwm->prescaler = 0;
> >  	pwm->period = 0;
> >  	pwm->duty = 0;
> > @@ -388,6 +388,16 @@ static const struct pwm_ops tpu_pwm_ops = {
> >   * Probe and remove
> >   */
> > 
> > +static void tpu_parse_pdata(struct tpu_device *tpu)
> > +{
> > +	struct tpu_pwm_platform_data *pdata = tpu->pdev->dev.platform_data;
> > +	unsigned int i;
> > +
> > +	for (i = 0; i < ARRAY_SIZE(tpu->polarities); ++i)
> > +		tpu->polarities[i] = pdata ? pdata->channels[i].polarity
> > +				   : PWM_POLARITY_NORMAL;
> > +}
> > +
> >  static int tpu_probe(struct platform_device *pdev)
> >  {
> >  	struct tpu_device *tpu;
> > @@ -400,7 +410,11 @@ static int tpu_probe(struct platform_device *pdev)
> >  		return -ENOMEM;
> >  	}
> > 
> > -	tpu->pdata = pdev->dev.platform_data;
> > +	spin_lock_init(&tpu->lock);
> > +	tpu->pdev = pdev;
> > +
> > +	/* Initialize device configuration from platform data. */
> > +	tpu_parse_pdata(tpu);
> > 
> >  	/* Map memory, get clock and pin control. */
> >  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > @@ -422,11 +436,10 @@ static int tpu_probe(struct platform_device *pdev)
> >  	/* Initialize and register the device. */
> >  	platform_set_drvdata(pdev, tpu);
> > 
> > -	spin_lock_init(&tpu->lock);
> > -	tpu->pdev = pdev;
> > -
> >  	tpu->chip.dev = &pdev->dev;
> >  	tpu->chip.ops = &tpu_pwm_ops;
> > +	tpu->chip.of_xlate = of_pwm_xlate_with_flags;
> > +	tpu->chip.of_pwm_n_cells = 3;
> >  	tpu->chip.base = -1;
> >  	tpu->chip.npwm = TPU_CHANNEL_MAX;
> > 
> > @@ -457,12 +470,26 @@ static int tpu_remove(struct platform_device *pdev)
> >  	return 0;
> >  }
> > 
> > +#ifdef CONFIG_OF
> > +static const struct of_device_id tpu_of_table[] = {
> > +	{ .compatible = "renesas,tpu-r8a73a4", },
> > +	{ .compatible = "renesas,tpu-r8a7740", },
> > +	{ .compatible = "renesas,tpu-r8a7790", },
> > +	{ .compatible = "renesas,tpu-sh7372", },
> > +	{ .compatible = "renesas,tpu", },
> > +	{ },
> > +};
> > +
> > +MODULE_DEVICE_TABLE(of, tpu_of_table);
> > +#endif
> > +
> >  static struct platform_driver tpu_driver = {
> >  	.probe		= tpu_probe,
> >  	.remove		= tpu_remove,
> >  	.driver		= {
> >  		.name	= "renesas-tpu-pwm",
> >  		.owner	= THIS_MODULE,
> > +		.of_match_table = of_match_ptr(tpu_of_table),

I'd like to point this out as an example of why I think aligning on the
= here is a bad idea. Eventually you're bound to add a field which is
longer than all the others and therefore can't be aligned consistently.
So instead of coming up with some kind of fancy formatting it often
turns out better to just use a regular single space around =. That you
can always consistently use, no matter how long the field names are.

To be clear, I don't expect you to change the patch because of this.

Thierry

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

Re: [PATCH] pwm: renesas-tpu: Add DT support

[Laurent Pinchart]

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

Hi Thierry,

On Monday 12 August 2013 09:21:58 Thierry Reding wrote:
> On Thu, Aug 08, 2013 at 12:44:42PM +0200, Laurent Pinchart wrote:
> > Hi Thierry,
> > 
> > I'd like to get this patch in v3.12, could you please take it in your tree
> > ?
>
> I'm Cc'ing the device tree bindings maintainers. Given that this uses
> only the standard PWM bindings in the first place I suppose it would be
> okay to take this in, but I'd like to run it past them to make sure.
> 
> > On Friday 26 July 2013 00:27:41 Laurent Pinchart wrote:
> > > Specify DT bindings for the TPU PWM controller and add OF support to the
> > > driver.
> > > 
> > > Signed-off-by: Laurent Pinchart
> > > <laurent.pinchart+renesas@ideasonboard.com>
> > > ---
> > > 
> > >  .../devicetree/bindings/pwm/renesas,tpu-pwm.txt    | 28 +++++++++++++++
> > >  drivers/pwm/pwm-renesas-tpu.c                      | 41 +++++++++++----
> > >  2 files changed, 62 insertions(+), 7 deletions(-)
> > >  create mode 100644
> > > 
> > > Documentation/devicetree/bindings/pwm/renesas,tpu-pwm.txt
> > > 
> > > This patch depends on the "[PATCH v2 0/4] Add PWM polarity flag macro
> > > for DT" series that is scheduled for merge in the PWM tree, and should
> > > thus go in via the same tree.
> > > 
> > > The code has been tested on an Armadillo800EVA with additional platform
> > > patches that I'm going to submit next.

[snip]

> > > diff --git a/drivers/pwm/pwm-renesas-tpu.c
> > > b/drivers/pwm/pwm-renesas-tpu.c
> > > index 2600892..3eeffff 100644
> > > --- a/drivers/pwm/pwm-renesas-tpu.c
> > > +++ b/drivers/pwm/pwm-renesas-tpu.c

[snip]

> > >  static struct platform_driver tpu_driver = {
> > >  	.probe		= tpu_probe,
> > >  	.remove		= tpu_remove,
> > >  	.driver		= {
> > >  		.name	= "renesas-tpu-pwm",
> > >  		.owner	= THIS_MODULE,
> > > +		.of_match_table = of_match_ptr(tpu_of_table),
> 
> I'd like to point this out as an example of why I think aligning on the
> = here is a bad idea. Eventually you're bound to add a field which is
> longer than all the others and therefore can't be aligned consistently.
> So instead of coming up with some kind of fancy formatting it often
> turns out better to just use a regular single space around =. That you
> can always consistently use, no matter how long the field names are.

Thanks, I'll keep that in mind.
 
> To be clear, I don't expect you to change the patch because of this.

OK.

-- 
Regards,

Laurent Pinchart

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

Re: [PATCH] pwm: renesas-tpu: Add DT support

[Stephen Warren]

On 07/25/2013 04:27 PM, Laurent Pinchart wrote:
> Specify DT bindings for the TPU PWM controller and add OF support to the
> driver.

> diff --git a/Documentation/devicetree/bindings/pwm/renesas,tpu-pwm.txt b/Documentation/devicetree/bindings/pwm/renesas,tpu-pwm.txt

> +  - compatible: should be one of the following.
> +    - "renesas,tpu-r8a73a4": for R8A77A4 (R-Mobile APE6) compatible PWM controller.
> +    - "renesas,tpu-r8a7740": for R8A7740 (R-Mobile A1) compatible PWM controller.

In the bindings I've seen, it's more typical for the compatible value to
be ${vendor},${soc}-${unit} than ${vendor},${unit}-${soc}. I guess I
don't know how common one format or the other is though.

Other than that, the binding,
Acked-by: Stephen Warren <swarren@nvidia.com>

Re: [PATCH] pwm: renesas-tpu: Add DT support

[Laurent Pinchart]

Hi Stephen,

On Monday 12 August 2013 11:43:01 Stephen Warren wrote:
> On 07/25/2013 04:27 PM, Laurent Pinchart wrote:
> > Specify DT bindings for the TPU PWM controller and add OF support to the
> > driver.
> > 
> > diff --git a/Documentation/devicetree/bindings/pwm/renesas,tpu-pwm.txt
> > b/Documentation/devicetree/bindings/pwm/renesas,tpu-pwm.txt
> > 
> > +  - compatible: should be one of the following.
> > +    - "renesas,tpu-r8a73a4": for R8A77A4 (R-Mobile APE6) compatible PWM
> > controller.
> > +    - "renesas,tpu-r8a7740": for R8A7740 (R-Mobile A1) compatible PWM
> > controller.
>
> In the bindings I've seen, it's more typical for the compatible value to
> be ${vendor},${soc}-${unit} than ${vendor},${unit}-${soc}. I guess I
> don't know how common one format or the other is though.

I'm personally fine with both. However, when using a version number, the 
format is ${vendor},${unit}-${version}. As we don't have an IP core version 
number we use the SoC name instead, so ${vendor},${unit}-${soc} would make 
sense. We should probably decide on one of the two alternatives and document 
it. 

> Other than that, the binding,
> Acked-by: Stephen Warren <swarren@nvidia.com>

-- 
Regards,

Laurent Pinchart

Re: [PATCH] pwm: renesas-tpu: Add DT support

[Thierry Reding]

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

On Fri, Jul 26, 2013 at 12:27:41AM +0200, Laurent Pinchart wrote:
> Specify DT bindings for the TPU PWM controller and add OF support to the
> driver.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
>  .../devicetree/bindings/pwm/renesas,tpu-pwm.txt    | 28 +++++++++++++++
>  drivers/pwm/pwm-renesas-tpu.c                      | 41 ++++++++++++++++++----
>  2 files changed, 62 insertions(+), 7 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/pwm/renesas,tpu-pwm.txt
> 
> This patch depends on the "[PATCH v2 0/4] Add PWM polarity flag macro for DT"
> series that is scheduled for merge in the PWM tree, and should thus go in via
> the same tree.
> 
> The code has been tested on an Armadillo800EVA with additional platform patches
> that I'm going to submit next.

Applied, thanks.

Thierry

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

Re: [PATCH] pwm: renesas-tpu: Add DT support

[Laurent Pinchart]

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

Hi Thierry,

On Wednesday 14 August 2013 11:44:36 Thierry Reding wrote:
> On Fri, Jul 26, 2013 at 12:27:41AM +0200, Laurent Pinchart wrote:
> > Specify DT bindings for the TPU PWM controller and add OF support to the
> > driver.
> > 
> > Signed-off-by: Laurent Pinchart
> > <laurent.pinchart+renesas@ideasonboard.com>
> > ---
> > 
> >  .../devicetree/bindings/pwm/renesas,tpu-pwm.txt    | 28 +++++++++++++++
> >  drivers/pwm/pwm-renesas-tpu.c                      | 41 +++++++++++++----
> >  2 files changed, 62 insertions(+), 7 deletions(-)
> >  create mode 100644
> >  Documentation/devicetree/bindings/pwm/renesas,tpu-pwm.txt> 
> > This patch depends on the "[PATCH v2 0/4] Add PWM polarity flag macro for
> > DT" series that is scheduled for merge in the PWM tree, and should thus
> > go in via the same tree.
> > 
> > The code has been tested on an Armadillo800EVA with additional platform
> > patches that I'm going to submit next.
> 
> Applied, thanks.

Thank you. I can't find the patches in git://gitorious.org/linux-pwm/linux-
pwm.git, have you applied them elsewhere ?

-- 
Regards,

Laurent Pinchart

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

Re: [PATCH] pwm: renesas-tpu: Add DT support

[Thierry Reding]

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

On Wed, Aug 21, 2013 at 01:36:38PM +0200, Laurent Pinchart wrote:
> Hi Thierry,
> 
> On Wednesday 14 August 2013 11:44:36 Thierry Reding wrote:
> > On Fri, Jul 26, 2013 at 12:27:41AM +0200, Laurent Pinchart wrote:
> > > Specify DT bindings for the TPU PWM controller and add OF support to the
> > > driver.
> > > 
> > > Signed-off-by: Laurent Pinchart
> > > <laurent.pinchart+renesas@ideasonboard.com>
> > > ---
> > > 
> > >  .../devicetree/bindings/pwm/renesas,tpu-pwm.txt    | 28 +++++++++++++++
> > >  drivers/pwm/pwm-renesas-tpu.c                      | 41 +++++++++++++----
> > >  2 files changed, 62 insertions(+), 7 deletions(-)
> > >  create mode 100644
> > >  Documentation/devicetree/bindings/pwm/renesas,tpu-pwm.txt> 
> > > This patch depends on the "[PATCH v2 0/4] Add PWM polarity flag macro for
> > > DT" series that is scheduled for merge in the PWM tree, and should thus
> > > go in via the same tree.
> > > 
> > > The code has been tested on an Armadillo800EVA with additional platform
> > > patches that I'm going to submit next.
> > 
> > Applied, thanks.
> 
> Thank you. I can't find the patches in git://gitorious.org/linux-pwm/linux-
> pwm.git, have you applied them elsewhere ?

Sorry, I had applied the patch, but forgotten to push it out. Should be
there now.

Thierry

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