| Message ID | 1409062589-12157-1-git-send-email-zyw@rock-chips.com |
|---|---|
| State | Superseded |
| Headers | show |
Chris, On Tue, Aug 26, 2014 at 7:16 AM, Chris Zhong <zyw@rock-chips.com> wrote: > Signed-off-by: Chris Zhong <zyw@rock-chips.com> > > --- > > Changes in v6: > Adviced by doug > - use correct argument call of_clk_add_provider in probe > > Changes in v5: > Adviced by doug > - add some error checking in probe > - move "rockchip,rk808.h" into the patch about dt-bindings > > Changes in v4: > Adviced by doug > - add "clock-output-names" propertiey > - add a header file "rockchip,rk808.h" > > Changes in v3: > - fix compile err > > Changes in v2: > Adviced by javier.martinez > - separated from rtc-rk808. > > drivers/clk/Kconfig | 9 +++ > drivers/clk/Makefile | 1 + > drivers/clk/clk-rk808.c | 163 +++++++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 173 insertions(+) > create mode 100644 drivers/clk/clk-rk808.c > > diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig > index cfd3af7..84e0590 100644 > --- a/drivers/clk/Kconfig > +++ b/drivers/clk/Kconfig > @@ -38,6 +38,15 @@ config COMMON_CLK_MAX77686 > ---help--- > This driver supports Maxim 77686 crystal oscillator clock. > > +config COMMON_CLK_RK808 > + tristate "Clock driver for RK808" > + depends on MFD_RK808 > + ---help--- > + This driver supports RK808 crystal oscillator clock. These > + multi-function devices have two fixed-rate oscillators, > + clocked at 32KHz each. Clkout1 is always on, Clkout2 can off > + by control register. > + > config COMMON_CLK_SI5351 > tristate "Clock driver for SiLabs 5351A/B/C" > depends on I2C > diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile > index f537a0b..99f53d5 100644 > --- a/drivers/clk/Makefile > +++ b/drivers/clk/Makefile > @@ -28,6 +28,7 @@ obj-$(CONFIG_ARCH_NOMADIK) += clk-nomadik.o > obj-$(CONFIG_ARCH_NSPIRE) += clk-nspire.o > obj-$(CONFIG_COMMON_CLK_PALMAS) += clk-palmas.o > obj-$(CONFIG_CLK_PPC_CORENET) += clk-ppc-corenet.o > +obj-$(CONFIG_COMMON_CLK_RK808) += clk-rk808.o > obj-$(CONFIG_COMMON_CLK_S2MPS11) += clk-s2mps11.o > obj-$(CONFIG_COMMON_CLK_SI5351) += clk-si5351.o > obj-$(CONFIG_COMMON_CLK_SI570) += clk-si570.o > diff --git a/drivers/clk/clk-rk808.c b/drivers/clk/clk-rk808.c > new file mode 100644 > index 0000000..21f8b54 > --- /dev/null > +++ b/drivers/clk/clk-rk808.c > @@ -0,0 +1,163 @@ > +/* > + * Clkout driver for Rockchip RK808 > + * > + * Copyright (c) 2014, Fuzhou Rockchip Electronics Co., Ltd > + * > + * Author: Chris Zhong <zyw@rock-chips.com> I probably would have removed "Author" here (like in other patches) since it's below in MODULE_AUTHOR. ...but I'm not a huge stickler for it. > + * > + * This program is free software; you can redistribute it and/or modify it > + * under the terms and conditions of the GNU General Public License, > + * version 2, as published by the Free Software Foundation. > + * > + * This program is distributed in the hope it will be useful, but WITHOUT > + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or > + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for > + * more details. > + */ > + > +#include <linux/clk.h> > +#include <linux/clk-provider.h> > +#include <linux/module.h> > +#include <linux/slab.h> > +#include <linux/platform_device.h> > +#include <linux/mfd/rk808.h> > +#include <linux/i2c.h> > + > +struct rk808_clkout { > + struct rk808 *rk808; > + struct clk_onecell_data clk_data; > + struct clk_hw clkout1_hw; > + struct clk_hw clkout2_hw; > +}; > + > +static unsigned long rk808_clkout_recalc_rate(struct clk_hw *hw, > + unsigned long parent_rate) > +{ > + return 32768; > +} > + > +static int rk808_clkout1_is_prepared(struct clk_hw *hw) > +{ > + return 1; > +} > + > +static int rk808_clkout2_control(struct clk_hw *hw, bool enable) > +{ > + struct rk808_clkout *rk808_clkout = container_of(hw, > + struct rk808_clkout, > + clkout2_hw); > + struct rk808 *rk808 = rk808_clkout->rk808; > + > + return regmap_update_bits(rk808->regmap, RK808_CLK32OUT_REG, > + CLK32KOUT2_EN, enable ? CLK32KOUT2_EN : 0); > +} > + > +static int rk808_clkout2_prepare(struct clk_hw *hw) > +{ > + return rk808_clkout2_control(hw, 1); nit: I'd pass "true" instead of 1. I should have noticed earlier. > +} > + > +static void rk808_clkout2_unprepare(struct clk_hw *hw) > +{ > + rk808_clkout2_control(hw, 0); nit: I'd pass "false" instead of 0. > +} > + > +static int rk808_clkout2_is_prepared(struct clk_hw *hw) > +{ > + struct rk808_clkout *rk808_clkout = container_of(hw, > + struct rk808_clkout, > + clkout2_hw); > + struct rk808 *rk808 = rk808_clkout->rk808; > + uint32_t val; > + > + int ret = regmap_read(rk808->regmap, RK808_CLK32OUT_REG, &val); > + > + if (ret < 0) > + return ret; > + > + return (val & CLK32KOUT2_EN) ? 1 : 0; > +} > + > +static const struct clk_ops rk808_clkout1_ops = { > + .is_prepared = rk808_clkout1_is_prepared, > + .recalc_rate = rk808_clkout_recalc_rate, > +}; > + > +static const struct clk_ops rk808_clkout2_ops = { > + .prepare = rk808_clkout2_prepare, > + .unprepare = rk808_clkout2_unprepare, > + .is_prepared = rk808_clkout2_is_prepared, > + .recalc_rate = rk808_clkout_recalc_rate, > +}; > + > +static int rk808_clkout_probe(struct platform_device *pdev) > +{ > + struct rk808 *rk808 = dev_get_drvdata(pdev->dev.parent); > + struct i2c_client *client = rk808->i2c; > + struct device_node *node = client->dev.of_node; > + struct clk_init_data init = {}; > + struct clk **clk_table; > + struct rk808_clkout *rk808_clkout; > + > + rk808_clkout = devm_kzalloc(&client->dev, > + sizeof(*rk808_clkout), GFP_KERNEL); > + if (!rk808_clkout) > + return -ENOMEM; > + > + rk808_clkout->rk808 = rk808; > + > + clk_table = devm_kzalloc(&client->dev, > + 2*sizeof(struct clk *), GFP_KERNEL); nit: space around "*". > + if (!clk_table) > + return -ENOMEM; > + > + init.flags = CLK_IS_ROOT; > + init.parent_names = NULL; > + init.num_parents = 0; > + init.name = "rk808-clkout1"; > + init.ops = &rk808_clkout1_ops; > + rk808_clkout->clkout1_hw.init = &init; > + > + /* optional override of the clockname */ > + of_property_read_string_index(node, "clock-output-names", > + 0, &init.name); > + > + clk_table[0] = devm_clk_register(&client->dev, > + &rk808_clkout->clkout1_hw); > + if (IS_ERR(clk_table[0])) > + return PTR_ERR(clk_table[0]); > + > + init.name = "rk808-clkout2"; > + init.ops = &rk808_clkout2_ops; > + rk808_clkout->clkout2_hw.init = &init; > + > + /* optional override of the clockname */ > + of_property_read_string_index(node, "clock-output-names", > + 1, &init.name); > + > + clk_table[1] = devm_clk_register(&client->dev, > + &rk808_clkout->clkout2_hw); > + if (IS_ERR(clk_table[1])) > + return PTR_ERR(clk_table[1]); > + > + rk808_clkout->clk_data.clks = clk_table; > + rk808_clkout->clk_data.clk_num = 2; > + > + return of_clk_add_provider(node, of_clk_src_onecell_get, > + &rk808_clkout->clk_data); > +} > + > +static struct platform_driver rk808_clkout_driver = { > + .probe = rk808_clkout_probe, > + .driver = { > + .name = "rk808-clkout", > + .owner = THIS_MODULE, nit: you could remove "THIS_MODULE" as Lee suggested in another patch. > + }, > +}; > + > +module_platform_driver(rk808_clkout_driver); > + > +MODULE_DESCRIPTION("Clkout driver for the rk808 series PMICs"); > +MODULE_AUTHOR("Chris Zhong <zyw@rock-chips.com>"); > +MODULE_LICENSE("GPL"); > +MODULE_ALIAS("platform:rk808-clkout"); Nothing above is really important and I've tested that this driver works both as a clock source for clocks on rk3288 and for use by clock clients, so feel free to add my: Reviewed-by: Doug Anderson <dianders@chromium.org> Tested-by: Doug Anderson <dianders@chromium.org>
On 26/08/14 19:02, Doug Anderson wrote: > Chris, > > On Tue, Aug 26, 2014 at 7:16 AM, Chris Zhong <zyw@rock-chips.com> wrote: >> Signed-off-by: Chris Zhong <zyw@rock-chips.com> >> >> --- >> >> Changes in v6: >> Adviced by doug >> - use correct argument call of_clk_add_provider in probe >> >> Changes in v5: >> Adviced by doug >> - add some error checking in probe >> - move "rockchip,rk808.h" into the patch about dt-bindings >> >> Changes in v4: >> Adviced by doug >> - add "clock-output-names" propertiey >> - add a header file "rockchip,rk808.h" >> >> Changes in v3: >> - fix compile err >> >> Changes in v2: >> Adviced by javier.martinez >> - separated from rtc-rk808. >> >> drivers/clk/Kconfig | 9 +++ >> drivers/clk/Makefile | 1 + >> drivers/clk/clk-rk808.c | 163 +++++++++++++++++++++++++++++++++++++++++++++++ >> 3 files changed, 173 insertions(+) >> create mode 100644 drivers/clk/clk-rk808.c >> >> diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig >> index cfd3af7..84e0590 100644 >> --- a/drivers/clk/Kconfig >> +++ b/drivers/clk/Kconfig >> @@ -38,6 +38,15 @@ config COMMON_CLK_MAX77686 >> ---help--- >> This driver supports Maxim 77686 crystal oscillator clock. >> >> +config COMMON_CLK_RK808 >> + tristate "Clock driver for RK808" >> + depends on MFD_RK808 >> + ---help--- >> + This driver supports RK808 crystal oscillator clock. These >> + multi-function devices have two fixed-rate oscillators, >> + clocked at 32KHz each. Clkout1 is always on, Clkout2 can off >> + by control register. >> + >> config COMMON_CLK_SI5351 >> tristate "Clock driver for SiLabs 5351A/B/C" >> depends on I2C >> diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile >> index f537a0b..99f53d5 100644 >> --- a/drivers/clk/Makefile >> +++ b/drivers/clk/Makefile >> @@ -28,6 +28,7 @@ obj-$(CONFIG_ARCH_NOMADIK) += clk-nomadik.o >> obj-$(CONFIG_ARCH_NSPIRE) += clk-nspire.o >> obj-$(CONFIG_COMMON_CLK_PALMAS) += clk-palmas.o >> obj-$(CONFIG_CLK_PPC_CORENET) += clk-ppc-corenet.o >> +obj-$(CONFIG_COMMON_CLK_RK808) += clk-rk808.o >> obj-$(CONFIG_COMMON_CLK_S2MPS11) += clk-s2mps11.o >> obj-$(CONFIG_COMMON_CLK_SI5351) += clk-si5351.o >> obj-$(CONFIG_COMMON_CLK_SI570) += clk-si570.o >> diff --git a/drivers/clk/clk-rk808.c b/drivers/clk/clk-rk808.c >> new file mode 100644 >> index 0000000..21f8b54 >> --- /dev/null >> +++ b/drivers/clk/clk-rk808.c >> @@ -0,0 +1,163 @@ >> +/* >> + * Clkout driver for Rockchip RK808 >> + * >> + * Copyright (c) 2014, Fuzhou Rockchip Electronics Co., Ltd >> + * >> + * Author: Chris Zhong <zyw@rock-chips.com> > > I probably would have removed "Author" here (like in other patches) > since it's below in MODULE_AUTHOR. ...but I'm not a huge stickler for > it. > > > >> + * >> + * This program is free software; you can redistribute it and/or modify it >> + * under the terms and conditions of the GNU General Public License, >> + * version 2, as published by the Free Software Foundation. >> + * >> + * This program is distributed in the hope it will be useful, but WITHOUT >> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or >> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for >> + * more details. >> + */ >> + >> +#include <linux/clk.h> >> +#include <linux/clk-provider.h> >> +#include <linux/module.h> >> +#include <linux/slab.h> >> +#include <linux/platform_device.h> >> +#include <linux/mfd/rk808.h> >> +#include <linux/i2c.h> >> + >> +struct rk808_clkout { >> + struct rk808 *rk808; >> + struct clk_onecell_data clk_data; >> + struct clk_hw clkout1_hw; >> + struct clk_hw clkout2_hw; >> +}; >> + >> +static unsigned long rk808_clkout_recalc_rate(struct clk_hw *hw, >> + unsigned long parent_rate) >> +{ >> + return 32768; >> +} >> + Given one of these is a fixed, ungatable clock then surely it would be easier to use a separate fixed clock node for that and then just have a none for the gated clock?
Am Mittwoch, 27. August 2014, 12:22:01 schrieb Ben Dooks: > >> diff --git a/drivers/clk/clk-rk808.c b/drivers/clk/clk-rk808.c > >> new file mode 100644 > >> index 0000000..21f8b54 > >> --- /dev/null > >> +++ b/drivers/clk/clk-rk808.c > >> @@ -0,0 +1,163 @@ > >> +/* > >> + * Clkout driver for Rockchip RK808 > >> + * > >> + * Copyright (c) 2014, Fuzhou Rockchip Electronics Co., Ltd > >> + * > >> + * Author: Chris Zhong <zyw@rock-chips.com> > > > > I probably would have removed "Author" here (like in other patches) > > since it's below in MODULE_AUTHOR. ...but I'm not a huge stickler for > > it. > > > >> + * > >> + * This program is free software; you can redistribute it and/or modify > >> it > >> + * under the terms and conditions of the GNU General Public License, > >> + * version 2, as published by the Free Software Foundation. > >> + * > >> + * This program is distributed in the hope it will be useful, but > >> WITHOUT > >> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or > >> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License > >> for + * more details. > >> + */ > >> + > >> +#include <linux/clk.h> > >> +#include <linux/clk-provider.h> > >> +#include <linux/module.h> > >> +#include <linux/slab.h> > >> +#include <linux/platform_device.h> > >> +#include <linux/mfd/rk808.h> > >> +#include <linux/i2c.h> > >> + > >> +struct rk808_clkout { > >> + struct rk808 *rk808; > >> + struct clk_onecell_data clk_data; > >> + struct clk_hw clkout1_hw; > >> + struct clk_hw clkout2_hw; > >> +}; > >> + > >> +static unsigned long rk808_clkout_recalc_rate(struct clk_hw *hw, > >> + unsigned long parent_rate) > >> +{ > >> + return 32768; > >> +} > >> + > > Given one of these is a fixed, ungatable clock then surely it would > be easier to use a separate fixed clock node for that and then just > have a none for the gated clock? Hmm, with the devicetree being supposed to describe the hardware, I guess this is debatable. As both clocks are provided by the rk808 chip, I guess the clock references for both also should point there and not some arbitary fixed clock somewhere else in the dt? But this view could also be to pendantic :-) . Maybe the dt maintainers can shed some light on this.
diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig index cfd3af7..84e0590 100644 --- a/drivers/clk/Kconfig +++ b/drivers/clk/Kconfig @@ -38,6 +38,15 @@ config COMMON_CLK_MAX77686 ---help--- This driver supports Maxim 77686 crystal oscillator clock. +config COMMON_CLK_RK808 + tristate "Clock driver for RK808" + depends on MFD_RK808 + ---help--- + This driver supports RK808 crystal oscillator clock. These + multi-function devices have two fixed-rate oscillators, + clocked at 32KHz each. Clkout1 is always on, Clkout2 can off + by control register. + config COMMON_CLK_SI5351 tristate "Clock driver for SiLabs 5351A/B/C" depends on I2C diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile index f537a0b..99f53d5 100644 --- a/drivers/clk/Makefile +++ b/drivers/clk/Makefile @@ -28,6 +28,7 @@ obj-$(CONFIG_ARCH_NOMADIK) += clk-nomadik.o obj-$(CONFIG_ARCH_NSPIRE) += clk-nspire.o obj-$(CONFIG_COMMON_CLK_PALMAS) += clk-palmas.o obj-$(CONFIG_CLK_PPC_CORENET) += clk-ppc-corenet.o +obj-$(CONFIG_COMMON_CLK_RK808) += clk-rk808.o obj-$(CONFIG_COMMON_CLK_S2MPS11) += clk-s2mps11.o obj-$(CONFIG_COMMON_CLK_SI5351) += clk-si5351.o obj-$(CONFIG_COMMON_CLK_SI570) += clk-si570.o diff --git a/drivers/clk/clk-rk808.c b/drivers/clk/clk-rk808.c new file mode 100644 index 0000000..21f8b54 --- /dev/null +++ b/drivers/clk/clk-rk808.c @@ -0,0 +1,163 @@ +/* + * Clkout driver for Rockchip RK808 + * + * Copyright (c) 2014, Fuzhou Rockchip Electronics Co., Ltd + * + * Author: Chris Zhong <zyw@rock-chips.com> + * + * This program is free software; you can redistribute it and/or modify it + * under the terms and conditions of the GNU General Public License, + * version 2, as published by the Free Software Foundation. + * + * This program is distributed in the hope it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + */ + +#include <linux/clk.h> +#include <linux/clk-provider.h> +#include <linux/module.h> +#include <linux/slab.h> +#include <linux/platform_device.h> +#include <linux/mfd/rk808.h> +#include <linux/i2c.h> + +struct rk808_clkout { + struct rk808 *rk808; + struct clk_onecell_data clk_data; + struct clk_hw clkout1_hw; + struct clk_hw clkout2_hw; +}; + +static unsigned long rk808_clkout_recalc_rate(struct clk_hw *hw, + unsigned long parent_rate) +{ + return 32768; +} + +static int rk808_clkout1_is_prepared(struct clk_hw *hw) +{ + return 1; +} + +static int rk808_clkout2_control(struct clk_hw *hw, bool enable) +{ + struct rk808_clkout *rk808_clkout = container_of(hw, + struct rk808_clkout, + clkout2_hw); + struct rk808 *rk808 = rk808_clkout->rk808; + + return regmap_update_bits(rk808->regmap, RK808_CLK32OUT_REG, + CLK32KOUT2_EN, enable ? CLK32KOUT2_EN : 0); +} + +static int rk808_clkout2_prepare(struct clk_hw *hw) +{ + return rk808_clkout2_control(hw, 1); +} + +static void rk808_clkout2_unprepare(struct clk_hw *hw) +{ + rk808_clkout2_control(hw, 0); +} + +static int rk808_clkout2_is_prepared(struct clk_hw *hw) +{ + struct rk808_clkout *rk808_clkout = container_of(hw, + struct rk808_clkout, + clkout2_hw); + struct rk808 *rk808 = rk808_clkout->rk808; + uint32_t val; + + int ret = regmap_read(rk808->regmap, RK808_CLK32OUT_REG, &val); + + if (ret < 0) + return ret; + + return (val & CLK32KOUT2_EN) ? 1 : 0; +} + +static const struct clk_ops rk808_clkout1_ops = { + .is_prepared = rk808_clkout1_is_prepared, + .recalc_rate = rk808_clkout_recalc_rate, +}; + +static const struct clk_ops rk808_clkout2_ops = { + .prepare = rk808_clkout2_prepare, + .unprepare = rk808_clkout2_unprepare, + .is_prepared = rk808_clkout2_is_prepared, + .recalc_rate = rk808_clkout_recalc_rate, +}; + +static int rk808_clkout_probe(struct platform_device *pdev) +{ + struct rk808 *rk808 = dev_get_drvdata(pdev->dev.parent); + struct i2c_client *client = rk808->i2c; + struct device_node *node = client->dev.of_node; + struct clk_init_data init = {}; + struct clk **clk_table; + struct rk808_clkout *rk808_clkout; + + rk808_clkout = devm_kzalloc(&client->dev, + sizeof(*rk808_clkout), GFP_KERNEL); + if (!rk808_clkout) + return -ENOMEM; + + rk808_clkout->rk808 = rk808; + + clk_table = devm_kzalloc(&client->dev, + 2*sizeof(struct clk *), GFP_KERNEL); + if (!clk_table) + return -ENOMEM; + + init.flags = CLK_IS_ROOT; + init.parent_names = NULL; + init.num_parents = 0; + init.name = "rk808-clkout1"; + init.ops = &rk808_clkout1_ops; + rk808_clkout->clkout1_hw.init = &init; + + /* optional override of the clockname */ + of_property_read_string_index(node, "clock-output-names", + 0, &init.name); + + clk_table[0] = devm_clk_register(&client->dev, + &rk808_clkout->clkout1_hw); + if (IS_ERR(clk_table[0])) + return PTR_ERR(clk_table[0]); + + init.name = "rk808-clkout2"; + init.ops = &rk808_clkout2_ops; + rk808_clkout->clkout2_hw.init = &init; + + /* optional override of the clockname */ + of_property_read_string_index(node, "clock-output-names", + 1, &init.name); + + clk_table[1] = devm_clk_register(&client->dev, + &rk808_clkout->clkout2_hw); + if (IS_ERR(clk_table[1])) + return PTR_ERR(clk_table[1]); + + rk808_clkout->clk_data.clks = clk_table; + rk808_clkout->clk_data.clk_num = 2; + + return of_clk_add_provider(node, of_clk_src_onecell_get, + &rk808_clkout->clk_data); +} + +static struct platform_driver rk808_clkout_driver = { + .probe = rk808_clkout_probe, + .driver = { + .name = "rk808-clkout", + .owner = THIS_MODULE, + }, +}; + +module_platform_driver(rk808_clkout_driver); + +MODULE_DESCRIPTION("Clkout driver for the rk808 series PMICs"); +MODULE_AUTHOR("Chris Zhong <zyw@rock-chips.com>"); +MODULE_LICENSE("GPL"); +MODULE_ALIAS("platform:rk808-clkout");
Signed-off-by: Chris Zhong <zyw@rock-chips.com> --- Changes in v6: Adviced by doug - use correct argument call of_clk_add_provider in probe Changes in v5: Adviced by doug - add some error checking in probe - move "rockchip,rk808.h" into the patch about dt-bindings Changes in v4: Adviced by doug - add "clock-output-names" propertiey - add a header file "rockchip,rk808.h" Changes in v3: - fix compile err Changes in v2: Adviced by javier.martinez - separated from rtc-rk808. drivers/clk/Kconfig | 9 +++ drivers/clk/Makefile | 1 + drivers/clk/clk-rk808.c | 163 +++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 173 insertions(+) create mode 100644 drivers/clk/clk-rk808.c