LinuxLists.cc - [PATCH] mfd: twl6040: Optional clk32k clock handling

2014-04-03 10:54:53

Subject: [PATCH] mfd: twl6040: Optional clk32k clock handling

In certain boards the source for the clk32k clock can be gated. In these
boards the clk32k clock can be provided to the driver and it is going to be
enabled/disabled when it is needed.
If the clk32k clock is not provided the driver will assume that it is always
running.

Signed-off-by: Peter Ujfalusi <[email protected]>
---
Documentation/devicetree/bindings/mfd/twl6040.txt | 2 ++
drivers/mfd/twl6040.c | 10 ++++++++++
include/linux/mfd/twl6040.h | 2 ++
3 files changed, 14 insertions(+)

diff --git a/Documentation/devicetree/bindings/mfd/twl6040.txt b/Documentation/devicetree/bindings/mfd/twl6040.txt
index 0f5dd709d752..a41157b5d930 100644
--- a/Documentation/devicetree/bindings/mfd/twl6040.txt
+++ b/Documentation/devicetree/bindings/mfd/twl6040.txt
@@ -19,6 +19,8 @@ Required properties:

Optional properties, nodes:
- enable-active-high: To power on the twl6040 during boot.
+- clocks: phandle to the clk32k clock provider
+- clock-names: Must be "clk32k"

Vibra functionality
Required properties:
diff --git a/drivers/mfd/twl6040.c b/drivers/mfd/twl6040.c
index 2e6504a8e1e3..12b314ea48dc 100644
--- a/drivers/mfd/twl6040.c
+++ b/drivers/mfd/twl6040.c
@@ -291,6 +291,8 @@ int twl6040_power(struct twl6040 *twl6040, int on)
if (twl6040->power_count++)
goto out;

+ clk_prepare_enable(twl6040->clk32k);
+
/* Allow writes to the chip */
regcache_cache_only(twl6040->regmap, false);

@@ -346,6 +348,8 @@ int twl6040_power(struct twl6040 *twl6040, int on)

twl6040->sysclk = 0;
twl6040->mclk = 0;
+
+ clk_disable_unprepare(twl6040->clk32k);
}

