Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1569291 > unrolled thread

[PATCH v5 00/11] pwm: imx: Provide atomic operation for IMX PWM driver

Back to article view | Back to linux.kernel

Contents

  [PATCH v5 00/11] pwm: imx: Provide atomic operation for IMX PWM driver Lukasz Majewski <lukma@denx.de> - 2017-01-29 23:00 +0100
    [PATCH v5 05/11] pwm: imx: Move PWMv2 software reset code to a separate function Lukasz Majewski <lukma@denx.de> - 2017-01-29 23:00 +0100
      Re: [PATCH v5 05/11] pwm: imx: Move PWMv2 software reset code to a  separate function Thierry Reding <thierry.reding@gmail.com> - 2017-01-30 08:30 +0100
    [PATCH v5 02/11] pwm: imx: remove ipg clock and enable per clock when required Lukasz Majewski <lukma@denx.de> - 2017-01-29 23:00 +0100
    [PATCH v5 11/11] pwm: imx: Add polarity inversion support to i.MX's PWMv2 Lukasz Majewski <lukma@denx.de> - 2017-01-29 23:00 +0100
    [PATCH v5 10/11] pwm: imx: doc: Update imx-pwm.txt documentation entry Lukasz Majewski <lukma@denx.de> - 2017-01-29 23:00 +0100
    [PATCH v5 03/11] pwm: imx: Add separate set of pwm ops for PWMv1 and PWMv2 Lukasz Majewski <lukma@denx.de> - 2017-01-29 23:00 +0100
      Re: [PATCH v5 03/11] pwm: imx: Add separate set of pwm ops for PWMv1  and PWMv2 Thierry Reding <thierry.reding@gmail.com> - 2017-01-30 08:30 +0100
        Re: [PATCH v5 03/11] pwm: imx: Add separate set of pwm ops for PWMv1  and PWMv2 Thierry Reding <thierry.reding@gmail.com> - 2017-01-30 08:50 +0100
          Re: [PATCH v5 03/11] pwm: imx: Add separate set of pwm ops for  PWMv1 and PWMv2 Lukasz Majewski <lukma@denx.de> - 2017-01-30 09:40 +0100
            Re: [PATCH v5 03/11] pwm: imx: Add separate set of pwm ops for PWMv1  and PWMv2 Thierry Reding <thierry.reding@gmail.com> - 2017-01-30 10:00 +0100
              Re: [PATCH v5 03/11] pwm: imx: Add separate set of pwm ops for  PWMv1 and PWMv2 Lukasz Majewski <lukma@denx.de> - 2017-01-30 10:00 +0100
                Re: [PATCH v5 03/11] pwm: imx: Add separate set of pwm ops for PWMv1  and PWMv2 Thierry Reding <thierry.reding@gmail.com> - 2017-01-30 10:10 +0100

#1569291 — [PATCH v5 00/11] pwm: imx: Provide atomic operation for IMX PWM driver

This patch set brings atomic operation to i.MX's PWMv2 driver.

This work has been supported and suggested by Boris Brezillon [1] and 
Stefan Agner, by showing how simple the transition could be :-) (in theory).

It has been divided into several steps:

- Remove ipg clock and enable per clock when required (as proposed by Boris 
  Brezillon)

- Provide different pwm ops for PWMv1 and PWMv2

- Separate PWMv1 commits from "generic" and non atomic PWM code

  NOTE: Since I do _not_ have board with PWMv1, I would like to ask somebody
        for testing

- Move some imx_config_v2 code to separate functions

- Provide PWM atomic implementation (the ->apply() driver) for PWMv2 in a
  single patch for better readability. This patch also includes some code
  to address potential issues on i.MX7 (lack of peripheral clock when accessing
  PWM registers).

- Remove redundant PWM code (disable, enable, config callbacks)

- Update proper documentation entries

- Provide support for polarity inversion on top of atomic PWM rework


Test HW:
--------
This patch set has been tested on i.MX6q board with
v4.10-rc5 kernel SHA1: 1b1bc42c1692e9b62756323c675a44cb1a1f9dbd

The PWM operation has been tested with pwm_bl backlight driver by changing
its brightness.

[1]: http://patchwork.ozlabs.org/patch/685402/

Lothar Wassmann (2):
  pwm: print error messages with pr_err() instead of pr_debug()
  pwm: core: make the PWM_POLARITY flag in DTB optional

Lukasz Majewski (8):
  pwm: imx: Add separate set of pwm ops for PWMv1 and PWMv2
  pwm: imx: Rewrite imx_pwm_*_v1 code to facilitate switch to atomic pwm
    operation
  pwm: imx: Move PWMv2 software reset code to a separate function
  pwm: imx: Move PWMv2 wait for fifo slot code to a separate function
  pwm: imx: Provide atomic PWM support for i.MX PWMv2
  pwm: imx: Remove redundant i.MX PWMv2 code
  pwm: imx: doc: Update imx-pwm.txt documentation entry
  pwm: imx: Add polarity inversion support to i.MX's PWMv2

Sascha Hauer (1):
  pwm: imx: remove ipg clock and enable per clock when required

 Documentation/devicetree/bindings/pwm/imx-pwm.txt |   6 +-
 drivers/pwm/core.c                                |  26 ++-
 drivers/pwm/pwm-imx.c                             | 269 ++++++++++------------
 3 files changed, 145 insertions(+), 156 deletions(-)

-- 
2.1.4

[toc] | [next] | [standalone]

