[PATCH 0/2] This series adds support for Rockchip SoCs integrated PWM.

[PATCH 0/2] This series adds support for Rockchip SoCs integrated PWM.

[caesar]

This patch would be make applying on the top of Beniamino's submission,
the Beniamino's submission come from [1].
[1]:
https://git.kernel.org/cgit/linux/kernel/git/thierry.reding/linux-pwm.git/log/?h=for-next

Beniamino's submission won't be used from next genenation RK SoCs.
So we have to add support to the new pwm version and VOP_PWM for next genenation RK SoCs.

Tested on RK3288 SDK board.

caesar (2):
 [PATCH 1/2] pwm: add this series patch to introduce for rk-pwm and vop-pwm
 [PATCH 2/2] pwm: add this series patch to support for rk-pwm and vop-pwm.

 .../devicetree/bindings/pwm/pwm-rockchip.txt       |  13 ++-
 drivers/pwm/pwm-rockchip.c                         | 108 +++++++++++++++++----
 2 files changed, 96 insertions(+), 25 deletions(-)

-- 
1.9.1
mZ

[PATCH 1/2] pwm: add this series patch to introduce for rk-pwm and vop-pwm.

[caesar]

Signed-off-by: caesar <caesar.wang@rock-chips.com>
---
 Documentation/devicetree/bindings/pwm/pwm-rockchip.txt | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/pwm/pwm-rockchip.txt b/Documentation/devicetree/bindings/pwm/pwm-rockchip.txt
index 3182126..bb6e7f1 100644
--- a/Documentation/devicetree/bindings/pwm/pwm-rockchip.txt
+++ b/Documentation/devicetree/bindings/pwm/pwm-rockchip.txt
@@ -1,11 +1,14 @@
 Rockchip PWM controller
 
 Required properties:
- - compatible: should be "rockchip,rk2928-pwm"
- - reg: physical base address and length of the controller's registers
- - clocks: phandle and clock specifier of the PWM reference clock
- - #pwm-cells: should be 2. See pwm.txt in this directory for a
-   description of the cell format.
+- compatible: should be "rockchip,<name>-pwm"should be one of following:
+    "rockchip,rk2928-pwm" - support the old version PWM for RK29xx,RK3066,RK3188 SoCs.
+    "rockchip,rk3288-pwm" - support the new version PWM for RK3036,RK3288 SoCs.
+    "rockchip,vop-pwm" - support rockchip integrated VOP-PWM for RK32 SoCs.
+- reg: physical base address and length of the controller's registers.
+- clocks: phandle and clock specifier of the PWM reference clock.
+- #pwm-cells: should be 2. See pwm.txt in this directory for a
+  description of the cell format.
 
 Example:
 
-- 
1.9.1

[PATCH 2/2] pwm: add this series patch to support for rk-pwm and vop-pwm.

[caesar]

Signed-off-by: caesar <caesar.wang@rock-chips.com>
---
 drivers/pwm/pwm-rockchip.c | 108 ++++++++++++++++++++++++++++++++++++---------
 1 file changed, 88 insertions(+), 20 deletions(-)

diff --git a/drivers/pwm/pwm-rockchip.c b/drivers/pwm/pwm-rockchip.c
index eec2145..59b0380 100644
--- a/drivers/pwm/pwm-rockchip.c
+++ b/drivers/pwm/pwm-rockchip.c
@@ -2,6 +2,7 @@
  * PWM driver for Rockchip SoCs
  *
  * Copyright (C) 2014 Beniamino Galvani <b.galvani@gmail.com>
+ * Copyright (C) 2014 Caesar Wang <caesar.wang@rock-chips.com>
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License
@@ -12,6 +13,8 @@
 #include <linux/io.h>
 #include <linux/module.h>
 #include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/of_address.h>
 #include <linux/platform_device.h>
 #include <linux/pwm.h>
 #include <linux/time.h>
@@ -23,14 +26,37 @@
 #define PWM_CTRL_TIMER_EN	(1 << 0)
 #define PWM_CTRL_OUTPUT_EN	(1 << 3)
 
+#define RK_PRESCALER		1
 #define PRESCALER		2
 
