From: "Benoît Thébaudeau" <benoit.thebaudeau@advansee.com>
To: Sascha Hauer <s.hauer@pengutronix.de>
Cc: HACHIMI Samir <shachimi@adeneo-embedded.com>,
shawn guo <shawn.guo@linaro.org>,
thierry reding <thierry.reding@avionic-design.de>,
linux-kernel@vger.kernel.org, kernel@pengutronix.de,
Philipp Zabel <p.zabel@pengutronix.de>,
linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 7/9] pwm i.MX: fix clock lookup
Date: Fri, 7 Sep 2012 11:57:18 +0200 (CEST) [thread overview]
Message-ID: <1500638147.3845571.1347011838934.JavaMail.root@advansee.com> (raw)
In-Reply-To: <20120907074138.GN26594@pengutronix.de>
On Friday, September 7, 2012 9:41:38 AM, Sascha Hauer wrote:
> On Thu, Sep 06, 2012 at 09:09:42PM +0200, Benoît Thébaudeau wrote:
> > On Thursday, September 6, 2012 8:42:56 PM, Sascha Hauer wrote:
> > > On Thu, Sep 06, 2012 at 08:31:58PM +0200, Benoît Thébaudeau
> > > wrote:
> > > > On Thursday, September 6, 2012 2:48:13 PM, Sascha Hauer wrote:
> > > > >
> > > > > + int ret;
> > > > > +
> > > > > + ret = clk_prepare_enable(imx->clk_ipg);
> > > > > + if (ret)
> > > > > + return ret;
> > > > >
> > > > > - return imx->config(chip, pwm, duty_ns, period_ns);
> > > > > + ret = imx->config(chip, pwm, duty_ns, period_ns);
> > > > > +
> > > > > + clk_disable_unprepare(imx->clk_ipg);
> > > > > +
> > > > > + return ret;
> > > > > }
> > > > >
> > > > > static int imx_pwm_enable(struct pwm_chip *chip, struct
> > > > > pwm_device
> > > > > *pwm)
> > > > > @@ -169,7 +179,7 @@ static int imx_pwm_enable(struct pwm_chip
> > > > > *chip,
> > > > > struct pwm_device *pwm)
> > > > > struct imx_chip *imx = to_imx_chip(chip);
> > > > > int ret;
> > > > >
> > > > > - ret = clk_prepare_enable(imx->clk);
> > > > > + ret = clk_prepare_enable(imx->clk_per);
> > > > > if (ret)
> > > > > return ret;
> > > >
> > > > Have you tested that this actually works on i.MX53?
> > > >
> > > > I have tested it successfully on i.MX35 (with a few additions
> > > > to
> > > > platform code).
> > > > But i.MX35 has a single bit controlling both PWM IPG and PER
> > > > clock
> > > > gates.
> > > >
> > > > On i.MX53, there are 2 separate control bits for these. So, if
> > > > ipg
> > > > clk is
> > > > strictly required to access PWM registers, even if per clk is
> > > > enabled, this code
> > > > should not work without adding
> > >
> > > I tested this on i.MX53, but you're right, this seems to be
> > > wrong.
> > > I'll
> > > recheck tomorrow.
> >
> > I've performed a few more tests with bare register accesses in the
> > bootloader to
> > see how the PWM IP behaves.
> >
> > On i.MX25, i.MX35 and i.MX51, read accesses always work, whatever
> > the state of
> > the IPG and PER clock gates.
> >
> > On i.MX25 and i.MX51, write accesses always work, whatever the
> > state of the IPG
> > and PER clock gates.
> >
> > On i.MX35, write accesses work if and only if the IPG and PER
> > clocks are not
> > gated off (single control bit for both).
>
> Ok, I tested on i.MX53 and i.MX27.
>
> On i.MX53 the registers are always accessible. I also measured with
> an
> oscilloscope that the ipg/per clock gates en/disable the PWM when
> it's
> configured for the corresponding clock.
>
> On i.MX27 register accesses also work regardless of the clock gates.
> Here we have a single clock gate which only gates the ipg clock.
>
> btw the i.MX6 has a single gate per pwm which are described as "PWMx
> clocks"
>
> So it seems while not 100% correct the current code seems to be safe.
Agreed.
Best regards,
Benoît
next prev parent reply other threads:[~2012-09-07 9:52 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-09-06 12:48 [PATCH v3] pwm i.MX: add devicetree support Sascha Hauer
2012-09-06 12:48 ` [PATCH 1/9] pwm i.MX: factor out SoC specific functions Sascha Hauer
2012-09-07 13:04 ` Thierry Reding
2012-09-06 12:48 ` [PATCH 2/9] pwm i.MX: remove unnecessary if in pwm_[en|dis]able Sascha Hauer
2012-09-06 12:48 ` [PATCH 3/9] pwm i.MX: add functions to enable/disable pwm Sascha Hauer
2012-09-06 12:48 ` [PATCH 4/9] pwm i.MX: Use module_platform_driver Sascha Hauer
2012-09-06 12:48 ` [PATCH 5/9] pwm i.MX: add devicetree support Sascha Hauer
2012-09-06 12:48 ` [PATCH 6/9] pwm i.MX: use per clock unconditionally Sascha Hauer
2012-09-06 12:48 ` [PATCH 7/9] pwm i.MX: fix clock lookup Sascha Hauer
2012-09-06 18:31 ` Benoît Thébaudeau
2012-09-06 18:42 ` Sascha Hauer
2012-09-06 19:09 ` Benoît Thébaudeau
2012-09-07 7:41 ` Sascha Hauer
2012-09-07 9:57 ` Benoît Thébaudeau [this message]
2012-09-06 12:48 ` [PATCH 8/9] ARM i.MX53: Add pwm support Sascha Hauer
2012-09-06 12:48 ` [PATCH 9/9] ARM i.MX: remove PWM platform support Sascha Hauer
2012-09-07 13:10 ` [PATCH v3] pwm i.MX: add devicetree support Thierry Reding
2012-09-07 17:14 ` Sascha Hauer
-- strict thread matches above, loose matches on Subject: below --
2012-08-28 11:48 i.MX pwm patches Sascha Hauer
2012-08-28 11:48 ` [PATCH 7/9] pwm i.MX: fix clock lookup Sascha Hauer
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1500638147.3845571.1347011838934.JavaMail.root@advansee.com \
--to=benoit.thebaudeau@advansee.com \
--cc=kernel@pengutronix.de \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=p.zabel@pengutronix.de \
--cc=s.hauer@pengutronix.de \
--cc=shachimi@adeneo-embedded.com \
--cc=shawn.guo@linaro.org \
--cc=thierry.reding@avionic-design.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).