#1569292 — [PATCH v5 05/11] pwm: imx: Move PWMv2 software reset code to a separate function

From: Lukasz Majewski <l.majewski@majess.pl>

The software reset code has been extracted from imx_pwm_config_v2 function
and moved to new one - imx_pwm_sw_reset().

This change reduces the overall size of imx_pwm_config_v2() and prepares
it for atomic PWM operation.

Suggested-by: Stefan Agner <stefan@agner.ch>
Suggested-by: Boris Brezillon <boris.brezillon@free-electrons.com>
Signed-off-by: Lukasz Majewski <l.majewski@majess.pl>

---
Changes for v5:
- None

Changes for v4:
- None

Changes for v3:
- None

Changes for v2:
- Add missing parenthesis
---
 drivers/pwm/pwm-imx.c | 31 +++++++++++++++++++++----------
 1 file changed, 21 insertions(+), 10 deletions(-)

diff --git a/drivers/pwm/pwm-imx.c b/drivers/pwm/pwm-imx.c
index 11e3f3e..f0d78f3 100644
--- a/drivers/pwm/pwm-imx.c
+++ b/drivers/pwm/pwm-imx.c
@@ -120,6 +120,25 @@ static void imx_pwm_disable_v1(struct pwm_chip *chip, struct pwm_device *pwm)
 	clk_disable_unprepare(imx->clk_per);
 }
 
