diff mbox series

[v6,10/10] clk: bd718x7: Initial support for ROHM bd71837/bd71847 PMIC clock

Message ID b1a7aa06389ccd46f60db3e63bdf1e9ce9781d49.1543922403.git.matti.vaittinen@fi.rohmeurope.com (mailing list archive)
State New, archived
Headers show
Series clk: clkdev/of_clk - add managed lookup and provider registrations | expand

Commit Message

Vaittinen, Matti Dec. 4, 2018, 11:39 a.m. UTC 
ROHM bd71837 and bd71847 contain 32768Hz clock gate. Support the clock
using generic clock framework. Note, only bd71837 is tested but bd71847
should be identical what comes to clk parts.

Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
---
 drivers/clk/Kconfig       |   7 +++
 drivers/clk/Makefile      |   1 +
 drivers/clk/clk-bd718x7.c | 131 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 139 insertions(+)
 create mode 100644 drivers/clk/clk-bd718x7.c

Comments

Stephen Boyd Dec. 5, 2018, 5:28 p.m. UTC | #1 
Quoting Matti Vaittinen (2018-12-04 03:39:38)
> diff --git a/drivers/clk/clk-bd718x7.c b/drivers/clk/clk-bd718x7.c
> new file mode 100644
> index 000000000000..d486859526ed
> --- /dev/null
> +++ b/drivers/clk/clk-bd718x7.c
> @@ -0,0 +1,131 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright (C) 2018 ROHM Semiconductors
> +// bd71837.c  -- ROHM BD71837MWV clock driver

This isn't even the name of the file. Please remove. Also, only the SPDX
tag is supposed to have // on it and the other things should be normal
/* */ comment style.

> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/err.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +#include <linux/mfd/rohm-bd718x7.h>
> +#include <linux/clk-provider.h>
> +#include <linux/clkdev.h>
> +#include <linux/regmap.h>
[....]
> +
> +static int bd71837_clk_probe(struct platform_device *pdev)
> +{
> +       struct bd718xx_clk *c;
> +       int rval = -ENOMEM;
> +       const char *parent_clk;
> +       struct device *parent = pdev->dev.parent;
> +       struct bd718xx *mfd = dev_get_drvdata(parent);
> +       struct clk_init_data init = {
> +               .name = "bd718xx-32k-out",
> +               .ops = &bd71837_clk_ops,
> +       };
> +
> +       c = devm_kzalloc(&pdev->dev, sizeof(*c), GFP_KERNEL);
> +       if (!c)
> +               return -ENOMEM;
> +
> +       init.num_parents = 1;
> +       parent_clk = of_clk_get_parent_name(parent->of_node, 0);
> +
> +       init.parent_names = &parent_clk;
> +       if (!parent_clk) {
> +               dev_err(&pdev->dev, "No parent clk found\n");
> +               return -EINVAL;
> +       }
> +
> +       c->reg = BD718XX_REG_OUT32K;
> +       c->mask = BD718XX_OUT32K_EN;
> +       c->mfd = mfd;
> +       c->pdev = pdev;
> +       c->hw.init = &init;
> +
> +       of_property_read_string_index(parent->of_node,
> +                                     "clock-output-names", 0, &init.name);
> +
> +       rval = devm_clk_hw_register(&pdev->dev, &c->hw);
> +       if (!rval) {
> +               rval = devm_clk_hw_register_clkdev(&pdev->dev,
> +                                                  &c->hw, init.name, NULL);

Do you plan to use the clkdev lookup? This driver looks fairly DT
dependent, so I would prefer to remove the clkdev part and only add it
later if anyone needs it.

> +               if (rval)
> +                       dev_warn(&pdev->dev, "Failed to register clkdev\n");
> +               if (parent->of_node) {
> +                       rval = devm_of_clk_add_hw_provider(&pdev->dev,
> +                                            of_clk_hw_simple_get, &c->hw);
> +                       if (rval)
> +                               dev_err(&pdev->dev,
> +                                       "adding clk provider failed\n");
> +               }
Vaittinen, Matti Dec. 5, 2018, 6:24 p.m. UTC | #2 
Thanks again Stephen.

On Wed, Dec 05, 2018 at 09:28:21AM -0800, Stephen Boyd wrote:
> Quoting Matti Vaittinen (2018-12-04 03:39:38)
> > diff --git a/drivers/clk/clk-bd718x7.c b/drivers/clk/clk-bd718x7.c
> > new file mode 100644
> > index 000000000000..d486859526ed
> > --- /dev/null
> > +++ b/drivers/clk/clk-bd718x7.c
> > @@ -0,0 +1,131 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +// Copyright (C) 2018 ROHM Semiconductors
> > +// bd71837.c  -- ROHM BD71837MWV clock driver
> 
> This isn't even the name of the file. Please remove. Also, only the SPDX
> tag is supposed to have // on it and the other things should be normal
> /* */ comment style.

I'll drop this name.

> 
> > +
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +#include <linux/init.h>
> > +#include <linux/err.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/slab.h>
> > +#include <linux/mfd/rohm-bd718x7.h>
> > +#include <linux/clk-provider.h>
> > +#include <linux/clkdev.h>
> > +#include <linux/regmap.h>
> [....]
> > +
> > +static int bd71837_clk_probe(struct platform_device *pdev)
> > +{
> > +       struct bd718xx_clk *c;
> > +       int rval = -ENOMEM;
> > +       const char *parent_clk;
> > +       struct device *parent = pdev->dev.parent;
> > +       struct bd718xx *mfd = dev_get_drvdata(parent);
> > +       struct clk_init_data init = {
> > +               .name = "bd718xx-32k-out",
> > +               .ops = &bd71837_clk_ops,
> > +       };
> > +
> > +       c = devm_kzalloc(&pdev->dev, sizeof(*c), GFP_KERNEL);
> > +       if (!c)
> > +               return -ENOMEM;
> > +
> > +       init.num_parents = 1;
> > +       parent_clk = of_clk_get_parent_name(parent->of_node, 0);
> > +
> > +       init.parent_names = &parent_clk;
> > +       if (!parent_clk) {
> > +               dev_err(&pdev->dev, "No parent clk found\n");
> > +               return -EINVAL;
> > +       }
> > +
> > +       c->reg = BD718XX_REG_OUT32K;
> > +       c->mask = BD718XX_OUT32K_EN;
> > +       c->mfd = mfd;
> > +       c->pdev = pdev;
> > +       c->hw.init = &init;
> > +
> > +       of_property_read_string_index(parent->of_node,
> > +                                     "clock-output-names", 0, &init.name);
> > +
> > +       rval = devm_clk_hw_register(&pdev->dev, &c->hw);
> > +       if (!rval) {
> > +               rval = devm_clk_hw_register_clkdev(&pdev->dev,
> > +                                                  &c->hw, init.name, NULL);
> 
> Do you plan to use the clkdev lookup? This driver looks fairly DT
> dependent, so I would prefer to remove the clkdev part and only add it
> later if anyone needs it.

Right. We depend heavily on DT so I guess I can drop the clkdev portion.

> > +               if (rval)
> > +                       dev_warn(&pdev->dev, "Failed to register clkdev\n");
> > +               if (parent->of_node) {
> > +                       rval = devm_of_clk_add_hw_provider(&pdev->dev,
> > +                                            of_clk_hw_simple_get, &c->hw);
> > +                       if (rval)
> > +                               dev_err(&pdev->dev,
> > +                                       "adding clk provider failed\n");
> > +               }
Pavel Machek Dec. 5, 2018, 8:17 p.m. UTC | #3 
On Wed 2018-12-05 09:28:21, Stephen Boyd wrote:
> Quoting Matti Vaittinen (2018-12-04 03:39:38)
> > diff --git a/drivers/clk/clk-bd718x7.c b/drivers/clk/clk-bd718x7.c
> > new file mode 100644
> > index 000000000000..d486859526ed
> > --- /dev/null
> > +++ b/drivers/clk/clk-bd718x7.c
> > @@ -0,0 +1,131 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +// Copyright (C) 2018 ROHM Semiconductors
> > +// bd71837.c  -- ROHM BD71837MWV clock driver
> 
> This isn't even the name of the file. Please remove. Also, only the SPDX
> tag is supposed to have // on it and the other things should be normal
> /* */ comment style.

Actually whole block commented by // is now considered ok.
									Pavel
diff mbox series

Patch

diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
index 81cdb4eaca07..2dc12bf75b1b 100644
--- a/drivers/clk/Kconfig
+++ b/drivers/clk/Kconfig
@@ -283,6 +283,13 @@  config COMMON_CLK_STM32H7
 	---help---
 	  Support for stm32h7 SoC family clocks
 
+config COMMON_CLK_BD718XX
+	tristate "Clock driver for ROHM BD718x7 PMIC"
+	depends on MFD_ROHM_BD718XX
+	help
+	  This driver supports ROHM BD71837 and ROHM BD71847
+	  PMICs clock gates.
+
 source "drivers/clk/actions/Kconfig"
 source "drivers/clk/bcm/Kconfig"
 source "drivers/clk/hisilicon/Kconfig"
diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index 72be7a38cff1..a47430b387db 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -21,6 +21,7 @@  endif
 obj-$(CONFIG_MACH_ASM9260)		+= clk-asm9260.o
 obj-$(CONFIG_COMMON_CLK_AXI_CLKGEN)	+= clk-axi-clkgen.o
 obj-$(CONFIG_ARCH_AXXIA)		+= clk-axm5516.o
+obj-$(CONFIG_COMMON_CLK_BD718XX)	+= clk-bd718x7.o
 obj-$(CONFIG_COMMON_CLK_CDCE706)	+= clk-cdce706.o
 obj-$(CONFIG_COMMON_CLK_CDCE925)	+= clk-cdce925.o
 obj-$(CONFIG_ARCH_CLPS711X)		+= clk-clps711x.o
diff --git a/drivers/clk/clk-bd718x7.c b/drivers/clk/clk-bd718x7.c
new file mode 100644
index 000000000000..d486859526ed
--- /dev/null
+++ b/drivers/clk/clk-bd718x7.c
@@ -0,0 +1,131 @@ 
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) 2018 ROHM Semiconductors
+// bd71837.c  -- ROHM BD71837MWV clock driver
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/err.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/mfd/rohm-bd718x7.h>
+#include <linux/clk-provider.h>
+#include <linux/clkdev.h>
+#include <linux/regmap.h>
+
+struct bd718xx_clk {
+	struct clk_hw hw;
+	u8 reg;
+	u8 mask;
+	struct platform_device *pdev;
+	struct bd718xx *mfd;
+};
+
+static int bd71837_clk_set(struct clk_hw *hw, int status)
+{
+	struct bd718xx_clk *c = container_of(hw, struct bd718xx_clk, hw);
+
+	return regmap_update_bits(c->mfd->regmap, c->reg, c->mask, status);
+}
+
+static void bd71837_clk_disable(struct clk_hw *hw)
+{
+	int rv;
+	struct bd718xx_clk *c = container_of(hw, struct bd718xx_clk, hw);
+
+	rv = bd71837_clk_set(hw, 0);
+	if (rv)
+		dev_dbg(&c->pdev->dev, "Failed to disable 32K clk (%d)\n", rv);
+}
+
+static int bd71837_clk_enable(struct clk_hw *hw)
+{
+	return bd71837_clk_set(hw, 1);
+}
+
+static int bd71837_clk_is_enabled(struct clk_hw *hw)
+{
+	int enabled;
+	int rval;
+	struct bd718xx_clk *c = container_of(hw, struct bd718xx_clk, hw);
+
+	rval = regmap_read(c->mfd->regmap, c->reg, &enabled);
+
+	if (rval)
+		return rval;
+
+	return enabled & c->mask;
+}
+
+static const struct clk_ops bd71837_clk_ops = {
+	.prepare = &bd71837_clk_enable,
+	.unprepare = &bd71837_clk_disable,
+	.is_prepared = &bd71837_clk_is_enabled,
+};
+
+static int bd71837_clk_probe(struct platform_device *pdev)
+{
+	struct bd718xx_clk *c;
+	int rval = -ENOMEM;
+	const char *parent_clk;
+	struct device *parent = pdev->dev.parent;
+	struct bd718xx *mfd = dev_get_drvdata(parent);
+	struct clk_init_data init = {
+		.name = "bd718xx-32k-out",
+		.ops = &bd71837_clk_ops,
+	};
+
+	c = devm_kzalloc(&pdev->dev, sizeof(*c), GFP_KERNEL);
+	if (!c)
+		return -ENOMEM;
+
+	init.num_parents = 1;
+	parent_clk = of_clk_get_parent_name(parent->of_node, 0);
+
+	init.parent_names = &parent_clk;
+	if (!parent_clk) {
+		dev_err(&pdev->dev, "No parent clk found\n");
+		return -EINVAL;
+	}
+
+	c->reg = BD718XX_REG_OUT32K;
+	c->mask = BD718XX_OUT32K_EN;
+	c->mfd = mfd;
+	c->pdev = pdev;
+	c->hw.init = &init;
+
+	of_property_read_string_index(parent->of_node,
+				      "clock-output-names", 0, &init.name);
+
+	rval = devm_clk_hw_register(&pdev->dev, &c->hw);
+	if (!rval) {
+		rval = devm_clk_hw_register_clkdev(&pdev->dev,
+						   &c->hw, init.name, NULL);
+		if (rval)
+			dev_warn(&pdev->dev, "Failed to register clkdev\n");
+		if (parent->of_node) {
+			rval = devm_of_clk_add_hw_provider(&pdev->dev,
+					     of_clk_hw_simple_get, &c->hw);
+			if (rval)
+				dev_err(&pdev->dev,
+					"adding clk provider failed\n");
+		}
+	} else {
+		dev_err(&pdev->dev, "failed to register 32K clk");
+	}
+
+	return rval;
+}
+
+static struct platform_driver bd71837_clk = {
+	.driver = {
+		.name = "bd718xx-clk",
+	},
+	.probe = bd71837_clk_probe,
+};
+
+module_platform_driver(bd71837_clk);
+
+MODULE_AUTHOR("Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>");
+MODULE_DESCRIPTION("BD71837 chip clk driver");
+MODULE_LICENSE("GPL");