RE: [PATCHv3 1/4] pwm: Add Freescale FTM PWM driver support
> Subject: Re: [PATCHv3 1/4] pwm: Add Freescale FTM PWM driver support
>
> On Fri, Sep 06, 2013 at 04:08:24PM +0800, Xiubo Li wrote:
> > The FTM PWM device can be found on Vybrid VF610 Tower and Layerscape
> LS-1 SoCs.
> >
> > +
> > +static int fsl_pwm_probe(struct platform_device *pdev) {
> > + int ret = 0;
> > + struct fsl_pwm_chip *fpc;
> > + struct resource *res;
> > +
> > + fpc = devm_kzalloc(>dev, sizeof(*fpc), GFP_KERNEL);
>
> You don't have to output a message when kzalloc fails, but still you
> should check for the result and return an error if necessary.
>
Okey, I will.
--
Xiubo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCHv3 1/4] pwm: Add Freescale FTM PWM driver support
On Fri, Sep 06, 2013 at 04:08:24PM +0800, Xiubo Li wrote:
> The FTM PWM device can be found on Vybrid VF610 Tower and Layerscape LS-1
> SoCs.
>
> +
> +static int fsl_pwm_probe(struct platform_device *pdev)
> +{
> + int ret = 0;
> + struct fsl_pwm_chip *fpc;
> + struct resource *res;
> +
> + fpc = devm_kzalloc(>dev, sizeof(*fpc), GFP_KERNEL);
You don't have to output a message when kzalloc fails, but still you
should check for the result and return an error if necessary.
With this fixed:
Reviewed-by: Sascha Hauer
Sascha
> +
> + ret = fsl_pwm_parse_clk_ps(fpc);
> + if (ret < 0)
> + return ret;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + fpc->base = devm_ioremap_resource(>dev, res);
> + if (IS_ERR(fpc->base)) {
> + ret = PTR_ERR(fpc->base);
> + return ret;
> + }
> +
> + fpc->pinctrl = devm_pinctrl_get(>dev);
> + if (IS_ERR(fpc->pinctrl)) {
> + ret = PTR_ERR(fpc->pinctrl);
> + return ret;
> + }
> +
> + fpc->chip.ops = _pwm_ops;
> + fpc->chip.of_xlate = of_pwm_xlate_with_flags;
> + fpc->chip.of_pwm_n_cells = 3;
> + fpc->chip.base = -1;
> + fpc->chip.npwm = FTM_MAX_CHANNEL;
> + ret = pwmchip_add(>chip);
> + if (ret < 0) {
> + dev_err(>dev, "failed to add PWM chip %d\n", ret);
> + return ret;
> + }
> +
> + platform_set_drvdata(pdev, fpc);
> +
> + return 0;
> +}
> +
> +static int fsl_pwm_remove(struct platform_device *pdev)
> +{
> + struct fsl_pwm_chip *fpc;
> +
> + fpc = platform_get_drvdata(pdev);
> +
> + return pwmchip_remove(>chip);
> +}
> +
> +static const struct of_device_id fsl_pwm_dt_ids[] = {
> + { .compatible = "fsl,vf610-ftm-pwm", },
> + { /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, fsl_pwm_dt_ids);
> +
> +static struct platform_driver fsl_pwm_driver = {
> + .driver = {
> + .name = "fsl-ftm-pwm",
> + .owner = THIS_MODULE,
> + .of_match_table = of_match_ptr(fsl_pwm_dt_ids),
> + },
> + .probe = fsl_pwm_probe,
> + .remove = fsl_pwm_remove,
> +};
> +module_platform_driver(fsl_pwm_driver);
> +
> +MODULE_DESCRIPTION("Freescale FTM PWM Driver");
> +MODULE_AUTHOR("Xiubo Li ");
> +MODULE_LICENSE("GPL");
> --
> 1.8.0
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
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- |
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCHv3 1/4] pwm: Add Freescale FTM PWM driver support
On Fri, Sep 06, 2013 at 04:08:24PM +0800, Xiubo Li wrote:
The FTM PWM device can be found on Vybrid VF610 Tower and Layerscape LS-1
SoCs.
+
+static int fsl_pwm_probe(struct platform_device *pdev)
+{
+ int ret = 0;
+ struct fsl_pwm_chip *fpc;
+ struct resource *res;
+
+ fpc = devm_kzalloc(pdev-dev, sizeof(*fpc), GFP_KERNEL);
You don't have to output a message when kzalloc fails, but still you
should check for the result and return an error if necessary.
With this fixed:
Reviewed-by: Sascha Hauer s.ha...@pengutronix.de
Sascha
+
+ ret = fsl_pwm_parse_clk_ps(fpc);
+ if (ret 0)
+ return ret;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ fpc-base = devm_ioremap_resource(pdev-dev, res);
+ if (IS_ERR(fpc-base)) {
+ ret = PTR_ERR(fpc-base);
+ return ret;
+ }
+
+ fpc-pinctrl = devm_pinctrl_get(pdev-dev);
+ if (IS_ERR(fpc-pinctrl)) {
+ ret = PTR_ERR(fpc-pinctrl);
+ return ret;
+ }
+
+ fpc-chip.ops = fsl_pwm_ops;
+ fpc-chip.of_xlate = of_pwm_xlate_with_flags;
+ fpc-chip.of_pwm_n_cells = 3;
+ fpc-chip.base = -1;
+ fpc-chip.npwm = FTM_MAX_CHANNEL;
+ ret = pwmchip_add(fpc-chip);
+ if (ret 0) {
+ dev_err(pdev-dev, failed to add PWM chip %d\n, ret);
+ return ret;
+ }
+
+ platform_set_drvdata(pdev, fpc);
+
+ return 0;
+}
+
+static int fsl_pwm_remove(struct platform_device *pdev)
+{
+ struct fsl_pwm_chip *fpc;
+
+ fpc = platform_get_drvdata(pdev);
+
+ return pwmchip_remove(fpc-chip);
+}
+
+static const struct of_device_id fsl_pwm_dt_ids[] = {
+ { .compatible = fsl,vf610-ftm-pwm, },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, fsl_pwm_dt_ids);
+
+static struct platform_driver fsl_pwm_driver = {
+ .driver = {
+ .name = fsl-ftm-pwm,
+ .owner = THIS_MODULE,
+ .of_match_table = of_match_ptr(fsl_pwm_dt_ids),
+ },
+ .probe = fsl_pwm_probe,
+ .remove = fsl_pwm_remove,
+};
+module_platform_driver(fsl_pwm_driver);
+
+MODULE_DESCRIPTION(Freescale FTM PWM Driver);
+MODULE_AUTHOR(Xiubo Li li.xi...@freescale.com);
+MODULE_LICENSE(GPL);
--
1.8.0
--
To unsubscribe from this list: send the line unsubscribe devicetree in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
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- |
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCHv3 1/4] pwm: Add Freescale FTM PWM driver support
Subject: Re: [PATCHv3 1/4] pwm: Add Freescale FTM PWM driver support
On Fri, Sep 06, 2013 at 04:08:24PM +0800, Xiubo Li wrote:
The FTM PWM device can be found on Vybrid VF610 Tower and Layerscape
LS-1 SoCs.
+
+static int fsl_pwm_probe(struct platform_device *pdev) {
+ int ret = 0;
+ struct fsl_pwm_chip *fpc;
+ struct resource *res;
+
+ fpc = devm_kzalloc(pdev-dev, sizeof(*fpc), GFP_KERNEL);
You don't have to output a message when kzalloc fails, but still you
should check for the result and return an error if necessary.
Okey, I will.
--
Xiubo
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCHv3 1/4] pwm: Add Freescale FTM PWM driver support
> Subject: Re: [PATCHv3 1/4] pwm: Add Freescale FTM PWM driver support > > On Mon, Sep 09, 2013 at 02:20:09PM +0200, Thierry Reding wrote: > > On Fri, Sep 06, 2013 at 04:08:24PM +0800, Xiubo Li wrote: > > > The FTM PWM device can be found on Vybrid VF610 Tower and Layerscape > LS-1 SoCs. > > > > > > Signed-off-by: Xiubo Li > > > Signed-off-by: Jingchang Lu > > > --- > > > drivers/pwm/Kconfig | 10 + > > > drivers/pwm/Makefile | 1 + > > > drivers/pwm/pwm-fsl-ftm.c | 505 > > > ++ > > > 3 files changed, 516 insertions(+) > > > create mode 100644 drivers/pwm/pwm-fsl-ftm.c > > > > This looks pretty good to me. I noticed that you didn't Cc Sascha who > > commented on this a lot. Can you please resend with him Cc'ed to make > > sure he sees this version of the series? > > I'm already aware of this, no need to resend. I'll have a look over it > tomorrow. > I'm very sorry, next time I will. Thanks very much. -- Best Regards, Xiubo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCHv3 1/4] pwm: Add Freescale FTM PWM driver support
On Mon, Sep 09, 2013 at 02:20:09PM +0200, Thierry Reding wrote: > On Fri, Sep 06, 2013 at 04:08:24PM +0800, Xiubo Li wrote: > > The FTM PWM device can be found on Vybrid VF610 Tower and Layerscape LS-1 > > SoCs. > > > > Signed-off-by: Xiubo Li > > Signed-off-by: Jingchang Lu > > --- > > drivers/pwm/Kconfig | 10 + > > drivers/pwm/Makefile | 1 + > > drivers/pwm/pwm-fsl-ftm.c | 505 > > ++ > > 3 files changed, 516 insertions(+) > > create mode 100644 drivers/pwm/pwm-fsl-ftm.c > > This looks pretty good to me. I noticed that you didn't Cc Sascha who > commented on this a lot. Can you please resend with him Cc'ed to make > sure he sees this version of the series? I'm already aware of this, no need to resend. I'll have a look over it tomorrow. 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- | -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCHv3 1/4] pwm: Add Freescale FTM PWM driver support
On Fri, Sep 06, 2013 at 04:08:24PM +0800, Xiubo Li wrote:
> The FTM PWM device can be found on Vybrid VF610 Tower and Layerscape LS-1
> SoCs.
>
> Signed-off-by: Xiubo Li
> Signed-off-by: Jingchang Lu
> ---
> drivers/pwm/Kconfig | 10 +
> drivers/pwm/Makefile | 1 +
> drivers/pwm/pwm-fsl-ftm.c | 505
> ++
> 3 files changed, 516 insertions(+)
> create mode 100644 drivers/pwm/pwm-fsl-ftm.c
This looks pretty good to me. I noticed that you didn't Cc Sascha who
commented on this a lot. Can you please resend with him Cc'ed to make
sure he sees this version of the series?
One thing jumped out at me, so I've commented on it below, but I will go
over the patch more carefully after you resend.
[...]
> diff --git a/drivers/pwm/pwm-fsl-ftm.c b/drivers/pwm/pwm-fsl-ftm.c
[...]
> +static int fsl_counter_clock_enable(struct fsl_pwm_chip *fpc)
> +{
[...]
> + switch (fpc->counter_clk_select) {
> + case FSL_COUNTER_CLK_SYS:
> + reg |= FTMSC_CLKSYS;
> + break;
> + case FSL_COUNTER_CLK_FIX:
> + reg |= FTMSC_CLKFIX;
> + break;
> + case FSL_COUNTER_CLK_EXT:
> + ret |= FTMSC_CLKEXT;
s/ret/reg/
Thierry
pgpYKHX8pb_Lw.pgp
Description: PGP signature
Re: [PATCHv3 1/4] pwm: Add Freescale FTM PWM driver support
On Fri, Sep 06, 2013 at 04:08:24PM +0800, Xiubo Li wrote:
The FTM PWM device can be found on Vybrid VF610 Tower and Layerscape LS-1
SoCs.
Signed-off-by: Xiubo Li li.xi...@freescale.com
Signed-off-by: Jingchang Lu b35...@freescale.com
---
drivers/pwm/Kconfig | 10 +
drivers/pwm/Makefile | 1 +
drivers/pwm/pwm-fsl-ftm.c | 505
++
3 files changed, 516 insertions(+)
create mode 100644 drivers/pwm/pwm-fsl-ftm.c
This looks pretty good to me. I noticed that you didn't Cc Sascha who
commented on this a lot. Can you please resend with him Cc'ed to make
sure he sees this version of the series?
One thing jumped out at me, so I've commented on it below, but I will go
over the patch more carefully after you resend.
[...]
diff --git a/drivers/pwm/pwm-fsl-ftm.c b/drivers/pwm/pwm-fsl-ftm.c
[...]
+static int fsl_counter_clock_enable(struct fsl_pwm_chip *fpc)
+{
[...]
+ switch (fpc-counter_clk_select) {
+ case FSL_COUNTER_CLK_SYS:
+ reg |= FTMSC_CLKSYS;
+ break;
+ case FSL_COUNTER_CLK_FIX:
+ reg |= FTMSC_CLKFIX;
+ break;
+ case FSL_COUNTER_CLK_EXT:
+ ret |= FTMSC_CLKEXT;
s/ret/reg/
Thierry
pgpYKHX8pb_Lw.pgp
Description: PGP signature
Re: [PATCHv3 1/4] pwm: Add Freescale FTM PWM driver support
On Mon, Sep 09, 2013 at 02:20:09PM +0200, Thierry Reding wrote: On Fri, Sep 06, 2013 at 04:08:24PM +0800, Xiubo Li wrote: The FTM PWM device can be found on Vybrid VF610 Tower and Layerscape LS-1 SoCs. Signed-off-by: Xiubo Li li.xi...@freescale.com Signed-off-by: Jingchang Lu b35...@freescale.com --- drivers/pwm/Kconfig | 10 + drivers/pwm/Makefile | 1 + drivers/pwm/pwm-fsl-ftm.c | 505 ++ 3 files changed, 516 insertions(+) create mode 100644 drivers/pwm/pwm-fsl-ftm.c This looks pretty good to me. I noticed that you didn't Cc Sascha who commented on this a lot. Can you please resend with him Cc'ed to make sure he sees this version of the series? I'm already aware of this, no need to resend. I'll have a look over it tomorrow. 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- | -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCHv3 1/4] pwm: Add Freescale FTM PWM driver support
Subject: Re: [PATCHv3 1/4] pwm: Add Freescale FTM PWM driver support On Mon, Sep 09, 2013 at 02:20:09PM +0200, Thierry Reding wrote: On Fri, Sep 06, 2013 at 04:08:24PM +0800, Xiubo Li wrote: The FTM PWM device can be found on Vybrid VF610 Tower and Layerscape LS-1 SoCs. Signed-off-by: Xiubo Li li.xi...@freescale.com Signed-off-by: Jingchang Lu b35...@freescale.com --- drivers/pwm/Kconfig | 10 + drivers/pwm/Makefile | 1 + drivers/pwm/pwm-fsl-ftm.c | 505 ++ 3 files changed, 516 insertions(+) create mode 100644 drivers/pwm/pwm-fsl-ftm.c This looks pretty good to me. I noticed that you didn't Cc Sascha who commented on this a lot. Can you please resend with him Cc'ed to make sure he sees this version of the series? I'm already aware of this, no need to resend. I'll have a look over it tomorrow. I'm very sorry, next time I will. Thanks very much. -- Best Regards, Xiubo -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
