| Message ID | 20220712100113.569042-6-ben.dooks@sifive.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [1/7] pwm: change &pci->dev to dev in probe | expand |
On Tue, Jul 12, 2022 at 11:01:11AM +0100, Ben Dooks wrote: > Add a configurable clock base rate for the pwm as when > being built for non-PCI the block may be sourced from > an internal clock. > > Signed-off-by: Ben Dooks <ben.dooks@sifive.com> > --- > drivers/pwm/pwm-dwc.c | 17 +++++++++++++++++ > 1 file changed, 17 insertions(+) > > diff --git a/drivers/pwm/pwm-dwc.c b/drivers/pwm/pwm-dwc.c > index 235cb730c888..aa0486b89bdd 100644 > --- a/drivers/pwm/pwm-dwc.c > +++ b/drivers/pwm/pwm-dwc.c > @@ -18,6 +18,7 @@ > #include <linux/kernel.h> > #include <linux/module.h> > #include <linux/pci.h> > +#include <linux/clk.h> > #include <linux/platform_device.h> > #include <linux/pm_runtime.h> > #include <linux/pwm.h> > @@ -35,7 +36,12 @@ > #define DWC_TIMERS_COMP_VERSION 0xac > > #define DWC_TIMERS_TOTAL 8 > + > +#ifndef CONFIG_OF > #define DWC_CLK_PERIOD_NS 10 > +#else > +#define DWC_CLK_PERIOD_NS dwc->clk_ns > +#endif Hmm, that looks wrong. If you have CONFIG_OF but use the pci device ... IMHO it would help readability if you used ifdef. When there is an #else branch anyhow, there is no reason to use the negative variant. > /* Timer Control Register */ > #define DWC_TIM_CTRL_EN BIT(0) > @@ -54,6 +60,8 @@ struct dwc_pwm_ctx { > struct dwc_pwm { > struct pwm_chip chip; > void __iomem *base; > + struct clk *clk; > + unsigned int clk_ns; > struct dwc_pwm_ctx ctx[DWC_TIMERS_TOTAL]; > }; > #define to_dwc_pwm(p) (container_of((p), struct dwc_pwm, chip)) > @@ -336,6 +344,14 @@ static int dwc_pwm_plat_probe(struct platform_device *pdev) > return dev_err_probe(dev, PTR_ERR(dwc->base), > "failed to map IO\n"); > > + dwc->clk = devm_clk_get(dev, "timer"); > + if (IS_ERR(dwc->clk)) > + return dev_err_probe(dev, PTR_ERR(dwc->clk), > + "failed to get timer clock\n"); > + > + clk_prepare_enable(dwc->clk); > + dwc->clk_ns = 1000000000 /clk_get_rate(dwc->clk); > + > ret = pwmchip_add(&dwc->chip); > if (ret) > return ret; Here you're missing clk_disable_unprepare(). (Alternatively use devm_clk_get_enabled().) > @@ -347,6 +363,7 @@ static int dwc_pwm_plat_remove(struct platform_device *pdev) > { > struct dwc_pwm *dwc = platform_get_drvdata(pdev); > > + clk_disable_unprepare(dwc->clk); > pwmchip_remove(&dwc->chip); The order is wrong here. You must only stop the clk when the pwmchip is removed. Until that the PWM is supposed to stay functional. > return 0; > } Best regards Uwe
diff --git a/drivers/pwm/pwm-dwc.c b/drivers/pwm/pwm-dwc.c index 235cb730c888..aa0486b89bdd 100644 --- a/drivers/pwm/pwm-dwc.c +++ b/drivers/pwm/pwm-dwc.c @@ -18,6 +18,7 @@ #include <linux/kernel.h> #include <linux/module.h> #include <linux/pci.h> +#include <linux/clk.h> #include <linux/platform_device.h> #include <linux/pm_runtime.h> #include <linux/pwm.h> @@ -35,7 +36,12 @@ #define DWC_TIMERS_COMP_VERSION 0xac #define DWC_TIMERS_TOTAL 8 + +#ifndef CONFIG_OF #define DWC_CLK_PERIOD_NS 10 +#else +#define DWC_CLK_PERIOD_NS dwc->clk_ns +#endif /* Timer Control Register */ #define DWC_TIM_CTRL_EN BIT(0) @@ -54,6 +60,8 @@ struct dwc_pwm_ctx { struct dwc_pwm { struct pwm_chip chip; void __iomem *base; + struct clk *clk; + unsigned int clk_ns; struct dwc_pwm_ctx ctx[DWC_TIMERS_TOTAL]; }; #define to_dwc_pwm(p) (container_of((p), struct dwc_pwm, chip)) @@ -336,6 +344,14 @@ static int dwc_pwm_plat_probe(struct platform_device *pdev) return dev_err_probe(dev, PTR_ERR(dwc->base), "failed to map IO\n"); + dwc->clk = devm_clk_get(dev, "timer"); + if (IS_ERR(dwc->clk)) + return dev_err_probe(dev, PTR_ERR(dwc->clk), + "failed to get timer clock\n"); + + clk_prepare_enable(dwc->clk); + dwc->clk_ns = 1000000000 /clk_get_rate(dwc->clk); + ret = pwmchip_add(&dwc->chip); if (ret) return ret; @@ -347,6 +363,7 @@ static int dwc_pwm_plat_remove(struct platform_device *pdev) { struct dwc_pwm *dwc = platform_get_drvdata(pdev); + clk_disable_unprepare(dwc->clk); pwmchip_remove(&dwc->chip); return 0; }
Add a configurable clock base rate for the pwm as when being built for non-PCI the block may be sourced from an internal clock. Signed-off-by: Ben Dooks <ben.dooks@sifive.com> --- drivers/pwm/pwm-dwc.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)