| Message ID | 1456439796-28546-2-git-send-email-fcooper@ti.com |
|---|---|
| State | Superseded |
| Headers | show |
+ Thierry, PWM subsystem maintainer. On Friday 26 February 2016 04:06 AM, Franklin S Cooper Jr wrote: > The PWMSS local clock gating registers have no real purpose on OMAP ARM > devices. These registers were left over registers from DSP IP where the > PRCM doesn't exist. There is a silicon bug where gating and ungating clocks > don't function properly. TRMs will be update to indicate that these > registers shouldn't be touched. > > Therefore, all code that accesses the PWMSS_CLKCONFIG or PWMSS_CLKSTATUS > will be removed by this patch with zero loss of functionality by the ECAP > and EPWM drivers. > > Signed-off-by: Franklin S Cooper Jr <fcooper@ti.com> > --- > Version 3 changes: > New patch > > drivers/pwm/pwm-tiecap.c | 28 -------------------------- > drivers/pwm/pwm-tiehrpwm.c | 29 --------------------------- > drivers/pwm/pwm-tipwmss.c | 49 ---------------------------------------------- > drivers/pwm/pwm-tipwmss.h | 39 ------------------------------------ > 4 files changed, 145 deletions(-) > delete mode 100644 drivers/pwm/pwm-tipwmss.h > > diff --git a/drivers/pwm/pwm-tiecap.c b/drivers/pwm/pwm-tiecap.c > index 616af76..9e0865d7 100644 > --- a/drivers/pwm/pwm-tiecap.c > +++ b/drivers/pwm/pwm-tiecap.c > @@ -27,8 +27,6 @@ > #include <linux/pwm.h> > #include <linux/of_device.h> > > -#include "pwm-tipwmss.h" > - > /* ECAP registers and bits definitions */ > #define CAP1 0x08 > #define CAP2 0x0C > @@ -206,7 +204,6 @@ static int ecap_pwm_probe(struct platform_device *pdev) > struct resource *r; > struct clk *clk; > struct ecap_pwm_chip *pc; > - u16 status; > > pc = devm_kzalloc(&pdev->dev, sizeof(*pc), GFP_KERNEL); > if (!pc) > @@ -243,40 +240,15 @@ static int ecap_pwm_probe(struct platform_device *pdev) > } > > pm_runtime_enable(&pdev->dev); > - pm_runtime_get_sync(&pdev->dev); > - > - status = pwmss_submodule_state_change(pdev->dev.parent, > - PWMSS_ECAPCLK_EN); > - if (!(status & PWMSS_ECAPCLK_EN_ACK)) { > - dev_err(&pdev->dev, "PWMSS config space clock enable failed\n"); > - ret = -EINVAL; > - goto pwmss_clk_failure; > - } > - > - pm_runtime_put_sync(&pdev->dev); > > platform_set_drvdata(pdev, pc); > return 0; > - > -pwmss_clk_failure: > - pm_runtime_put_sync(&pdev->dev); > - pm_runtime_disable(&pdev->dev); > - pwmchip_remove(&pc->chip); > - return ret; > } > > static int ecap_pwm_remove(struct platform_device *pdev) > { > struct ecap_pwm_chip *pc = platform_get_drvdata(pdev); > > - pm_runtime_get_sync(&pdev->dev); > - /* > - * Due to hardware misbehaviour, acknowledge of the stop_req > - * is missing. Hence checking of the status bit skipped. > - */ > - pwmss_submodule_state_change(pdev->dev.parent, PWMSS_ECAPCLK_STOP_REQ); > - pm_runtime_put_sync(&pdev->dev); > - > pm_runtime_disable(&pdev->dev); > return pwmchip_remove(&pc->chip); > } > diff --git a/drivers/pwm/pwm-tiehrpwm.c b/drivers/pwm/pwm-tiehrpwm.c > index 6a41e66..e09b1f0 100644 > --- a/drivers/pwm/pwm-tiehrpwm.c > +++ b/drivers/pwm/pwm-tiehrpwm.c > @@ -27,8 +27,6 @@ > #include <linux/pm_runtime.h> > #include <linux/of_device.h> > > -#include "pwm-tipwmss.h" > - > /* EHRPWM registers and bits definitions */ > > /* Time base module registers */ > @@ -437,7 +435,6 @@ static int ehrpwm_pwm_probe(struct platform_device *pdev) > struct resource *r; > struct clk *clk; > struct ehrpwm_pwm_chip *pc; > - u16 status; > > pc = devm_kzalloc(&pdev->dev, sizeof(*pc), GFP_KERNEL); > if (!pc) > @@ -487,27 +484,9 @@ static int ehrpwm_pwm_probe(struct platform_device *pdev) > } > > pm_runtime_enable(&pdev->dev); > - pm_runtime_get_sync(&pdev->dev); > - > - status = pwmss_submodule_state_change(pdev->dev.parent, > - PWMSS_EPWMCLK_EN); > - if (!(status & PWMSS_EPWMCLK_EN_ACK)) { > - dev_err(&pdev->dev, "PWMSS config space clock enable failed\n"); > - ret = -EINVAL; > - goto pwmss_clk_failure; > - } > - > - pm_runtime_put_sync(&pdev->dev); > > platform_set_drvdata(pdev, pc); > return 0; > - > -pwmss_clk_failure: > - pm_runtime_put_sync(&pdev->dev); > - pm_runtime_disable(&pdev->dev); > - pwmchip_remove(&pc->chip); > - clk_unprepare(pc->tbclk); > - return ret; > } > > static int ehrpwm_pwm_remove(struct platform_device *pdev) > @@ -516,14 +495,6 @@ static int ehrpwm_pwm_remove(struct platform_device *pdev) > > clk_unprepare(pc->tbclk); > > - pm_runtime_get_sync(&pdev->dev); > - /* > - * Due to hardware misbehaviour, acknowledge of the stop_req > - * is missing. Hence checking of the status bit skipped. > - */ > - pwmss_submodule_state_change(pdev->dev.parent, PWMSS_EPWMCLK_STOP_REQ); > - pm_runtime_put_sync(&pdev->dev); > - > pm_runtime_put_sync(&pdev->dev); > pm_runtime_disable(&pdev->dev); > return pwmchip_remove(&pc->chip); > diff --git a/drivers/pwm/pwm-tipwmss.c b/drivers/pwm/pwm-tipwmss.c > index 5cf65a1..829f499 100644 > --- a/drivers/pwm/pwm-tipwmss.c > +++ b/drivers/pwm/pwm-tipwmss.c > @@ -22,32 +22,6 @@ > #include <linux/pm_runtime.h> > #include <linux/of_device.h> > > -#include "pwm-tipwmss.h" > - > -#define PWMSS_CLKCONFIG 0x8 /* Clock gating reg */ > -#define PWMSS_CLKSTATUS 0xc /* Clock gating status reg */ > - > -struct pwmss_info { > - void __iomem *mmio_base; > - struct mutex pwmss_lock; > - u16 pwmss_clkconfig; > -}; > - > -u16 pwmss_submodule_state_change(struct device *dev, int set) > -{ > - struct pwmss_info *info = dev_get_drvdata(dev); > - u16 val; > - > - mutex_lock(&info->pwmss_lock); > - val = readw(info->mmio_base + PWMSS_CLKCONFIG); > - val |= set; > - writew(val , info->mmio_base + PWMSS_CLKCONFIG); > - mutex_unlock(&info->pwmss_lock); > - > - return readw(info->mmio_base + PWMSS_CLKSTATUS); > -} > -EXPORT_SYMBOL(pwmss_submodule_state_change); > - > static const struct of_device_id pwmss_of_match[] = { > { .compatible = "ti,am33xx-pwmss" }, > {}, > @@ -57,24 +31,10 @@ MODULE_DEVICE_TABLE(of, pwmss_of_match); > static int pwmss_probe(struct platform_device *pdev) > { > int ret; > - struct resource *r; > - struct pwmss_info *info; > struct device_node *node = pdev->dev.of_node; > > - info = devm_kzalloc(&pdev->dev, sizeof(*info), GFP_KERNEL); > - if (!info) > - return -ENOMEM; > - > - mutex_init(&info->pwmss_lock); > - > - r = platform_get_resource(pdev, IORESOURCE_MEM, 0); > - info->mmio_base = devm_ioremap_resource(&pdev->dev, r); > - if (IS_ERR(info->mmio_base)) > - return PTR_ERR(info->mmio_base); > - > pm_runtime_enable(&pdev->dev); > pm_runtime_get_sync(&pdev->dev); > - platform_set_drvdata(pdev, info); > > /* Populate all the child nodes here... */ > ret = of_platform_populate(node, NULL, NULL, &pdev->dev); > @@ -86,30 +46,21 @@ static int pwmss_probe(struct platform_device *pdev) > > static int pwmss_remove(struct platform_device *pdev) > { > - struct pwmss_info *info = platform_get_drvdata(pdev); > - > pm_runtime_put_sync(&pdev->dev); > pm_runtime_disable(&pdev->dev); > - mutex_destroy(&info->pwmss_lock); > return 0; > } > > #ifdef CONFIG_PM_SLEEP > static int pwmss_suspend(struct device *dev) > { > - struct pwmss_info *info = dev_get_drvdata(dev); > - > - info->pwmss_clkconfig = readw(info->mmio_base + PWMSS_CLKCONFIG); > pm_runtime_put_sync(dev); > return 0; > } > > static int pwmss_resume(struct device *dev) > { > - struct pwmss_info *info = dev_get_drvdata(dev); > - > pm_runtime_get_sync(dev); > - writew(info->pwmss_clkconfig, info->mmio_base + PWMSS_CLKCONFIG); > return 0; > } > #endif > diff --git a/drivers/pwm/pwm-tipwmss.h b/drivers/pwm/pwm-tipwmss.h > deleted file mode 100644 > index 10ad804..0000000 > --- a/drivers/pwm/pwm-tipwmss.h > +++ /dev/null > @@ -1,39 +0,0 @@ > -/* > - * TI PWM Subsystem driver > - * > - * Copyright (C) 2012 Texas Instruments Incorporated - http://www.ti.com/ > - * > - * This program is free software; you can redistribute it and/or modify > - * it under the terms of the GNU General Public License as published by > - * the Free Software Foundation; either version 2 of the License, or > - * (at your option) any later version. > - * > - * This program is distributed in the hope that it will be useful, > - * but WITHOUT ANY WARRANTY; without even the implied warranty of > - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > - * GNU General Public License for more details. > - * > - */ > - > -#ifndef __TIPWMSS_H > -#define __TIPWMSS_H > - > -/* PWM substem clock gating */ > -#define PWMSS_ECAPCLK_EN BIT(0) > -#define PWMSS_ECAPCLK_STOP_REQ BIT(1) > -#define PWMSS_EPWMCLK_EN BIT(8) > -#define PWMSS_EPWMCLK_STOP_REQ BIT(9) > - > -#define PWMSS_ECAPCLK_EN_ACK BIT(0) > -#define PWMSS_EPWMCLK_EN_ACK BIT(8) > - > -#ifdef CONFIG_PWM_TIPWMSS > -extern u16 pwmss_submodule_state_change(struct device *dev, int set); > -#else > -static inline u16 pwmss_submodule_state_change(struct device *dev, int set) > -{ > - /* return success status value */ > - return 0xFFFF; > -} > -#endif > -#endif /* __TIPWMSS_H */ > -- To unsubscribe from this list: send the line "unsubscribe linux-pwm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
* Sekhar Nori <nsekhar@ti.com> [160226 02:27]:
> + Thierry, PWM subsystem maintainer.
The driver changes need to be resent to Thierry please.
Regards,
Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-pwm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
* Franklin S Cooper Jr <fcooper@ti.com> [160225 14:37]: > The PWMSS local clock gating registers have no real purpose on OMAP ARM > devices. These registers were left over registers from DSP IP where the > PRCM doesn't exist. There is a silicon bug where gating and ungating clocks > don't function properly. TRMs will be update to indicate that these > registers shouldn't be touched. > > Therefore, all code that accesses the PWMSS_CLKCONFIG or PWMSS_CLKSTATUS > will be removed by this patch with zero loss of functionality by the ECAP > and EPWM drivers. > @@ -243,40 +240,15 @@ static int ecap_pwm_probe(struct platform_device *pdev) > } > > pm_runtime_enable(&pdev->dev); > - pm_runtime_get_sync(&pdev->dev); > - > - status = pwmss_submodule_state_change(pdev->dev.parent, > - PWMSS_ECAPCLK_EN); > - if (!(status & PWMSS_ECAPCLK_EN_ACK)) { > - dev_err(&pdev->dev, "PWMSS config space clock enable failed\n"); > - ret = -EINVAL; > - goto pwmss_clk_failure; > - } > - > - pm_runtime_put_sync(&pdev->dev); > > platform_set_drvdata(pdev, pc); > return 0; > - > -pwmss_clk_failure: > - pm_runtime_put_sync(&pdev->dev); > - pm_runtime_disable(&pdev->dev); > - pwmchip_remove(&pc->chip); > - return ret; > } Hmm but why are you also removing the pm_runtime calls? Those actually do take care of gating the clocks via the interconnect level code that is hwmod in this case. Regards, Tony -- To unsubscribe from this list: send the line "unsubscribe linux-pwm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Tony, On 02/29/2016 04:04 PM, Tony Lindgren wrote: > * Franklin S Cooper Jr <fcooper@ti.com> [160225 14:37]: >> The PWMSS local clock gating registers have no real purpose on OMAP ARM >> devices. These registers were left over registers from DSP IP where the >> PRCM doesn't exist. There is a silicon bug where gating and ungating clocks >> don't function properly. TRMs will be update to indicate that these >> registers shouldn't be touched. >> >> Therefore, all code that accesses the PWMSS_CLKCONFIG or PWMSS_CLKSTATUS >> will be removed by this patch with zero loss of functionality by the ECAP >> and EPWM drivers. >> @@ -243,40 +240,15 @@ static int ecap_pwm_probe(struct platform_device *pdev) >> } >> >> pm_runtime_enable(&pdev->dev); >> - pm_runtime_get_sync(&pdev->dev); >> - >> - status = pwmss_submodule_state_change(pdev->dev.parent, >> - PWMSS_ECAPCLK_EN); >> - if (!(status & PWMSS_ECAPCLK_EN_ACK)) { >> - dev_err(&pdev->dev, "PWMSS config space clock enable failed\n"); >> - ret = -EINVAL; >> - goto pwmss_clk_failure; >> - } >> - >> - pm_runtime_put_sync(&pdev->dev); >> >> platform_set_drvdata(pdev, pc); >> return 0; >> - >> -pwmss_clk_failure: >> - pm_runtime_put_sync(&pdev->dev); >> - pm_runtime_disable(&pdev->dev); >> - pwmchip_remove(&pc->chip); >> - return ret; >> } > Hmm but why are you also removing the pm_runtime calls? Those > actually do take care of gating the clocks via the interconnect > level code that is hwmod in this case. I removed all PM runtime calls that revolved around pwmss_submodule_state_change. Originally the driver would do a pm_runtime_get_sync then call pwmss_submodule_state_change and then immediately call pm_runtime_put_sync. Without pwmss_submodule_state_change those calls would be meaningless. I also removed pm_runtime calls in error paths that no longer existed. Within ecap and epwm driver pm_runtime_get is done when pwm_enable is called and pm_runtime_sync calls are done when pwm_disable is called. Similar pm_runtime_get and pm_runtime_sync calls are done in functions that ended up touching register's within the IP. > > Regards, > > Tony -- To unsubscribe from this list: send the line "unsubscribe linux-pwm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
* Franklin S Cooper Jr. <fcooper@ti.com> [160229 14:31]: > On 02/29/2016 04:04 PM, Tony Lindgren wrote: > > Hmm but why are you also removing the pm_runtime calls? Those > > actually do take care of gating the clocks via the interconnect > > level code that is hwmod in this case. > I removed all PM runtime calls that revolved around > pwmss_submodule_state_change. Originally the driver would do > a pm_runtime_get_sync then call pwmss_submodule_state_change > and then immediately call pm_runtime_put_sync. Without > pwmss_submodule_state_change those calls would be > meaningless. I also removed pm_runtime calls in error paths > that no longer existed. Typically the interconnect level code can gate the clkctrl bit for the module with PM runtime even with no other driver specific registers. If you remove the pm_runtime calls, that does not happen. Also, how do you know this change does not affect the other SoC variants using the same driver? Regards, Tony -- To unsubscribe from this list: send the line "unsubscribe linux-pwm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 02/29/2016 04:55 PM, Tony Lindgren wrote: > * Franklin S Cooper Jr. <fcooper@ti.com> [160229 14:31]: >> On 02/29/2016 04:04 PM, Tony Lindgren wrote: >>> Hmm but why are you also removing the pm_runtime calls? Those >>> actually do take care of gating the clocks via the interconnect >>> level code that is hwmod in this case. >> I removed all PM runtime calls that revolved around >> pwmss_submodule_state_change. Originally the driver would do >> a pm_runtime_get_sync then call pwmss_submodule_state_change >> and then immediately call pm_runtime_put_sync. Without >> pwmss_submodule_state_change those calls would be >> meaningless. I also removed pm_runtime calls in error paths >> that no longer existed. > Typically the interconnect level code can gate the clkctrl bit > for the module with PM runtime even with no other driver specific > registers. If you remove the pm_runtime calls, that does not > happen. So the clocks should be unlocked when ever the IP registers are being read/written or if the peripheral is being used for example the pwm signal is being generated. All these cases are already being handled. Using ecap driver as an example. Pm_runtime_get_sync is called within ecap_pwm_enable when the pwm signal is to be generated. Pm_runtime_put_sync is called when the pwm signal is to be stopped. When either the pwm signal polarity is set or pwm configuration is made then a pm_runtime_get_sync and pm_runtime_put_sync are called within the same function surrounding calls to the IP's registers. Probe is calling pm_runtime_enable while remove is calling pm_runtime_disable. So the correct pm_runtime calls are being made from what I can see. I'm not sure I understand the concern since removing those calls aren't creating any kind of imbalance. If I'm not addressing your concern please give me an example of where you see a possible issue. > Also, how do you know this change does not affect the other > SoC variants using the same driver? I've tested these changes on AM335x GP and AM437x GP evms. AM335x and AM437x were the only other users of this driver. Sorry I should of documented this in my cover-letter. > Regards, > > Tony -- To unsubscribe from this list: send the line "unsubscribe linux-pwm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
* Franklin S Cooper Jr. <fcooper@ti.com> [160229 15:12]: > > > On 02/29/2016 04:55 PM, Tony Lindgren wrote: > > * Franklin S Cooper Jr. <fcooper@ti.com> [160229 14:31]: > >> On 02/29/2016 04:04 PM, Tony Lindgren wrote: > >>> Hmm but why are you also removing the pm_runtime calls? Those > >>> actually do take care of gating the clocks via the interconnect > >>> level code that is hwmod in this case. > >> I removed all PM runtime calls that revolved around > >> pwmss_submodule_state_change. Originally the driver would do > >> a pm_runtime_get_sync then call pwmss_submodule_state_change > >> and then immediately call pm_runtime_put_sync. Without > >> pwmss_submodule_state_change those calls would be > >> meaningless. I also removed pm_runtime calls in error paths > >> that no longer existed. > > Typically the interconnect level code can gate the clkctrl bit > > for the module with PM runtime even with no other driver specific > > registers. If you remove the pm_runtime calls, that does not > > happen. > > So the clocks should be unlocked when ever the IP registers are > being read/written or if the peripheral is being used for > example > the pwm signal is being generated. All these cases are already > being handled. > > Using ecap driver as an example. > > Pm_runtime_get_sync is called within ecap_pwm_enable when > the pwm signal is to be generated. Pm_runtime_put_sync is called > when the pwm signal is to be stopped. > > When either the pwm signal polarity is set or pwm > configuration is made > then a pm_runtime_get_sync and pm_runtime_put_sync are > called within > the same function surrounding calls to the IP's registers. > > Probe is calling pm_runtime_enable while remove is calling > pm_runtime_disable. OK good to hear you have considered this. The above answers my questions then thanks. > So the correct pm_runtime calls are being made from what I > can see. > I'm not sure I understand the concern since removing those > calls aren't > creating any kind of imbalance. OK thanks for checking. > If I'm not addressing your concern please give me an example > of where > you see a possible issue. No that's fine. I thought you're ripping out all of the the pm_runtime based on just looking at the patch :) > > Also, how do you know this change does not affect the other > > SoC variants using the same driver? > > I've tested these changes on AM335x GP and AM437x GP evms. > AM335x > and AM437x were the only other users of this driver. Sorry > I should of > documented this in my cover-letter. OK good to hear. Thanks, Tony -- To unsubscribe from this list: send the line "unsubscribe linux-pwm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 02/29/2016 05:20 PM, Tony Lindgren wrote: > * Franklin S Cooper Jr. <fcooper@ti.com> [160229 15:12]: >> >> On 02/29/2016 04:55 PM, Tony Lindgren wrote: >>> * Franklin S Cooper Jr. <fcooper@ti.com> [160229 14:31]: >>>> On 02/29/2016 04:04 PM, Tony Lindgren wrote: >>>>> Hmm but why are you also removing the pm_runtime calls? Those >>>>> actually do take care of gating the clocks via the interconnect >>>>> level code that is hwmod in this case. >>>> I removed all PM runtime calls that revolved around >>>> pwmss_submodule_state_change. Originally the driver would do >>>> a pm_runtime_get_sync then call pwmss_submodule_state_change >>>> and then immediately call pm_runtime_put_sync. Without >>>> pwmss_submodule_state_change those calls would be >>>> meaningless. I also removed pm_runtime calls in error paths >>>> that no longer existed. >>> Typically the interconnect level code can gate the clkctrl bit >>> for the module with PM runtime even with no other driver specific >>> registers. If you remove the pm_runtime calls, that does not >>> happen. >> So the clocks should be unlocked when ever the IP registers are >> being read/written or if the peripheral is being used for >> example >> the pwm signal is being generated. All these cases are already >> being handled. >> >> Using ecap driver as an example. >> >> Pm_runtime_get_sync is called within ecap_pwm_enable when >> the pwm signal is to be generated. Pm_runtime_put_sync is called >> when the pwm signal is to be stopped. >> >> When either the pwm signal polarity is set or pwm >> configuration is made >> then a pm_runtime_get_sync and pm_runtime_put_sync are >> called within >> the same function surrounding calls to the IP's registers. >> >> Probe is calling pm_runtime_enable while remove is calling >> pm_runtime_disable. > OK good to hear you have considered this. The above answers > my questions then thanks. > >> So the correct pm_runtime calls are being made from what I >> can see. >> I'm not sure I understand the concern since removing those >> calls aren't >> creating any kind of imbalance. > OK thanks for checking. > >> If I'm not addressing your concern please give me an example >> of where >> you see a possible issue. > No that's fine. I thought you're ripping out all of the the > pm_runtime based on just looking at the patch :) > >>> Also, how do you know this change does not affect the other >>> SoC variants using the same driver? >> I've tested these changes on AM335x GP and AM437x GP evms. >> AM335x >> and AM437x were the only other users of this driver. Sorry >> I should of >> documented this in my cover-letter. > OK good to hear. > > Thanks, > > Tony I know there are some comments regarding other patches in this patchset but this patch is unrelated and can be pulled in separately. Any objections to this or should I just resubmit this patch by itself? -- To unsubscribe from this list: send the line "unsubscribe linux-pwm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
* Franklin S Cooper Jr. <fcooper@ti.com> [160302 11:41]: > I know there are some comments regarding other patches in > this patchset but this patch is unrelated and can be pulled > in separately. Any objections to this or should I just > resubmit this patch by itself? No objections from me at least. If something needs to change at the driver level, it sounds like a different set of patches then. Regards, Tony -- To unsubscribe from this list: send the line "unsubscribe linux-pwm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/pwm/pwm-tiecap.c b/drivers/pwm/pwm-tiecap.c index 616af76..9e0865d7 100644 --- a/drivers/pwm/pwm-tiecap.c +++ b/drivers/pwm/pwm-tiecap.c @@ -27,8 +27,6 @@ #include <linux/pwm.h> #include <linux/of_device.h> -#include "pwm-tipwmss.h" - /* ECAP registers and bits definitions */ #define CAP1 0x08 #define CAP2 0x0C @@ -206,7 +204,6 @@ static int ecap_pwm_probe(struct platform_device *pdev) struct resource *r; struct clk *clk; struct ecap_pwm_chip *pc; - u16 status; pc = devm_kzalloc(&pdev->dev, sizeof(*pc), GFP_KERNEL); if (!pc) @@ -243,40 +240,15 @@ static int ecap_pwm_probe(struct platform_device *pdev) } pm_runtime_enable(&pdev->dev); - pm_runtime_get_sync(&pdev->dev); - - status = pwmss_submodule_state_change(pdev->dev.parent, - PWMSS_ECAPCLK_EN); - if (!(status & PWMSS_ECAPCLK_EN_ACK)) { - dev_err(&pdev->dev, "PWMSS config space clock enable failed\n"); - ret = -EINVAL; - goto pwmss_clk_failure; - } - - pm_runtime_put_sync(&pdev->dev); platform_set_drvdata(pdev, pc); return 0; - -pwmss_clk_failure: - pm_runtime_put_sync(&pdev->dev); - pm_runtime_disable(&pdev->dev); - pwmchip_remove(&pc->chip); - return ret; } static int ecap_pwm_remove(struct platform_device *pdev) { struct ecap_pwm_chip *pc = platform_get_drvdata(pdev); - pm_runtime_get_sync(&pdev->dev); - /* - * Due to hardware misbehaviour, acknowledge of the stop_req - * is missing. Hence checking of the status bit skipped. - */ - pwmss_submodule_state_change(pdev->dev.parent, PWMSS_ECAPCLK_STOP_REQ); - pm_runtime_put_sync(&pdev->dev); - pm_runtime_disable(&pdev->dev); return pwmchip_remove(&pc->chip); } diff --git a/drivers/pwm/pwm-tiehrpwm.c b/drivers/pwm/pwm-tiehrpwm.c index 6a41e66..e09b1f0 100644 --- a/drivers/pwm/pwm-tiehrpwm.c +++ b/drivers/pwm/pwm-tiehrpwm.c @@ -27,8 +27,6 @@ #include <linux/pm_runtime.h> #include <linux/of_device.h> -#include "pwm-tipwmss.h" - /* EHRPWM registers and bits definitions */ /* Time base module registers */ @@ -437,7 +435,6 @@ static int ehrpwm_pwm_probe(struct platform_device *pdev) struct resource *r; struct clk *clk; struct ehrpwm_pwm_chip *pc; - u16 status; pc = devm_kzalloc(&pdev->dev, sizeof(*pc), GFP_KERNEL); if (!pc) @@ -487,27 +484,9 @@ static int ehrpwm_pwm_probe(struct platform_device *pdev) } pm_runtime_enable(&pdev->dev); - pm_runtime_get_sync(&pdev->dev); - - status = pwmss_submodule_state_change(pdev->dev.parent, - PWMSS_EPWMCLK_EN); - if (!(status & PWMSS_EPWMCLK_EN_ACK)) { - dev_err(&pdev->dev, "PWMSS config space clock enable failed\n"); - ret = -EINVAL; - goto pwmss_clk_failure; - } - - pm_runtime_put_sync(&pdev->dev); platform_set_drvdata(pdev, pc); return 0; - -pwmss_clk_failure: - pm_runtime_put_sync(&pdev->dev); - pm_runtime_disable(&pdev->dev); - pwmchip_remove(&pc->chip); - clk_unprepare(pc->tbclk); - return ret; } static int ehrpwm_pwm_remove(struct platform_device *pdev) @@ -516,14 +495,6 @@ static int ehrpwm_pwm_remove(struct platform_device *pdev) clk_unprepare(pc->tbclk); - pm_runtime_get_sync(&pdev->dev); - /* - * Due to hardware misbehaviour, acknowledge of the stop_req - * is missing. Hence checking of the status bit skipped. - */ - pwmss_submodule_state_change(pdev->dev.parent, PWMSS_EPWMCLK_STOP_REQ); - pm_runtime_put_sync(&pdev->dev); - pm_runtime_put_sync(&pdev->dev); pm_runtime_disable(&pdev->dev); return pwmchip_remove(&pc->chip); diff --git a/drivers/pwm/pwm-tipwmss.c b/drivers/pwm/pwm-tipwmss.c index 5cf65a1..829f499 100644 --- a/drivers/pwm/pwm-tipwmss.c +++ b/drivers/pwm/pwm-tipwmss.c @@ -22,32 +22,6 @@ #include <linux/pm_runtime.h> #include <linux/of_device.h> -#include "pwm-tipwmss.h" - -#define PWMSS_CLKCONFIG 0x8 /* Clock gating reg */ -#define PWMSS_CLKSTATUS 0xc /* Clock gating status reg */ - -struct pwmss_info { - void __iomem *mmio_base; - struct mutex pwmss_lock; - u16 pwmss_clkconfig; -}; - -u16 pwmss_submodule_state_change(struct device *dev, int set) -{ - struct pwmss_info *info = dev_get_drvdata(dev); - u16 val; - - mutex_lock(&info->pwmss_lock); - val = readw(info->mmio_base + PWMSS_CLKCONFIG); - val |= set; - writew(val , info->mmio_base + PWMSS_CLKCONFIG); - mutex_unlock(&info->pwmss_lock); - - return readw(info->mmio_base + PWMSS_CLKSTATUS); -} -EXPORT_SYMBOL(pwmss_submodule_state_change); - static const struct of_device_id pwmss_of_match[] = { { .compatible = "ti,am33xx-pwmss" }, {}, @@ -57,24 +31,10 @@ MODULE_DEVICE_TABLE(of, pwmss_of_match); static int pwmss_probe(struct platform_device *pdev) { int ret; - struct resource *r; - struct pwmss_info *info; struct device_node *node = pdev->dev.of_node; - info = devm_kzalloc(&pdev->dev, sizeof(*info), GFP_KERNEL); - if (!info) - return -ENOMEM; - - mutex_init(&info->pwmss_lock); - - r = platform_get_resource(pdev, IORESOURCE_MEM, 0); - info->mmio_base = devm_ioremap_resource(&pdev->dev, r); - if (IS_ERR(info->mmio_base)) - return PTR_ERR(info->mmio_base); - pm_runtime_enable(&pdev->dev); pm_runtime_get_sync(&pdev->dev); - platform_set_drvdata(pdev, info); /* Populate all the child nodes here... */ ret = of_platform_populate(node, NULL, NULL, &pdev->dev); @@ -86,30 +46,21 @@ static int pwmss_probe(struct platform_device *pdev) static int pwmss_remove(struct platform_device *pdev) { - struct pwmss_info *info = platform_get_drvdata(pdev); - pm_runtime_put_sync(&pdev->dev); pm_runtime_disable(&pdev->dev); - mutex_destroy(&info->pwmss_lock); return 0; } #ifdef CONFIG_PM_SLEEP static int pwmss_suspend(struct device *dev) { - struct pwmss_info *info = dev_get_drvdata(dev); - - info->pwmss_clkconfig = readw(info->mmio_base + PWMSS_CLKCONFIG); pm_runtime_put_sync(dev); return 0; } static int pwmss_resume(struct device *dev) { - struct pwmss_info *info = dev_get_drvdata(dev); - pm_runtime_get_sync(dev); - writew(info->pwmss_clkconfig, info->mmio_base + PWMSS_CLKCONFIG); return 0; } #endif diff --git a/drivers/pwm/pwm-tipwmss.h b/drivers/pwm/pwm-tipwmss.h deleted file mode 100644 index 10ad804..0000000 --- a/drivers/pwm/pwm-tipwmss.h +++ /dev/null @@ -1,39 +0,0 @@ -/* - * TI PWM Subsystem driver - * - * Copyright (C) 2012 Texas Instruments Incorporated - http://www.ti.com/ - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation; either version 2 of the License, or - * (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - */ - -#ifndef __TIPWMSS_H -#define __TIPWMSS_H - -/* PWM substem clock gating */ -#define PWMSS_ECAPCLK_EN BIT(0) -#define PWMSS_ECAPCLK_STOP_REQ BIT(1) -#define PWMSS_EPWMCLK_EN BIT(8) -#define PWMSS_EPWMCLK_STOP_REQ BIT(9) - -#define PWMSS_ECAPCLK_EN_ACK BIT(0) -#define PWMSS_EPWMCLK_EN_ACK BIT(8) - -#ifdef CONFIG_PWM_TIPWMSS -extern u16 pwmss_submodule_state_change(struct device *dev, int set); -#else -static inline u16 pwmss_submodule_state_change(struct device *dev, int set) -{ - /* return success status value */ - return 0xFFFF; -} -#endif -#endif /* __TIPWMSS_H */
The PWMSS local clock gating registers have no real purpose on OMAP ARM devices. These registers were left over registers from DSP IP where the PRCM doesn't exist. There is a silicon bug where gating and ungating clocks don't function properly. TRMs will be update to indicate that these registers shouldn't be touched. Therefore, all code that accesses the PWMSS_CLKCONFIG or PWMSS_CLKSTATUS will be removed by this patch with zero loss of functionality by the ECAP and EPWM drivers. Signed-off-by: Franklin S Cooper Jr <fcooper@ti.com> --- Version 3 changes: New patch drivers/pwm/pwm-tiecap.c | 28 -------------------------- drivers/pwm/pwm-tiehrpwm.c | 29 --------------------------- drivers/pwm/pwm-tipwmss.c | 49 ---------------------------------------------- drivers/pwm/pwm-tipwmss.h | 39 ------------------------------------ 4 files changed, 145 deletions(-) delete mode 100644 drivers/pwm/pwm-tipwmss.h