+#define PWM_CONTINUOUS		(1 << 1)
+#define PWM_DUTY_POSITIVE		(1 << 3)
+#define PWM_INACTIVE_NEGATIVE		(0 << 4)
+#define PWM_OUTPUT_LEFT		(0 << 5)
+#define PWM_LP_DISABLE		(0 << 8)
+
+#define RK_PWM_ENABLE		(1 << 0)
+
+#define VOP_PWM_CTRL		0x00		/* VOP-PWM Control Register */
+#define VOP_PWM_CNTR		0x0c
+
+
 struct rockchip_pwm_chip {
 	struct pwm_chip chip;
 	struct clk *clk;
+	const struct rockchip_pwm_data *data;
 	void __iomem *base;
 };
 
+struct rockchip_pwm_data {
+	u32 duty;
+	u32 period;
+	u32 reg_cntr;
+	u32 reg_ctrl;
+	int prescaler;
+	u32 enable_conf;
+};
+
 static inline struct rockchip_pwm_chip *to_rockchip_pwm_chip(struct pwm_chip *c)
 {
 	return container_of(c, struct rockchip_pwm_chip, chip);
@@ -52,20 +78,20 @@ static int rockchip_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
 	 * default prescaler value for all practical clock rate values.
 	 */
 	div = clk_rate * period_ns;
-	do_div(div, PRESCALER * NSEC_PER_SEC);
+	do_div(div, pc->data->prescaler * NSEC_PER_SEC);
 	period = div;
 
 	div = clk_rate * duty_ns;
-	do_div(div, PRESCALER * NSEC_PER_SEC);
+	do_div(div, pc->data->prescaler * NSEC_PER_SEC);
 	duty = div;
 
 	ret = clk_enable(pc->clk);
 	if (ret)
 		return ret;
 
-	writel(period, pc->base + PWM_LRC);
-	writel(duty, pc->base + PWM_HRC);
-	writel(0, pc->base + PWM_CNTR);
+	writel(period, pc->base + pc->data->period);
+	writel(duty, pc->base + pc->data->duty);
+	writel(0, pc->base + pc->data->reg_cntr);
 
 	clk_disable(pc->clk);
 
@@ -82,9 +108,9 @@ static int rockchip_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
 	if (ret)
 		return ret;
 
-	val = readl_relaxed(pc->base + PWM_CTRL);
-	val |= PWM_CTRL_OUTPUT_EN | PWM_CTRL_TIMER_EN;
-	writel_relaxed(val, pc->base + PWM_CTRL);
+	val = readl_relaxed(pc->base + pc->data->reg_ctrl);
+	val |= pc->data->enable_conf;
+	writel_relaxed(val, pc->base + pc->data->reg_ctrl);
 
 	return 0;
 }
@@ -94,9 +120,9 @@ static void rockchip_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm)
 	struct rockchip_pwm_chip *pc = to_rockchip_pwm_chip(chip);
 	u32 val;
 
-	val = readl_relaxed(pc->base + PWM_CTRL);
-	val &= ~(PWM_CTRL_OUTPUT_EN | PWM_CTRL_TIMER_EN);
-	writel_relaxed(val, pc->base + PWM_CTRL);
+	val = readl_relaxed(pc->base + pc->data->reg_ctrl);
+	val &= ~pc->data->enable_conf;
+	writel_relaxed(val, pc->base + pc->data->reg_ctrl);
 
 	clk_disable(pc->clk);
 }
@@ -108,8 +134,48 @@ static const struct pwm_ops rockchip_pwm_ops = {
 	.owner = THIS_MODULE,
 };
 