+static void imx_pwm_sw_reset(struct pwm_chip *chip)
+{
+	struct imx_chip *imx = to_imx_chip(chip);
+	struct device *dev = chip->dev;
+	int wait_count = 0;
+	u32 cr;
+
+	writel(MX3_PWMCR_SWR, imx->mmio_base + MX3_PWMCR);
+	do {
+		usleep_range(200, 1000);
+		cr = readl(imx->mmio_base + MX3_PWMCR);
+	} while ((cr & MX3_PWMCR_SWR) &&
+		 (wait_count++ < MX3_PWM_SWR_LOOP));
+
+	if (cr & MX3_PWMCR_SWR)
+		dev_warn(dev, "software reset timeout\n");
+}
+
+
 static int imx_pwm_config_v2(struct pwm_chip *chip,
 		struct pwm_device *pwm, int duty_ns, int period_ns)
 {
@@ -129,7 +148,7 @@ static int imx_pwm_config_v2(struct pwm_chip *chip,
 	unsigned long period_cycles, duty_cycles, prescale;
 	unsigned int period_ms;
 	bool enable = pwm_is_enabled(pwm);
-	int wait_count = 0, fifoav;
+	int fifoav;
 	u32 cr, sr;
 
 	/*
@@ -152,15 +171,7 @@ static int imx_pwm_config_v2(struct pwm_chip *chip,
 				dev_warn(dev, "there is no free FIFO slot\n");
 		}
 	} else {
-		writel(MX3_PWMCR_SWR, imx->mmio_base + MX3_PWMCR);
-		do {
-			usleep_range(200, 1000);
-			cr = readl(imx->mmio_base + MX3_PWMCR);
-		} while ((cr & MX3_PWMCR_SWR) &&
-			 (wait_count++ < MX3_PWM_SWR_LOOP));
-
-		if (cr & MX3_PWMCR_SWR)
-			dev_warn(dev, "software reset timeout\n");
+		imx_pwm_sw_reset(chip);
 	}
 
 	c = clk_get_rate(imx->clk_per);
-- 
2.1.4

[toc] | [prev] | [next] | [standalone]

#1569438 — Re: [PATCH v5 05/11] pwm: imx: Move PWMv2 software reset code to a separate function

[Multipart message — attachments visible in raw view] — view raw

On Sun, Jan 29, 2017 at 10:54:09PM +0100, Lukasz Majewski wrote:
> From: Lukasz Majewski <l.majewski@majess.pl>
> 
> The software reset code has been extracted from imx_pwm_config_v2 function
> and moved to new one - imx_pwm_sw_reset().
> 
> This change reduces the overall size of imx_pwm_config_v2() and prepares
> it for atomic PWM operation.
> 
> Suggested-by: Stefan Agner <stefan@agner.ch>
> Suggested-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> Signed-off-by: Lukasz Majewski <l.majewski@majess.pl>
> 
> ---
> Changes for v5:
> - None
> 
> Changes for v4:
> - None
> 
> Changes for v3:
> - None
> 
> Changes for v2:
> - Add missing parenthesis
> ---
>  drivers/pwm/pwm-imx.c | 31 +++++++++++++++++++++----------
>  1 file changed, 21 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/pwm/pwm-imx.c b/drivers/pwm/pwm-imx.c
> index 11e3f3e..f0d78f3 100644
> --- a/drivers/pwm/pwm-imx.c
> +++ b/drivers/pwm/pwm-imx.c
> @@ -120,6 +120,25 @@ static void imx_pwm_disable_v1(struct pwm_chip *chip, struct pwm_device *pwm)
>  	clk_disable_unprepare(imx->clk_per);
>  }
>  
> +static void imx_pwm_sw_reset(struct pwm_chip *chip)
> +{
> +	struct imx_chip *imx = to_imx_chip(chip);
> +	struct device *dev = chip->dev;
> +	int wait_count = 0;
> +	u32 cr;
> +
> +	writel(MX3_PWMCR_SWR, imx->mmio_base + MX3_PWMCR);
> +	do {
> +		usleep_range(200, 1000);
> +		cr = readl(imx->mmio_base + MX3_PWMCR);
> +	} while ((cr & MX3_PWMCR_SWR) &&
> +		 (wait_count++ < MX3_PWM_SWR_LOOP));

I think you could replace this by one of the accessors from
linux/iopoll.h, but that can be a separate patch.

Thierry

[toc] | [prev] | [next] | [standalone]

#1569293 — [PATCH v5 02/11] pwm: imx: remove ipg clock and enable per clock when required

From: Sascha Hauer <s.hauer@pengutronix.de>

The use of the ipg clock was introduced with commit 7b27c160c681
("pwm: i.MX: fix clock lookup").
In the commit message it was claimed that the ipg clock is enabled for
register accesses. This is true for the ->config() callback, but not
for the ->set_enable() callback. Given that the ipg clock is not
consistently enabled for all register accesses we can assume that either
it is not required at all or that the current code does not work.
Remove the ipg clock code for now so that it's no longer in the way of
refactoring the driver.

On the other hand, the imx7 IP requires the peripheral clock to be
enabled before accessing its registers. Since ->config() can be called
when the PWM is disabled (in which case, the peripheral clock is also
disabled), we need to surround the imx->config() with
clk_prepare_enable(per_clk)/clk_disable_unprepare(per_clk) calls.

Note that the driver was working fine for the i.MX 7 IP so far because
the ipg and peripheral clock use the same hardware clock gate, which
guaranteed peripheral clock activation even when ->config() was called
when the PWM was disabled.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
Cc: Philipp Zabel <p.zabel@pengutronix.de>
Reviewed-by: Stefan Agner <stefan@agner.ch>
Tested-by: Stefan Agner <stefan@agner.ch>
---
Changes in v5:
- Minor edit of commmit message

Changes in v4:
- Enable per clk before calling imx->config()

Changes in v3:
- New patch
---
 drivers/pwm/pwm-imx.c | 12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/drivers/pwm/pwm-imx.c b/drivers/pwm/pwm-imx.c
index d600fd5..b1d1e50 100644
--- a/drivers/pwm/pwm-imx.c
+++ b/drivers/pwm/pwm-imx.c
@@ -49,7 +49,6 @@
 
 struct imx_chip {
 	struct clk	*clk_per;
-	struct clk	*clk_ipg;
 
 	void __iomem	*mmio_base;
 
@@ -206,13 +205,13 @@ static int imx_pwm_config(struct pwm_chip *chip,
 	struct imx_chip *imx = to_imx_chip(chip);
 	int ret;
 
-	ret = clk_prepare_enable(imx->clk_ipg);
+	ret = clk_prepare_enable(imx->clk_per);
 	if (ret)
 		return ret;
 
 	ret = imx->config(chip, pwm, duty_ns, period_ns);
 
-	clk_disable_unprepare(imx->clk_ipg);
+	clk_disable_unprepare(imx->clk_per);
 
 	return ret;
 }
@@ -293,13 +292,6 @@ static int imx_pwm_probe(struct platform_device *pdev)
 		return PTR_ERR(imx->clk_per);
 	}
 
-	imx->clk_ipg = devm_clk_get(&pdev->dev, "ipg");
-	if (IS_ERR(imx->clk_ipg)) {
-		dev_err(&pdev->dev, "getting ipg clock failed with %ld\n",
-				PTR_ERR(imx->clk_ipg));
-		return PTR_ERR(imx->clk_ipg);
-	}
-
 	imx->chip.ops = &imx_pwm_ops;
 	imx->chip.dev = &pdev->dev;
 	imx->chip.base = -1;
-- 
2.1.4

[toc] | [prev] | [next] | [standalone]

#1569294 — [PATCH v5 11/11] pwm: imx: Add polarity inversion support to i.MX's PWMv2

With this patch the polarity settings for i.MX's PWMv2 is now supported
on top of atomic PWM setting

Signed-off-by: Bhuvanchandra DV <bhuvanchandra.dv@toradex.com>
Signed-off-by: Lukasz Majewski <l.majewski@majess.pl>
---
Changes for v5:
- Adjust to corrected ->apply_v2() code

Changes for v4:
- MX3_PWMCR_POUTC setting adjustements after apply_v2() previous changes

Changes for v3:
- None

Changes for v2:
- New patch
---
 drivers/pwm/pwm-imx.c | 22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/drivers/pwm/pwm-imx.c b/drivers/pwm/pwm-imx.c
index e037023..9f7e787 100644
--- a/drivers/pwm/pwm-imx.c
+++ b/drivers/pwm/pwm-imx.c
@@ -38,6 +38,7 @@
 #define MX3_PWMCR_DOZEEN		(1 << 24)
 #define MX3_PWMCR_WAITEN		(1 << 23)
 #define MX3_PWMCR_DBGEN			(1 << 22)
+#define MX3_PWMCR_POUTC			(1 << 18)
 #define MX3_PWMCR_CLKSRC_IPG_HIGH	(2 << 16)
 #define MX3_PWMCR_CLKSRC_IPG		(1 << 16)
 #define MX3_PWMCR_SWR			(1 << 3)
@@ -164,6 +165,7 @@ static int imx_pwm_apply_v2(struct pwm_chip *chip, struct pwm_device *pwm,
 	struct pwm_state cstate;
 	unsigned long long c;
 	int ret;
+	u32 cr;
 
 	pwm_get_state(pwm, &cstate);
 
@@ -208,12 +210,15 @@ static int imx_pwm_apply_v2(struct pwm_chip *chip, struct pwm_device *pwm,
 		writel(duty_cycles, imx->mmio_base + MX3_PWMSAR);
 		writel(period_cycles, imx->mmio_base + MX3_PWMPR);
 
-		writel(MX3_PWMCR_PRESCALER(prescale) |
-		       MX3_PWMCR_DOZEEN | MX3_PWMCR_WAITEN |
-		       MX3_PWMCR_DBGEN | MX3_PWMCR_CLKSRC_IPG_HIGH |
-		       MX3_PWMCR_EN,
-		       imx->mmio_base + MX3_PWMCR);
+		cr = MX3_PWMCR_PRESCALER(prescale) |
+			MX3_PWMCR_DOZEEN | MX3_PWMCR_WAITEN |
+			MX3_PWMCR_DBGEN | MX3_PWMCR_CLKSRC_IPG_HIGH |
+			MX3_PWMCR_EN;
+
+		if (state->polarity == PWM_POLARITY_INVERSED)
+			cr |= MX3_PWMCR_POUTC;
 
+		writel(cr, imx->mmio_base + MX3_PWMCR);
 	} else if (cstate.enabled) {
 		writel(0, imx->mmio_base + MX3_PWMCR);
 
@@ -236,6 +241,7 @@ static struct pwm_ops imx_pwm_ops_v2 = {
 };
 
 struct imx_pwm_data {
+	bool polarity_supported;
 	struct pwm_ops *pwm_ops;
 };
 
@@ -244,6 +250,7 @@ static struct imx_pwm_data imx_pwm_data_v1 = {
 };
 
 static struct imx_pwm_data imx_pwm_data_v2 = {
+	.polarity_supported = true,
 	.pwm_ops = &imx_pwm_ops_v2,
 };
 
@@ -284,6 +291,11 @@ static int imx_pwm_probe(struct platform_device *pdev)
 	imx->chip.base = -1;
 	imx->chip.npwm = 1;
 	imx->chip.can_sleep = true;
+	if (data->polarity_supported) {
+		dev_dbg(&pdev->dev, "PWM supports output inversion\n");
+		imx->chip.of_xlate = of_pwm_xlate_with_flags;
+		imx->chip.of_pwm_n_cells = 3;
+	}
 
 	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	imx->mmio_base = devm_ioremap_resource(&pdev->dev, r);
-- 
2.1.4

[toc] | [prev] | [next] | [standalone]

#1569295 — [PATCH v5 10/11] pwm: imx: doc: Update imx-pwm.txt documentation entry

From: Lukasz Majewski <l.majewski@majess.pl>

The imx-pwm.txt documentation update as a preparation for polarity
support.

Signed-off-by: Bhuvanchandra DV <bhuvanchandra.dv@toradex.com>
Signed-off-by: Lukasz Majewski <l.majewski@majess.pl>
Acked-by: Rob Herring <robh@kernel.org>
---
Changes for v5:
- None

Changes for v4:
- None

Changes for v3:
- None

Changes for v2:
- New patch
---
 Documentation/devicetree/bindings/pwm/imx-pwm.txt | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/pwm/imx-pwm.txt b/Documentation/devicetree/bindings/pwm/imx-pwm.txt
index e00c2e9..c61bdf8 100644
--- a/Documentation/devicetree/bindings/pwm/imx-pwm.txt
+++ b/Documentation/devicetree/bindings/pwm/imx-pwm.txt
@@ -6,8 +6,8 @@ Required properties:
   - "fsl,imx1-pwm" for PWM compatible with the one integrated on i.MX1
   - "fsl,imx27-pwm" for PWM compatible with the one integrated on i.MX27
 - reg: physical base address and length of the controller's registers
-- #pwm-cells: should be 2. See pwm.txt in this directory for a description of
-  the cells format.
+- #pwm-cells: 2 for i.MX1 and 3 for i.MX27 and newer SoCs. See pwm.txt
+  in this directory for a description of the cells format.
 - clocks : Clock specifiers for both ipg and per clocks.
 - clock-names : Clock names should include both "ipg" and "per"
 See the clock consumer binding,
@@ -17,7 +17,7 @@ See the clock consumer binding,
 Example:
 
 pwm1: pwm@53fb4000 {
-	#pwm-cells = <2>;
+	#pwm-cells = <3>;
 	compatible = "fsl,imx53-pwm", "fsl,imx27-pwm";
 	reg = <0x53fb4000 0x4000>;
 	clocks = <&clks IMX5_CLK_PWM1_IPG_GATE>,
-- 
2.1.4

[toc] | [prev] | [next] | [standalone]

#1569296 — [PATCH v5 03/11] pwm: imx: Add separate set of pwm ops for PWMv1 and PWMv2

From: Lukasz Majewski <l.majewski@majess.pl>

This patch provides separate set of pwm ops utilized by
i.MX's PWMv1 and PWMv2.

Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
Signed-off-by: Bhuvanchandra DV <bhuvanchandra.dv@toradex.com>
Signed-off-by: Lukasz Majewski <l.majewski@majess.pl>
Acked-by: Shawn Guo <shawn.guo@linaro.org>
Reviewed-by: Sascha Hauer <s.hauer@pengutronix.de>
---
Changes for v5:
- None

Changes for v4:
- None

Changes for v3:
- Adjust the code to work with ipg clock removed

Changes for v2:
- New patch
---
 drivers/pwm/pwm-imx.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/pwm/pwm-imx.c b/drivers/pwm/pwm-imx.c
index b1d1e50..0fa480d 100644
--- a/drivers/pwm/pwm-imx.c
+++ b/drivers/pwm/pwm-imx.c
@@ -239,7 +239,14 @@ static void imx_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm)
 	clk_disable_unprepare(imx->clk_per);
 }
 
-static struct pwm_ops imx_pwm_ops = {
+static struct pwm_ops imx_pwm_ops_v1 = {
+	.enable = imx_pwm_enable,
+	.disable = imx_pwm_disable,
+	.config = imx_pwm_config,
+	.owner = THIS_MODULE,
+};
+
+static struct pwm_ops imx_pwm_ops_v2 = {
 	.enable = imx_pwm_enable,
 	.disable = imx_pwm_disable,
 	.config = imx_pwm_config,
@@ -250,16 +257,19 @@ struct imx_pwm_data {
 	int (*config)(struct pwm_chip *chip,
 		struct pwm_device *pwm, int duty_ns, int period_ns);
 	void (*set_enable)(struct pwm_chip *chip, bool enable);
+	struct pwm_ops *pwm_ops;
 };
 
 static struct imx_pwm_data imx_pwm_data_v1 = {
 	.config = imx_pwm_config_v1,
 	.set_enable = imx_pwm_set_enable_v1,
+	.pwm_ops = &imx_pwm_ops_v1,
 };
 
 static struct imx_pwm_data imx_pwm_data_v2 = {
 	.config = imx_pwm_config_v2,
 	.set_enable = imx_pwm_set_enable_v2,
+	.pwm_ops = &imx_pwm_ops_v2,
 };
 
 static const struct of_device_id imx_pwm_dt_ids[] = {
@@ -281,6 +291,8 @@ static int imx_pwm_probe(struct platform_device *pdev)
 	if (!of_id)
 		return -ENODEV;
 
+	data = of_id->data;
+
 	imx = devm_kzalloc(&pdev->dev, sizeof(*imx), GFP_KERNEL);
 	if (imx == NULL)
 		return -ENOMEM;
@@ -292,7 +304,7 @@ static int imx_pwm_probe(struct platform_device *pdev)
 		return PTR_ERR(imx->clk_per);
 	}
 
-	imx->chip.ops = &imx_pwm_ops;
+	imx->chip.ops = data->pwm_ops;
 	imx->chip.dev = &pdev->dev;
 	imx->chip.base = -1;
 	imx->chip.npwm = 1;
@@ -303,7 +315,6 @@ static int imx_pwm_probe(struct platform_device *pdev)
 	if (IS_ERR(imx->mmio_base))
 		return PTR_ERR(imx->mmio_base);
 
-	data = of_id->data;
 	imx->config = data->config;
 	imx->set_enable = data->set_enable;
 
-- 
2.1.4

[toc] | [prev] | [next] | [standalone]

#1569437 — Re: [PATCH v5 03/11] pwm: imx: Add separate set of pwm ops for PWMv1 and PWMv2

[Multipart message — attachments visible in raw view] — view raw

On Sun, Jan 29, 2017 at 10:54:07PM +0100, Lukasz Majewski wrote:
> From: Lukasz Majewski <l.majewski@majess.pl>
> 
> This patch provides separate set of pwm ops utilized by
> i.MX's PWMv1 and PWMv2.
> 
> Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
> Signed-off-by: Bhuvanchandra DV <bhuvanchandra.dv@toradex.com>
> Signed-off-by: Lukasz Majewski <l.majewski@majess.pl>
> Acked-by: Shawn Guo <shawn.guo@linaro.org>
> Reviewed-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
> Changes for v5:
> - None
> 
> Changes for v4:
> - None
> 
> Changes for v3:
> - Adjust the code to work with ipg clock removed
> 
> Changes for v2:
> - New patch
> ---
>  drivers/pwm/pwm-imx.c | 17 ++++++++++++++---
>  1 file changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/pwm/pwm-imx.c b/drivers/pwm/pwm-imx.c
> index b1d1e50..0fa480d 100644
> --- a/drivers/pwm/pwm-imx.c
> +++ b/drivers/pwm/pwm-imx.c
> @@ -239,7 +239,14 @@ static void imx_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm)
>  	clk_disable_unprepare(imx->clk_per);
>  }
>  
> -static struct pwm_ops imx_pwm_ops = {
> +static struct pwm_ops imx_pwm_ops_v1 = {
> +	.enable = imx_pwm_enable,
> +	.disable = imx_pwm_disable,
> +	.config = imx_pwm_config,
> +	.owner = THIS_MODULE,
> +};
> +
> +static struct pwm_ops imx_pwm_ops_v2 = {

Can't these two be const? No need to respin for only this, just let me
know and I can make the change while applying.

Thierry

[toc] | [prev] | [next] | [standalone]

#1569441 — Re: [PATCH v5 03/11] pwm: imx: Add separate set of pwm ops for PWMv1 and PWMv2

[Multipart message — attachments visible in raw view] — view raw

On Mon, Jan 30, 2017 at 08:23:12AM +0100, Thierry Reding wrote:
> On Sun, Jan 29, 2017 at 10:54:07PM +0100, Lukasz Majewski wrote:
> > From: Lukasz Majewski <l.majewski@majess.pl>
> > 
> > This patch provides separate set of pwm ops utilized by
> > i.MX's PWMv1 and PWMv2.
> > 
> > Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
> > Signed-off-by: Bhuvanchandra DV <bhuvanchandra.dv@toradex.com>
> > Signed-off-by: Lukasz Majewski <l.majewski@majess.pl>
> > Acked-by: Shawn Guo <shawn.guo@linaro.org>
> > Reviewed-by: Sascha Hauer <s.hauer@pengutronix.de>
> > ---
> > Changes for v5:
> > - None
> > 
> > Changes for v4:
> > - None
> > 
> > Changes for v3:
> > - Adjust the code to work with ipg clock removed
> > 
> > Changes for v2:
> > - New patch
> > ---
> >  drivers/pwm/pwm-imx.c | 17 ++++++++++++++---
> >  1 file changed, 14 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/pwm/pwm-imx.c b/drivers/pwm/pwm-imx.c
> > index b1d1e50..0fa480d 100644
> > --- a/drivers/pwm/pwm-imx.c
> > +++ b/drivers/pwm/pwm-imx.c
> > @@ -239,7 +239,14 @@ static void imx_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm)
> >  	clk_disable_unprepare(imx->clk_per);
> >  }
> >  
> > -static struct pwm_ops imx_pwm_ops = {
> > +static struct pwm_ops imx_pwm_ops_v1 = {
> > +	.enable = imx_pwm_enable,
> > +	.disable = imx_pwm_disable,
> > +	.config = imx_pwm_config,
> > +	.owner = THIS_MODULE,
> > +};
> > +
> > +static struct pwm_ops imx_pwm_ops_v2 = {
> 
> Can't these two be const? No need to respin for only this, just let me
> know and I can make the change while applying.

Nevermind that. I just remembered that I had picked up a patch to make
the original imx_pwm_ops a const and things still work fine if I make
both of the above const, so I just had to manually apply your patch, but
other than that it seems fine. Let me apply the rest of this set and
push out. It'd be great if you could check afterwards that it's all
still what you expect.

Thierry

[toc] | [prev] | [next] | [standalone]

#1569466 — Re: [PATCH v5 03/11] pwm: imx: Add separate set of pwm ops for PWMv1 and PWMv2

[Multipart message — attachments visible in raw view] — view raw

Hi Thierry,

> On Mon, Jan 30, 2017 at 08:23:12AM +0100, Thierry Reding wrote:
> > On Sun, Jan 29, 2017 at 10:54:07PM +0100, Lukasz Majewski wrote:
> > > From: Lukasz Majewski <l.majewski@majess.pl>
> > > 
> > > This patch provides separate set of pwm ops utilized by
> > > i.MX's PWMv1 and PWMv2.
> > > 
> > > Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
> > > Signed-off-by: Bhuvanchandra DV <bhuvanchandra.dv@toradex.com>
> > > Signed-off-by: Lukasz Majewski <l.majewski@majess.pl>
> > > Acked-by: Shawn Guo <shawn.guo@linaro.org>
> > > Reviewed-by: Sascha Hauer <s.hauer@pengutronix.de>
> > > ---
> > > Changes for v5:
> > > - None
> > > 
> > > Changes for v4:
> > > - None
> > > 
> > > Changes for v3:
> > > - Adjust the code to work with ipg clock removed
> > > 
> > > Changes for v2:
> > > - New patch
> > > ---
> > >  drivers/pwm/pwm-imx.c | 17 ++++++++++++++---
> > >  1 file changed, 14 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/drivers/pwm/pwm-imx.c b/drivers/pwm/pwm-imx.c
> > > index b1d1e50..0fa480d 100644
> > > --- a/drivers/pwm/pwm-imx.c
> > > +++ b/drivers/pwm/pwm-imx.c
> > > @@ -239,7 +239,14 @@ static void imx_pwm_disable(struct pwm_chip
> > > *chip, struct pwm_device *pwm)
> > > clk_disable_unprepare(imx->clk_per); }
> > >  
> > > -static struct pwm_ops imx_pwm_ops = {
> > > +static struct pwm_ops imx_pwm_ops_v1 = {
> > > +	.enable = imx_pwm_enable,
> > > +	.disable = imx_pwm_disable,
> > > +	.config = imx_pwm_config,
> > > +	.owner = THIS_MODULE,
> > > +};
> > > +
> > > +static struct pwm_ops imx_pwm_ops_v2 = {
> > 
> > Can't these two be const? No need to respin for only this, just let
> > me know and I can make the change while applying.
> 
> Nevermind that. I just remembered that I had picked up a patch to make
> the original imx_pwm_ops a const and things still work fine if I make
> both of the above const, so I just had to manually apply your patch,
> but other than that it seems fine. Let me apply the rest of this set
> and push out. It'd be great if you could check afterwards that it's
> all still what you expect.

I will do that. Thanks for integrating the patch series :-).

> 
> Thierry


Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de

[toc] | [prev] | [next] | [standalone]

#1569476 — Re: [PATCH v5 03/11] pwm: imx: Add separate set of pwm ops for PWMv1 and PWMv2

[Multipart message — attachments visible in raw view] — view raw

On Mon, Jan 30, 2017 at 09:36:49AM +0100, Lukasz Majewski wrote:
> Hi Thierry,
> 
> > On Mon, Jan 30, 2017 at 08:23:12AM +0100, Thierry Reding wrote:
> > > On Sun, Jan 29, 2017 at 10:54:07PM +0100, Lukasz Majewski wrote:
> > > > From: Lukasz Majewski <l.majewski@majess.pl>
> > > > 
> > > > This patch provides separate set of pwm ops utilized by
> > > > i.MX's PWMv1 and PWMv2.
> > > > 
> > > > Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
> > > > Signed-off-by: Bhuvanchandra DV <bhuvanchandra.dv@toradex.com>
> > > > Signed-off-by: Lukasz Majewski <l.majewski@majess.pl>
> > > > Acked-by: Shawn Guo <shawn.guo@linaro.org>
> > > > Reviewed-by: Sascha Hauer <s.hauer@pengutronix.de>
> > > > ---
> > > > Changes for v5:
> > > > - None
> > > > 
> > > > Changes for v4:
> > > > - None
> > > > 
> > > > Changes for v3:
> > > > - Adjust the code to work with ipg clock removed
> > > > 
> > > > Changes for v2:
> > > > - New patch
> > > > ---
> > > >  drivers/pwm/pwm-imx.c | 17 ++++++++++++++---
> > > >  1 file changed, 14 insertions(+), 3 deletions(-)
> > > > 
> > > > diff --git a/drivers/pwm/pwm-imx.c b/drivers/pwm/pwm-imx.c
> > > > index b1d1e50..0fa480d 100644
> > > > --- a/drivers/pwm/pwm-imx.c
> > > > +++ b/drivers/pwm/pwm-imx.c
> > > > @@ -239,7 +239,14 @@ static void imx_pwm_disable(struct pwm_chip
> > > > *chip, struct pwm_device *pwm)
> > > > clk_disable_unprepare(imx->clk_per); }
> > > >  
> > > > -static struct pwm_ops imx_pwm_ops = {
> > > > +static struct pwm_ops imx_pwm_ops_v1 = {
> > > > +	.enable = imx_pwm_enable,
> > > > +	.disable = imx_pwm_disable,
> > > > +	.config = imx_pwm_config,
> > > > +	.owner = THIS_MODULE,
> > > > +};
> > > > +
> > > > +static struct pwm_ops imx_pwm_ops_v2 = {
> > > 
> > > Can't these two be const? No need to respin for only this, just let
> > > me know and I can make the change while applying.
> > 
> > Nevermind that. I just remembered that I had picked up a patch to make
> > the original imx_pwm_ops a const and things still work fine if I make
> > both of the above const, so I just had to manually apply your patch,
> > but other than that it seems fine. Let me apply the rest of this set
> > and push out. It'd be great if you could check afterwards that it's
> > all still what you expect.
> 
> I will do that. Thanks for integrating the patch series :-).

Thanks for sticking with it. I know the initial patches for optional
polarity support have been around for years, and it took a really long
time for this all to come together.

But I think the end result is sound and looks really good.

The one remaining bit that I'm not 100% happy about is that the v1
support is not atomic while the v2 support is. Not a blocker, but it
looks as if it should be easy to convert over v1 as well. Any takers?

Thierry

[toc] | [prev] | [next] | [standalone]

#1569478 — Re: [PATCH v5 03/11] pwm: imx: Add separate set of pwm ops for PWMv1 and PWMv2

[Multipart message — attachments visible in raw view] — view raw

Hi Thierry,

> On Mon, Jan 30, 2017 at 09:36:49AM +0100, Lukasz Majewski wrote:
> > Hi Thierry,
> > 
> > > On Mon, Jan 30, 2017 at 08:23:12AM +0100, Thierry Reding wrote:
> > > > On Sun, Jan 29, 2017 at 10:54:07PM +0100, Lukasz Majewski wrote:
> > > > > From: Lukasz Majewski <l.majewski@majess.pl>
> > > > > 
> > > > > This patch provides separate set of pwm ops utilized by
> > > > > i.MX's PWMv1 and PWMv2.
> > > > > 
> > > > > Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
> > > > > Signed-off-by: Bhuvanchandra DV <bhuvanchandra.dv@toradex.com>
> > > > > Signed-off-by: Lukasz Majewski <l.majewski@majess.pl>
> > > > > Acked-by: Shawn Guo <shawn.guo@linaro.org>
> > > > > Reviewed-by: Sascha Hauer <s.hauer@pengutronix.de>
> > > > > ---
> > > > > Changes for v5:
> > > > > - None
> > > > > 
> > > > > Changes for v4:
> > > > > - None
> > > > > 
> > > > > Changes for v3:
> > > > > - Adjust the code to work with ipg clock removed
> > > > > 
> > > > > Changes for v2:
> > > > > - New patch
> > > > > ---
> > > > >  drivers/pwm/pwm-imx.c | 17 ++++++++++++++---
> > > > >  1 file changed, 14 insertions(+), 3 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/pwm/pwm-imx.c b/drivers/pwm/pwm-imx.c
> > > > > index b1d1e50..0fa480d 100644
> > > > > --- a/drivers/pwm/pwm-imx.c
> > > > > +++ b/drivers/pwm/pwm-imx.c
> > > > > @@ -239,7 +239,14 @@ static void imx_pwm_disable(struct
> > > > > pwm_chip *chip, struct pwm_device *pwm)
> > > > > clk_disable_unprepare(imx->clk_per); }
> > > > >  
> > > > > -static struct pwm_ops imx_pwm_ops = {
> > > > > +static struct pwm_ops imx_pwm_ops_v1 = {
> > > > > +	.enable = imx_pwm_enable,
> > > > > +	.disable = imx_pwm_disable,
> > > > > +	.config = imx_pwm_config,
> > > > > +	.owner = THIS_MODULE,
> > > > > +};
> > > > > +
> > > > > +static struct pwm_ops imx_pwm_ops_v2 = {
> > > > 
> > > > Can't these two be const? No need to respin for only this, just
> > > > let me know and I can make the change while applying.
> > > 
> > > Nevermind that. I just remembered that I had picked up a patch to
> > > make the original imx_pwm_ops a const and things still work fine
> > > if I make both of the above const, so I just had to manually
> > > apply your patch, but other than that it seems fine. Let me apply
> > > the rest of this set and push out. It'd be great if you could
> > > check afterwards that it's all still what you expect.
> > 
> > I will do that. Thanks for integrating the patch series :-).
> 
> Thanks for sticking with it. I know the initial patches for optional
> polarity support have been around for years, and it took a really long
> time for this all to come together.
> 
> But I think the end result is sound and looks really good.

You are welcome :-)

> 
> The one remaining bit that I'm not 100% happy about is that the v1
> support is not atomic while the v2 support is.

Here the only limitation is the lack of v1 HW.

> Not a blocker, but it
> looks as if it should be easy to convert over v1 as well. Any takers?
> 
> Thierry


Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de

[toc] | [prev] | [next] | [standalone]

#1569485 — Re: [PATCH v5 03/11] pwm: imx: Add separate set of pwm ops for PWMv1 and PWMv2

[Multipart message — attachments visible in raw view] — view raw

On Mon, Jan 30, 2017 at 09:55:04AM +0100, Lukasz Majewski wrote:
> Hi Thierry,
> 
> > On Mon, Jan 30, 2017 at 09:36:49AM +0100, Lukasz Majewski wrote:
> > > Hi Thierry,
> > > 
> > > > On Mon, Jan 30, 2017 at 08:23:12AM +0100, Thierry Reding wrote:
> > > > > On Sun, Jan 29, 2017 at 10:54:07PM +0100, Lukasz Majewski wrote:
> > > > > > From: Lukasz Majewski <l.majewski@majess.pl>
> > > > > > 
> > > > > > This patch provides separate set of pwm ops utilized by
> > > > > > i.MX's PWMv1 and PWMv2.
> > > > > > 
> > > > > > Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
> > > > > > Signed-off-by: Bhuvanchandra DV <bhuvanchandra.dv@toradex.com>
> > > > > > Signed-off-by: Lukasz Majewski <l.majewski@majess.pl>
> > > > > > Acked-by: Shawn Guo <shawn.guo@linaro.org>
> > > > > > Reviewed-by: Sascha Hauer <s.hauer@pengutronix.de>
> > > > > > ---
> > > > > > Changes for v5:
> > > > > > - None
> > > > > > 
> > > > > > Changes for v4:
> > > > > > - None
> > > > > > 
> > > > > > Changes for v3:
> > > > > > - Adjust the code to work with ipg clock removed
> > > > > > 
> > > > > > Changes for v2:
> > > > > > - New patch
> > > > > > ---
> > > > > >  drivers/pwm/pwm-imx.c | 17 ++++++++++++++---
> > > > > >  1 file changed, 14 insertions(+), 3 deletions(-)
> > > > > > 
> > > > > > diff --git a/drivers/pwm/pwm-imx.c b/drivers/pwm/pwm-imx.c
> > > > > > index b1d1e50..0fa480d 100644
> > > > > > --- a/drivers/pwm/pwm-imx.c
> > > > > > +++ b/drivers/pwm/pwm-imx.c
> > > > > > @@ -239,7 +239,14 @@ static void imx_pwm_disable(struct
> > > > > > pwm_chip *chip, struct pwm_device *pwm)
> > > > > > clk_disable_unprepare(imx->clk_per); }
> > > > > >  
> > > > > > -static struct pwm_ops imx_pwm_ops = {
> > > > > > +static struct pwm_ops imx_pwm_ops_v1 = {
> > > > > > +	.enable = imx_pwm_enable,
> > > > > > +	.disable = imx_pwm_disable,
> > > > > > +	.config = imx_pwm_config,
> > > > > > +	.owner = THIS_MODULE,
> > > > > > +};
> > > > > > +
> > > > > > +static struct pwm_ops imx_pwm_ops_v2 = {
> > > > > 
> > > > > Can't these two be const? No need to respin for only this, just
> > > > > let me know and I can make the change while applying.
> > > > 
> > > > Nevermind that. I just remembered that I had picked up a patch to
> > > > make the original imx_pwm_ops a const and things still work fine
> > > > if I make both of the above const, so I just had to manually
> > > > apply your patch, but other than that it seems fine. Let me apply
> > > > the rest of this set and push out. It'd be great if you could
> > > > check afterwards that it's all still what you expect.
> > > 
> > > I will do that. Thanks for integrating the patch series :-).
> > 
> > Thanks for sticking with it. I know the initial patches for optional
> > polarity support have been around for years, and it took a really long
> > time for this all to come together.
> > 
> > But I think the end result is sound and looks really good.
> 
> You are welcome :-)
> 
> > 
> > The one remaining bit that I'm not 100% happy about is that the v1
> > support is not atomic while the v2 support is.
> 
> Here the only limitation is the lack of v1 HW.

That doesn't have to be a blocker. If you're willing to invest some more
work to do the additional conversion (I think it would be a fairly minor
change, looking at the existing v1 code), I'm sure we can find someone
with the hardware to test it.

Sascha, Lothar, Fabio, Shawn: do you guys have access to v1 hardware, or
know of anyone who might?

Thierry

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web