From: javier.martinez@collabora•co.uk (Javier Martinez Canillas)
To: linux-arm-kernel@lists•infradead.org
Subject: [PATCH v5 05/14] clk: Add generic driver for Maxim PMIC clocks
Date: Wed, 02 Jul 2014 12:13:29 +0200 [thread overview]
Message-ID: <53B3DB49.6020604@collabora.co.uk> (raw)
In-Reply-To: <20140701172616.32686.44374@quantum>
Hello Mike,
On 07/01/2014 07:26 PM, Mike Turquette wrote:
> Quoting Yadwinder Singh Brar (2014-06-29 21:01:36)
>> Hi Javier,
>>
>> On Thu, Jun 26, 2014 at 11:45 PM, Javier Martinez Canillas
>> <javier.martinez@collabora•co.uk> wrote:
>> > Maxim Integrated Power Management ICs are very similar with
>> > regard to their clock outputs. Most of the clock drivers for
>> > these chips are duplicating code and are simpler enough that
>> > can be converted to use a generic driver to consolidate code
>> > and avoid duplication.
>> >
>> > Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora•co.uk>
>> > Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung•com>
>> > ---
>> >
>> > Changes since v4:
>> > - Return recalc 0 if clock isn't enabled in Suggested by Yadwinder Singh Brar.
>> >
>>
>> It seems you didn't implement or posted same patch again :) .
>>
>> > Changes since v3:
>> > - Add current copyright information. Suggested by Krzysztof Kozlowski
>> > - Do a single allocation for struct max_gen_clk. Suggested by Krzysztof Kozlowski
>> > - Add EXPORT_SYMBOL() for exported symbols. Suggested by Krzysztof Kozlowski
>> >
>> > drivers/clk/Kconfig | 3 +
>> > drivers/clk/Makefile | 1 +
>> > drivers/clk/clk-max-gen.c | 195 ++++++++++++++++++++++++++++++++++++++++++++++
>> > drivers/clk/clk-max-gen.h | 32 ++++++++
>> > 4 files changed, 231 insertions(+)
>> > create mode 100644 drivers/clk/clk-max-gen.c
>> > create mode 100644 drivers/clk/clk-max-gen.h
>> >
>>
>> [ .. ]
>>
>> > +
>> > +static unsigned long max_gen_recalc_rate(struct clk_hw *hw,
>> > + unsigned long parent_rate)
>> > +{
>> > + return 32768;
>> > +}
>>
>> Its still same here.
>
> Changing this would be a new behavior. I do not know of any other clock
> drivers that conditionally returns a rate of 0 based on whether or not
> the clock is gated.
>
After Yadwinder feedback I searched for clock drivers that returned 0 when the
clock was not enabled/prepared and found for example drivers/clk/clk-s2mps11.c:
static unsigned long s2mps11_clk_recalc_rate(struct clk_hw *hw,
unsigned long parent_rate)
{
struct s2mps11_clk *s2mps11 = to_s2mps11_clk(hw);
if (s2mps11->enabled)
return 32768;
else
return 0;
}
> It is also buggy since calls to clk_enable and clk_disable do not invoke
> .recalc_rate, so the rate of your clock would not be updated from the
> framework's perspective until some later point where you call
> clk_set_rate or something.
>
s2ps11->enabled is set in the driver's clk_ops .prepare and .unprepare function
handlers and calls to clk_prepare and clk_unprepare also don't seems to invoke
.recalc_rate so I guess that driver is wrong as well and should just return the
clock rate unconditionally?
> If your driver needs to know whether or not the clock is enabled then we
> could introduce a new bool clk_is_enabled(struct clk *clk); to clk.h,
> but I'd rather not do that. Instead if a driver needs a clock then it
> calls clk_enable on it without any knowledge about the internal state of
> the clock enable_count.
>
> Regards,
> Mike
>
Thanks a lot for the explanation, I'll revert that change then and return the
clock rate unconditionally on the next version of the patch-set.
Best regards,
Javier
next prev parent reply other threads:[~2014-07-02 10:13 UTC|newest]
Thread overview: 42+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-06-26 18:15 [PATCH v5 00/14] Add Maxim 77802 PMIC support Javier Martinez Canillas
2014-06-26 18:15 ` [PATCH v5 01/14] mfd: max77686: Convert to use regmap_irq Javier Martinez Canillas
2014-06-26 18:15 ` [PATCH v5 02/14] mfd: max77686: Allow the max77686 rtc to wakeup the system Javier Martinez Canillas
2014-06-27 9:21 ` Lee Jones
2014-06-27 9:32 ` Javier Martinez Canillas
2014-06-26 18:15 ` [PATCH v5 03/14] clk: max77686: Add DT include for MAX77686 PMIC clock Javier Martinez Canillas
2014-06-27 7:48 ` Andreas Färber
2014-06-27 7:53 ` Javier Martinez Canillas
2014-06-27 8:26 ` Andreas Färber
2014-06-27 8:54 ` Javier Martinez Canillas
2014-07-01 17:30 ` Mike Turquette
2014-06-26 18:15 ` [PATCH v5 04/14] clk: max77686: Improve Maxim 77686 PMIC clocks binding Javier Martinez Canillas
2014-07-01 17:29 ` Mike Turquette
2014-07-02 10:17 ` Javier Martinez Canillas
2014-07-02 15:21 ` Mike Turquette
2014-06-26 18:15 ` [PATCH v5 05/14] clk: Add generic driver for Maxim PMIC clocks Javier Martinez Canillas
2014-06-30 4:01 ` Yadwinder Singh Brar
2014-06-30 10:58 ` Javier Martinez Canillas
2014-06-30 11:35 ` Russell King - ARM Linux
2014-06-30 16:00 ` Javier Martinez Canillas
2014-07-01 17:26 ` Mike Turquette
2014-07-02 10:13 ` Javier Martinez Canillas [this message]
2014-07-02 10:19 ` Krzysztof Kozlowski
2014-06-26 18:15 ` [PATCH v5 06/14] clk: max77686: Convert to the generic max clock driver Javier Martinez Canillas
2014-06-26 18:15 ` [PATCH v5 07/14] mfd: Add driver for Maxim 77802 Power Management IC Javier Martinez Canillas
2014-07-01 15:15 ` Lee Jones
2014-07-01 15:55 ` Javier Martinez Canillas
2014-06-26 18:15 ` [PATCH v5 08/14] mfd: max77802: Add DT binding documentation Javier Martinez Canillas
2014-06-27 8:06 ` Andreas Färber
2014-06-27 8:50 ` Javier Martinez Canillas
2014-06-26 18:15 ` [PATCH v5 09/14] regmap: Add regmap_reg_copy function Javier Martinez Canillas
2014-06-26 18:15 ` [PATCH v5 10/14] regulator: Add driver for Maxim 77802 PMIC regulators Javier Martinez Canillas
2014-06-27 9:26 ` Lee Jones
2014-06-27 9:42 ` Javier Martinez Canillas
2014-06-26 18:15 ` [PATCH v5 11/14] clk: Add driver for Maxim 77802 PMIC clocks Javier Martinez Canillas
2014-06-26 18:15 ` [PATCH v5 12/14] clk: max77802: Add DT binding documentation Javier Martinez Canillas
2014-06-27 7:52 ` Andreas Färber
2014-06-27 7:55 ` Javier Martinez Canillas
2014-06-26 18:15 ` [PATCH v5 13/14] rtc: Add driver for Maxim 77802 PMIC Real-Time-Clock Javier Martinez Canillas
2014-06-26 18:15 ` [PATCH v5 14/14] ARM: dts: Add max77802 to exynos5420-peach-pit and exynos5800-peach-pi Javier Martinez Canillas
2014-07-02 9:20 ` Tushar Behera
2014-07-02 9:22 ` Javier Martinez Canillas
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=53B3DB49.6020604@collabora.co.uk \
--to=javier.martinez@collabora$(echo .)co.uk \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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