Hi Stefan, > Hi Stefan, > > > On 2016-12-26 23:55, Lukasz Majewski wrote: > > > From: Sascha Hauer > > > > > > The use of the ipg clock was introduced with commit 7b27c160c681 > > > ("pwm: i.MX: fix clock lookup"). > > > In the commit message it was claimed that the ipg clock is enabled > > > for register accesses. This is true for the ->config() callback, > > > but not for the ->set_enable() callback. Given that the ipg clock > > > is not consistently enabled for all register accesses we can > > > assume that either it is not required at all or that the current > > > code does not work. Remove the ipg clock code for now so that > > > it's no longer in the way of refactoring the driver. > > > > Hi Lukasz, > > > > Has my concern addressed in any way with this resend? > > https://lkml.org/lkml/2016/11/22/729 > > Unfortunately not, since I don't have iMX7 for testing. > > > > > Breaking hardware is usually not an option :-) > > Yes, I know, but > > Please look on the patch set from my perspective: > > I originally wanted to add polarity inversion to PWM. Then, there was > the request from you and Boris to go with "atomicity" support, so I > converted the driver to support it. > > This patch set has been resent on purpose at the end of merge window, > so we do have some time to fix it if it would be accepted to -next > tree (or any other PWM related one). Moreover, the burden for > preparing patches would be smaller - since we all have agreed that > "atomicity" is a more than welcome feature. > > > > > > I checked the i.MX 7 reference manual again, and in this case the > > peripheral access clock is a clock line named "ipg_clk_s" (Table > > 12-20), with a clock root "PWM1_CLK_ROOT" (Table 5-12). In i.MX 7 > > all clocks are behind a single gate, so in fact it does not matter > > which clock we take. Given that others have peripheral access > > behind the "pwm" gate, I guess we should take the "pwm" gate... > > > If possible please prepare a patch. It would be the best solution. If I might ask - are you willing to prepare patch to fix iMX7 or shall I roll back to the ipg code already present in main line ? Best regards, Łukasz Majewski > > Thanks in advance, > Łukasz Majewski > > > > > -- > > Stefan > > > > > > > > Signed-off-by: Sascha Hauer > > > Cc: Philipp Zabel > > > --- > > > [commit message text refactored by Lukasz Majewski > > > ] --- > > > Changes for v3: > > > - New patch > > > --- > > > drivers/pwm/pwm-imx.c | 19 +------------------ > > > 1 file changed, 1 insertion(+), 18 deletions(-) > > > > > > diff --git a/drivers/pwm/pwm-imx.c b/drivers/pwm/pwm-imx.c > > > index d600fd5..70609ef2 100644 > > > --- a/drivers/pwm/pwm-imx.c > > > +++ b/drivers/pwm/pwm-imx.c > > > @@ -49,7 +49,6 @@ > > > > > > struct imx_chip { > > > struct clk *clk_per; > > > - struct clk *clk_ipg; > > > > > > void __iomem *mmio_base; > > > > > > @@ -204,17 +203,8 @@ static int imx_pwm_config(struct pwm_chip > > > *chip, struct pwm_device *pwm, int duty_ns, int period_ns) > > > { > > > struct imx_chip *imx = to_imx_chip(chip); > > > - int ret; > > > - > > > - ret = clk_prepare_enable(imx->clk_ipg); > > > - if (ret) > > > - return ret; > > > > > > - ret = imx->config(chip, pwm, duty_ns, period_ns); > > > - > > > - clk_disable_unprepare(imx->clk_ipg); > > > - > > > - return ret; > > > + return imx->config(chip, pwm, duty_ns, period_ns); > > > } > > > > > > static int imx_pwm_enable(struct pwm_chip *chip, struct > > > pwm_device *pwm) @@ -293,13 +283,6 @@ static int > > > imx_pwm_probe(struct platform_device *pdev) return > > > PTR_ERR(imx->clk_per); } > > > > > > - imx->clk_ipg = devm_clk_get(&pdev->dev, "ipg"); > > > - if (IS_ERR(imx->clk_ipg)) { > > > - dev_err(&pdev->dev, "getting ipg clock failed > > > with %ld\n", > > > - PTR_ERR(imx->clk_ipg)); > > > - return PTR_ERR(imx->clk_ipg); > > > - } > > > - > > > imx->chip.ops = &imx_pwm_ops; > > > imx->chip.dev = &pdev->dev; > > > imx->chip.base = -1; >