From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
To: Lei YU <mine260309@gmail.com>, Guenter Roeck <linux@roeck-us.net>
Cc: Jean Delvare <jdelvare@suse.com>, Joel Stanley <joel@jms.id.au>,
Andrew Jeffery <andrew@aj.id.au>,
Hardware Monitoring <linux-hwmon@vger.kernel.org>,
linux-arm-kernel@lists.infradead.org,
linux-aspeed@lists.ozlabs.org,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] hwmon: (aspeed-pwm-tacho) Use 24MHz clock
Date: Wed, 09 May 2018 14:03:04 +1000 [thread overview]
Message-ID: <3117a5230fe7849e66d2d354f04ec3f24fccca0f.camel@kernel.crashing.org> (raw)
In-Reply-To: <CAARXrt=ZA7sz_CvNUxHRvie3YfErU-5d5ewHry=mgxb6Uz-+TA@mail.gmail.com>
On Wed, 2018-05-09 at 11:50 +0800, Lei YU wrote:
> On Wed, May 9, 2018 at 11:43 AM, Guenter Roeck <linux@roeck-us.net> wrote:
> >
> > I am not going to accept this change, period. This is not one, it is five
> > steps
> > backward. If "aspeed_set_clock_source(priv->regmap, 0)" changes the clock
> > speed,
> > or the clock source, read it later, and attach to the correct clock. If that
> > doesn't work, fix the problem in the clock subsystem. Hacking the driver is
> > just
> > plain wrong.
> >
> > Also, if the idea in DT is to provide a different clock to the watchdog on
> > purpose,
> > maybe the call to "aspeed_set_clock_source(priv->regmap, 0)" is wrong.
>
> Exactly!
> My first change (not sent to mailing list) is to remove the call of
> "aspeed_set_clock_source(priv->regmap, 0)", instead, checking the clock
> source, and use either 24M or the memory controller clock.
> But that make things a bit more complicated and Aspeed suggests to use 24M
> clock. So I sent this simplified change.
>
> It's OK to not accept this change, the fix in DT will fix the issue as well.
Another option is to give *both* clock sources in the DT, since in HW
they both eventually reach the IP block, and have the driver select
which one it wants to use (that itself can also be in the DT).
That way you can select 0 if you want and still use clk_get_rate() to
know how fast it's ticking.
Cheers,
Ben.
prev parent reply other threads:[~2018-05-09 4:04 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-05-08 9:39 [PATCH] hwmon: (aspeed-pwm-tacho) Use 24MHz clock Lei YU
2018-05-08 13:51 ` Guenter Roeck
2018-05-09 3:18 ` Lei YU
2018-05-09 3:43 ` Guenter Roeck
2018-05-09 3:50 ` Lei YU
2018-05-09 4:03 ` Benjamin Herrenschmidt [this message]
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=3117a5230fe7849e66d2d354f04ec3f24fccca0f.camel@kernel.crashing.org \
--to=benh@kernel.crashing.org \
--cc=andrew@aj.id.au \
--cc=jdelvare@suse.com \
--cc=joel@jms.id.au \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-aspeed@lists.ozlabs.org \
--cc=linux-hwmon@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux@roeck-us.net \
--cc=mine260309@gmail.com \
/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