| Message ID | 1381238480-18852-1-git-send-email-ldewangan@nvidia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 10/08/2013 08:21 AM, Laxman Dewangan wrote: > Palmas devices has two clock output CLK32K_KG and CLK32K_KG_AUDIO not all palmas devices have 2 clocks - example: tps659038 > which can be enable/disable through software. > > Add clock driver support for the Palmas 32KHz clocks. > > Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com> > --- > Changes from V1: > - Call prepare if the clk is needed for boot-enable or sleep control. > - change is_enabled to is_prepared. > - Added ops palmas_clks_recalc_rate. > - Added of_clk_add_provider(). > > Changes from V2: > - Convert magic number to defines. > > .../devicetree/bindings/clock/clk-palmas.txt | 45 +++ > drivers/clk/Kconfig | 7 + > drivers/clk/Makefile | 1 + > drivers/clk/clk-palmas.c | 340 ++++++++++++++++++++ http://www.spinics.net/lists/devicetree/msg04855.html Do we do 2 patches now? one seperate for binding and implementation? What is our current preference now a days? > 4 files changed, 393 insertions(+), 0 deletions(-) > create mode 100644 Documentation/devicetree/bindings/clock/clk-palmas.txt > create mode 100644 drivers/clk/clk-palmas.c > > diff --git a/Documentation/devicetree/bindings/clock/clk-palmas.txt b/Documentation/devicetree/bindings/clock/clk-palmas.txt > new file mode 100644 > index 0000000..c247538 > --- /dev/null > +++ b/Documentation/devicetree/bindings/clock/clk-palmas.txt > @@ -0,0 +1,45 @@ > +* Palmas 32KHz clocks * > + > +Palmas device has two clock output pins for 32KHz, KG and KG_AUDIO. > + > +This binding uses the common clock binding ./clock-bindings.txt. proper link would be to provide Documentation/devicetree/bindings/clock/clock-bindings.txt ? > + > +Clock 32KHz KG is output 0 of the driver and clock 32KHz is output 1. > + > +Required properties: > +- compatible : shall be "ti,palmas-clk". To handle variants of Palmas chips in production, you'd want to be specific here clk32k_kg and clk32k_kg_audio. > +- #clock-cells : from common clock binding; shall be set to 1. > + > +Optional subnode: > + The clock node has optional subnode to provide the init configuration of > + clocks like boot enable, sleep control. > + > + The subnode name is fixed and it should be > + clk32k_kg for the 32KHz KG clock. > + clk32k_kg_audio for the 32KHz KG_AUDIO clock. > + > + Optional subnode properties: > + ti,clock-boot-enable: Enable clock at the time of booting. Dumb question: Why is this needed? should'nt relevant drivers do a clk_get to enable the relevant clocks? > + ti,external-sleep-control: The clock is enable/disabled by state > + of external enable input pins ENABLE, ENABLE2 and NSLEEP. > + The valid value for the external pins are: > + 1 for ENABLE1 > + 2 for ENABLE2 > + 3 for NSLEEP. could we not have macros for readability? > + Option 0 or missing this property means the clock is > + enabled/disabled via register access and these pins do > + not have any control. > + > +Example: > + clock { > + compatible = "ti,palmas-clk"; > + #clock-cells = <1>; > + clk32k_kg { > + ti,clock-boot-enable; > + ti,external-sleep-control = <3>; > + }; > + > + clk32k_kg_audio { > + ti,clock-boot-enable; > + }; > + }; > diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig > index 279407a..6b8c233 100644 > --- a/drivers/clk/Kconfig > +++ b/drivers/clk/Kconfig > @@ -93,6 +93,13 @@ config CLK_PPC_CORENET > This adds the clock driver support for Freescale PowerPC corenet > platforms using common clock framework. > > +config COMMON_CLK_PALMAS > + tristate "Clock driver for TI Palmas devices" > + depends on MFD_PALMAS > + ---help--- > + This driver supports TI Palmas devices 32KHz output KG and KG_AUDIO > + using common clock framework. > + > endmenu > > source "drivers/clk/mvebu/Kconfig" > diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile > index 7b11106..32e04cf 100644 > --- a/drivers/clk/Makefile > +++ b/drivers/clk/Makefile > @@ -43,3 +43,4 @@ obj-$(CONFIG_COMMON_CLK_SI5351) += clk-si5351.o > obj-$(CONFIG_COMMON_CLK_S2MPS11) += clk-s2mps11.o > obj-$(CONFIG_CLK_TWL6040) += clk-twl6040.o > obj-$(CONFIG_CLK_PPC_CORENET) += clk-ppc-corenet.o > +obj-$(CONFIG_COMMON_CLK_PALMAS) += clk-palmas.o > diff --git a/drivers/clk/clk-palmas.c b/drivers/clk/clk-palmas.c > new file mode 100644 > index 0000000..3e070b2 > --- /dev/null > +++ b/drivers/clk/clk-palmas.c > @@ -0,0 +1,340 @@ > +/* > + * Clock driver for Palmas device. > + * > + * Copyright (c) 2013, NVIDIA Corporation. > + * > + * Author: Laxman Dewangan <ldewangan@nvidia.com> > + * > + * This program is free software; you can redistribute it and/or > + * modify it under the terms of the GNU General Public License as > + * published by the Free Software Foundation version 2. > + * > + * This program is distributed "as is" WITHOUT ANY WARRANTY of any kind, > + * whether express or implied; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + * General Public License for more details. > + * > + * You should have received a copy of the GNU General Public License > + * along with this program; if not, write to the Free Software > + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA > + * 02111-1307, USA > + */ > + > +#include <linux/clk.h> > +#include <linux/clkdev.h> > +#include <linux/clk-provider.h> > +#include <linux/mfd/palmas.h> > +#include <linux/module.h> > +#include <linux/of.h> > +#include <linux/of_device.h> > +#include <linux/platform_device.h> > +#include <linux/slab.h> > + > +enum PALMAS_CLOCK32K { > + PALMAS_CLOCK32KG, > + PALMAS_CLOCK32KG_AUDIO, > + > + /* Last entry */ > + PALMAS_CLOCK32K_NR, > +}; you should be able to get rid of this entirely > + > +#define PALMAS_CLOCK_DT_EXT_CONTROL_ENABLE1 1 > +#define PALMAS_CLOCK_DT_EXT_CONTROL_ENABLE2 2 > +#define PALMAS_CLOCK_DT_EXT_CONTROL_NSLEEP 3 > + > +struct palmas_clks; > + > +struct palmas_clk32k_desc { > + const char *clk_name; > + unsigned int control_reg; > + unsigned int enable_mask; > + unsigned int sleep_mask; > + unsigned int sleep_reqstr_id; > +}; > + > +struct palmas_clock_info { > + struct clk *clk; > + struct clk_hw hw; > + struct palmas_clk32k_desc *clk_desc; > + struct palmas_clks *palmas_clk; > + int ext_control_pin; > + bool boot_enable; > +}; > + > +struct palmas_clks { > + struct device *dev; > + struct palmas *palmas; > + struct clk_onecell_data clk_data; > + struct palmas_clock_info clk_info[PALMAS_CLOCK32K_NR]; > +}; > + > +static struct palmas_clk32k_desc palmas_clk32k_descs[] = { > + { > + .clk_name = "clk32k_kg", > + .control_reg = PALMAS_CLK32KG_CTRL, > + .enable_mask = PALMAS_CLK32KG_CTRL_MODE_ACTIVE, > + .sleep_mask = PALMAS_CLK32KG_CTRL_MODE_SLEEP, > + .sleep_reqstr_id = PALMAS_EXTERNAL_REQSTR_ID_CLK32KG, > + }, { > + .clk_name = "clk32k_kg_audio", > + .control_reg = PALMAS_CLK32KGAUDIO_CTRL, > + .enable_mask = PALMAS_CLK32KG_CTRL_MODE_ACTIVE, > + .sleep_mask = PALMAS_CLK32KG_CTRL_MODE_SLEEP, > + .sleep_reqstr_id = PALMAS_EXTERNAL_REQSTR_ID_CLK32KGAUDIO, > + }, > +}; > + > +static inline struct palmas_clock_info *to_palmas_clks_info(struct clk_hw *hw) > +{ > + return container_of(hw, struct palmas_clock_info, hw); > +} > + > +static unsigned long palmas_clks_recalc_rate(struct clk_hw *hw, > + unsigned long parent_rate) > +{ > + return 32768; > +} > + > +static int palmas_clks_prepare(struct clk_hw *hw) > +{ > + struct palmas_clock_info *cinfo = to_palmas_clks_info(hw); > + struct palmas_clks *palmas_clks = cinfo->palmas_clk; > + int ret; > + > + ret = palmas_update_bits(palmas_clks->palmas, PALMAS_RESOURCE_BASE, > + cinfo->clk_desc->control_reg, > + cinfo->clk_desc->enable_mask, > + cinfo->clk_desc->enable_mask); > + if (ret < 0) > + dev_err(palmas_clks->dev, "Reg 0x%02x update failed, %d\n", > + cinfo->clk_desc->control_reg, ret); > + > + return ret; > +} > + > +static void palmas_clks_unprepare(struct clk_hw *hw) > +{ > + struct palmas_clock_info *cinfo = to_palmas_clks_info(hw); > + struct palmas_clks *palmas_clks = cinfo->palmas_clk; > + int ret; > + > + /* > + * Clock can be disabled through external pin if it is externally > + * controlled. > + */ > + if (cinfo->ext_control_pin) > + return; > + > + ret = palmas_update_bits(palmas_clks->palmas, PALMAS_RESOURCE_BASE, > + cinfo->clk_desc->control_reg, > + cinfo->clk_desc->enable_mask, 0); > + if (ret < 0) > + dev_err(palmas_clks->dev, "Reg 0x%02x update failed, %d\n", > + cinfo->clk_desc->control_reg, ret); > + > +} > + > +static int palmas_clks_is_prepared(struct clk_hw *hw) > +{ > + struct palmas_clock_info *cinfo = to_palmas_clks_info(hw); > + struct palmas_clks *palmas_clks = cinfo->palmas_clk; > + int ret; > + u32 val; > + > + if (cinfo->ext_control_pin) > + return 1; > + > + ret = palmas_read(palmas_clks->palmas, PALMAS_RESOURCE_BASE, > + cinfo->clk_desc->control_reg, &val); > + if (ret < 0) { > + dev_err(palmas_clks->dev, "Reg 0x%02x read failed, %d\n", > + cinfo->clk_desc->control_reg, ret); > + return ret; > + } > + return !!(val & cinfo->clk_desc->enable_mask); > +} > + > +static struct clk_ops palmas_clks_ops = { > + .prepare = palmas_clks_prepare, > + .unprepare = palmas_clks_unprepare, > + .is_prepared = palmas_clks_is_prepared, > + .recalc_rate = palmas_clks_recalc_rate, > +}; > + > +static struct clk_init_data palmas_clks_hw_init[PALMAS_CLOCK32K_NR] = { > + [PALMAS_CLOCK32KG] = { > + .name = "clk32k_kg", > + .ops = &palmas_clks_ops, > + .flags = CLK_IS_ROOT | CLK_IGNORE_UNUSED, > + }, > + [PALMAS_CLOCK32KG_AUDIO] = { > + .name = "clk32k_kg_audio", > + .ops = &palmas_clks_ops, > + .flags = CLK_IS_ROOT | CLK_IGNORE_UNUSED, > + }, > +}; if we had two compatible properties, then we would not need this. > + > +static int palmas_clks_get_clk_data(struct platform_device *pdev, > + struct palmas_clks *palmas_clks) > +{ > + struct device_node *node = pdev->dev.of_node; > + struct device_node *child; > + struct palmas_clock_info *cinfo; > + unsigned int prop; > + int ret; > + int i; > + > + for (i = 0; i < PALMAS_CLOCK32K_NR; ++i) { > + child = of_get_child_by_name(node, > + palmas_clk32k_descs[i].clk_name); > + if (!child) > + continue; > + > + cinfo = &palmas_clks->clk_info[i]; > + cinfo->boot_enable = of_property_read_bool(child, > + "ti,clock-boot-enable"); > + ret = of_property_read_u32(child, "ti,external-sleep-control", > + &prop); > + if (!ret) { > + switch (prop) { > + case PALMAS_CLOCK_DT_EXT_CONTROL_ENABLE1: > + prop = PALMAS_EXT_CONTROL_ENABLE1; > + break; > + case PALMAS_CLOCK_DT_EXT_CONTROL_ENABLE2: > + prop = PALMAS_EXT_CONTROL_ENABLE2; > + break; > + case PALMAS_CLOCK_DT_EXT_CONTROL_NSLEEP: > + prop = PALMAS_EXT_CONTROL_NSLEEP; > + break; > + default: > + WARN_ON(1); > + dev_warn(&pdev->dev, > + "%s: Invalid ext control option: %u\n", > + child->name, prop); > + prop = 0; > + break; > + } > + cinfo->ext_control_pin = prop; > + } > + } > + > + return 0; > +} > + > +static int palmas_clks_init_configure(struct palmas_clock_info *cinfo) > +{ > + struct palmas_clks *palmas_clks = cinfo->palmas_clk; > + int ret; > + > + if (cinfo->boot_enable || cinfo->ext_control_pin) { > + ret = clk_prepare(cinfo->clk); > + if (ret < 0) { > + dev_err(palmas_clks->dev, > + "Clock prep failed, %d\n", ret); > + return ret; > + } > + } > + > + ret = palmas_update_bits(palmas_clks->palmas, PALMAS_RESOURCE_BASE, > + cinfo->clk_desc->control_reg, > + cinfo->clk_desc->sleep_mask, 0); > + if (ret < 0) { > + dev_err(palmas_clks->dev, "Reg 0x%02x update failed, %d\n", > + cinfo->clk_desc->control_reg, ret); > + return ret; > + } > + > + if (cinfo->ext_control_pin) { > + ret = palmas_ext_control_req_config(palmas_clks->palmas, > + cinfo->clk_desc->sleep_reqstr_id, > + cinfo->ext_control_pin, true); > + if (ret < 0) { > + dev_err(palmas_clks->dev, > + "Ext config for %s failed, %d\n", > + cinfo->clk_desc->clk_name, ret); > + return ret; > + } > + } > + > + return ret; > +} > + > +static int palmas_clks_probe(struct platform_device *pdev) > +{ > + struct palmas *palmas = dev_get_drvdata(pdev->dev.parent); > + struct palmas_clock_info *cinfo; > + struct palmas_clks *palmas_clks; > + struct clk *clk; > + int i, ret; > + > + palmas_clks = devm_kzalloc(&pdev->dev, sizeof(*palmas_clks), > + GFP_KERNEL); > + if (!palmas_clks) > + return -ENOMEM; > + > + palmas_clks->clk_data.clks = devm_kzalloc(&pdev->dev, > + PALMAS_CLOCK32K_NR * sizeof(palmas_clks->clk_data.clks), > + GFP_KERNEL); > + if (!palmas_clks->clk_data.clks) > + return -ENOMEM; > + > + palmas_clks_get_clk_data(pdev, palmas_clks); > + platform_set_drvdata(pdev, palmas_clks); > + > + palmas_clks->dev = &pdev->dev; > + palmas_clks->palmas = palmas; > + > + for (i = 0; i < PALMAS_CLOCK32K_NR; i++) { > + cinfo = &palmas_clks->clk_info[i]; > + cinfo->clk_desc = &palmas_clk32k_descs[i]; > + cinfo->hw.init = &palmas_clks_hw_init[i]; > + cinfo->palmas_clk = palmas_clks; > + clk = devm_clk_register(&pdev->dev, &cinfo->hw); > + if (IS_ERR(clk)) { > + ret = PTR_ERR(clk); > + dev_err(&pdev->dev, "Fail to register clock %s, %d\n", > + palmas_clk32k_descs[i].clk_name, ret); > + return ret; > + } > + > + cinfo->clk = clk; > + palmas_clks->clk_data.clks[i] = clk; > + palmas_clks->clk_data.clk_num++; > + palmas_clks_init_configure(cinfo); we dont handle error here? > + } > + > + ret = of_clk_add_provider(pdev->dev.of_node, of_clk_src_simple_get, > + &palmas_clks->clk_data); > + if (ret < 0) > + dev_err(&pdev->dev, "Fail to add clock driver, %d\n", ret); > + return ret; > +} > + > +static int palmas_clks_remove(struct platform_device *pdev) > +{ > + of_clk_del_provider(pdev->dev.of_node); > + return 0; > +} > + > +static struct of_device_id of_palmas_clks_match_tbl[] = { > + { .compatible = "ti,palmas-clk", }, you could use match_data to map required data into the system.. > + {}, > +}; > +MODULE_DEVICE_TABLE(of, of_palmas_clks_match_tbl); > + > +static struct platform_driver palmas_clks_driver = { > + .driver = { > + .name = "palmas-clk", > + .owner = THIS_MODULE, > + .of_match_table = of_palmas_clks_match_tbl, > + }, > + .probe = palmas_clks_probe, > + .remove = palmas_clks_remove, > +}; > + > +module_platform_driver(palmas_clks_driver); > + > +MODULE_DESCRIPTION("Clock driver for Palmas Series Devices"); > +MODULE_ALIAS("platform:palmas-clk"); > +MODULE_AUTHOR("Laxman Dewangan <ldewangan@nvidia.com>"); > +MODULE_LICENSE("GPL v2"); > I wonder if we can simplify this with CLK_OF_DECLARE - I suppose it wont work if of_clk_init(NULL); was invoked previously.
Thanks Nishanth for review. On Tuesday 08 October 2013 06:59 PM, Nishanth Menon wrote: > On 10/08/2013 08:21 AM, Laxman Dewangan wrote: >> Palmas devices has two clock output CLK32K_KG and CLK32K_KG_AUDIO > not all palmas devices have 2 clocks - example: tps659038 This is for generic palmas and I have seen it for TPS65913, TPS65914, TPS80036. If the generic one is not compatible then it need to add device specific and at that time, it is require to update the binding document accordingly. > | 7 + > drivers/clk/Makefile | 1 + > drivers/clk/clk-palmas.c | 340 ++++++++++++++++++++ > > http://www.spinics.net/lists/devicetree/msg04855.html > Do we do 2 patches now? one seperate for binding and implementation? > What is our current preference now a days? Currently it is implementation + binding doc in one patch. > >> Palmas device has two clock output pins for 32KHz, KG and KG_AUDIO. >> + >> +This binding uses the common clock binding ./clock-bindings.txt. > proper link would be to provide > Documentation/devicetree/bindings/clock/clock-bindings.txt ? Hmm, other patch I got feedback from DT maintainers to do not use the absolute path as document directory may change >> + >> +Clock 32KHz KG is output 0 of the driver and clock 32KHz is output 1. >> + >> +Required properties: >> +- compatible : shall be "ti,palmas-clk". > To handle variants of Palmas chips in production, you'd want to be > specific here clk32k_kg and clk32k_kg_audio. The compatible is the device sub module level, not the clock level. Same thing we are following on regulators. > >> + >> + Optional subnode properties: >> + ti,clock-boot-enable: Enable clock at the time of booting. > Dumb question: Why is this needed? should'nt relevant drivers do a > clk_get to enable the relevant clocks? If some board needs this clock to be always available for rest of system to work without any specific driver then this flag is useful. > >> + ti,external-sleep-control: The clock is enable/disabled by state >> + of external enable input pins ENABLE, ENABLE2 and NSLEEP. >> + The valid value for the external pins are: >> + 1 for ENABLE1 >> + 2 for ENABLE2 >> + 3 for NSLEEP. > could we not have macros for readability? I am thinking of adding the palmas for dt-binding and then change on multiple places. I will post patches for this. the patch will go on dt tree as include/dt-bindings and then on documents file and then on actually DTS. I will work towards this but scoping out of this patch. > >> + >> + >> +enum PALMAS_CLOCK32K { >> + PALMAS_CLOCK32KG, >> + PALMAS_CLOCK32KG_AUDIO, >> + >> + /* Last entry */ >> + PALMAS_CLOCK32K_NR, >> +}; > you should be able to get rid of this entirely Probably yes but it is easy to read (atleast for me). > > + cinfo->clk = clk; > + palmas_clks->clk_data.clks[i] = clk; > + palmas_clks->clk_data.clk_num++; > + palmas_clks_init_configure(cinfo); > we dont handle error here? Intentionally I ignore error, just print and continue the registration. > >> + >> +MODULE_AUTHOR("Laxman Dewangan <ldewangan@nvidia.com>"); >> +MODULE_LICENSE("GPL v2"); >> > I wonder if we can simplify this with CLK_OF_DECLARE - I suppose it > wont work if of_clk_init(NULL); was invoked previously. This driver has dependency over the mfd driver and hence until mfd driver invoked and get registered, this driver should not be called. The platform driver registration is done in mfd. As per my understanding and referring the other code, CLK_OF_DECLARE is useful if there is no such dependency. Please correct me if this is not true.
On Tue, Oct 08, 2013 at 03:39:54PM +0100, Laxman Dewangan wrote: > Thanks Nishanth for review. > > On Tuesday 08 October 2013 06:59 PM, Nishanth Menon wrote: > > On 10/08/2013 08:21 AM, Laxman Dewangan wrote: > >> Palmas devices has two clock output CLK32K_KG and CLK32K_KG_AUDIO > > not all palmas devices have 2 clocks - example: tps659038 > > This is for generic palmas and I have seen it for TPS65913, TPS65914, > TPS80036. If the generic one is not compatible then it need to add > device specific and at that time, it is require to update the binding > document accordingly. We've just been given an example of where it's not compatible. > > > | 7 + > > drivers/clk/Makefile | 1 + > > drivers/clk/clk-palmas.c | 340 ++++++++++++++++++++ > > > > http://www.spinics.net/lists/devicetree/msg04855.html > > Do we do 2 patches now? one seperate for binding and implementation? > > What is our current preference now a days? > > Currently it is implementation + binding doc in one patch. I believe the general consensus is that separate DT patches are preferred. > > > > >> Palmas device has two clock output pins for 32KHz, KG and KG_AUDIO. > >> + > >> +This binding uses the common clock binding ./clock-bindings.txt. > > proper link would be to provide > > Documentation/devicetree/bindings/clock/clock-bindings.txt ? > > Hmm, other patch I got feedback from DT maintainers to do not use the > absolute path as document directory may change I'd not seen this, but I don't have a strong preference either way. This document will go under Documentation/devicetree/bindings/clock, so a relative path seems Ok. > > >> + > >> +Clock 32KHz KG is output 0 of the driver and clock 32KHz is output 1. > >> + > >> +Required properties: > >> +- compatible : shall be "ti,palmas-clk". > > To handle variants of Palmas chips in production, you'd want to be > > specific here clk32k_kg and clk32k_kg_audio. > > The compatible is the device sub module level, not the clock level. Same > thing we are following on regulators. Yes, we don't need a separate compatible string for each clock within a block providing those clocks. > > > > > >> + > >> + Optional subnode properties: > >> + ti,clock-boot-enable: Enable clock at the time of booting. > > Dumb question: Why is this needed? should'nt relevant drivers do a > > clk_get to enable the relevant clocks? > > If some board needs this clock to be always available for rest of system > to work without any specific driver then this flag is useful. Do we _actually_ need this right now, or is this hypothetical? If we don't need it now, remove it. If you think we need it know, please describe exactly why (i.e. what device needs the clock to work, why does this affect the rest of the board if we don't ahve a driver for that device, why don't we just write a driver for that device). > > > > > >> + ti,external-sleep-control: The clock is enable/disabled by state > >> + of external enable input pins ENABLE, ENABLE2 and NSLEEP. > >> + The valid value for the external pins are: > >> + 1 for ENABLE1 > >> + 2 for ENABLE2 > >> + 3 for NSLEEP. I asked this on the last version (before having noticed this one). What actually drives those pins to control the clock(s)? Is this for setting the clock to be controlled by the external pin, or is the clock hard-wired to a particular pin? Thanks, Mark.
On 10/08/2013 09:39 AM, Laxman Dewangan wrote: > Thanks Nishanth for review. > > On Tuesday 08 October 2013 06:59 PM, Nishanth Menon wrote: >> On 10/08/2013 08:21 AM, Laxman Dewangan wrote: >>> Palmas devices has two clock output CLK32K_KG and CLK32K_KG_AUDIO >> not all palmas devices have 2 clocks - example: tps659038 > > This is for generic palmas and I have seen it for TPS65913, TPS65914, > TPS80036. If the generic one is not compatible then it need to add > device specific and at that time, it is require to update the binding > document accordingly. ?? you do have two clocks inside the device they should be represented as two compatible entities - that simplifies everyone's life. > >> | 7 + >> drivers/clk/Makefile | 1 + >> drivers/clk/clk-palmas.c | 340 ++++++++++++++++++++ >> >> http://www.spinics.net/lists/devicetree/msg04855.html >> Do we do 2 patches now? one seperate for binding and implementation? >> What is our current preference now a days? > > Currently it is implementation + binding doc in one patch. > >> >>> Palmas device has two clock output pins for 32KHz, KG and KG_AUDIO. >>> + >>> +This binding uses the common clock binding ./clock-bindings.txt. >> proper link would be to provide >> Documentation/devicetree/bindings/clock/clock-bindings.txt ? > > Hmm, other patch I got feedback from DT maintainers to do not use the > absolute path as document directory may change > >>> + >>> +Clock 32KHz KG is output 0 of the driver and clock 32KHz is output 1. >>> + >>> +Required properties: >>> +- compatible : shall be "ti,palmas-clk". >> To handle variants of Palmas chips in production, you'd want to be >> specific here clk32k_kg and clk32k_kg_audio. > > The compatible is the device sub module level, not the clock level. Same > thing we are following on regulators. not exactly the same problem as regulator IMHO here. > > >> >>> + >>> + Optional subnode properties: >>> + ti,clock-boot-enable: Enable clock at the time of booting. >> Dumb question: Why is this needed? should'nt relevant drivers do a >> clk_get to enable the relevant clocks? > > If some board needs this clock to be always available for rest of system > to work without any specific driver then this flag is useful. that is the wrong way of using this. > > >> >>> + ti,external-sleep-control: The clock is enable/disabled by state >>> + of external enable input pins ENABLE, ENABLE2 and NSLEEP. >>> + The valid value for the external pins are: >>> + 1 for ENABLE1 >>> + 2 for ENABLE2 >>> + 3 for NSLEEP. >> could we not have macros for readability? > > I am thinking of adding the palmas for dt-binding and then change on > multiple places. I will post patches for this. > the patch will go on dt tree as include/dt-bindings and then on > documents file and then on actually DTS. I will work towards this but > scoping out of this patch. > > why not do it here? and provide explanation - we dont want to deal with backward compatible dtbs etc later on. >> >>> + >>> + >>> +enum PALMAS_CLOCK32K { >>> + PALMAS_CLOCK32KG, >>> + PALMAS_CLOCK32KG_AUDIO, >>> + >>> + /* Last entry */ >>> + PALMAS_CLOCK32K_NR, >>> +}; >> you should be able to get rid of this entirely > > Probably yes but it is easy to read (atleast for me). you can get rid of it entirely by using appropriate matches. > > >> >> + cinfo->clk = clk; >> + palmas_clks->clk_data.clks[i] = clk; >> + palmas_clks->clk_data.clk_num++; >> + palmas_clks_init_configure(cinfo); >> we dont handle error here? > > Intentionally I ignore error, just print and continue the registration. not acceptable: since the failure indicates setups are broken, adding a provider is not valid even, sorry, NAK as a result. > > >> >>> + >>> +MODULE_AUTHOR("Laxman Dewangan <ldewangan@nvidia.com>"); >>> +MODULE_LICENSE("GPL v2"); >>> >> I wonder if we can simplify this with CLK_OF_DECLARE - I suppose it >> wont work if of_clk_init(NULL); was invoked previously. > > This driver has dependency over the mfd driver and hence until mfd > driver invoked and get registered, this driver should not be called. The > platform driver registration is done in mfd. > As per my understanding and referring the other code, CLK_OF_DECLARE is > useful if there is no such dependency. Please correct me if this is not > true. > that is what i was wondering - since it is a clock source....
On 10/08/2013 10:14 AM, Nishanth Menon wrote: > On 10/08/2013 09:39 AM, Laxman Dewangan wrote: >> Thanks Nishanth for review. >> >> On Tuesday 08 October 2013 06:59 PM, Nishanth Menon wrote: >>> On 10/08/2013 08:21 AM, Laxman Dewangan wrote: >>>> Palmas devices has two clock output CLK32K_KG and CLK32K_KG_AUDIO >>> not all palmas devices have 2 clocks - example: tps659038 >> >> This is for generic palmas and I have seen it for TPS65913, TPS65914, >> TPS80036. If the generic one is not compatible then it need to add >> device specific and at that time, it is require to update the binding >> document accordingly. > > ?? you do have two clocks inside the device they should be represented > as two compatible entities - that simplifies everyone's life. I think the terminology you're using here is quite confusing. Are you talking about having two different compatible values for two different HW designs, where those different designs implement different sets of clocks (which makes sense), or two different DT nodes for two different clocks (which IMHO doesn't always, unless those different clocks *truly* are separate IP blocks with completely independent register regions, and where those IP blocks are likely to be re-used as-is in other chips).
On 10/08/2013 12:08 PM, Stephen Warren wrote: > On 10/08/2013 10:14 AM, Nishanth Menon wrote: >> On 10/08/2013 09:39 AM, Laxman Dewangan wrote: >>> Thanks Nishanth for review. >>> >>> On Tuesday 08 October 2013 06:59 PM, Nishanth Menon wrote: >>>> On 10/08/2013 08:21 AM, Laxman Dewangan wrote: >>>>> Palmas devices has two clock output CLK32K_KG and CLK32K_KG_AUDIO >>>> not all palmas devices have 2 clocks - example: tps659038 >>> >>> This is for generic palmas and I have seen it for TPS65913, TPS65914, >>> TPS80036. If the generic one is not compatible then it need to add >>> device specific and at that time, it is require to update the binding >>> document accordingly. >> >> ?? you do have two clocks inside the device they should be represented >> as two compatible entities - that simplifies everyone's life. > > I think the terminology you're using here is quite confusing. > > Are you talking about having two different compatible values for two > different HW designs, where those different designs implement different > sets of clocks (which makes sense), or two different DT nodes for two > different clocks (which IMHO doesn't always, unless those different > clocks *truly* are separate IP blocks with completely independent > register regions, and where those IP blocks are likely to be re-used > as-is in other chips). > clk32k and clk32k_audio are two different resources and since these are two different resource instances - a "compatible" matching an actual device is my suggestion. clk32k and clk32k_audio are two different resources because they have their specific set of controls registers and may even be independently present in a Palmas variant. To highlight this: The example of tps659038 where clk32k is not present, but clk32k_audio is present (and happens to be disabled by default - thanks to an OTP on the chip - on platform like DRA7-evm, it is used to for 32k clk for wlan -currently hacked in u-boot using plain i2c writes[1] - yes it is yucky). Obviously, there are many ways to implement this. based on the current implementation, it indicates that if i create a node with "ti,palmas-clk" -i'd create two clocks - that is wrong for tps659038. Now (with the current approach), if I have to create a one clock for tps659038, i have to fix the for adding clock providers, add up "ti,tps659038-clk" etc.. it is doable - but IMHO, I dont need to do it with only the relevant nodes in dts. Further, it has no way to indicate that device X uses clock Y using clocks =<&xyz> either. [1] DRA7-evm u-boot hack. i2c dev 0 i2c probe i2c mw 0x58 0xfb 0x2b (select secondary function for GPIO_5) i2c mw 0x58 0xd5 0x01 (enable audio clock)
On 10/08/2013 11:33 AM, Nishanth Menon wrote: > On 10/08/2013 12:08 PM, Stephen Warren wrote: >> On 10/08/2013 10:14 AM, Nishanth Menon wrote: >>> On 10/08/2013 09:39 AM, Laxman Dewangan wrote: >>>> Thanks Nishanth for review. >>>> >>>> On Tuesday 08 October 2013 06:59 PM, Nishanth Menon wrote: >>>>> On 10/08/2013 08:21 AM, Laxman Dewangan wrote: >>>>>> Palmas devices has two clock output CLK32K_KG and CLK32K_KG_AUDIO >>>>> not all palmas devices have 2 clocks - example: tps659038 >>>> >>>> This is for generic palmas and I have seen it for TPS65913, TPS65914, >>>> TPS80036. If the generic one is not compatible then it need to add >>>> device specific and at that time, it is require to update the binding >>>> document accordingly. >>> >>> ?? you do have two clocks inside the device they should be represented >>> as two compatible entities - that simplifies everyone's life. >> >> I think the terminology you're using here is quite confusing. >> >> Are you talking about having two different compatible values for two >> different HW designs, where those different designs implement different >> sets of clocks (which makes sense), or two different DT nodes for two >> different clocks (which IMHO doesn't always, unless those different >> clocks *truly* are separate IP blocks with completely independent >> register regions, and where those IP blocks are likely to be re-used >> as-is in other chips). > > clk32k and clk32k_audio are two different resources and since these > are two different resource instances - a "compatible" matching an > actual device is my suggestion. The fact that two clocks are two different resources isn't at all relevant to DT structure. HW module design is what's relevant. > clk32k and clk32k_audio are two different resources because they have > their specific set of controls registers and may even be independently > present in a Palmas variant. That's a better argument, assuming that: The registers for those two clocks aren't randomly interleaved with other registers within the HW module. That would imply that the clock registers aren't independant HW blocks. > To highlight this: The example of tps659038 where clk32k is not > present, but clk32k_audio is present (and happens to be disabled by > default - thanks to an OTP on the chip - on platform like DRA7-evm, it > is used to for 32k clk for wlan -currently hacked in u-boot using > plain i2c writes[1] - yes it is yucky). That can easily be handled by having separate compatible values for a monolithic overall Palmas or Palmas-clock node/HW-block. The fact that different chips are different doesn't, in and of itself, need to influence whether the different clocks are represented as different DT nodes. > Obviously, there are many ways to implement this. based on the current > implementation, it indicates that if i create a node with > "ti,palmas-clk" -i'd create two clocks - that is wrong for tps659038. > > Now (with the current approach), if I have to create a one clock for > tps659038, i have to fix the for adding clock providers, add up > "ti,tps659038-clk" etc.. it is doable - but IMHO, I dont need to do it > with only the relevant nodes in dts. > > Further, it has no way to indicate that device X uses clock Y using > clocks =<&xyz> either. Sorry, I just don't understand that. If a clock provider provides two clocks, it could number then e.g. 0 and 1. Clock consumers would reference those IDs. If a different chip that uses the same binding only supports one of those two clocks, just have the driver return an error if the DT attempts to use/reference the invalid clock ID; nothign could be simpler.
On Tue, Oct 8, 2013 at 3:43 PM, Stephen Warren <swarren@wwwdotorg.org> wrote: > On 10/08/2013 11:33 AM, Nishanth Menon wrote: >> On 10/08/2013 12:08 PM, Stephen Warren wrote: >>> On 10/08/2013 10:14 AM, Nishanth Menon wrote: >>>> On 10/08/2013 09:39 AM, Laxman Dewangan wrote: >>>>> Thanks Nishanth for review. >>>>> >>>>> On Tuesday 08 October 2013 06:59 PM, Nishanth Menon wrote: >>>>>> On 10/08/2013 08:21 AM, Laxman Dewangan wrote: >>>>>>> Palmas devices has two clock output CLK32K_KG and CLK32K_KG_AUDIO >>>>>> not all palmas devices have 2 clocks - example: tps659038 >>>>> >>>>> This is for generic palmas and I have seen it for TPS65913, TPS65914, >>>>> TPS80036. If the generic one is not compatible then it need to add >>>>> device specific and at that time, it is require to update the binding >>>>> document accordingly. >>>> >>>> ?? you do have two clocks inside the device they should be represented >>>> as two compatible entities - that simplifies everyone's life. >>> >>> I think the terminology you're using here is quite confusing. >>> >>> Are you talking about having two different compatible values for two >>> different HW designs, where those different designs implement different >>> sets of clocks (which makes sense), or two different DT nodes for two >>> different clocks (which IMHO doesn't always, unless those different >>> clocks *truly* are separate IP blocks with completely independent >>> register regions, and where those IP blocks are likely to be re-used >>> as-is in other chips). >> >> clk32k and clk32k_audio are two different resources and since these >> are two different resource instances - a "compatible" matching an >> actual device is my suggestion. > > The fact that two clocks are two different resources isn't at all > relevant to DT structure. HW module design is what's relevant. > >> clk32k and clk32k_audio are two different resources because they have >> their specific set of controls registers and may even be independently >> present in a Palmas variant. > > That's a better argument, assuming that: The registers for those two > clocks aren't randomly interleaved with other registers within the HW > module. That would imply that the clock registers aren't independant HW > blocks. You would be unpleasantly surprised if register offsets is a standard to determine if IP blocks are independent instances or not. it is just integration level decision, and unfortunately, not all h/w integration guys care a lot about s/w :(. > >> To highlight this: The example of tps659038 where clk32k is not >> present, but clk32k_audio is present (and happens to be disabled by >> default - thanks to an OTP on the chip - on platform like DRA7-evm, it >> is used to for 32k clk for wlan -currently hacked in u-boot using >> plain i2c writes[1] - yes it is yucky). > > That can easily be handled by having separate compatible values for a > monolithic overall Palmas or Palmas-clock node/HW-block. The fact that > different chips are different doesn't, in and of itself, need to > influence whether the different clocks are represented as different DT > nodes. So, the fact that a clock does not exist on a variation of palmas is not sufficient proof that the blocks are independent. I am not sure, given the lack of public documentation, short of sharing rtl (which I cant ;) ), i might let this debate flame out.. > >> Obviously, there are many ways to implement this. based on the current >> implementation, it indicates that if i create a node with >> "ti,palmas-clk" -i'd create two clocks - that is wrong for tps659038. >> >> Now (with the current approach), if I have to create a one clock for >> tps659038, i have to fix the for adding clock providers, add up >> "ti,tps659038-clk" etc.. it is doable - but IMHO, I dont need to do it >> with only the relevant nodes in dts. >> >> Further, it has no way to indicate that device X uses clock Y using >> clocks =<&xyz> either. > > Sorry, I just don't understand that. > > If a clock provider provides two clocks, it could number then e.g. 0 and > 1. Clock consumers would reference those IDs. If a different chip that > uses the same binding only supports one of those two clocks, just have > the driver return an error if the DT attempts to use/reference the > invalid clock ID; nothign could be simpler. fair enough. Regards, Nishanth Menon
On Tuesday 08 October 2013 08:58 PM, Mark Rutland wrote: > On Tue, Oct 08, 2013 at 03:39:54PM +0100, Laxman Dewangan wrote: >> >>>> + >>>> + Optional subnode properties: >>>> + ti,clock-boot-enable: Enable clock at the time of booting. >>> Dumb question: Why is this needed? should'nt relevant drivers do a >>> clk_get to enable the relevant clocks? >> If some board needs this clock to be always available for rest of system >> to work without any specific driver then this flag is useful. > Do we _actually_ need this right now, or is this hypothetical? > > If we don't need it now, remove it. If you think we need it know, please > describe exactly why (i.e. what device needs the clock to work, why does > this affect the rest of the board if we don't ahve a driver for that > device, why don't we just write a driver for that device). > Ok, I will remove it. Going with nothing free of cost for the driver/system and client need to call the proper APIs. >> >>>> + ti,external-sleep-control: The clock is enable/disabled by state >>>> + of external enable input pins ENABLE, ENABLE2 and NSLEEP. >>>> + The valid value for the external pins are: >>>> + 1 for ENABLE1 >>>> + 2 for ENABLE2 >>>> + 3 for NSLEEP. > I asked this on the last version (before having noticed this one). What > actually drives those pins to control the clock(s)? > > Is this for setting the clock to be controlled by the external pin, or > is the clock hard-wired to a particular pin? > This is for setting the clock to be controlled by the external pin.
diff --git a/Documentation/devicetree/bindings/clock/clk-palmas.txt b/Documentation/devicetree/bindings/clock/clk-palmas.txt new file mode 100644 index 0000000..c247538 --- /dev/null +++ b/Documentation/devicetree/bindings/clock/clk-palmas.txt @@ -0,0 +1,45 @@ +* Palmas 32KHz clocks * + +Palmas device has two clock output pins for 32KHz, KG and KG_AUDIO. + +This binding uses the common clock binding ./clock-bindings.txt. + +Clock 32KHz KG is output 0 of the driver and clock 32KHz is output 1. + +Required properties: +- compatible : shall be "ti,palmas-clk". +- #clock-cells : from common clock binding; shall be set to 1. + +Optional subnode: + The clock node has optional subnode to provide the init configuration of + clocks like boot enable, sleep control. + + The subnode name is fixed and it should be + clk32k_kg for the 32KHz KG clock. + clk32k_kg_audio for the 32KHz KG_AUDIO clock. + + Optional subnode properties: + ti,clock-boot-enable: Enable clock at the time of booting. + ti,external-sleep-control: The clock is enable/disabled by state + of external enable input pins ENABLE, ENABLE2 and NSLEEP. + The valid value for the external pins are: + 1 for ENABLE1 + 2 for ENABLE2 + 3 for NSLEEP. + Option 0 or missing this property means the clock is + enabled/disabled via register access and these pins do + not have any control. + +Example: + clock { + compatible = "ti,palmas-clk"; + #clock-cells = <1>; + clk32k_kg { + ti,clock-boot-enable; + ti,external-sleep-control = <3>; + }; + + clk32k_kg_audio { + ti,clock-boot-enable; + }; + }; diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig index 279407a..6b8c233 100644 --- a/drivers/clk/Kconfig +++ b/drivers/clk/Kconfig @@ -93,6 +93,13 @@ config CLK_PPC_CORENET This adds the clock driver support for Freescale PowerPC corenet platforms using common clock framework. +config COMMON_CLK_PALMAS + tristate "Clock driver for TI Palmas devices" + depends on MFD_PALMAS + ---help--- + This driver supports TI Palmas devices 32KHz output KG and KG_AUDIO + using common clock framework. + endmenu source "drivers/clk/mvebu/Kconfig" diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile index 7b11106..32e04cf 100644 --- a/drivers/clk/Makefile +++ b/drivers/clk/Makefile @@ -43,3 +43,4 @@ obj-$(CONFIG_COMMON_CLK_SI5351) += clk-si5351.o obj-$(CONFIG_COMMON_CLK_S2MPS11) += clk-s2mps11.o obj-$(CONFIG_CLK_TWL6040) += clk-twl6040.o obj-$(CONFIG_CLK_PPC_CORENET) += clk-ppc-corenet.o +obj-$(CONFIG_COMMON_CLK_PALMAS) += clk-palmas.o diff --git a/drivers/clk/clk-palmas.c b/drivers/clk/clk-palmas.c new file mode 100644 index 0000000..3e070b2 --- /dev/null +++ b/drivers/clk/clk-palmas.c @@ -0,0 +1,340 @@ +/* + * Clock driver for Palmas device. + * + * Copyright (c) 2013, NVIDIA Corporation. + * + * Author: Laxman Dewangan <ldewangan@nvidia.com> + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License as + * published by the Free Software Foundation version 2. + * + * This program is distributed "as is" WITHOUT ANY WARRANTY of any kind, + * whether express or implied; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA + * 02111-1307, USA + */ + +#include <linux/clk.h> +#include <linux/clkdev.h> +#include <linux/clk-provider.h> +#include <linux/mfd/palmas.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/of_device.h> +#include <linux/platform_device.h> +#include <linux/slab.h> + +enum PALMAS_CLOCK32K { + PALMAS_CLOCK32KG, + PALMAS_CLOCK32KG_AUDIO, + + /* Last entry */ + PALMAS_CLOCK32K_NR, +}; + +#define PALMAS_CLOCK_DT_EXT_CONTROL_ENABLE1 1 +#define PALMAS_CLOCK_DT_EXT_CONTROL_ENABLE2 2 +#define PALMAS_CLOCK_DT_EXT_CONTROL_NSLEEP 3 + +struct palmas_clks; + +struct palmas_clk32k_desc { + const char *clk_name; + unsigned int control_reg; + unsigned int enable_mask; + unsigned int sleep_mask; + unsigned int sleep_reqstr_id; +}; + +struct palmas_clock_info { + struct clk *clk; + struct clk_hw hw; + struct palmas_clk32k_desc *clk_desc; + struct palmas_clks *palmas_clk; + int ext_control_pin; + bool boot_enable; +}; + +struct palmas_clks { + struct device *dev; + struct palmas *palmas; + struct clk_onecell_data clk_data; + struct palmas_clock_info clk_info[PALMAS_CLOCK32K_NR]; +}; + +static struct palmas_clk32k_desc palmas_clk32k_descs[] = { + { + .clk_name = "clk32k_kg", + .control_reg = PALMAS_CLK32KG_CTRL, + .enable_mask = PALMAS_CLK32KG_CTRL_MODE_ACTIVE, + .sleep_mask = PALMAS_CLK32KG_CTRL_MODE_SLEEP, + .sleep_reqstr_id = PALMAS_EXTERNAL_REQSTR_ID_CLK32KG, + }, { + .clk_name = "clk32k_kg_audio", + .control_reg = PALMAS_CLK32KGAUDIO_CTRL, + .enable_mask = PALMAS_CLK32KG_CTRL_MODE_ACTIVE, + .sleep_mask = PALMAS_CLK32KG_CTRL_MODE_SLEEP, + .sleep_reqstr_id = PALMAS_EXTERNAL_REQSTR_ID_CLK32KGAUDIO, + }, +}; + +static inline struct palmas_clock_info *to_palmas_clks_info(struct clk_hw *hw) +{ + return container_of(hw, struct palmas_clock_info, hw); +} + +static unsigned long palmas_clks_recalc_rate(struct clk_hw *hw, + unsigned long parent_rate) +{ + return 32768; +} + +static int palmas_clks_prepare(struct clk_hw *hw) +{ + struct palmas_clock_info *cinfo = to_palmas_clks_info(hw); + struct palmas_clks *palmas_clks = cinfo->palmas_clk; + int ret; + + ret = palmas_update_bits(palmas_clks->palmas, PALMAS_RESOURCE_BASE, + cinfo->clk_desc->control_reg, + cinfo->clk_desc->enable_mask, + cinfo->clk_desc->enable_mask); + if (ret < 0) + dev_err(palmas_clks->dev, "Reg 0x%02x update failed, %d\n", + cinfo->clk_desc->control_reg, ret); + + return ret; +} + +static void palmas_clks_unprepare(struct clk_hw *hw) +{ + struct palmas_clock_info *cinfo = to_palmas_clks_info(hw); + struct palmas_clks *palmas_clks = cinfo->palmas_clk; + int ret; + + /* + * Clock can be disabled through external pin if it is externally + * controlled. + */ + if (cinfo->ext_control_pin) + return; + + ret = palmas_update_bits(palmas_clks->palmas, PALMAS_RESOURCE_BASE, + cinfo->clk_desc->control_reg, + cinfo->clk_desc->enable_mask, 0); + if (ret < 0) + dev_err(palmas_clks->dev, "Reg 0x%02x update failed, %d\n", + cinfo->clk_desc->control_reg, ret); + +} + +static int palmas_clks_is_prepared(struct clk_hw *hw) +{ + struct palmas_clock_info *cinfo = to_palmas_clks_info(hw); + struct palmas_clks *palmas_clks = cinfo->palmas_clk; + int ret; + u32 val; + + if (cinfo->ext_control_pin) + return 1; + + ret = palmas_read(palmas_clks->palmas, PALMAS_RESOURCE_BASE, + cinfo->clk_desc->control_reg, &val); + if (ret < 0) { + dev_err(palmas_clks->dev, "Reg 0x%02x read failed, %d\n", + cinfo->clk_desc->control_reg, ret); + return ret; + } + return !!(val & cinfo->clk_desc->enable_mask); +} + +static struct clk_ops palmas_clks_ops = { + .prepare = palmas_clks_prepare, + .unprepare = palmas_clks_unprepare, + .is_prepared = palmas_clks_is_prepared, + .recalc_rate = palmas_clks_recalc_rate, +}; + +static struct clk_init_data palmas_clks_hw_init[PALMAS_CLOCK32K_NR] = { + [PALMAS_CLOCK32KG] = { + .name = "clk32k_kg", + .ops = &palmas_clks_ops, + .flags = CLK_IS_ROOT | CLK_IGNORE_UNUSED, + }, + [PALMAS_CLOCK32KG_AUDIO] = { + .name = "clk32k_kg_audio", + .ops = &palmas_clks_ops, + .flags = CLK_IS_ROOT | CLK_IGNORE_UNUSED, + }, +}; + +static int palmas_clks_get_clk_data(struct platform_device *pdev, + struct palmas_clks *palmas_clks) +{ + struct device_node *node = pdev->dev.of_node; + struct device_node *child; + struct palmas_clock_info *cinfo; + unsigned int prop; + int ret; + int i; + + for (i = 0; i < PALMAS_CLOCK32K_NR; ++i) { + child = of_get_child_by_name(node, + palmas_clk32k_descs[i].clk_name); + if (!child) + continue; + + cinfo = &palmas_clks->clk_info[i]; + cinfo->boot_enable = of_property_read_bool(child, + "ti,clock-boot-enable"); + ret = of_property_read_u32(child, "ti,external-sleep-control", + &prop); + if (!ret) { + switch (prop) { + case PALMAS_CLOCK_DT_EXT_CONTROL_ENABLE1: + prop = PALMAS_EXT_CONTROL_ENABLE1; + break; + case PALMAS_CLOCK_DT_EXT_CONTROL_ENABLE2: + prop = PALMAS_EXT_CONTROL_ENABLE2; + break; + case PALMAS_CLOCK_DT_EXT_CONTROL_NSLEEP: + prop = PALMAS_EXT_CONTROL_NSLEEP; + break; + default: + WARN_ON(1); + dev_warn(&pdev->dev, + "%s: Invalid ext control option: %u\n", + child->name, prop); + prop = 0; + break; + } + cinfo->ext_control_pin = prop; + } + } + + return 0; +} + +static int palmas_clks_init_configure(struct palmas_clock_info *cinfo) +{ + struct palmas_clks *palmas_clks = cinfo->palmas_clk; + int ret; + + if (cinfo->boot_enable || cinfo->ext_control_pin) { + ret = clk_prepare(cinfo->clk); + if (ret < 0) { + dev_err(palmas_clks->dev, + "Clock prep failed, %d\n", ret); + return ret; + } + } + + ret = palmas_update_bits(palmas_clks->palmas, PALMAS_RESOURCE_BASE, + cinfo->clk_desc->control_reg, + cinfo->clk_desc->sleep_mask, 0); + if (ret < 0) { + dev_err(palmas_clks->dev, "Reg 0x%02x update failed, %d\n", + cinfo->clk_desc->control_reg, ret); + return ret; + } + + if (cinfo->ext_control_pin) { + ret = palmas_ext_control_req_config(palmas_clks->palmas, + cinfo->clk_desc->sleep_reqstr_id, + cinfo->ext_control_pin, true); + if (ret < 0) { + dev_err(palmas_clks->dev, + "Ext config for %s failed, %d\n", + cinfo->clk_desc->clk_name, ret); + return ret; + } + } + + return ret; +} + +static int palmas_clks_probe(struct platform_device *pdev) +{ + struct palmas *palmas = dev_get_drvdata(pdev->dev.parent); + struct palmas_clock_info *cinfo; + struct palmas_clks *palmas_clks; + struct clk *clk; + int i, ret; + + palmas_clks = devm_kzalloc(&pdev->dev, sizeof(*palmas_clks), + GFP_KERNEL); + if (!palmas_clks) + return -ENOMEM; + + palmas_clks->clk_data.clks = devm_kzalloc(&pdev->dev, + PALMAS_CLOCK32K_NR * sizeof(palmas_clks->clk_data.clks), + GFP_KERNEL); + if (!palmas_clks->clk_data.clks) + return -ENOMEM; + + palmas_clks_get_clk_data(pdev, palmas_clks); + platform_set_drvdata(pdev, palmas_clks); + + palmas_clks->dev = &pdev->dev; + palmas_clks->palmas = palmas; + + for (i = 0; i < PALMAS_CLOCK32K_NR; i++) { + cinfo = &palmas_clks->clk_info[i]; + cinfo->clk_desc = &palmas_clk32k_descs[i]; + cinfo->hw.init = &palmas_clks_hw_init[i]; + cinfo->palmas_clk = palmas_clks; + clk = devm_clk_register(&pdev->dev, &cinfo->hw); + if (IS_ERR(clk)) { + ret = PTR_ERR(clk); + dev_err(&pdev->dev, "Fail to register clock %s, %d\n", + palmas_clk32k_descs[i].clk_name, ret); + return ret; + } + + cinfo->clk = clk; + palmas_clks->clk_data.clks[i] = clk; + palmas_clks->clk_data.clk_num++; + palmas_clks_init_configure(cinfo); + } + + ret = of_clk_add_provider(pdev->dev.of_node, of_clk_src_simple_get, + &palmas_clks->clk_data); + if (ret < 0) + dev_err(&pdev->dev, "Fail to add clock driver, %d\n", ret); + return ret; +} + +static int palmas_clks_remove(struct platform_device *pdev) +{ + of_clk_del_provider(pdev->dev.of_node); + return 0; +} + +static struct of_device_id of_palmas_clks_match_tbl[] = { + { .compatible = "ti,palmas-clk", }, + {}, +}; +MODULE_DEVICE_TABLE(of, of_palmas_clks_match_tbl); + +static struct platform_driver palmas_clks_driver = { + .driver = { + .name = "palmas-clk", + .owner = THIS_MODULE, + .of_match_table = of_palmas_clks_match_tbl, + }, + .probe = palmas_clks_probe, + .remove = palmas_clks_remove, +}; + +module_platform_driver(palmas_clks_driver); + +MODULE_DESCRIPTION("Clock driver for Palmas Series Devices"); +MODULE_ALIAS("platform:palmas-clk"); +MODULE_AUTHOR("Laxman Dewangan <ldewangan@nvidia.com>"); +MODULE_LICENSE("GPL v2");
Palmas devices has two clock output CLK32K_KG and CLK32K_KG_AUDIO which can be enable/disable through software. Add clock driver support for the Palmas 32KHz clocks. Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com> --- Changes from V1: - Call prepare if the clk is needed for boot-enable or sleep control. - change is_enabled to is_prepared. - Added ops palmas_clks_recalc_rate. - Added of_clk_add_provider(). Changes from V2: - Convert magic number to defines. .../devicetree/bindings/clock/clk-palmas.txt | 45 +++ drivers/clk/Kconfig | 7 + drivers/clk/Makefile | 1 + drivers/clk/clk-palmas.c | 340 ++++++++++++++++++++ 4 files changed, 393 insertions(+), 0 deletions(-) create mode 100644 Documentation/devicetree/bindings/clock/clk-palmas.txt create mode 100644 drivers/clk/clk-palmas.c