| Message ID | 1389859585-14006-3-git-send-email-LW@KARO-electronics.de |
|---|---|
| State | New |
| Headers | show |
On Thu, Jan 16, 2014 at 09:06:25AM +0100, Lothar Waßmann wrote: > The i.MX PWM controller supports inverting the polarity of the PWM > output. Make this feature available in the pxm-imx driver. > > Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de> > --- > Documentation/devicetree/bindings/pwm/imx-pwm.txt | 5 +- > drivers/pwm/pwm-imx.c | 42 +++++++++++++++++++++ > 2 files changed, 45 insertions(+), 2 deletions(-) > > diff --git a/Documentation/devicetree/bindings/pwm/imx-pwm.txt b/Documentation/devicetree/bindings/pwm/imx-pwm.txt > index b50d7a6d..d0b04b5 100644 > --- a/Documentation/devicetree/bindings/pwm/imx-pwm.txt > +++ b/Documentation/devicetree/bindings/pwm/imx-pwm.txt > @@ -3,8 +3,9 @@ Freescale i.MX PWM controller > Required properties: > - compatible: should be "fsl,<soc>-pwm" > - 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: may be 2 for backwards compatibility or 3 to support > + switching the output polarity. See pwm.txt in this directory for a > + description of the cells format. > - interrupts: The interrupt for the pwm controller > > Example: > diff --git a/drivers/pwm/pwm-imx.c b/drivers/pwm/pwm-imx.c > index 3b00a82..05461bb 100644 > --- a/drivers/pwm/pwm-imx.c > +++ b/drivers/pwm/pwm-imx.c > @@ -36,6 +36,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_EN (1 << 0) > @@ -138,6 +139,9 @@ static int imx_pwm_config_v2(struct pwm_chip *chip, > if (test_bit(PWMF_ENABLED, &pwm->flags)) > cr |= MX3_PWMCR_EN; > > + if (pwm->polarity == PWM_POLARITY_INVERSED) > + cr |= MX3_PWMCR_POUTC; > + > writel(cr, imx->mmio_base + MX3_PWMCR); > > return 0; > @@ -155,6 +159,11 @@ static void imx_pwm_set_enable_v2(struct pwm_chip *chip, bool enable) > else > val &= ~MX3_PWMCR_EN; > > + if (chip->pwms[0].polarity == PWM_POLARITY_INVERSED) > + val |= MX3_PWMCR_POUTC; > + else > + val &= ~MX3_PWMCR_POUTC; > + > writel(val, imx->mmio_base + MX3_PWMCR); > } > > @@ -198,6 +207,17 @@ static void imx_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm) > clk_disable_unprepare(imx->clk_per); > } > > +static int imx_pwm_set_polarity(struct pwm_chip *chip, struct pwm_device *pwm, > + enum pwm_polarity polarity) > +{ > + struct imx_chip *imx = to_imx_chip(chip); > + > + dev_dbg(imx->chip.dev, "%s: polarity set to %s\n", __func__, > + polarity == PWM_POLARITY_INVERSED ? "inverted" : "normal"); > + > + return 0; > +} > + > static struct pwm_ops imx_pwm_ops = { > .enable = imx_pwm_enable, > .disable = imx_pwm_disable, > @@ -209,6 +229,7 @@ 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); > + unsigned output_polarity:1; > }; > > static struct imx_pwm_data imx_pwm_data_v1 = { > @@ -219,6 +240,7 @@ static struct imx_pwm_data imx_pwm_data_v1 = { > static struct imx_pwm_data imx_pwm_data_v2 = { > .config = imx_pwm_config_v2, > .set_enable = imx_pwm_set_enable_v2, > + .output_polarity = 1, > }; > > static const struct of_device_id imx_pwm_dt_ids[] = { > @@ -271,6 +293,26 @@ static int imx_pwm_probe(struct platform_device *pdev) > return PTR_ERR(imx->mmio_base); > > data = of_id->data; > + if (data->output_polarity) { > + const struct device_node *np = pdev->dev.of_node; > + u32 num_cells; > + > + dev_dbg(&pdev->dev, "PWM supports output inversion\n"); > + ret = of_property_read_u32(np, "#pwm-cells", &num_cells); > + if (ret < 0) { > + dev_err(&pdev->dev, "missing property '#pwm-cells'\n"); > + return ret; > + } > + if (num_cells == 3) { > + imx_pwm_ops.set_polarity = imx_pwm_set_polarity; > + imx->chip.of_xlate = of_pwm_xlate_with_flags; > + } else if (num_cells != 2) { > + dev_err(&pdev->dev, "'#pwm-cells' must be <2> or <3>\n"); > + return -EINVAL; > + } > + imx->chip.of_pwm_n_cells = num_cells; > + } Can't this be done in the PWM core? Right now the PWM core checks for of_pwm_n_cells before calling ->of_xlate. IMO this check should be done in the of_xlate hook. Then of_pwm_simple_xlate and of_pwm_xlate_with_flags can be merged into something like: static struct pwm_device * of_pwm_simple_xlate(struct pwm_chip *pc, const struct of_phandle_args *args) { struct pwm_device *pwm; int ret; if (pc->of_pwm_n_cells < 2) return ERR_PTR(-EINVAL); if (args->args[0] >= pc->npwm) return ERR_PTR(-EINVAL); pwm = pwm_request_from_chip(pc, args->args[0], NULL); if (IS_ERR(pwm)) return pwm; if (pc->of_pwm_n_cells >= 3) { if (args->args[2] & PWM_POLARITY_INVERTED) ret = pwm_set_polarity(pwm, PWM_POLARITY_INVERSED); else ret = pwm_set_polarity(pwm, PWM_POLARITY_NORMAL); if (ret) return ret; } pwm_set_period(pwm, args->args[1]); return pwm; } Sascha
Hi, Sascha Hauer wrote: > On Thu, Jan 16, 2014 at 09:06:25AM +0100, Lothar Waßmann wrote: > > The i.MX PWM controller supports inverting the polarity of the PWM > > output. Make this feature available in the pxm-imx driver. > > > > Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de> > > --- > > Documentation/devicetree/bindings/pwm/imx-pwm.txt | 5 +- > > drivers/pwm/pwm-imx.c | 42 +++++++++++++++++++++ > > 2 files changed, 45 insertions(+), 2 deletions(-) > > > > diff --git a/Documentation/devicetree/bindings/pwm/imx-pwm.txt b/Documentation/devicetree/bindings/pwm/imx-pwm.txt > > index b50d7a6d..d0b04b5 100644 > > --- a/Documentation/devicetree/bindings/pwm/imx-pwm.txt > > +++ b/Documentation/devicetree/bindings/pwm/imx-pwm.txt > > @@ -3,8 +3,9 @@ Freescale i.MX PWM controller > > Required properties: > > - compatible: should be "fsl,<soc>-pwm" > > - 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: may be 2 for backwards compatibility or 3 to support > > + switching the output polarity. See pwm.txt in this directory for a > > + description of the cells format. > > - interrupts: The interrupt for the pwm controller > > > > Example: > > diff --git a/drivers/pwm/pwm-imx.c b/drivers/pwm/pwm-imx.c > > index 3b00a82..05461bb 100644 > > --- a/drivers/pwm/pwm-imx.c > > +++ b/drivers/pwm/pwm-imx.c > > @@ -36,6 +36,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_EN (1 << 0) > > @@ -138,6 +139,9 @@ static int imx_pwm_config_v2(struct pwm_chip *chip, > > if (test_bit(PWMF_ENABLED, &pwm->flags)) > > cr |= MX3_PWMCR_EN; > > > > + if (pwm->polarity == PWM_POLARITY_INVERSED) > > + cr |= MX3_PWMCR_POUTC; > > + > > writel(cr, imx->mmio_base + MX3_PWMCR); > > > > return 0; > > @@ -155,6 +159,11 @@ static void imx_pwm_set_enable_v2(struct pwm_chip *chip, bool enable) > > else > > val &= ~MX3_PWMCR_EN; > > > > + if (chip->pwms[0].polarity == PWM_POLARITY_INVERSED) > > + val |= MX3_PWMCR_POUTC; > > + else > > + val &= ~MX3_PWMCR_POUTC; > > + > > writel(val, imx->mmio_base + MX3_PWMCR); > > } > > > > @@ -198,6 +207,17 @@ static void imx_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm) > > clk_disable_unprepare(imx->clk_per); > > } > > > > +static int imx_pwm_set_polarity(struct pwm_chip *chip, struct pwm_device *pwm, > > + enum pwm_polarity polarity) > > +{ > > + struct imx_chip *imx = to_imx_chip(chip); > > + > > + dev_dbg(imx->chip.dev, "%s: polarity set to %s\n", __func__, > > + polarity == PWM_POLARITY_INVERSED ? "inverted" : "normal"); > > + > > + return 0; > > +} > > + > > static struct pwm_ops imx_pwm_ops = { > > .enable = imx_pwm_enable, > > .disable = imx_pwm_disable, > > @@ -209,6 +229,7 @@ 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); > > + unsigned output_polarity:1; > > }; > > > > static struct imx_pwm_data imx_pwm_data_v1 = { > > @@ -219,6 +240,7 @@ static struct imx_pwm_data imx_pwm_data_v1 = { > > static struct imx_pwm_data imx_pwm_data_v2 = { > > .config = imx_pwm_config_v2, > > .set_enable = imx_pwm_set_enable_v2, > > + .output_polarity = 1, > > }; > > > > static const struct of_device_id imx_pwm_dt_ids[] = { > > @@ -271,6 +293,26 @@ static int imx_pwm_probe(struct platform_device *pdev) > > return PTR_ERR(imx->mmio_base); > > > > data = of_id->data; > > + if (data->output_polarity) { > > + const struct device_node *np = pdev->dev.of_node; > > + u32 num_cells; > > + > > + dev_dbg(&pdev->dev, "PWM supports output inversion\n"); > > + ret = of_property_read_u32(np, "#pwm-cells", &num_cells); > > + if (ret < 0) { > > + dev_err(&pdev->dev, "missing property '#pwm-cells'\n"); > > + return ret; > > + } > > + if (num_cells == 3) { > > + imx_pwm_ops.set_polarity = imx_pwm_set_polarity; > > + imx->chip.of_xlate = of_pwm_xlate_with_flags; > > + } else if (num_cells != 2) { > > + dev_err(&pdev->dev, "'#pwm-cells' must be <2> or <3>\n"); > > + return -EINVAL; > > + } > > + imx->chip.of_pwm_n_cells = num_cells; > > + } > > Can't this be done in the PWM core? Right now the PWM core checks for > of_pwm_n_cells before calling ->of_xlate. IMO this check should be done > in the of_xlate hook. Then of_pwm_simple_xlate and > of_pwm_xlate_with_flags can be merged into something like: > This wouldn't buy much without a material change to of_pwm_get(). The function of_parse_phandle_with_args() called by of_pwm_get() requires the number of args in the pwms property be greater or equal to the #pwm-cells property in the pwm node. Thus, the interesting case of having #pwm-cells = <3> without changing the existing users is prohibited by of_parse_phandle_with_args(). of_pwm_get() would have to be changed to something like below to allow this: struct pwm_device *of_pwm_get(struct device_node *np, const char *con_id) { struct pwm_device *pwm = NULL; struct of_phandle_args args; struct pwm_chip *pc; int index = 0; int err; struct property *prop; u32 num_cells; int i; const __be32 *list; if (con_id) { index = of_property_match_string(np, "pwm-names", con_id); if (index < 0) return ERR_PTR(index); } args.np = of_parse_phandle(np, "pwms", index); if (!args.np) { pr_err("%s(): property 'pwms' not found in '%s'\n", __func__, np->full_name); return ERR_PTR(-ENOENT); } err = of_property_read_u32(args.np, "#pwm-cells", &num_cells); if (err) { pr_err("%s(): could not read property '#pwm-cells' in '%s': %d\n", __func__, args.np->full_name, err); return ERR_PTR(err); } prop = of_find_property(np, "pwms", NULL); if (WARN_ON(!prop)) return ERR_PTR(-EINVAL); args.args_count = prop->length / sizeof(u32) - 1; list = prop->value; for (i = 0; i < args.args_count; i++) args.args[i] = be32_to_cpup(++list); pc = of_node_to_pwmchip(args.np); if (IS_ERR(pc)) { pr_err("%s(): PWM chip not found\n", __func__); pwm = ERR_CAST(pc); goto put; } [...] Lothar Waßmann
I thinking more of the following. I haven't tested it, but it has a negative
diffstat, so it must be good ;)
Sascha
----------------------------------------------------------------
Sascha Hauer (2):
PWM: let of_xlate handlers check args count
PWM: handle additional flags in of_pwm_simple_xlate
drivers/pwm/core.c | 41 +++++++----------------------------------
drivers/pwm/pwm-atmel-tcb.c | 2 --
drivers/pwm/pwm-renesas-tpu.c | 2 --
drivers/pwm/pwm-samsung.c | 3 ---
drivers/pwm/pwm-tiecap.c | 2 --
drivers/pwm/pwm-tiehrpwm.c | 2 --
drivers/pwm/pwm-vt8500.c | 2 --
include/linux/pwm.h | 3 ---
8 files changed, 7 insertions(+), 50 deletions(-)
Hi, Sascha Hauer wrote: > of_pwm_n_cells for the of_xlate handler is stored in struct pwm_chip, > but it is only ever used by the of_xlate handler itsel. Remove > of_pwm_n_cells from struct pwm_chip and let the handler do the argument > count checking to simplify the code. > This still does not make the PWM_POLARITY flag in the pwms node optional as was the goal because of_parse_phandle_with_args() requires at least #pwm-cells arguments in the node. So, with a DT configuration like: pwm0: pwm@0 { #pwm-cells = <3>; }; backlight { pwms = <&pwm0 0 100000>; }; the driver will bail out at of_parse_phandle_with_args() in of_pwm_get() with the error message: "/backlight: arguments longer than property" and never reach your clever xlate function. Thus you will still need to replace of_parse_phandle_with_args() with different code that copies most but not all of the functionality. Lothar Waßmann
On Thu, Jan 23, 2014 at 11:56:32AM +0100, Lothar Waßmann wrote: > Hi, > > Sascha Hauer wrote: > > of_pwm_n_cells for the of_xlate handler is stored in struct pwm_chip, > > but it is only ever used by the of_xlate handler itsel. Remove > > of_pwm_n_cells from struct pwm_chip and let the handler do the argument > > count checking to simplify the code. > > > This still does not make the PWM_POLARITY flag in the pwms node > optional as was the goal because of_parse_phandle_with_args() requires > at least #pwm-cells arguments in the node. > > So, with a DT configuration like: > pwm0: pwm@0 { > #pwm-cells = <3>; > }; > backlight { > pwms = <&pwm0 0 100000>; > }; We misunderstood each other. My goal was to allow the driver to also work with old devicetrees which specify #pwm-cells = <2>, not to allow inconsistent devicetrees like the snippet above. Sascha
On Thu, Jan 23, 2014 at 11:52:03AM +0000, Russell King - ARM Linux wrote: > On Thu, Jan 23, 2014 at 08:37:14AM +0100, Lothar Waßmann wrote: > > This wouldn't buy much without a material change to of_pwm_get(). > > The function of_parse_phandle_with_args() called by of_pwm_get() > > requires the number of args in the pwms property be greater or equal to > > the #pwm-cells property in the pwm node. Thus, the interesting case of > > having #pwm-cells = <3> without changing the existing users is > > prohibited by of_parse_phandle_with_args(). > > I really don't think that's a problem we need to be concerned with at > the moment. What we need is for the kernel to be able to parse files > with #pwm-cells = <2> with the pwms property containing two arguments, > and when they're updated to #pwm-cells = <3> with the pwms property > containing three arguments. > > Yes, that means all the board dt files need to be updated at the same > time to include the additional argument, but I don't see that as a big > problem. > > What we do need to do is to adjust the PWM parsing code such that it's > possible to use either specification without causing any side effects. > > I would test this, but as u-boot is rather fscked at the moment and the > networking has broken on my cubox-i as a result... and it seems that the > u-boot developers have pissed off cubox-i u-boot hackers soo much that > they've dropped u-boot in favour of barebox... Oh, and another reason... the u-boot video settings are totally and utterly buggered to the point that it doesn't produce correct timings, and it seems that u-boot people have zero interest in fixing that, so u-boot mainline is basically refusing to fix this - another reason to stay away from it. (1024x768 @ 60Hz produces 70Hz refresh on iMX6Q here - I've seen it produce 51Hz on iMX6S, both of which are far enough out that lots of display devices will not accept it as a valid signal.)
On Thu, Jan 23, 2014 at 11:52:03AM +0000, Russell King - ARM Linux wrote: > On Thu, Jan 23, 2014 at 08:37:14AM +0100, Lothar Waßmann wrote: > > This wouldn't buy much without a material change to of_pwm_get(). > > The function of_parse_phandle_with_args() called by of_pwm_get() > > requires the number of args in the pwms property be greater or equal to > > the #pwm-cells property in the pwm node. Thus, the interesting case of > > having #pwm-cells = <3> without changing the existing users is > > prohibited by of_parse_phandle_with_args(). > > I really don't think that's a problem we need to be concerned with at > the moment. What we need is for the kernel to be able to parse files > with #pwm-cells = <2> with the pwms property containing two arguments, > and when they're updated to #pwm-cells = <3> with the pwms property > containing three arguments. > > Yes, that means all the board dt files need to be updated at the same > time to include the additional argument, but I don't see that as a big > problem. > > What we do need to do is to adjust the PWM parsing code such that it's > possible to use either specification without causing any side effects. > > I would test this, but as u-boot is rather fscked at the moment and the > networking has broken on my cubox-i as a result... and it seems that the > u-boot developers have pissed off cubox-i u-boot hackers soo much that > they've dropped u-boot in favour of barebox... Okay, finally confirmed that this works with #pwm-cells = 2.
On Thu, Jan 23, 2014 at 12:04:44PM +0100, Sascha Hauer wrote: > On Thu, Jan 23, 2014 at 11:56:32AM +0100, Lothar Waßmann wrote: > > Hi, > > > > Sascha Hauer wrote: > > > of_pwm_n_cells for the of_xlate handler is stored in struct pwm_chip, > > > but it is only ever used by the of_xlate handler itsel. Remove > > > of_pwm_n_cells from struct pwm_chip and let the handler do the argument > > > count checking to simplify the code. > > > > > This still does not make the PWM_POLARITY flag in the pwms node > > optional as was the goal because of_parse_phandle_with_args() requires > > at least #pwm-cells arguments in the node. > > > > So, with a DT configuration like: > > pwm0: pwm@0 { > > #pwm-cells = <3>; > > }; > > backlight { > > pwms = <&pwm0 0 100000>; > > }; > > We misunderstood each other. My goal was to allow the driver to also > work with old devicetrees which specify #pwm-cells = <2>, not to allow > inconsistent devicetrees like the snippet above. In which case, the patch I've posted seems to do that job too... I'm just about to test out the three-cell version.
On Thu, Jan 23, 2014 at 04:53:50PM +0000, Russell King - ARM Linux wrote: > On Thu, Jan 23, 2014 at 12:04:44PM +0100, Sascha Hauer wrote: > > On Thu, Jan 23, 2014 at 11:56:32AM +0100, Lothar Waßmann wrote: > > > Hi, > > > > > > Sascha Hauer wrote: > > > > of_pwm_n_cells for the of_xlate handler is stored in struct pwm_chip, > > > > but it is only ever used by the of_xlate handler itsel. Remove > > > > of_pwm_n_cells from struct pwm_chip and let the handler do the argument > > > > count checking to simplify the code. > > > > > > > This still does not make the PWM_POLARITY flag in the pwms node > > > optional as was the goal because of_parse_phandle_with_args() requires > > > at least #pwm-cells arguments in the node. > > > > > > So, with a DT configuration like: > > > pwm0: pwm@0 { > > > #pwm-cells = <3>; > > > }; > > > backlight { > > > pwms = <&pwm0 0 100000>; > > > }; > > > > We misunderstood each other. My goal was to allow the driver to also > > work with old devicetrees which specify #pwm-cells = <2>, not to allow > > inconsistent devicetrees like the snippet above. > > In which case, the patch I've posted seems to do that job too... I'm > just about to test out the three-cell version. Okay, this works, but there's a problem with pwm-leds. When the duty cycle is set to zero (when you set the brightness to zero) pwm-leds decides to disable the PWM after configuring it. This causes the PWM output to be driven low, causing the LED to go to maximum brightness. So, using the inversion at PWM level doesn't work. To make this work correctly, we really need pwm-leds to do the inversion rather than setting the inversion bit in hardware.
Hi, Russell King - ARM Linux wrote: > On Thu, Jan 23, 2014 at 04:53:50PM +0000, Russell King - ARM Linux wrote: > > On Thu, Jan 23, 2014 at 12:04:44PM +0100, Sascha Hauer wrote: > > > On Thu, Jan 23, 2014 at 11:56:32AM +0100, Lothar Waßmann wrote: > > > > Hi, > > > > > > > > Sascha Hauer wrote: > > > > > of_pwm_n_cells for the of_xlate handler is stored in struct pwm_chip, > > > > > but it is only ever used by the of_xlate handler itsel. Remove > > > > > of_pwm_n_cells from struct pwm_chip and let the handler do the argument > > > > > count checking to simplify the code. > > > > > > > > > This still does not make the PWM_POLARITY flag in the pwms node > > > > optional as was the goal because of_parse_phandle_with_args() requires > > > > at least #pwm-cells arguments in the node. > > > > > > > > So, with a DT configuration like: > > > > pwm0: pwm@0 { > > > > #pwm-cells = <3>; > > > > }; > > > > backlight { > > > > pwms = <&pwm0 0 100000>; > > > > }; > > > > > > We misunderstood each other. My goal was to allow the driver to also > > > work with old devicetrees which specify #pwm-cells = <2>, not to allow > > > inconsistent devicetrees like the snippet above. > > > > In which case, the patch I've posted seems to do that job too... I'm > > just about to test out the three-cell version. > > Okay, this works, but there's a problem with pwm-leds. > > When the duty cycle is set to zero (when you set the brightness to zero) > pwm-leds decides to disable the PWM after configuring it. This causes > the PWM output to be driven low, causing the LED to go to maximum > brightness. > > So, using the inversion at PWM level doesn't work. > The problem is that the driver calls pwm_disable() when the duty cycle is 0. This sets the PWM output low independent from the output polarity setting. > To make this work correctly, we really need pwm-leds to do the inversion > rather than setting the inversion bit in hardware. > The same holds for the pwm-backlight driver. The easiest fix would be not to call pwm_disable() even for a zero duty cycle. Lothar Waßmann
diff --git a/Documentation/devicetree/bindings/pwm/imx-pwm.txt b/Documentation/devicetree/bindings/pwm/imx-pwm.txt index b50d7a6d..d0b04b5 100644 --- a/Documentation/devicetree/bindings/pwm/imx-pwm.txt +++ b/Documentation/devicetree/bindings/pwm/imx-pwm.txt @@ -3,8 +3,9 @@ Freescale i.MX PWM controller Required properties: - compatible: should be "fsl,<soc>-pwm" - 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: may be 2 for backwards compatibility or 3 to support + switching the output polarity. See pwm.txt in this directory for a + description of the cells format. - interrupts: The interrupt for the pwm controller Example: diff --git a/drivers/pwm/pwm-imx.c b/drivers/pwm/pwm-imx.c index 3b00a82..05461bb 100644 --- a/drivers/pwm/pwm-imx.c +++ b/drivers/pwm/pwm-imx.c @@ -36,6 +36,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_EN (1 << 0) @@ -138,6 +139,9 @@ static int imx_pwm_config_v2(struct pwm_chip *chip, if (test_bit(PWMF_ENABLED, &pwm->flags)) cr |= MX3_PWMCR_EN; + if (pwm->polarity == PWM_POLARITY_INVERSED) + cr |= MX3_PWMCR_POUTC; + writel(cr, imx->mmio_base + MX3_PWMCR); return 0; @@ -155,6 +159,11 @@ static void imx_pwm_set_enable_v2(struct pwm_chip *chip, bool enable) else val &= ~MX3_PWMCR_EN; + if (chip->pwms[0].polarity == PWM_POLARITY_INVERSED) + val |= MX3_PWMCR_POUTC; + else + val &= ~MX3_PWMCR_POUTC; + writel(val, imx->mmio_base + MX3_PWMCR); } @@ -198,6 +207,17 @@ static void imx_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm) clk_disable_unprepare(imx->clk_per); } +static int imx_pwm_set_polarity(struct pwm_chip *chip, struct pwm_device *pwm, + enum pwm_polarity polarity) +{ + struct imx_chip *imx = to_imx_chip(chip); + + dev_dbg(imx->chip.dev, "%s: polarity set to %s\n", __func__, + polarity == PWM_POLARITY_INVERSED ? "inverted" : "normal"); + + return 0; +} + static struct pwm_ops imx_pwm_ops = { .enable = imx_pwm_enable, .disable = imx_pwm_disable, @@ -209,6 +229,7 @@ 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); + unsigned output_polarity:1; }; static struct imx_pwm_data imx_pwm_data_v1 = { @@ -219,6 +240,7 @@ static struct imx_pwm_data imx_pwm_data_v1 = { static struct imx_pwm_data imx_pwm_data_v2 = { .config = imx_pwm_config_v2, .set_enable = imx_pwm_set_enable_v2, + .output_polarity = 1, }; static const struct of_device_id imx_pwm_dt_ids[] = { @@ -271,6 +293,26 @@ static int imx_pwm_probe(struct platform_device *pdev) return PTR_ERR(imx->mmio_base); data = of_id->data; + if (data->output_polarity) { + const struct device_node *np = pdev->dev.of_node; + u32 num_cells; + + dev_dbg(&pdev->dev, "PWM supports output inversion\n"); + ret = of_property_read_u32(np, "#pwm-cells", &num_cells); + if (ret < 0) { + dev_err(&pdev->dev, "missing property '#pwm-cells'\n"); + return ret; + } + if (num_cells == 3) { + imx_pwm_ops.set_polarity = imx_pwm_set_polarity; + imx->chip.of_xlate = of_pwm_xlate_with_flags; + } else if (num_cells != 2) { + dev_err(&pdev->dev, "'#pwm-cells' must be <2> or <3>\n"); + return -EINVAL; + } + imx->chip.of_pwm_n_cells = num_cells; + } + imx->config = data->config; imx->set_enable = data->set_enable;
The i.MX PWM controller supports inverting the polarity of the PWM output. Make this feature available in the pxm-imx driver. Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de> --- Documentation/devicetree/bindings/pwm/imx-pwm.txt | 5 +- drivers/pwm/pwm-imx.c | 42 +++++++++++++++++++++ 2 files changed, 45 insertions(+), 2 deletions(-)