* [PATCHv2 0/2] pwm: imx: support polarity inversion @ 2014-01-16 8:06 Lothar Waßmann 2014-01-16 8:06 ` [PATCHv2 1/2] pwm: imx: indentation cleanup Lothar Waßmann 2014-01-16 8:06 ` [PATCHv2 2/2] pwm: imx: support polarity inversion Lothar Waßmann 0 siblings, 2 replies; 17+ messages in thread From: Lothar Waßmann @ 2014-01-16 8:06 UTC (permalink / raw) To: linux-arm-kernel, Shawn Guo, Sascha Hauer, Thierry Reding, Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, Rob Landley, linux-pwm, devicetree, linux-doc, linux-kernel The first patch does a minor source cleanup without any functional change. Changes wrt. v1: - make PWM_POLARITY flag optional, so that the driver will work with either the existing DT binding or with support for polarity inversion. ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCHv2 1/2] pwm: imx: indentation cleanup 2014-01-16 8:06 [PATCHv2 0/2] pwm: imx: support polarity inversion Lothar Waßmann @ 2014-01-16 8:06 ` Lothar Waßmann 2014-01-16 8:06 ` [PATCHv2 2/2] pwm: imx: support polarity inversion Lothar Waßmann 1 sibling, 0 replies; 17+ messages in thread From: Lothar Waßmann @ 2014-01-16 8:06 UTC (permalink / raw) To: linux-arm-kernel, Shawn Guo, Sascha Hauer, Thierry Reding, Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, Rob Landley, linux-pwm, devicetree, linux-doc, linux-kernel Cc: Lothar Waßmann Consistently use TABs for indentation and the same indentation level. No functional change. Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de> --- drivers/pwm/pwm-imx.c | 26 +++++++++++++------------- 1 files changed, 13 insertions(+), 13 deletions(-) diff --git a/drivers/pwm/pwm-imx.c b/drivers/pwm/pwm-imx.c index cc47733..3b00a82 100644 --- a/drivers/pwm/pwm-imx.c +++ b/drivers/pwm/pwm-imx.c @@ -21,24 +21,24 @@ /* i.MX1 and i.MX21 share the same PWM function block: */ -#define MX1_PWMC 0x00 /* PWM Control Register */ -#define MX1_PWMS 0x04 /* PWM Sample Register */ -#define MX1_PWMP 0x08 /* PWM Period Register */ +#define MX1_PWMC 0x00 /* PWM Control Register */ +#define MX1_PWMS 0x04 /* PWM Sample Register */ +#define MX1_PWMP 0x08 /* PWM Period Register */ -#define MX1_PWMC_EN (1 << 4) +#define MX1_PWMC_EN (1 << 4) /* i.MX27, i.MX31, i.MX35 share the same PWM function block: */ -#define MX3_PWMCR 0x00 /* PWM Control Register */ -#define MX3_PWMSAR 0x0C /* PWM Sample Register */ -#define MX3_PWMPR 0x10 /* PWM Period Register */ -#define MX3_PWMCR_PRESCALER(x) (((x - 1) & 0xFFF) << 4) -#define MX3_PWMCR_DOZEEN (1 << 24) -#define MX3_PWMCR_WAITEN (1 << 23) +#define MX3_PWMCR 0x00 /* PWM Control Register */ +#define MX3_PWMSAR 0x0C /* PWM Sample Register */ +#define MX3_PWMPR 0x10 /* PWM Period Register */ +#define MX3_PWMCR_PRESCALER(x) (((x - 1) & 0xFFF) << 4) +#define MX3_PWMCR_DOZEEN (1 << 24) +#define MX3_PWMCR_WAITEN (1 << 23) #define MX3_PWMCR_DBGEN (1 << 22) -#define MX3_PWMCR_CLKSRC_IPG_HIGH (2 << 16) -#define MX3_PWMCR_CLKSRC_IPG (1 << 16) -#define MX3_PWMCR_EN (1 << 0) +#define MX3_PWMCR_CLKSRC_IPG_HIGH (2 << 16) +#define MX3_PWMCR_CLKSRC_IPG (1 << 16) +#define MX3_PWMCR_EN (1 << 0) struct imx_chip { struct clk *clk_per; -- 1.7.2.5 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCHv2 2/2] pwm: imx: support polarity inversion 2014-01-16 8:06 [PATCHv2 0/2] pwm: imx: support polarity inversion Lothar Waßmann 2014-01-16 8:06 ` [PATCHv2 1/2] pwm: imx: indentation cleanup Lothar Waßmann @ 2014-01-16 8:06 ` Lothar Waßmann 2014-01-16 16:03 ` Sascha Hauer 1 sibling, 1 reply; 17+ messages in thread From: Lothar Waßmann @ 2014-01-16 8:06 UTC (permalink / raw) To: linux-arm-kernel, Shawn Guo, Sascha Hauer, Thierry Reding, Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, Rob Landley, linux-pwm, devicetree, linux-doc, linux-kernel Cc: Lothar Waßmann 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; + } + imx->config = data->config; imx->set_enable = data->set_enable; -- 1.7.2.5 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCHv2 2/2] pwm: imx: support polarity inversion 2014-01-16 8:06 ` [PATCHv2 2/2] pwm: imx: support polarity inversion Lothar Waßmann @ 2014-01-16 16:03 ` Sascha Hauer 2014-01-23 7:37 ` Lothar Waßmann 0 siblings, 1 reply; 17+ messages in thread From: Sascha Hauer @ 2014-01-16 16:03 UTC (permalink / raw) To: Lothar Waßmann Cc: Mark Rutland, linux-pwm, Rob Landley, Pawel Moll, Ian Campbell, linux-doc, linux-kernel, devicetree, Rob Herring, Thierry Reding, Sascha Hauer, Kumar Gala, Shawn Guo, linux-arm-kernel 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 -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCHv2 2/2] pwm: imx: support polarity inversion 2014-01-16 16:03 ` Sascha Hauer @ 2014-01-23 7:37 ` Lothar Waßmann 2014-01-23 9:04 ` Sascha Hauer 2014-01-23 11:52 ` [PATCHv2 2/2] pwm: imx: support polarity inversion Russell King - ARM Linux 0 siblings, 2 replies; 17+ messages in thread From: Lothar Waßmann @ 2014-01-23 7:37 UTC (permalink / raw) To: Sascha Hauer Cc: linux-arm-kernel, Shawn Guo, Sascha Hauer, Thierry Reding, Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, Rob Landley, linux-pwm, devicetree, linux-doc, linux-kernel 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 -- ___________________________________________________________ Ka-Ro electronics GmbH | Pascalstraße 22 | D - 52076 Aachen Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10 Geschäftsführer: Matthias Kaussen Handelsregistereintrag: Amtsgericht Aachen, HRB 4996 www.karo-electronics.de | info@karo-electronics.de ___________________________________________________________ ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCHv2 2/2] pwm: imx: support polarity inversion 2014-01-23 7:37 ` Lothar Waßmann @ 2014-01-23 9:04 ` Sascha Hauer 2014-01-23 9:04 ` [PATCH 1/2] PWM: let of_xlate handlers check args count Sascha Hauer 2014-01-23 9:04 ` [PATCH 2/2] PWM: handle additional flags in of_pwm_simple_xlate Sascha Hauer 2014-01-23 11:52 ` [PATCHv2 2/2] pwm: imx: support polarity inversion Russell King - ARM Linux 1 sibling, 2 replies; 17+ messages in thread From: Sascha Hauer @ 2014-01-23 9:04 UTC (permalink / raw) To: Lothar Waßmann Cc: Mark Rutland, linux-pwm, Pawel Moll, Ian Campbell, linux-kernel, Rob Herring, Thierry Reding, Rob Landley, Kumar Gala, Shawn Guo, linux-arm-kernel 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(-) ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 1/2] PWM: let of_xlate handlers check args count 2014-01-23 9:04 ` Sascha Hauer @ 2014-01-23 9:04 ` Sascha Hauer 2014-01-23 10:56 ` Lothar Waßmann 2014-01-23 9:04 ` [PATCH 2/2] PWM: handle additional flags in of_pwm_simple_xlate Sascha Hauer 1 sibling, 1 reply; 17+ messages in thread From: Sascha Hauer @ 2014-01-23 9:04 UTC (permalink / raw) To: Lothar Waßmann Cc: linux-arm-kernel, Shawn Guo, Thierry Reding, Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, Rob Landley, linux-pwm, linux-kernel, Sascha Hauer 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. Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> --- drivers/pwm/core.c | 15 +++------------ drivers/pwm/pwm-atmel-tcb.c | 1 - drivers/pwm/pwm-renesas-tpu.c | 1 - drivers/pwm/pwm-samsung.c | 1 - drivers/pwm/pwm-tiecap.c | 1 - drivers/pwm/pwm-tiehrpwm.c | 1 - drivers/pwm/pwm-vt8500.c | 1 - 7 files changed, 3 insertions(+), 18 deletions(-) diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c index 2ca9504..c882051 100644 --- a/drivers/pwm/core.c +++ b/drivers/pwm/core.c @@ -136,7 +136,7 @@ of_pwm_xlate_with_flags(struct pwm_chip *pc, const struct of_phandle_args *args) { struct pwm_device *pwm; - if (pc->of_pwm_n_cells < 3) + if (args->args_count != 3) return ERR_PTR(-EINVAL); if (args->args[0] >= pc->npwm) @@ -162,7 +162,7 @@ of_pwm_simple_xlate(struct pwm_chip *pc, const struct of_phandle_args *args) { struct pwm_device *pwm; - if (pc->of_pwm_n_cells < 2) + if (args->args_count != 2) return ERR_PTR(-EINVAL); if (args->args[0] >= pc->npwm) @@ -182,10 +182,8 @@ static void of_pwmchip_add(struct pwm_chip *chip) if (!chip->dev || !chip->dev->of_node) return; - if (!chip->of_xlate) { + if (!chip->of_xlate) chip->of_xlate = of_pwm_simple_xlate; - chip->of_pwm_n_cells = 2; - } of_node_get(chip->dev->of_node); } @@ -536,13 +534,6 @@ struct pwm_device *of_pwm_get(struct device_node *np, const char *con_id) goto put; } - if (args.args_count != pc->of_pwm_n_cells) { - pr_debug("%s: wrong #pwm-cells for %s\n", np->full_name, - args.np->full_name); - pwm = ERR_PTR(-EINVAL); - goto put; - } - pwm = pc->of_xlate(pc, &args); if (IS_ERR(pwm)) goto put; diff --git a/drivers/pwm/pwm-atmel-tcb.c b/drivers/pwm/pwm-atmel-tcb.c index f3dcd02..55fabf8 100644 --- a/drivers/pwm/pwm-atmel-tcb.c +++ b/drivers/pwm/pwm-atmel-tcb.c @@ -395,7 +395,6 @@ static int atmel_tcb_pwm_probe(struct platform_device *pdev) tcbpwm->chip.dev = &pdev->dev; tcbpwm->chip.ops = &atmel_tcb_pwm_ops; tcbpwm->chip.of_xlate = of_pwm_xlate_with_flags; - tcbpwm->chip.of_pwm_n_cells = 3; tcbpwm->chip.base = -1; tcbpwm->chip.npwm = NPWM; tcbpwm->tc = tc; diff --git a/drivers/pwm/pwm-renesas-tpu.c b/drivers/pwm/pwm-renesas-tpu.c index aff6ba9..0a8adb6 100644 --- a/drivers/pwm/pwm-renesas-tpu.c +++ b/drivers/pwm/pwm-renesas-tpu.c @@ -434,7 +434,6 @@ static int tpu_probe(struct platform_device *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; diff --git a/drivers/pwm/pwm-samsung.c b/drivers/pwm/pwm-samsung.c index b59639e..8d8dced 100644 --- a/drivers/pwm/pwm-samsung.c +++ b/drivers/pwm/pwm-samsung.c @@ -487,7 +487,6 @@ static int pwm_samsung_probe(struct platform_device *pdev) return ret; chip->chip.of_xlate = of_pwm_xlate_with_flags; - chip->chip.of_pwm_n_cells = 3; } else { if (!pdev->dev.platform_data) { dev_err(&pdev->dev, "no platform data specified\n"); diff --git a/drivers/pwm/pwm-tiecap.c b/drivers/pwm/pwm-tiecap.c index 4e5c3d1..4d7a01a 100644 --- a/drivers/pwm/pwm-tiecap.c +++ b/drivers/pwm/pwm-tiecap.c @@ -229,7 +229,6 @@ static int ecap_pwm_probe(struct platform_device *pdev) pc->chip.dev = &pdev->dev; pc->chip.ops = &ecap_pwm_ops; pc->chip.of_xlate = of_pwm_xlate_with_flags; - pc->chip.of_pwm_n_cells = 3; pc->chip.base = -1; pc->chip.npwm = 1; diff --git a/drivers/pwm/pwm-tiehrpwm.c b/drivers/pwm/pwm-tiehrpwm.c index a4d8f51..2c2621a 100644 --- a/drivers/pwm/pwm-tiehrpwm.c +++ b/drivers/pwm/pwm-tiehrpwm.c @@ -460,7 +460,6 @@ static int ehrpwm_pwm_probe(struct platform_device *pdev) pc->chip.dev = &pdev->dev; pc->chip.ops = &ehrpwm_pwm_ops; pc->chip.of_xlate = of_pwm_xlate_with_flags; - pc->chip.of_pwm_n_cells = 3; pc->chip.base = -1; pc->chip.npwm = NUM_PWM_CHANNEL; diff --git a/drivers/pwm/pwm-vt8500.c b/drivers/pwm/pwm-vt8500.c index 323125a..5472051 100644 --- a/drivers/pwm/pwm-vt8500.c +++ b/drivers/pwm/pwm-vt8500.c @@ -219,7 +219,6 @@ static int vt8500_pwm_probe(struct platform_device *pdev) chip->chip.dev = &pdev->dev; chip->chip.ops = &vt8500_pwm_ops; chip->chip.of_xlate = of_pwm_xlate_with_flags; - chip->chip.of_pwm_n_cells = 3; chip->chip.base = -1; chip->chip.npwm = VT8500_NR_PWMS; -- 1.8.5.2 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 1/2] PWM: let of_xlate handlers check args count 2014-01-23 9:04 ` [PATCH 1/2] PWM: let of_xlate handlers check args count Sascha Hauer @ 2014-01-23 10:56 ` Lothar Waßmann 2014-01-23 11:04 ` Sascha Hauer 0 siblings, 1 reply; 17+ messages in thread From: Lothar Waßmann @ 2014-01-23 10:56 UTC (permalink / raw) To: Sascha Hauer Cc: linux-arm-kernel, Shawn Guo, Thierry Reding, Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, Rob Landley, linux-pwm, linux-kernel 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 -- ___________________________________________________________ Ka-Ro electronics GmbH | Pascalstraße 22 | D - 52076 Aachen Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10 Geschäftsführer: Matthias Kaussen Handelsregistereintrag: Amtsgericht Aachen, HRB 4996 www.karo-electronics.de | info@karo-electronics.de ___________________________________________________________ ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/2] PWM: let of_xlate handlers check args count 2014-01-23 10:56 ` Lothar Waßmann @ 2014-01-23 11:04 ` Sascha Hauer 2014-01-23 16:53 ` Russell King - ARM Linux 0 siblings, 1 reply; 17+ messages in thread From: Sascha Hauer @ 2014-01-23 11:04 UTC (permalink / raw) To: Lothar Waßmann Cc: linux-arm-kernel, Shawn Guo, Thierry Reding, Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, Rob Landley, linux-pwm, linux-kernel 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 -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/2] PWM: let of_xlate handlers check args count 2014-01-23 11:04 ` Sascha Hauer @ 2014-01-23 16:53 ` Russell King - ARM Linux 2014-01-23 17:36 ` Russell King - ARM Linux 0 siblings, 1 reply; 17+ messages in thread From: Russell King - ARM Linux @ 2014-01-23 16:53 UTC (permalink / raw) To: Sascha Hauer Cc: Lothar Waßmann, Mark Rutland, linux-pwm, Pawel Moll, Ian Campbell, linux-kernel, Rob Herring, Thierry Reding, Rob Landley, Kumar Gala, Shawn Guo, linux-arm-kernel 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. -- FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad. Estimate before purchase was "up to 13.2Mbit". ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/2] PWM: let of_xlate handlers check args count 2014-01-23 16:53 ` Russell King - ARM Linux @ 2014-01-23 17:36 ` Russell King - ARM Linux 2014-01-24 5:42 ` Lothar Waßmann 0 siblings, 1 reply; 17+ messages in thread From: Russell King - ARM Linux @ 2014-01-23 17:36 UTC (permalink / raw) To: Sascha Hauer Cc: Mark Rutland, linux-pwm, Pawel Moll, Ian Campbell, linux-kernel, Rob Herring, Thierry Reding, Rob Landley, Kumar Gala, Shawn Guo, linux-arm-kernel, Lothar Waßmann 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. -- FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad. Estimate before purchase was "up to 13.2Mbit". ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/2] PWM: let of_xlate handlers check args count 2014-01-23 17:36 ` Russell King - ARM Linux @ 2014-01-24 5:42 ` Lothar Waßmann 2014-01-24 7:41 ` Sascha Hauer 0 siblings, 1 reply; 17+ messages in thread From: Lothar Waßmann @ 2014-01-24 5:42 UTC (permalink / raw) To: Russell King - ARM Linux Cc: Sascha Hauer, Mark Rutland, linux-pwm, Pawel Moll, Ian Campbell, linux-kernel, Rob Herring, Thierry Reding, Rob Landley, Kumar Gala, Shawn Guo, linux-arm-kernel 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 -- ___________________________________________________________ Ka-Ro electronics GmbH | Pascalstraße 22 | D - 52076 Aachen Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10 Geschäftsführer: Matthias Kaussen Handelsregistereintrag: Amtsgericht Aachen, HRB 4996 www.karo-electronics.de | info@karo-electronics.de ___________________________________________________________ ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/2] PWM: let of_xlate handlers check args count 2014-01-24 5:42 ` Lothar Waßmann @ 2014-01-24 7:41 ` Sascha Hauer 0 siblings, 0 replies; 17+ messages in thread From: Sascha Hauer @ 2014-01-24 7:41 UTC (permalink / raw) To: Lothar Waßmann Cc: Russell King - ARM Linux, Mark Rutland, linux-pwm, Pawel Moll, Ian Campbell, linux-kernel, Rob Herring, Thierry Reding, Rob Landley, Kumar Gala, Shawn Guo, linux-arm-kernel On Fri, Jan 24, 2014 at 06:42:54AM +0100, Lothar Waßmann wrote: > Hi, > > > 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. IMO that's the right thing to do anyway due to the different PWM hardware controllers we have. I'm thinking about the following patch for some time. Sascha --------------------8<-------------------------- From 9ebbc3d72c71bd97d7fc4458f60ae3ecd5876984 Mon Sep 17 00:00:00 2001 From: Sascha Hauer <s.hauer@pengutronix.de> Date: Fri, 24 Jan 2014 08:34:16 +0100 Subject: [PATCH] PWM: Document disabled PWM output as undefined When disabled PWM hardware reacts differently. Some controllers just stop with their current value, others produce a constant high or low output, sometimes depending on the output inversion bit. Update the documentation to reflect that and request from the PWM consumer drivers to set a constant high/low value with duty cycles of 0/100% instead of disabling the PWM. Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> --- Documentation/pwm.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Documentation/pwm.txt b/Documentation/pwm.txt index 93cb979..b7e8a31 100644 --- a/Documentation/pwm.txt +++ b/Documentation/pwm.txt @@ -44,6 +44,8 @@ After being requested, a PWM has to be configured using: int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns); To start/stop toggling the PWM output use pwm_enable()/pwm_disable(). +The output of a disabled PWM is undefined. Set the duty cycle to 100% +for a constant high output and to 0 for constant low output. Using PWMs with the sysfs interface ----------------------------------- -- 1.8.5.2 -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 2/2] PWM: handle additional flags in of_pwm_simple_xlate 2014-01-23 9:04 ` Sascha Hauer 2014-01-23 9:04 ` [PATCH 1/2] PWM: let of_xlate handlers check args count Sascha Hauer @ 2014-01-23 9:04 ` Sascha Hauer 1 sibling, 0 replies; 17+ messages in thread From: Sascha Hauer @ 2014-01-23 9:04 UTC (permalink / raw) To: Lothar Waßmann Cc: linux-arm-kernel, Shawn Guo, Thierry Reding, Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, Rob Landley, linux-pwm, linux-kernel, Sascha Hauer Let of_pwm_simple_xlate behave like of_pwm_xlate_with_flags when the argument count is 3. This makes of_pwm_xlate_with_flags unncessary. Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> --- drivers/pwm/core.c | 30 ++++++------------------------ drivers/pwm/pwm-atmel-tcb.c | 1 - drivers/pwm/pwm-renesas-tpu.c | 1 - drivers/pwm/pwm-samsung.c | 2 -- drivers/pwm/pwm-tiecap.c | 1 - drivers/pwm/pwm-tiehrpwm.c | 1 - drivers/pwm/pwm-vt8500.c | 1 - include/linux/pwm.h | 3 --- 8 files changed, 6 insertions(+), 34 deletions(-) diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c index c882051..329b1e5 100644 --- a/drivers/pwm/core.c +++ b/drivers/pwm/core.c @@ -131,12 +131,12 @@ static int pwm_device_request(struct pwm_device *pwm, const char *label) return 0; } -struct pwm_device * -of_pwm_xlate_with_flags(struct pwm_chip *pc, const struct of_phandle_args *args) +static struct pwm_device * +of_pwm_simple_xlate(struct pwm_chip *pc, const struct of_phandle_args *args) { struct pwm_device *pwm; - if (args->args_count != 3) + if (args->args_count < 2) return ERR_PTR(-EINVAL); if (args->args[0] >= pc->npwm) @@ -148,6 +148,9 @@ of_pwm_xlate_with_flags(struct pwm_chip *pc, const struct of_phandle_args *args) pwm_set_period(pwm, args->args[1]); + if (args->args_count < 3) + return 0; + if (args->args[2] & PWM_POLARITY_INVERTED) pwm_set_polarity(pwm, PWM_POLARITY_INVERSED); else @@ -155,27 +158,6 @@ of_pwm_xlate_with_flags(struct pwm_chip *pc, const struct of_phandle_args *args) return pwm; } -EXPORT_SYMBOL_GPL(of_pwm_xlate_with_flags); - -static struct pwm_device * -of_pwm_simple_xlate(struct pwm_chip *pc, const struct of_phandle_args *args) -{ - struct pwm_device *pwm; - - if (args->args_count != 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; - - pwm_set_period(pwm, args->args[1]); - - return pwm; -} static void of_pwmchip_add(struct pwm_chip *chip) { diff --git a/drivers/pwm/pwm-atmel-tcb.c b/drivers/pwm/pwm-atmel-tcb.c index 55fabf8..e1e5a20 100644 --- a/drivers/pwm/pwm-atmel-tcb.c +++ b/drivers/pwm/pwm-atmel-tcb.c @@ -394,7 +394,6 @@ static int atmel_tcb_pwm_probe(struct platform_device *pdev) tcbpwm->chip.dev = &pdev->dev; tcbpwm->chip.ops = &atmel_tcb_pwm_ops; - tcbpwm->chip.of_xlate = of_pwm_xlate_with_flags; tcbpwm->chip.base = -1; tcbpwm->chip.npwm = NPWM; tcbpwm->tc = tc; diff --git a/drivers/pwm/pwm-renesas-tpu.c b/drivers/pwm/pwm-renesas-tpu.c index 0a8adb6..82dcab9 100644 --- a/drivers/pwm/pwm-renesas-tpu.c +++ b/drivers/pwm/pwm-renesas-tpu.c @@ -433,7 +433,6 @@ static int tpu_probe(struct platform_device *pdev) tpu->chip.dev = &pdev->dev; tpu->chip.ops = &tpu_pwm_ops; - tpu->chip.of_xlate = of_pwm_xlate_with_flags; tpu->chip.base = -1; tpu->chip.npwm = TPU_CHANNEL_MAX; diff --git a/drivers/pwm/pwm-samsung.c b/drivers/pwm/pwm-samsung.c index 8d8dced..465f9ee 100644 --- a/drivers/pwm/pwm-samsung.c +++ b/drivers/pwm/pwm-samsung.c @@ -485,8 +485,6 @@ static int pwm_samsung_probe(struct platform_device *pdev) ret = pwm_samsung_parse_dt(chip); if (ret) return ret; - - chip->chip.of_xlate = of_pwm_xlate_with_flags; } else { if (!pdev->dev.platform_data) { dev_err(&pdev->dev, "no platform data specified\n"); diff --git a/drivers/pwm/pwm-tiecap.c b/drivers/pwm/pwm-tiecap.c index 4d7a01a..27cde03 100644 --- a/drivers/pwm/pwm-tiecap.c +++ b/drivers/pwm/pwm-tiecap.c @@ -228,7 +228,6 @@ static int ecap_pwm_probe(struct platform_device *pdev) pc->chip.dev = &pdev->dev; pc->chip.ops = &ecap_pwm_ops; - pc->chip.of_xlate = of_pwm_xlate_with_flags; pc->chip.base = -1; pc->chip.npwm = 1; diff --git a/drivers/pwm/pwm-tiehrpwm.c b/drivers/pwm/pwm-tiehrpwm.c index 2c2621a..4e566df 100644 --- a/drivers/pwm/pwm-tiehrpwm.c +++ b/drivers/pwm/pwm-tiehrpwm.c @@ -459,7 +459,6 @@ static int ehrpwm_pwm_probe(struct platform_device *pdev) pc->chip.dev = &pdev->dev; pc->chip.ops = &ehrpwm_pwm_ops; - pc->chip.of_xlate = of_pwm_xlate_with_flags; pc->chip.base = -1; pc->chip.npwm = NUM_PWM_CHANNEL; diff --git a/drivers/pwm/pwm-vt8500.c b/drivers/pwm/pwm-vt8500.c index 5472051..30def61 100644 --- a/drivers/pwm/pwm-vt8500.c +++ b/drivers/pwm/pwm-vt8500.c @@ -218,7 +218,6 @@ static int vt8500_pwm_probe(struct platform_device *pdev) chip->chip.dev = &pdev->dev; chip->chip.ops = &vt8500_pwm_ops; - chip->chip.of_xlate = of_pwm_xlate_with_flags; chip->chip.base = -1; chip->chip.npwm = VT8500_NR_PWMS; diff --git a/include/linux/pwm.h b/include/linux/pwm.h index f0feafd..2447d6f 100644 --- a/include/linux/pwm.h +++ b/include/linux/pwm.h @@ -188,9 +188,6 @@ struct pwm_device *pwm_request_from_chip(struct pwm_chip *chip, unsigned int index, const char *label); -struct pwm_device *of_pwm_xlate_with_flags(struct pwm_chip *pc, - const struct of_phandle_args *args); - struct pwm_device *pwm_get(struct device *dev, const char *con_id); struct pwm_device *of_pwm_get(struct device_node *np, const char *con_id); void pwm_put(struct pwm_device *pwm); -- 1.8.5.2 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCHv2 2/2] pwm: imx: support polarity inversion 2014-01-23 7:37 ` Lothar Waßmann 2014-01-23 9:04 ` Sascha Hauer @ 2014-01-23 11:52 ` Russell King - ARM Linux 2014-01-23 12:33 ` Russell King - ARM Linux 2014-01-23 16:44 ` Russell King - ARM Linux 1 sibling, 2 replies; 17+ messages in thread From: Russell King - ARM Linux @ 2014-01-23 11:52 UTC (permalink / raw) To: Lothar Waßmann Cc: Sascha Hauer, Mark Rutland, linux-pwm, Rob Landley, Pawel Moll, Ian Campbell, linux-doc, linux-kernel, devicetree, Rob Herring, Thierry Reding, Sascha Hauer, Kumar Gala, Shawn Guo, linux-arm-kernel 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... drivers/pwm/core.c | 59 +++++++++++++++++++++++++++++------------------------ include/linux/pwm.h | 2 ++ 2 files changed, 34 insertions(+), 27 deletions(-) diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c index 2ca95042a0b9..40adbce8ef0c 100644 --- a/drivers/pwm/core.c +++ b/drivers/pwm/core.c @@ -132,14 +132,11 @@ static int pwm_device_request(struct pwm_device *pwm, const char *label) } struct pwm_device * -of_pwm_xlate_with_flags(struct pwm_chip *pc, const struct of_phandle_args *args) +of_pwm_xlate(struct pwm_chip *pc, const struct of_phandle_args *args) { struct pwm_device *pwm; - if (pc->of_pwm_n_cells < 3) - return ERR_PTR(-EINVAL); - - if (args->args[0] >= pc->npwm) + if (args->args_count < 2) return ERR_PTR(-EINVAL); pwm = pwm_request_from_chip(pc, args->args[0], NULL); @@ -148,33 +145,45 @@ of_pwm_xlate_with_flags(struct pwm_chip *pc, const struct of_phandle_args *args) pwm_set_period(pwm, args->args[1]); - if (args->args[2] & PWM_POLARITY_INVERTED) - pwm_set_polarity(pwm, PWM_POLARITY_INVERSED); - else - pwm_set_polarity(pwm, PWM_POLARITY_NORMAL); + if (args->args_count > 2) { + int err; + + if (args->args[2] & PWM_POLARITY_INVERTED) + err = pwm_set_polarity(pwm, PWM_POLARITY_INVERSED); + else + err = pwm_set_polarity(pwm, PWM_POLARITY_NORMAL); + + pwm_put(pwm); + return ERR_PTR(err); + } return pwm; } +EXPORT_SYMBOL(of_pwm_xlate); + +struct pwm_device * +of_pwm_xlate_with_flags(struct pwm_chip *pc, const struct of_phandle_args *args) +{ + if (pc->of_pwm_n_cells < 3) + return ERR_PTR(-EINVAL); + + if (args->args_count != pc->of_pwm_n_cells) + return ERR_PTR(-EINVAL); + + return of_pwm_xlate(pc, args); +} EXPORT_SYMBOL_GPL(of_pwm_xlate_with_flags); static struct pwm_device * of_pwm_simple_xlate(struct pwm_chip *pc, const struct of_phandle_args *args) { - struct pwm_device *pwm; - if (pc->of_pwm_n_cells < 2) return ERR_PTR(-EINVAL); - if (args->args[0] >= pc->npwm) + if (args->args_count != pc->of_pwm_n_cells) return ERR_PTR(-EINVAL); - pwm = pwm_request_from_chip(pc, args->args[0], NULL); - if (IS_ERR(pwm)) - return pwm; - - pwm_set_period(pwm, args->args[1]); - - return pwm; + return of_pwm_xlate(pc, args); } static void of_pwmchip_add(struct pwm_chip *chip) @@ -536,16 +545,12 @@ struct pwm_device *of_pwm_get(struct device_node *np, const char *con_id) goto put; } - if (args.args_count != pc->of_pwm_n_cells) { - pr_debug("%s: wrong #pwm-cells for %s\n", np->full_name, - args.np->full_name); - pwm = ERR_PTR(-EINVAL); - goto put; - } - pwm = pc->of_xlate(pc, &args); - if (IS_ERR(pwm)) + if (IS_ERR(pwm)) { + pr_debug("%s: of_xlate failed for %s: %d\n", np->full_name, + args.np->full_name, (int)PTR_ERR(pwm)); goto put; + } /* * If a consumer name was not given, try to look it up from the diff --git a/include/linux/pwm.h b/include/linux/pwm.h index f0feafd184a0..14a823f77c31 100644 --- a/include/linux/pwm.h +++ b/include/linux/pwm.h @@ -188,6 +188,8 @@ struct pwm_device *pwm_request_from_chip(struct pwm_chip *chip, unsigned int index, const char *label); +struct pwm_device *of_pwm_xlate(struct pwm_chip *pc, + const struct of_phandle_args *args); struct pwm_device *of_pwm_xlate_with_flags(struct pwm_chip *pc, const struct of_phandle_args *args); -- FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad. Estimate before purchase was "up to 13.2Mbit". ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCHv2 2/2] pwm: imx: support polarity inversion 2014-01-23 11:52 ` [PATCHv2 2/2] pwm: imx: support polarity inversion Russell King - ARM Linux @ 2014-01-23 12:33 ` Russell King - ARM Linux 2014-01-23 16:44 ` Russell King - ARM Linux 1 sibling, 0 replies; 17+ messages in thread From: Russell King - ARM Linux @ 2014-01-23 12:33 UTC (permalink / raw) To: Lothar Waßmann Cc: Mark Rutland, linux-pwm, Sascha Hauer, Pawel Moll, Ian Campbell, Sascha Hauer, linux-doc, linux-kernel, devicetree, Rob Herring, Thierry Reding, Rob Landley, Kumar Gala, Shawn Guo, linux-arm-kernel 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.) -- FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad. Estimate before purchase was "up to 13.2Mbit". ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCHv2 2/2] pwm: imx: support polarity inversion 2014-01-23 11:52 ` [PATCHv2 2/2] pwm: imx: support polarity inversion Russell King - ARM Linux 2014-01-23 12:33 ` Russell King - ARM Linux @ 2014-01-23 16:44 ` Russell King - ARM Linux 1 sibling, 0 replies; 17+ messages in thread From: Russell King - ARM Linux @ 2014-01-23 16:44 UTC (permalink / raw) To: Lothar Waßmann Cc: Mark Rutland, linux-pwm, Sascha Hauer, Pawel Moll, Ian Campbell, Sascha Hauer, linux-doc, linux-kernel, devicetree, Rob Herring, Thierry Reding, Rob Landley, Kumar Gala, Shawn Guo, linux-arm-kernel 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. -- FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad. Estimate before purchase was "up to 13.2Mbit". ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2014-01-24 7:41 UTC | newest] Thread overview: 17+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-01-16 8:06 [PATCHv2 0/2] pwm: imx: support polarity inversion Lothar Waßmann 2014-01-16 8:06 ` [PATCHv2 1/2] pwm: imx: indentation cleanup Lothar Waßmann 2014-01-16 8:06 ` [PATCHv2 2/2] pwm: imx: support polarity inversion Lothar Waßmann 2014-01-16 16:03 ` Sascha Hauer 2014-01-23 7:37 ` Lothar Waßmann 2014-01-23 9:04 ` Sascha Hauer 2014-01-23 9:04 ` [PATCH 1/2] PWM: let of_xlate handlers check args count Sascha Hauer 2014-01-23 10:56 ` Lothar Waßmann 2014-01-23 11:04 ` Sascha Hauer 2014-01-23 16:53 ` Russell King - ARM Linux 2014-01-23 17:36 ` Russell King - ARM Linux 2014-01-24 5:42 ` Lothar Waßmann 2014-01-24 7:41 ` Sascha Hauer 2014-01-23 9:04 ` [PATCH 2/2] PWM: handle additional flags in of_pwm_simple_xlate Sascha Hauer 2014-01-23 11:52 ` [PATCHv2 2/2] pwm: imx: support polarity inversion Russell King - ARM Linux 2014-01-23 12:33 ` Russell King - ARM Linux 2014-01-23 16:44 ` Russell King - ARM Linux
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).