Re: [PATCH RFC] clk-gpio-gate: use prepare/unprepare to do the GPIO work

[Jyri Sarha <jsarha@ti.com>]

On 08/11/15 10:54, Michael Allwright wrote:
> On 11 August 2015 at 09:24, Jyri Sarha <jsarha@ti.com> wrote:
>> On 08/10/15 17:50, Michael Allwright wrote:
>>>
>>>   From 19512587a6799c3cb96dc3f95d989b7c778a6f8c Mon Sep 17 00:00:00 2001
>>> From: Michael Allwright <allsey87@gmail.com>
>>> Date: Mon, 10 Aug 2015 16:44:05 +0200
>>> Subject: [PATCH] clk-gpio-gate: use prepare/unprepare ops to control clock
>>>    enabling via gpiod_set_value_cansleep. This allows for GPIO expanders on
>>> I2C
>>>    and SPI buses
>>>
>>
>> Hi,
>> I sounds a bit weird if a clock gets turned on already at the prepare state,
>> but I can see that this is the simples solution to your problem. Anyway, at
>> least this should not be the default behavior. If this is really widely
>> needed then adding a flag to device tree binding for changing the behavior
>> would sound more feasible, but then again I am no common clock framework
>> expert. Better to turn to CCF maintainers for that.
>>
>> BTW, if you propose some clk-gpio-gate changes to mainline, you should know
>> that the clk-gpio-gate.c was replaced with clk-gpio.c in linux-next. The new
>> file contains both gpio-gate and gpio-mux functionality. I think this is the
>> patch set was applied:
>> http://www.spinics.net/lists/linux-clk/msg01280.html
>>
>> Best regards,
>> Jyri
>
> Thanks for the reply Jyri, if you have a look over on slide #18 of
> http://elinux.org/images/b/b8/Elc2013_Clement.pdf - I think this use
> case has been considered and that actually doing the enable work in
> prepare could be an acceptable solution. How do/should we get in
> contact with the CCF maintainers?
>
> Cheers,
>
> Michael
>

Checking from MAINTAINERS file in linux git:
COMMON CLK FRAMEWORK
M:      Michael Turquette <mturquette@baylibre.com>
M:      Stephen Boyd <sboyd@codeaurora.org>
L:      linux-clk@vger.kernel.org
T:      git git://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git
S:      Maintained
F:      drivers/clk/
X:      drivers/clk/clkdev.c
F:      include/linux/clk-pr*
F:      include/linux/clk/

Cheers,
Jyri


>>> ---$ grep -A 3 "COMMON CLK FRAMEWORK" MAINTAINERS
COMMON CLK FRAMEWORK
M:      Michael Turquette <mturquette@baylibre.com>
M:      Stephen Boyd <sboyd@codeaurora.org>
L:      linux-clk@vger.kernel.org

>>>    drivers/clk/clk-gpio-gate.c | 31 ++++++++++++++++++++++++-------
>>>    1 file changed, 24 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/drivers/clk/clk-gpio-gate.c b/drivers/clk/clk-gpio-gate.c
>>> index a71cabe..432edd9 100644
>>> --- a/drivers/clk/clk-gpio-gate.c
>>> +++ b/drivers/clk/clk-gpio-gate.c
>>> @@ -22,8 +22,8 @@
>>>     * DOC: basic gpio gated clock which can be enabled and disabled
>>>     *      with gpio output
>>>     * Traits of this clock:
>>> - * prepare - clk_(un)prepare only ensures parent is (un)prepared
>>> - * enable - clk_enable and clk_disable are functional & control gpio
>>> + * prepare - clk_(un)prepare ensures parent is (un)prepared and control
>>> gpio
>>> + * enable - clk_enable and clk_disable only ensures parent is
>>> enabled/disabled
>>>     * rate - inherits rate from parent.  No clk_set_rate support
>>>     * parent - fixed parent.  No clk_set_parent support
>>>     */
>>> @@ -32,28 +32,45 @@
>>>
>>>    static int clk_gpio_gate_enable(struct clk_hw *hw)
>>>    {
>>> +    return 0;
>>> +}
>>> +
>>> +static void clk_gpio_gate_disable(struct clk_hw *hw)
>>> +{
>>> +}
>>> +
>>> +static int clk_gpio_gate_is_enabled(struct clk_hw *hw)
>>> +{
>>> +    return 0;
>>> +}
>>> +
>>> +static int clk_gpio_gate_prepare(struct clk_hw *hw)
>>> +{
>>>        struct clk_gpio *clk = to_clk_gpio(hw);
>>>
>>> -    gpiod_set_value(clk->gpiod, 1);
>>> +    gpiod_set_value_cansleep(clk->gpiod, 1);
>>>
>>>        return 0;
>>>    }
>>>
>>> -static void clk_gpio_gate_disable(struct clk_hw *hw)
>>> +static void clk_gpio_gate_unprepare(struct clk_hw *hw)
>>>    {
>>>        struct clk_gpio *clk = to_clk_gpio(hw);
>>>
>>> -    gpiod_set_value(clk->gpiod, 0);
>>> +    gpiod_set_value_cansleep(clk->gpiod, 0);
>>>    }
>>>
>>> -static int clk_gpio_gate_is_enabled(struct clk_hw *hw)
>>> +static int clk_gpio_gate_is_prepared(struct clk_hw *hw)
>>>    {
>>>        struct clk_gpio *clk = to_clk_gpio(hw);
>>>
>>> -    return gpiod_get_value(clk->gpiod);
>>> +    return gpiod_get_value_cansleep(clk->gpiod);
>>>    }
>>>
>>>    const struct clk_ops clk_gpio_gate_ops = {
>>> +    .prepare = clk_gpio_gate_prepare,
>>> +    .unprepare = clk_gpio_gate_unprepare,
>>> +    .is_prepared = clk_gpio_gate_is_prepared,
>>>        .enable = clk_gpio_gate_enable,
>>>        .disable = clk_gpio_gate_disable,
>>>        .is_enabled = clk_gpio_gate_is_enabled,
>>>
>>