+static struct rockchip_pwm_data pwm_data_v1 = {
+	.duty = PWM_HRC,
+	.period = PWM_LRC,
+	.reg_cntr = PWM_CNTR,
+	.reg_ctrl = PWM_CTRL,
+	.prescaler = PRESCALER,
+	.enable_conf = PWM_CTRL_OUTPUT_EN | PWM_CTRL_TIMER_EN,
+};
+
+static struct rockchip_pwm_data pwm_data_v2 = {
+	.duty = PWM_LRC,
+	.period = PWM_HRC,
+	.reg_cntr = PWM_CNTR,
+	.reg_ctrl = PWM_CTRL,
+	.prescaler = RK_PRESCALER,
+	.enable_conf = PWM_OUTPUT_LEFT | PWM_LP_DISABLE | RK_PWM_ENABLE |
+		PWM_CONTINUOUS | PWM_DUTY_POSITIVE | PWM_INACTIVE_NEGATIVE,
+};
+
+static struct rockchip_pwm_data pwm_data_vop = {
+	.duty = PWM_LRC,
+	.period = PWM_HRC,
+	.reg_cntr = VOP_PWM_CNTR,
+	.reg_ctrl = VOP_PWM_CTRL,
+	.prescaler = RK_PRESCALER,
+	.enable_conf = PWM_OUTPUT_LEFT | PWM_LP_DISABLE | RK_PWM_ENABLE |
+		PWM_CONTINUOUS | PWM_DUTY_POSITIVE | PWM_INACTIVE_NEGATIVE,
+};
+
+static const struct of_device_id rockchip_pwm_dt_ids[] = {
+	{ .compatible = "rockchip,rk2928-pwm", .data = &pwm_data_v1},
+	{ .compatible = "rockchip,rk3288-pwm", .data = &pwm_data_v2},
+	{ .compatible = "rockchip,vop-pwm", .data = &pwm_data_vop},
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, rockchip_pwm_dt_ids);
+
 static int rockchip_pwm_probe(struct platform_device *pdev)
 {
+	const struct of_device_id *of_id =
+	    of_match_device(rockchip_pwm_dt_ids, &pdev->dev);
+	struct device_node *np = pdev->dev.of_node;
 	struct rockchip_pwm_chip *pc;
 	struct resource *r;
 	int ret;
@@ -119,9 +185,12 @@ static int rockchip_pwm_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	pc->base = devm_ioremap_resource(&pdev->dev, r);
-	if (IS_ERR(pc->base))
-		return PTR_ERR(pc->base);
+	pc->base = of_iomap(np, 0);
+	if (!pc->base) {
+		dev_err(&pdev->dev, "failed to map controller\n");
+		ret = -ENOMEM;
+		goto fail_map;
+	}
 
 	pc->clk = devm_clk_get(&pdev->dev, NULL);
 	if (IS_ERR(pc->clk))
@@ -133,6 +202,7 @@ static int rockchip_pwm_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, pc);
 
+	pc->data = of_id->data;
 	pc->chip.dev = &pdev->dev;
 	pc->chip.ops = &rockchip_pwm_ops;
 	pc->chip.base = -1;
@@ -145,6 +215,10 @@ static int rockchip_pwm_probe(struct platform_device *pdev)
 	}
 
 	return ret;
+
+fail_map:
+	iounmap(pc->base);
+	return ret;
 }
 
 static int rockchip_pwm_remove(struct platform_device *pdev)
@@ -156,12 +230,6 @@ static int rockchip_pwm_remove(struct platform_device *pdev)
 	return pwmchip_remove(&pc->chip);
 }
 