out:
@@ -644,6 +648,12 @@ static int twl6040_probe(struct i2c_client *client,

i2c_set_clientdata(client, twl6040);

+ twl6040->clk32k = devm_clk_get(&client->dev, "clk32k");
+ if (IS_ERR(twl6040->clk32k)) {
+ dev_info(&client->dev, "clk32k is not handled\n");
+ twl6040->clk32k = NULL;
+ }
+
twl6040->supplies[0].supply = "vio";
twl6040->supplies[1].supply = "v2v1";
ret = devm_regulator_bulk_get(&client->dev, TWL6040_NUM_SUPPLIES,
diff --git a/include/linux/mfd/twl6040.h b/include/linux/mfd/twl6040.h
index a69d16b30c18..8f9fc3d26e6d 100644
--- a/include/linux/mfd/twl6040.h
+++ b/include/linux/mfd/twl6040.h
@@ -28,6 +28,7 @@
#include <linux/interrupt.h>
#include <linux/mfd/core.h>
#include <linux/regulator/consumer.h>
+#include <linux/clk.h>

#define TWL6040_REG_ASICID 0x01
#define TWL6040_REG_ASICREV 0x02
@@ -223,6 +224,7 @@ struct twl6040 {
struct regmap *regmap;
struct regmap_irq_chip_data *irq_data;
struct regulator_bulk_data supplies[2]; /* supplies for vio, v2v1 */
+ struct clk *clk32k;
struct mutex mutex;
struct mutex irq_mutex;
struct mfd_cell cells[TWL6040_CELLS];
--
1.9.1

2014-04-03 10:55:14

by Peter Ujfalusi

[permalink] [raw]

Subject: [PATCH] mfd: twl6040: Optional clk32k clock handling

2014-04-24 09:15:40

by Peter Ujfalusi

[permalink] [raw]

Subject: Re: [PATCH] mfd: twl6040: Optional clk32k clock handling

Lee,

On 04/03/2014 01:54 PM, Peter Ujfalusi wrote:
> In certain boards the source for the clk32k clock can be gated. In these
> boards the clk32k clock can be provided to the driver and it is going to be
> enabled/disabled when it is needed.
> If the clk32k clock is not provided the driver will assume that it is always
> running.

Do you want me to resend this patch, it seams to be missing from next still.

Thanks,
P?ter

>
> Signed-off-by: Peter Ujfalusi <[email protected]>
> ---
> Documentation/devicetree/bindings/mfd/twl6040.txt | 2 ++
> drivers/mfd/twl6040.c | 10 ++++++++++
> include/linux/mfd/twl6040.h | 2 ++
> 3 files changed, 14 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/mfd/twl6040.txt b/Documentation/devicetree/bindings/mfd/twl6040.txt
> index 0f5dd709d752..a41157b5d930 100644
> --- a/Documentation/devicetree/bindings/mfd/twl6040.txt
> +++ b/Documentation/devicetree/bindings/mfd/twl6040.txt
> @@ -19,6 +19,8 @@ Required properties:
>
> Optional properties, nodes:
> - enable-active-high: To power on the twl6040 during boot.
> +- clocks: phandle to the clk32k clock provider
> +- clock-names: Must be "clk32k"
>
> Vibra functionality
> Required properties:
> diff --git a/drivers/mfd/twl6040.c b/drivers/mfd/twl6040.c
> index 2e6504a8e1e3..12b314ea48dc 100644
> --- a/drivers/mfd/twl6040.c
> +++ b/drivers/mfd/twl6040.c
> @@ -291,6 +291,8 @@ int twl6040_power(struct twl6040 *twl6040, int on)
> if (twl6040->power_count++)
> goto out;
>
> + clk_prepare_enable(twl6040->clk32k);
> +
> /* Allow writes to the chip */
> regcache_cache_only(twl6040->regmap, false);
>
> @@ -346,6 +348,8 @@ int twl6040_power(struct twl6040 *twl6040, int on)
>
> twl6040->sysclk = 0;
> twl6040->mclk = 0;
> +
> + clk_disable_unprepare(twl6040->clk32k);
> }
>
> out:
> @@ -644,6 +648,12 @@ static int twl6040_probe(struct i2c_client *client,
>
> i2c_set_clientdata(client, twl6040);
>
> + twl6040->clk32k = devm_clk_get(&client->dev, "clk32k");
> + if (IS_ERR(twl6040->clk32k)) {
> + dev_info(&client->dev, "clk32k is not handled\n");
> + twl6040->clk32k = NULL;
> + }
> +
> twl6040->supplies[0].supply = "vio";
> twl6040->supplies[1].supply = "v2v1";
> ret = devm_regulator_bulk_get(&client->dev, TWL6040_NUM_SUPPLIES,
> diff --git a/include/linux/mfd/twl6040.h b/include/linux/mfd/twl6040.h
> index a69d16b30c18..8f9fc3d26e6d 100644
> --- a/include/linux/mfd/twl6040.h
> +++ b/include/linux/mfd/twl6040.h
> @@ -28,6 +28,7 @@
> #include <linux/interrupt.h>
> #include <linux/mfd/core.h>
> #include <linux/regulator/consumer.h>
> +#include <linux/clk.h>
>
> #define TWL6040_REG_ASICID 0x01
> #define TWL6040_REG_ASICREV 0x02
> @@ -223,6 +224,7 @@ struct twl6040 {
> struct regmap *regmap;
> struct regmap_irq_chip_data *irq_data;
> struct regulator_bulk_data supplies[2]; /* supplies for vio, v2v1 */
> + struct clk *clk32k;
> struct mutex mutex;
> struct mutex irq_mutex;
> struct mfd_cell cells[TWL6040_CELLS];
>

2014-04-28 10:36:39

by Lee Jones

[permalink] [raw]

Subject: Re: [PATCH] mfd: twl6040: Optional clk32k clock handling

> In certain boards the source for the clk32k clock can be gated. In these
> boards the clk32k clock can be provided to the driver and it is going to be
> enabled/disabled when it is needed.
> If the clk32k clock is not provided the driver will assume that it is always
> running.
>
> Signed-off-by: Peter Ujfalusi <[email protected]>
> ---
> Documentation/devicetree/bindings/mfd/twl6040.txt | 2 ++
> drivers/mfd/twl6040.c | 10 ++++++++++
> include/linux/mfd/twl6040.h | 2 ++
> 3 files changed, 14 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/mfd/twl6040.txt b/Documentation/devicetree/bindings/mfd/twl6040.txt
> index 0f5dd709d752..a41157b5d930 100644
> --- a/Documentation/devicetree/bindings/mfd/twl6040.txt
> +++ b/Documentation/devicetree/bindings/mfd/twl6040.txt
> @@ -19,6 +19,8 @@ Required properties:
>
> Optional properties, nodes:
> - enable-active-high: To power on the twl6040 during boot.
> +- clocks: phandle to the clk32k clock provider
> +- clock-names: Must be "clk32k"
>
> Vibra functionality
> Required properties:
> diff --git a/drivers/mfd/twl6040.c b/drivers/mfd/twl6040.c
> index 2e6504a8e1e3..12b314ea48dc 100644
> --- a/drivers/mfd/twl6040.c
> +++ b/drivers/mfd/twl6040.c
> @@ -291,6 +291,8 @@ int twl6040_power(struct twl6040 *twl6040, int on)
> if (twl6040->power_count++)
> goto out;
>
> + clk_prepare_enable(twl6040->clk32k);
> +
> /* Allow writes to the chip */
> regcache_cache_only(twl6040->regmap, false);
>
> @@ -346,6 +348,8 @@ int twl6040_power(struct twl6040 *twl6040, int on)
>
> twl6040->sysclk = 0;
> twl6040->mclk = 0;
> +
> + clk_disable_unprepare(twl6040->clk32k);
> }
>
> out:
> @@ -644,6 +648,12 @@ static int twl6040_probe(struct i2c_client *client,
>
> i2c_set_clientdata(client, twl6040);
>
> + twl6040->clk32k = devm_clk_get(&client->dev, "clk32k");
> + if (IS_ERR(twl6040->clk32k)) {
> + dev_info(&client->dev, "clk32k is not handled\n");
> + twl6040->clk32k = NULL;

So what happens if you pass a NULL clk reference to clk_prepare_enable()?

--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

2014-04-28 10:47:00

by Peter Ujfalusi

[permalink] [raw]

Subject: Re: [PATCH] mfd: twl6040: Optional clk32k clock handling

On 04/28/2014 01:36 PM, Lee Jones wrote:
>> In certain boards the source for the clk32k clock can be gated. In these
>> boards the clk32k clock can be provided to the driver and it is going to be
>> enabled/disabled when it is needed.
>> If the clk32k clock is not provided the driver will assume that it is always
>> running.
>>
>> Signed-off-by: Peter Ujfalusi <[email protected]>
>> ---
>> Documentation/devicetree/bindings/mfd/twl6040.txt | 2 ++
>> drivers/mfd/twl6040.c | 10 ++++++++++
>> include/linux/mfd/twl6040.h | 2 ++
>> 3 files changed, 14 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/mfd/twl6040.txt b/Documentation/devicetree/bindings/mfd/twl6040.txt
>> index 0f5dd709d752..a41157b5d930 100644
>> --- a/Documentation/devicetree/bindings/mfd/twl6040.txt
>> +++ b/Documentation/devicetree/bindings/mfd/twl6040.txt
>> @@ -19,6 +19,8 @@ Required properties:
>>
>> Optional properties, nodes:
>> - enable-active-high: To power on the twl6040 during boot.
>> +- clocks: phandle to the clk32k clock provider
>> +- clock-names: Must be "clk32k"
>>
>> Vibra functionality
>> Required properties:
>> diff --git a/drivers/mfd/twl6040.c b/drivers/mfd/twl6040.c
>> index 2e6504a8e1e3..12b314ea48dc 100644
>> --- a/drivers/mfd/twl6040.c
>> +++ b/drivers/mfd/twl6040.c
>> @@ -291,6 +291,8 @@ int twl6040_power(struct twl6040 *twl6040, int on)
>> if (twl6040->power_count++)
>> goto out;
>>
>> + clk_prepare_enable(twl6040->clk32k);
>> +
>> /* Allow writes to the chip */
>> regcache_cache_only(twl6040->regmap, false);
>>
>> @@ -346,6 +348,8 @@ int twl6040_power(struct twl6040 *twl6040, int on)
>>
>> twl6040->sysclk = 0;
>> twl6040->mclk = 0;
>> +
>> + clk_disable_unprepare(twl6040->clk32k);
>> }
>>
>> out:
>> @@ -644,6 +648,12 @@ static int twl6040_probe(struct i2c_client *client,
>>
>> i2c_set_clientdata(client, twl6040);
>>
>> + twl6040->clk32k = devm_clk_get(&client->dev, "clk32k");
>> + if (IS_ERR(twl6040->clk32k)) {
>> + dev_info(&client->dev, "clk32k is not handled\n");
>> + twl6040->clk32k = NULL;
>
> So what happens if you pass a NULL clk reference to clk_prepare_enable()?

We have checks for !clk in all cases within the clk_* API. If we pass NULL,
nothing will happen and from the calling driver point of view it will look
like a success.
AFAIK this is the way to handle non esential/optional clocks without if()s in
the driver code around the clock handling.

--
Péter

2014-04-28 12:51:01

by Lee Jones

[permalink] [raw]

Subject: Re: [PATCH] mfd: twl6040: Optional clk32k clock handling

On Mon, 28 Apr 2014, Peter Ujfalusi wrote:

> On 04/28/2014 01:36 PM, Lee Jones wrote:
> >> In certain boards the source for the clk32k clock can be gated. In these
> >> boards the clk32k clock can be provided to the driver and it is going to be
> >> enabled/disabled when it is needed.
> >> If the clk32k clock is not provided the driver will assume that it is always
> >> running.
> >>
> >> Signed-off-by: Peter Ujfalusi <[email protected]>
> >> ---
> >> Documentation/devicetree/bindings/mfd/twl6040.txt | 2 ++
> >> drivers/mfd/twl6040.c | 10 ++++++++++
> >> include/linux/mfd/twl6040.h | 2 ++
> >> 3 files changed, 14 insertions(+)
> >>
> >> diff --git a/Documentation/devicetree/bindings/mfd/twl6040.txt b/Documentation/devicetree/bindings/mfd/twl6040.txt
> >> index 0f5dd709d752..a41157b5d930 100644
> >> --- a/Documentation/devicetree/bindings/mfd/twl6040.txt
> >> +++ b/Documentation/devicetree/bindings/mfd/twl6040.txt
> >> @@ -19,6 +19,8 @@ Required properties:
> >>
> >> Optional properties, nodes:
> >> - enable-active-high: To power on the twl6040 during boot.
> >> +- clocks: phandle to the clk32k clock provider
> >> +- clock-names: Must be "clk32k"
> >>
> >> Vibra functionality
> >> Required properties:
> >> diff --git a/drivers/mfd/twl6040.c b/drivers/mfd/twl6040.c
> >> index 2e6504a8e1e3..12b314ea48dc 100644
> >> --- a/drivers/mfd/twl6040.c
> >> +++ b/drivers/mfd/twl6040.c
> >> @@ -291,6 +291,8 @@ int twl6040_power(struct twl6040 *twl6040, int on)
> >> if (twl6040->power_count++)
> >> goto out;
> >>
> >> + clk_prepare_enable(twl6040->clk32k);
> >> +
> >> /* Allow writes to the chip */
> >> regcache_cache_only(twl6040->regmap, false);
> >>
> >> @@ -346,6 +348,8 @@ int twl6040_power(struct twl6040 *twl6040, int on)
> >>
> >> twl6040->sysclk = 0;
> >> twl6040->mclk = 0;
> >> +
> >> + clk_disable_unprepare(twl6040->clk32k);
> >> }
> >>
> >> out:
> >> @@ -644,6 +648,12 @@ static int twl6040_probe(struct i2c_client *client,
> >>
> >> i2c_set_clientdata(client, twl6040);
> >>
> >> + twl6040->clk32k = devm_clk_get(&client->dev, "clk32k");
> >> + if (IS_ERR(twl6040->clk32k)) {
> >> + dev_info(&client->dev, "clk32k is not handled\n");
> >> + twl6040->clk32k = NULL;
> >
> > So what happens if you pass a NULL clk reference to clk_prepare_enable()?
>
> We have checks for !clk in all cases within the clk_* API. If we pass NULL,
> nothing will happen and from the calling driver point of view it will look
> like a success.
> AFAIK this is the way to handle non esential/optional clocks without if()s in
> the driver code around the clock handling.

Okay, so long as those checks are there, we're all good.

--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

2014-04-28 12:51:26

by Lee Jones

[permalink] [raw]