Wrong PWM channel

[qianfan-Zhao]

The PWM driver can't know the right pwm channel if CONFIG_OF enabaled.

This is how the driver calc pwm channel:

int ch = pwm->hwpwm + chip->base;

But there are no such rules between pwm channel and chip->base.

I used 2 pwm channels on my board and have such device node in dts:

pwm0: pwm0@b8007000 {
    pinctrl-names = "default";
    pinctrl-0 = <&pinctrl_pwm0_PB>;
    status = "okay";
};

pwm2: pwm2@b8007000 {
    pinctrl-names = "default";
    pinctrl-0 = <&pinctrl_pwm2_PD>;
    status = "okay";
};

I had modified the driver to dump hwpwm and base.

# ls
pwmchip0  pwmchip1
# echo 0 > pwmchip0/export
# echo 0 > pwmchip1/export
# echo 1 > pwmchip0/pwm0/period
pwm->hwpwm: 0, chip->base: 0
# echo 1 > pwmchip1/pwm0/period
pwm->hwpwm: 0, chip->base: 1

So the driver will configure PWM0 and PWM1, but the really channel should be PWM0 and PWM2.

[qianfan-Zhao]

This is a solution:

From 985246f43d97012a26947e6e37b3dd2b6d86c2a0 Mon Sep 17 00:00:00 2001
From: qianfan Zhao <qianfanguijin@163.com>
Date: Wed, 27 Mar 2019 19:18:18 +0800
Subject: [PATCH] nuc972: fix pwm channel calculation

Fixes: #34

Signed-off-by: qianfan Zhao <qianfanguijin@163.com>
---
 arch/arm/boot/dts/nuc970.dtsi | 32 --------------------------------
 drivers/pwm/pwm-nuc970.c      | 12 ++++++------
 2 files changed, 6 insertions(+), 38 deletions(-)

diff --git a/arch/arm/boot/dts/nuc970.dtsi b/arch/arm/boot/dts/nuc970.dtsi
index e0f9f1e9d60..cbacf2e524b 100644
--- a/arch/arm/boot/dts/nuc970.dtsi
+++ b/arch/arm/boot/dts/nuc970.dtsi
@@ -1379,38 +1379,6 @@
            reg = <0xb8007000 0x100>;
            interrupts = <60 4 1>;
            map-addr = <0xf8007000>;
-           pinctrl-names = "default";
-           pinctrl-0 = <&pinctrl_pwm0_PA>;
-           status = "disabled";
-       };
-
-       pwm1: pwm1@b8007000 {
-           compatible = "nuvoton,nuc970-pwm";
-           reg = <0xb8007000 0x100>;
-           interrupts = <60 4 1>;
-           map-addr = <0xf8007000>;
-           pinctrl-names = "default";
-           pinctrl-0 = <&pinctrl_pwm1_PA>;
-           status = "disabled";
-       };
-
-       pwm2: pwm2@b8007000 {
-           compatible = "nuvoton,nuc970-pwm";
-           reg = <0xb8007000 0x100>;
-           interrupts = <60 4 1>;
-           map-addr = <0xf8007000>;
-           pinctrl-names = "default";
-           pinctrl-0 = <&pinctrl_pwm2_PA>;
-           status = "disabled";
-       };
-
-       pwm3: pwm3@b8007000 {
-           compatible = "nuvoton,nuc970-pwm";
-           reg = <0xb8007000 0x100>;
-           interrupts = <60 4 1>;
-           map-addr = <0xf8007000>;
-           pinctrl-names = "default";
-           pinctrl-0 = <&pinctrl_pwm3_PA>;
            status = "disabled";
        };

diff --git a/drivers/pwm/pwm-nuc970.c b/drivers/pwm/pwm-nuc970.c
index 45f7ee94df3..c96278d3f5a 100644
--- a/drivers/pwm/pwm-nuc970.c
+++ b/drivers/pwm/pwm-nuc970.c
@@ -59,8 +59,8 @@ static void pwm_dbg(void)
 static int nuc970_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
 {
    //struct nuc970_chip *nuc970 = to_nuc970_chip(chip);
-   int ch = pwm->hwpwm + chip->base;
    unsigned long flags, cnr, cmr;
+   int ch = pwm->hwpwm;

    local_irq_save(flags);

@@ -100,7 +100,7 @@ static int nuc970_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
 static void nuc970_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm)
 {
    //struct nuc970_chip *nuc970 = to_nuc970_chip(chip);
-   int ch = pwm->hwpwm + chip->base;
+   int ch = pwm->hwpwm;
    unsigned long flags;

    local_irq_save(flags);
@@ -127,7 +127,7 @@ static int nuc970_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
    struct nuc970_chip *nuc970 = to_nuc970_chip(chip);
    unsigned long period, duty, prescale;
    unsigned long flags;
-   int ch = pwm->hwpwm + chip->base;
+   int ch = pwm->hwpwm;

    // Get PCLK, calculate valid parameter range.
    prescale = clk_get_rate(nuc970->clk) / 1000000 - 1;
@@ -135,7 +135,7 @@ static int nuc970_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
    // now pwm time unit is 1000ns.
    period = (period_ns + 500) / 1000;
    duty = (duty_ns + 500) / 1000;
-   
+
    // don't want the minus 1 below change the value to -1 (0xFFFF)
    if(period == 0)
        period = 1;
@@ -194,8 +194,8 @@ static int nuc970_pwm_probe(struct platform_device *pdev)
    nuc970->chip.ops = &nuc970_pwm_ops;
    //nuc970->chip.of_xlate = of_pwm_xlate_with_flags;
    //nuc970->chip.of_pwm_n_cells = 3;
-   nuc970->chip.base = pdev->id;
-   nuc970->chip.npwm = 1;
+   nuc970->chip.base = -1;
+   nuc970->chip.npwm = 4;

    nuc970->clk = clk_get(NULL, "pwm");
    if (IS_ERR(nuc970->clk)) {
-- 
2.17.1

Add PWM0 and PWM2 in dts:

        pwm0: pwm0@b8007000 {
            pinctrl-names = "default";
            pinctrl-0 = <&pinctrl_pwm0_PB &pinctrl_pwm2_PD>;
            status = "okay";
        };

[yachen]

We tried to set the channel number in probe() functio by nuc970->chip.base = pdev->id

And the id is defined in arch/arm/mach-nuc970/dev.c. We'll check why the id is not properly set if CONFIG_OF is set. Thanks.

Sincerely,

Yi-An Chen

[yachen]

Fixed in 329f50237a85e801f39814c7392269504dd94efe, thanks.