-static const struct of_device_id rockchip_pwm_dt_ids[] = {
-	{ .compatible = "rockchip,rk2928-pwm" },
-	{ /* sentinel */ }
-};
-MODULE_DEVICE_TABLE(of, rockchip_pwm_dt_ids);
-
 static struct platform_driver rockchip_pwm_driver = {
 	.driver = {
 		.name = "rockchip-pwm",
-- 
1.9.1

Re: [PATCH 2/2] pwm: add this series patch to support for rk-pwm and vop-pwm.

[Beniamino Galvani]

On Thu, Jul 17, 2014 at 02:08:14PM +0800, caesar wrote:
> Signed-off-by: caesar <caesar.wang@rock-chips.com>

Hi Caesar,

just a couple of comments below.

> ---
>  drivers/pwm/pwm-rockchip.c | 108 ++++++++++++++++++++++++++++++++++++---------
>  1 file changed, 88 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/pwm/pwm-rockchip.c b/drivers/pwm/pwm-rockchip.c
> index eec2145..59b0380 100644
> --- a/drivers/pwm/pwm-rockchip.c
> +++ b/drivers/pwm/pwm-rockchip.c
> @@ -2,6 +2,7 @@
>   * PWM driver for Rockchip SoCs
>   *
>   * Copyright (C) 2014 Beniamino Galvani <b.galvani@gmail.com>
> + * Copyright (C) 2014 Caesar Wang <caesar.wang@rock-chips.com>
>   *
>   * This program is free software; you can redistribute it and/or
>   * modify it under the terms of the GNU General Public License
> @@ -12,6 +13,8 @@
>  #include <linux/io.h>
>  #include <linux/module.h>
>  #include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/of_address.h>

These two should be swapped to keep the alphabetical order of the
includes.

>  #include <linux/platform_device.h>
>  #include <linux/pwm.h>
>  #include <linux/time.h>
> @@ -23,14 +26,37 @@
>  #define PWM_CTRL_TIMER_EN	(1 << 0)
>  #define PWM_CTRL_OUTPUT_EN	(1 << 3)
> +

[...]

>  static int rockchip_pwm_probe(struct platform_device *pdev)
>  {
> +	const struct of_device_id *of_id =
> +	    of_match_device(rockchip_pwm_dt_ids, &pdev->dev);
> +	struct device_node *np = pdev->dev.of_node;
>  	struct rockchip_pwm_chip *pc;
>  	struct resource *r;
>  	int ret;
> @@ -119,9 +185,12 @@ static int rockchip_pwm_probe(struct platform_device *pdev)
>  		return -ENOMEM;
>  
>  	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> -	pc->base = devm_ioremap_resource(&pdev->dev, r);
> -	if (IS_ERR(pc->base))
> -		return PTR_ERR(pc->base);
> +	pc->base = of_iomap(np, 0);
> +	if (!pc->base) {
> +		dev_err(&pdev->dev, "failed to map controller\n");
> +		ret = -ENOMEM;
> +		goto fail_map;
> +	}

I think that this change is not needed. devm_ioremap_resource()
guarantees an automatic unmapping when the device is destroyed.

Moreover, when of_iomap() fails you don't need to call iounmap().

Beniamino

>  
>  	pc->clk = devm_clk_get(&pdev->dev, NULL);
>  	if (IS_ERR(pc->clk))
> @@ -133,6 +202,7 @@ static int rockchip_pwm_probe(struct platform_device *pdev)
>  
>  	platform_set_drvdata(pdev, pc);
>  
> +	pc->data = of_id->data;
>  	pc->chip.dev = &pdev->dev;
>  	pc->chip.ops = &rockchip_pwm_ops;
>  	pc->chip.base = -1;
> @@ -145,6 +215,10 @@ static int rockchip_pwm_probe(struct platform_device *pdev)
>  	}
>  
>  	return ret;
> +
> +fail_map:
> +	iounmap(pc->base);
> +	return ret;
>  }
>  
>  static int rockchip_pwm_remove(struct platform_device *pdev)
> @@ -156,12 +230,6 @@ static int rockchip_pwm_remove(struct platform_device *pdev)
>  	return pwmchip_remove(&pc->chip);
>  }
>  
> -static const struct of_device_id rockchip_pwm_dt_ids[] = {
> -	{ .compatible = "rockchip,rk2928-pwm" },
> -	{ /* sentinel */ }
> -};
> -MODULE_DEVICE_TABLE(of, rockchip_pwm_dt_ids);
> -
>  static struct platform_driver rockchip_pwm_driver = {
>  	.driver = {
>  		.name = "rockchip-pwm",
> -- 
> 1.9.1
> 

Re: [PATCH 2/2] pwm: add this series patch to support for rk-pwm and vop-pwm.

[caesar]

Hi Beniamino,

于 2014年07月18日 03:24, Beniamino Galvani 写道:
> On Thu, Jul 17, 2014 at 02:08:14PM +0800, caesar wrote:
>> Signed-off-by: caesar<caesar.wang@rock-chips.com>
> Hi Caesar,
>
> just a couple of comments below.
>
>> ---
>>   drivers/pwm/pwm-rockchip.c | 108 ++++++++++++++++++++++++++++++++++++---------
>>   1 file changed, 88 insertions(+), 20 deletions(-)
>>
>> diff --git a/drivers/pwm/pwm-rockchip.c b/drivers/pwm/pwm-rockchip.c
>> index eec2145..59b0380 100644
>> --- a/drivers/pwm/pwm-rockchip.c
>> +++ b/drivers/pwm/pwm-rockchip.c
>> @@ -2,6 +2,7 @@
>>    * PWM driver for Rockchip SoCs
>>    *
>>    * Copyright (C) 2014 Beniamino Galvani<b.galvani@gmail.com>
>> + * Copyright (C) 2014 Caesar Wang<caesar.wang@rock-chips.com>
>>    *
>>    * This program is free software; you can redistribute it and/or
>>    * modify it under the terms of the GNU General Public License
>> @@ -12,6 +13,8 @@
>>   #include <linux/io.h>
>>   #include <linux/module.h>
>>   #include <linux/of.h>
>> +#include <linux/of_device.h>
>> +#include <linux/of_address.h>
> These two should be swapped to keep the alphabetical order of the
> includes.
ok,II will fix it.
>>   #include <linux/platform_device.h>
>>   #include <linux/pwm.h>
>>   #include <linux/time.h>
>> @@ -23,14 +26,37 @@
>>   #define PWM_CTRL_TIMER_EN	(1 << 0)
>>   #define PWM_CTRL_OUTPUT_EN	(1 << 3)
>> +
> [...]
:-( ok,I will remove it.
>>   static int rockchip_pwm_probe(struct platform_device *pdev)
>>   {
>> +	const struct of_device_id *of_id =
>> +	    of_match_device(rockchip_pwm_dt_ids, &pdev->dev);
>> +	struct device_node *np = pdev->dev.of_node;
>>   	struct rockchip_pwm_chip *pc;
>>   	struct resource *r;
>>   	int ret;
>> @@ -119,9 +185,12 @@ static int rockchip_pwm_probe(struct platform_device *pdev)
>>   		return -ENOMEM;
>>   
>>   	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> -	pc->base = devm_ioremap_resource(&pdev->dev, r);
>> -	if (IS_ERR(pc->base))
>> -		return PTR_ERR(pc->base);
>> +	pc->base = of_iomap(np, 0);
>> +	if (!pc->base) {
>> +		dev_err(&pdev->dev, "failed to map controller\n");
>> +		ret = -ENOMEM;
>> +		goto fail_map;
>> +	}
> I think that this change is not needed. devm_ioremap_resource()
> guarantees an automatic unmapping when the device is destroyed.
>
> Moreover, when of_iomap() fails you don't need to call iounmap().
>
> Beniamino
VOP-PWM base has be requested for lcdc.
When I use devm_ioremap_resource(), the vop-pwm will request region fail.

Example:.931020] rockchip-pwm ff9401a0.pwm: can't request region for 
resource [mem 0xff9401a0-0xff9401af] /pwm@ff9401a0.
So ,I have to charge it.

I will be simplyfied by having:
- pc->base = devm_ioremap_resource(&pdev->dev, r);
+ if (!strcmp(of_id->compatible, "rockchip,vop-pwm"))
+ pc->base = devm_ioremap(&pdev->dev, r->start, resource_size(r));
+ else
+ pc->base = devm_ioremap_resource(&pdev->dev, r);

Maybe, Could you give me better suggestions for it?

Caesar

>>   
>>   	pc->clk = devm_clk_get(&pdev->dev, NULL);
>>   	if (IS_ERR(pc->clk))
>> @@ -133,6 +202,7 @@ static int rockchip_pwm_probe(struct platform_device *pdev)
>>   
>>   	platform_set_drvdata(pdev, pc);
>>   
>> +	pc->data = of_id->data;
>>   	pc->chip.dev = &pdev->dev;
>>   	pc->chip.ops = &rockchip_pwm_ops;
>>   	pc->chip.base = -1;
>> @@ -145,6 +215,10 @@ static int rockchip_pwm_probe(struct platform_device *pdev)
>>   	}
>>   
>>   	return ret;
>> +
>> +fail_map:
>> +	iounmap(pc->base);
>> +	return ret;
>>   }
>>   
>>   static int rockchip_pwm_remove(struct platform_device *pdev)
>> @@ -156,12 +230,6 @@ static int rockchip_pwm_remove(struct platform_device *pdev)
>>   	return pwmchip_remove(&pc->chip);
>>   }
>>   
>> -static const struct of_device_id rockchip_pwm_dt_ids[] = {
>> -	{ .compatible = "rockchip,rk2928-pwm" },
>> -	{ /* sentinel */ }
>> -};
>> -MODULE_DEVICE_TABLE(of, rockchip_pwm_dt_ids);
>> -
>>   static struct platform_driver rockchip_pwm_driver = {
>>   	.driver = {
>>   		.name = "rockchip-pwm",
>> -- 
>> 1.9.1
>>
>

Re: [PATCH 2/2] pwm: add this series patch to support for rk-pwm and vop-pwm.

[Thierry Reding]

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

On Fri, Jul 18, 2014 at 01:05:56PM +0800, caesar wrote:
> 于 2014年07月18日 03:24, Beniamino Galvani 写道:
> >On Thu, Jul 17, 2014 at 02:08:14PM +0800, caesar wrote:
[...]
> >>@@ -119,9 +185,12 @@ static int rockchip_pwm_probe(struct platform_device *pdev)
> >>  		return -ENOMEM;
> >>  	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> >>-	pc->base = devm_ioremap_resource(&pdev->dev, r);
> >>-	if (IS_ERR(pc->base))
> >>-		return PTR_ERR(pc->base);
> >>+	pc->base = of_iomap(np, 0);
> >>+	if (!pc->base) {
> >>+		dev_err(&pdev->dev, "failed to map controller\n");
> >>+		ret = -ENOMEM;
> >>+		goto fail_map;
> >>+	}
> >I think that this change is not needed. devm_ioremap_resource()
> >guarantees an automatic unmapping when the device is destroyed.
> >
> >Moreover, when of_iomap() fails you don't need to call iounmap().
> >
> >Beniamino
> VOP-PWM base has be requested for lcdc.

Why?

> When I use devm_ioremap_resource(), the vop-pwm will request region fail.
> 
> Example:.931020] rockchip-pwm ff9401a0.pwm: can't request region for
> resource [mem 0xff9401a0-0xff9401af] /pwm@ff9401a0.
> So ,I have to charge it.
> 
> I will be simplyfied by having:
> - pc->base = devm_ioremap_resource(&pdev->dev, r);
> + if (!strcmp(of_id->compatible, "rockchip,vop-pwm"))
> + pc->base = devm_ioremap(&pdev->dev, r->start, resource_size(r));
> + else
> + pc->base = devm_ioremap_resource(&pdev->dev, r);
> 
> Maybe, Could you give me better suggestions for it?

The right thing to do is not have two drivers access the same device.
Why does lcdc request the PWM register region?

Thierry

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

Re: [PATCH 2/2] pwm: add this series patch to support for rk-pwm and vop-pwm.

[caesar]

Hi Thierry,

于 2014年07月18日 18:03, Thierry Reding 写道:
> On Fri, Jul 18, 2014 at 01:05:56PM +0800, caesar wrote:
>> 于 2014年07月18日 03:24, Beniamino Galvani 写道:
>>> On Thu, Jul 17, 2014 at 02:08:14PM +0800, caesar wrote:
> [...]
>>>> @@ -119,9 +185,12 @@ static int rockchip_pwm_probe(struct platform_device *pdev)
>>>>   		return -ENOMEM;
>>>>   	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>>> -	pc->base = devm_ioremap_resource(&pdev->dev, r);
>>>> -	if (IS_ERR(pc->base))
>>>> -		return PTR_ERR(pc->base);
>>>> +	pc->base = of_iomap(np, 0);
>>>> +	if (!pc->base) {
>>>> +		dev_err(&pdev->dev, "failed to map controller\n");
>>>> +		ret = -ENOMEM;
>>>> +		goto fail_map;
>>>> +	}
>>> I think that this change is not needed. devm_ioremap_resource()
>>> guarantees an automatic unmapping when the device is destroyed.
>>>
>>> Moreover, when of_iomap() fails you don't need to call iounmap().
>>>
>>> Beniamino
>> VOP-PWM base has be requested for lcdc.
> Why?
VOP is  a seperate controllers for rk3288 and genenation SoCs.
The VOP contains lcdc,pwm......

For example :Rk3288 SOC address mapping

lcdc0: lcdc@ff930000                     vop0pwm: pwm@ff9301a0
reg = <0xff930000 0x10000>       reg = <0xff9301a0 0x10>;
offset = 0x10000                            offset = 0x10

lcdc1: lcdc@ff940000                    vop1pwm: pwm@ff9401a0
reg = <0xff940000 0x10000>      reg = <0xff9401a0 0x10>;
offset = 0x10000                           offset = 0x10

In a word, lcdc and vop-pwm has shared the map address.
>> When I use devm_ioremap_resource(), the vop-pwm will request region fail.
>>
>> Example:.931020] rockchip-pwm ff9401a0.pwm: can't request region for
>> resource [mem 0xff9401a0-0xff9401af] /pwm@ff9401a0.
>> So ,I have to charge it.
>>
>> I will be simplyfied by having:
>> - pc->base = devm_ioremap_resource(&pdev->dev, r);
>> + if (!strcmp(of_id->compatible, "rockchip,vop-pwm"))
>> + pc->base = devm_ioremap(&pdev->dev, r->start, resource_size(r));
>> + else
>> + pc->base = devm_ioremap_resource(&pdev->dev, r);
>>
>> Maybe, Could you give me better suggestions for it?
> The right thing to do is not have two drivers access the same device.
> Why does lcdc request the PWM register region?
>
> Thierry
Ditto,
Maybe,Could you give me a better suggestion to do it?