[PATCH 0/5] clk: Combine MAX77686 and MAX77802 driver and extend for MAX77620

[PATCH 0/5] clk: Combine MAX77686 and MAX77802 driver and extend for MAX77620

[Laxman Dewangan]

The clock IP used on the Maxim PMICs max77686 and max77802 are
same. The configuration of clock register is also same except
the number of clocks.
    
Part of common code utilisation, there is 3 files for these chips
clock driver, one for common and two files for driver registration.
    
Combine both drivers into single file and move common code into
same common file reduces the 2 files and make max77686 and max77802
clock driver in single fine. This driver does not depends on the
parent driver structure. The regmap handle is acquired through
regmap APIs for the register access.

After comining the driver and DT files, extend the support to MAX77620.


This series is based on discussion from the series
	clk: max77620: Add clock driver for MAX77620/MAX20024
On this series, it is proposed to combine all files together.


Laxman Dewangan (5):
  clk: max77686: Combine Maxim max77686 and max77802 driver
  clk: Combine DT binding doc for max77686 and max77802
  clk: max77686: Add DT binding details for PMIC MAX77620
  clk: max77686: Add support for MAX77620 clocks
  ARM: config: Get rid of config COMMON_CLK_MAX77802

 .../devicetree/bindings/clock/maxim,max77686.txt   | 108 +++++++--
 .../devicetree/bindings/clock/maxim,max77802.txt   |  44 ----
 arch/arm/configs/exynos_defconfig                  |   1 -
 arch/arm/configs/multi_v7_defconfig                |   1 -
 drivers/clk/Kconfig                                |  18 +-
 drivers/clk/Makefile                               |   2 -
 drivers/clk/clk-max-gen.c                          | 194 ----------------
 drivers/clk/clk-max-gen.h                          |  32 ---
 drivers/clk/clk-max77686.c                         | 257 +++++++++++++++++++--
 drivers/clk/clk-max77802.c                         |  96 --------
 include/dt-bindings/mfd/max77620.h                 |   4 +
 11 files changed, 335 insertions(+), 422 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/clock/maxim,max77802.txt
 delete mode 100644 drivers/clk/clk-max-gen.c
 delete mode 100644 drivers/clk/clk-max-gen.h
 delete mode 100644 drivers/clk/clk-max77802.c

-- 
2.1.4

[PATCH 1/5] clk: max77686: Combine Maxim max77686 and max77802 driver

[Laxman Dewangan]

The clock IP used on the Maxim PMICs max77686 and max77802 are
same. The configuration of clock register is also same except
the number of clocks.

Part of common code utilisation, there is 3 files for these chips
clock driver, one for common and two files for driver registration.

Combine both drivers into single file and move common code into
same common file reduces the 2 files and make max77686 and max77802
clock driver in single fine. This driver does not depends on the
parent driver structure. The regmap handle is acquired through
regmap APIs for the register access.

This combination of driver helps on adding clock driver for different
Maxim PMICs which has similar clock IP like MAX77620 and MAX20024.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
CC: Krzysztof Kozlowski <k.kozlowski@samsung.com>
CC: Javier Martinez Canillas <javier@dowhile0.org>
---
 drivers/clk/Kconfig        |  15 +--
 drivers/clk/Makefile       |   2 -
 drivers/clk/clk-max-gen.c  | 194 ------------------------------------
 drivers/clk/clk-max-gen.h  |  32 ------
 drivers/clk/clk-max77686.c | 241 +++++++++++++++++++++++++++++++++++++++++----
 drivers/clk/clk-max77802.c |  96 ------------------
 6 files changed, 225 insertions(+), 355 deletions(-)
 delete mode 100644 drivers/clk/clk-max-gen.c
 delete mode 100644 drivers/clk/clk-max-gen.h
 delete mode 100644 drivers/clk/clk-max77802.c

diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
index 98efbfc..6afad74 100644
--- a/drivers/clk/Kconfig
+++ b/drivers/clk/Kconfig
@@ -31,22 +31,11 @@ config COMMON_CLK_WM831X
 
 source "drivers/clk/versatile/Kconfig"
 
-config COMMON_CLK_MAX_GEN
-        bool
-
 config COMMON_CLK_MAX77686
-	tristate "Clock driver for Maxim 77686 MFD"
-	depends on MFD_MAX77686
-	select COMMON_CLK_MAX_GEN
-	---help---
-	  This driver supports Maxim 77686 crystal oscillator clock. 
-
-config COMMON_CLK_MAX77802
-	tristate "Clock driver for Maxim 77802 PMIC"
+	tristate "Clock driver for Maxim 77686/77802 MFD"
 	depends on MFD_MAX77686
-	select COMMON_CLK_MAX_GEN
 	---help---
-	  This driver supports Maxim 77802 crystal oscillator clock.
+	  This driver supports Maxim 77686/77802 crystal oscillator clock.
 
 config COMMON_CLK_RK808
 	tristate "Clock driver for RK808"
diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index dcc5e69..0a3a91b 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -26,9 +26,7 @@ obj-$(CONFIG_ARCH_CLPS711X)		+= clk-clps711x.o
 obj-$(CONFIG_ARCH_EFM32)		+= clk-efm32gg.o
 obj-$(CONFIG_ARCH_HIGHBANK)		+= clk-highbank.o
 obj-$(CONFIG_MACH_LOONGSON32)		+= clk-ls1x.o
-obj-$(CONFIG_COMMON_CLK_MAX_GEN)	+= clk-max-gen.o
 obj-$(CONFIG_COMMON_CLK_MAX77686)	+= clk-max77686.o
-obj-$(CONFIG_COMMON_CLK_MAX77802)	+= clk-max77802.o
 obj-$(CONFIG_ARCH_MB86S7X)		+= clk-mb86s7x.o
 obj-$(CONFIG_ARCH_MOXART)		+= clk-moxart.o
 obj-$(CONFIG_ARCH_NOMADIK)		+= clk-nomadik.o
diff --git a/drivers/clk/clk-max-gen.c b/drivers/clk/clk-max-gen.c
deleted file mode 100644
index 35af9cb..0000000
--- a/drivers/clk/clk-max-gen.c
+++ /dev/null
@@ -1,194 +0,0 @@
-/*
- * clk-max-gen.c - Generic clock driver for Maxim PMICs clocks
- *
- * Copyright (C) 2014 Google, Inc
- *
- * Copyright (C) 2012 Samsung Electornics
- * Jonghwa Lee <jonghwa3.lee@samsung.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;  either version 2 of the  License, or (at your
- * option) any later version.
- *
- * This program is distributed in the hope that 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.
- *
- * This driver is based on clk-max77686.c
- *
- */
-
-#include <linux/kernel.h>
-#include <linux/slab.h>
-#include <linux/err.h>
-#include <linux/regmap.h>
-#include <linux/platform_device.h>
-#include <linux/clk-provider.h>
-#include <linux/mutex.h>
-#include <linux/clkdev.h>
-#include <linux/of.h>
-#include <linux/export.h>
-
-#include "clk-max-gen.h"
-
-struct max_gen_clk {
-	struct regmap *regmap;
-	u32 mask;
-	u32 reg;
-	struct clk_hw hw;
-};
-
-static struct max_gen_clk *to_max_gen_clk(struct clk_hw *hw)
-{
-	return container_of(hw, struct max_gen_clk, hw);
-}
-
-static int max_gen_clk_prepare(struct clk_hw *hw)
-{
-	struct max_gen_clk *max_gen = to_max_gen_clk(hw);
-
-	return regmap_update_bits(max_gen->regmap, max_gen->reg,
-				  max_gen->mask, max_gen->mask);
-}
-
-static void max_gen_clk_unprepare(struct clk_hw *hw)
-{
-	struct max_gen_clk *max_gen = to_max_gen_clk(hw);
-
-	regmap_update_bits(max_gen->regmap, max_gen->reg,
-			   max_gen->mask, ~max_gen->mask);
-}
-
-static int max_gen_clk_is_prepared(struct clk_hw *hw)
-{
-	struct max_gen_clk *max_gen = to_max_gen_clk(hw);
-	int ret;
-	u32 val;
-
-	ret = regmap_read(max_gen->regmap, max_gen->reg, &val);
-
-	if (ret < 0)
-		return -EINVAL;
-
-	return val & max_gen->mask;
-}
-
-static unsigned long max_gen_recalc_rate(struct clk_hw *hw,
-					 unsigned long parent_rate)
-{
-	return 32768;
-}
-
-struct clk_ops max_gen_clk_ops = {
-	.prepare	= max_gen_clk_prepare,
-	.unprepare	= max_gen_clk_unprepare,
-	.is_prepared	= max_gen_clk_is_prepared,
-	.recalc_rate	= max_gen_recalc_rate,
-};
-EXPORT_SYMBOL_GPL(max_gen_clk_ops);
-
-static struct clk *max_gen_clk_register(struct device *dev,
-					struct max_gen_clk *max_gen)
-{
-	struct clk *clk;
-	struct clk_hw *hw = &max_gen->hw;
-	int ret;
-
-	clk = devm_clk_register(dev, hw);
-	if (IS_ERR(clk))
-		return clk;
-
-	ret = clk_register_clkdev(clk, hw->init->name, NULL);
-
-	if (ret)
-		return ERR_PTR(ret);
-
-	return clk;
-}
-
-int max_gen_clk_probe(struct platform_device *pdev, struct regmap *regmap,
-		      u32 reg, struct clk_init_data *clks_init, int num_init)
-{
-	int i, ret;
-	struct max_gen_clk *max_gen_clks;
-	struct clk **clocks;
-	struct device *dev = pdev->dev.parent;
-	const char *clk_name;
-	struct clk_init_data *init;
-
-	clocks = devm_kzalloc(dev, sizeof(struct clk *) * num_init, GFP_KERNEL);
-	if (!clocks)
-		return -ENOMEM;
-
-	max_gen_clks = devm_kzalloc(dev, sizeof(struct max_gen_clk)
-				    * num_init, GFP_KERNEL);
-	if (!max_gen_clks)
-		return -ENOMEM;
-
-	for (i = 0; i < num_init; i++) {
-		max_gen_clks[i].regmap = regmap;
-		max_gen_clks[i].mask = 1 << i;
-		max_gen_clks[i].reg = reg;
-
-		init = devm_kzalloc(dev, sizeof(*init), GFP_KERNEL);
-		if (!init)
-			return -ENOMEM;
-
-		if (dev->of_node &&
-		    !of_property_read_string_index(dev->of_node,
-						   "clock-output-names",
-						   i, &clk_name))
-			init->name = clk_name;
-		else
-			init->name = clks_init[i].name;
-
-		init->ops = clks_init[i].ops;
-		init->flags = clks_init[i].flags;
-
-		max_gen_clks[i].hw.init = init;
-
-		clocks[i] = max_gen_clk_register(dev, &max_gen_clks[i]);
-		if (IS_ERR(clocks[i])) {
-			ret = PTR_ERR(clocks[i]);
-			dev_err(dev, "failed to register %s\n",
-				max_gen_clks[i].hw.init->name);
-			return ret;
-		}
-	}
-
-	platform_set_drvdata(pdev, clocks);
-
-	if (dev->of_node) {
-		struct clk_onecell_data *of_data;
-
-		of_data = devm_kzalloc(dev, sizeof(*of_data), GFP_KERNEL);
-		if (!of_data)
-			return -ENOMEM;
-
-		of_data->clks = clocks;
-		of_data->clk_num = num_init;
-		ret = of_clk_add_provider(dev->of_node, of_clk_src_onecell_get,
-					  of_data);
-
-		if (ret) {
-			dev_err(dev, "failed to register OF clock provider\n");
-			return ret;
-		}
-	}
-
-	return 0;
-}
-EXPORT_SYMBOL_GPL(max_gen_clk_probe);
-
-int max_gen_clk_remove(struct platform_device *pdev, int num_init)
-{
-	struct device *dev = pdev->dev.parent;
-
-	if (dev->of_node)
-		of_clk_del_provider(dev->of_node);
-
-	return 0;
-}
-EXPORT_SYMBOL_GPL(max_gen_clk_remove);
diff --git a/drivers/clk/clk-max-gen.h b/drivers/clk/clk-max-gen.h
deleted file mode 100644
index 997e86f..0000000
--- a/drivers/clk/clk-max-gen.h
+++ /dev/null
@@ -1,32 +0,0 @@
-/*
- * clk-max-gen.h - Generic clock driver for Maxim PMICs clocks
- *
- * Copyright (C) 2014 Google, Inc
- *
- * 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;  either version 2 of the  License, or (at your
- * option) any later version.
- *
- * This program is distributed in the hope that 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.
- *
- */
-
-#ifndef __CLK_MAX_GEN_H__
-#define __CLK_MAX_GEN_H__
-
-#include <linux/types.h>
-#include <linux/device.h>
-#include <linux/clkdev.h>
-#include <linux/regmap.h>
-#include <linux/platform_device.h>
-
-int max_gen_clk_probe(struct platform_device *pdev, struct regmap *regmap,
-		      u32 reg, struct clk_init_data *clks_init, int num_init);
-int max_gen_clk_remove(struct platform_device *pdev, int num_init);
-extern struct clk_ops max_gen_clk_ops;
-
-#endif /* __CLK_MAX_GEN_H__ */
diff --git a/drivers/clk/clk-max77686.c b/drivers/clk/clk-max77686.c
index 9b6f277..31ba726 100644
--- a/drivers/clk/clk-max77686.c
+++ b/drivers/clk/clk-max77686.c
@@ -1,5 +1,5 @@
 /*
- * clk-max77686.c - Clock driver for Maxim 77686
+ * clk-max77686.c - Clock driver for Maxim 77686/MAX77802
  *
  * Copyright (C) 2012 Samsung Electornics
  * Jonghwa Lee <jonghwa3.lee@samsung.com>
@@ -13,13 +13,7 @@
  * 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.
- *
- * 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/kernel.h>
 #include <linux/slab.h>
 #include <linux/err.h>
@@ -30,41 +24,252 @@
 #include <linux/clk-provider.h>
 #include <linux/mutex.h>
 #include <linux/clkdev.h>
+#include <linux/of.h>
+#include <linux/regmap.h>
+#include <linux/mutex.h>
 
 #include <dt-bindings/clock/maxim,max77686.h>
-#include "clk-max-gen.h"
+#include <dt-bindings/clock/maxim,max77802.h>
 
-static struct clk_init_data max77686_clks_init[MAX77686_CLKS_NUM] = {
+#define MAX77802_CLOCK_LOW_JITTER_SHIFT 0x3
+
+enum chip_name {
+	CHIP_MAX77686,
+	CHIP_MAX77802,
+};
+
+struct max_gen_hw_clk_data {
+	const char *name;
+	u32 reg;
+	u32 mask;
+	u32 flags;
+};
+
+struct max_gen_clk_data {
+	struct regmap *regmap;
+	struct clk_init_data clk_idata;
+	struct clk_hw hw;
+	u32 reg;
+	u32 mask;
+};
+
+struct max_gen_clk_driver_info {
+	enum chip_name chip;
+	struct clk **clks;
+	struct max_gen_clk_data *max_clk_data;
+	struct clk_onecell_data of_data;
+};
+
+static struct max_gen_hw_clk_data max77686_hw_clks_info[MAX77686_CLKS_NUM] = {
 	[MAX77686_CLK_AP] = {
 		.name = "32khz_ap",
-		.ops = &max_gen_clk_ops,
+		.reg = MAX77686_REG_32KHZ,
+		.mask = BIT(MAX77686_CLK_AP),
 	},
 	[MAX77686_CLK_CP] = {
 		.name = "32khz_cp",
-		.ops = &max_gen_clk_ops,
+		.reg = MAX77686_REG_32KHZ,
+		.mask = BIT(MAX77686_CLK_CP),
 	},
 	[MAX77686_CLK_PMIC] = {
 		.name = "32khz_pmic",
-		.ops = &max_gen_clk_ops,
+		.reg = MAX77686_REG_32KHZ,
+		.mask = BIT(MAX77686_CLK_PMIC),
+	},
+};
+
+static struct max_gen_hw_clk_data max77802_hw_clks_info[MAX77802_CLKS_NUM] = {
+	[MAX77802_CLK_32K_AP] = {
+		.name = "32khz_ap",
+		.reg = MAX77802_REG_32KHZ,
+		.mask = BIT(MAX77802_CLK_32K_AP),
+	},
+	[MAX77802_CLK_32K_CP] = {
+		.name = "32khz_cp",
+		.reg = MAX77802_REG_32KHZ,
+		.mask = BIT(MAX77802_CLK_32K_CP),
 	},
 };
 
+static struct max_gen_clk_data *to_max_gen_clk_data(struct clk_hw *hw)
+{
+	return container_of(hw, struct max_gen_clk_data, hw);
+}
+
+static int max_gen_clk_prepare(struct clk_hw *hw)
+{
+	struct max_gen_clk_data *max_gen = to_max_gen_clk_data(hw);
+
+	return regmap_update_bits(max_gen->regmap, max_gen->reg,
+				  max_gen->mask, max_gen->mask);
+}
+
+static void max_gen_clk_unprepare(struct clk_hw *hw)
+{
+	struct max_gen_clk_data *max_gen = to_max_gen_clk_data(hw);
+
+	regmap_update_bits(max_gen->regmap, max_gen->reg,
+			   max_gen->mask, ~max_gen->mask);
+}
+
+static int max_gen_clk_is_prepared(struct clk_hw *hw)
+{
+	struct max_gen_clk_data *max_gen = to_max_gen_clk_data(hw);
+	int ret;
+	u32 val;
+
+	ret = regmap_read(max_gen->regmap, max_gen->reg, &val);
+
+	if (ret < 0)
+		return -EINVAL;
+
+	return val & max_gen->mask;
+}
+
+static unsigned long max_gen_recalc_rate(struct clk_hw *hw,
+					 unsigned long parent_rate)
+{
+	return 32768;
+}
+
+static struct clk_ops max_gen_clk_ops = {
+	.prepare	= max_gen_clk_prepare,
+	.unprepare	= max_gen_clk_unprepare,
+	.is_prepared	= max_gen_clk_is_prepared,
+	.recalc_rate	= max_gen_recalc_rate,
+};
+
 static int max77686_clk_probe(struct platform_device *pdev)
 {
-	struct max77686_dev *iodev = dev_get_drvdata(pdev->dev.parent);
+	struct device *parent = pdev->dev.parent;
+	struct device *dev = &pdev->dev;
+	const struct platform_device_id *id = platform_get_device_id(pdev);
+	struct max_gen_clk_driver_info *drv_info;
+	struct max_gen_clk_data *max_clk_data;
+	const struct max_gen_hw_clk_data *hw_clks;
+	struct regmap *regmap;
+	int i, ret, num_clks;
+
+	drv_info = devm_kzalloc(dev, sizeof(*drv_info), GFP_KERNEL);
+	if (!drv_info)
+		return -ENOMEM;
+
+	regmap = dev_get_regmap(parent, NULL);
+	if (!regmap) {
+		dev_err(dev, "Failed to get rtc regmap\n");
+		return -ENODEV;
+	}
+
+	drv_info->chip = id->driver_data;
+
+	switch (drv_info->chip) {
+	case CHIP_MAX77686:
+		num_clks = MAX77686_CLKS_NUM;
+		hw_clks = max77686_hw_clks_info;
+		break;
+	case CHIP_MAX77802:
+		num_clks = MAX77802_CLKS_NUM;
+		hw_clks = max77802_hw_clks_info;
+		break;
+	default:
+		dev_err(dev, "Unknown Chip ID\n");
+		return -EINVAL;
+	};
 
-	return max_gen_clk_probe(pdev, iodev->regmap, MAX77686_REG_32KHZ,
-				 max77686_clks_init, MAX77686_CLKS_NUM);
+	drv_info->max_clk_data = devm_kcalloc(dev, num_clks,
+					      sizeof(*drv_info->max_clk_data),
+					      GFP_KERNEL);
+	if (!drv_info->max_clk_data)
+		return -ENOMEM;
+
+	drv_info->clks = devm_kcalloc(dev, num_clks,
+				      sizeof(*drv_info->clks), GFP_KERNEL);
+	if (!drv_info->clks)
+		return -ENOMEM;
+
+	for (i = 0; i < num_clks; i++) {
+		struct clk *clk;
+		const char *clk_name;
+
+		max_clk_data = &drv_info->max_clk_data[i];
+
+		max_clk_data->regmap = regmap;
+		max_clk_data->mask = hw_clks[i].mask;
+		max_clk_data->reg = hw_clks[i].reg;
+		max_clk_data->clk_idata.flags = hw_clks[i].flags;
+		max_clk_data->clk_idata.ops = &max_gen_clk_ops;
+
+		if (parent->of_node &&
+		    !of_property_read_string_index(parent->of_node,
+						   "clock-output-names",
+						   i, &clk_name))
+			max_clk_data->clk_idata.name = clk_name;
+		else
+			max_clk_data->clk_idata.name = hw_clks[i].name;
+
+		max_clk_data->hw.init = &max_clk_data->clk_idata;
+
+		clk = devm_clk_register(dev, &max_clk_data->hw);
+		if (IS_ERR(clk)) {
+			ret = PTR_ERR(clk);
+			dev_err(dev, "Failed to clock register: %d\n", ret);
+			return ret;
+		}
+
+		ret = clk_register_clkdev(clk, max_clk_data->clk_idata.name,
+					  NULL);
+		if (ret < 0) {
+			dev_err(dev, "Failed to clkdev register: %d\n", ret);
+			return ret;
+		}
+		drv_info->clks[i] = clk;
+	}
+
+	platform_set_drvdata(pdev, drv_info);
+
+	if (parent->of_node) {
+		drv_info->of_data.clks = drv_info->clks;
+		drv_info->of_data.clk_num = num_clks;
+		ret = of_clk_add_provider(parent->of_node,
+					  of_clk_src_onecell_get,
+					  &drv_info->of_data);
+
+		if (ret < 0) {
+			dev_err(dev, "Failed to register OF clock provider: %d\n",
+				ret);
+			return ret;
+		}
+	}
+
+	/* MAX77802: Enable low-jitter mode on the 32khz clocks. */
+	if (drv_info->chip == CHIP_MAX77802) {
+		ret = regmap_update_bits(regmap, MAX77802_REG_32KHZ,
+					 1 << MAX77802_CLOCK_LOW_JITTER_SHIFT,
+					 1 << MAX77802_CLOCK_LOW_JITTER_SHIFT);
+		if (ret < 0) {
+			dev_err(dev, "Failed to set low-jitter mode: %d\n",
+				ret);
+			return ret;
+		}
+	}
+
+	return 0;
 }
 
 static int max77686_clk_remove(struct platform_device *pdev)
 {
-	return max_gen_clk_remove(pdev, MAX77686_CLKS_NUM);
+	struct device *parent = pdev->dev.parent;
+
+	if (parent->of_node)
+		of_clk_del_provider(parent->of_node);
+
+	return 0;
 }
 
 static const struct platform_device_id max77686_clk_id[] = {
-	{ "max77686-clk", 0},
-	{ },
+	{ "max77686-clk", .driver_data = (kernel_ulong_t)CHIP_MAX77686, },
+	{ "max77802-clk", .driver_data = (kernel_ulong_t)CHIP_MAX77802, },
+	{},
 };
 MODULE_DEVICE_TABLE(platform, max77686_clk_id);
 
diff --git a/drivers/clk/clk-max77802.c b/drivers/clk/clk-max77802.c
deleted file mode 100644
index 355dd2e..0000000
--- a/drivers/clk/clk-max77802.c
+++ /dev/null
@@ -1,96 +0,0 @@
-/*
- * clk-max77802.c - Clock driver for Maxim 77802
- *
- * Copyright (C) 2014 Google, Inc
- *
- * Copyright (C) 2012 Samsung Electornics
- * Jonghwa Lee <jonghwa3.lee@samsung.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;  either version 2 of the  License, or (at your
- * option) any later version.
- *
- * This program is distributed in the hope that 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.
- *
- * This driver is based on clk-max77686.c
- */
-
-#include <linux/kernel.h>
-#include <linux/slab.h>
-#include <linux/err.h>
-#include <linux/module.h>
-#include <linux/platform_device.h>
-#include <linux/mfd/max77686-private.h>
-#include <linux/clk-provider.h>
-#include <linux/mutex.h>
-#include <linux/clkdev.h>
-
-#include <dt-bindings/clock/maxim,max77802.h>
-#include "clk-max-gen.h"
-
-#define MAX77802_CLOCK_OPMODE_MASK	0x1
-#define MAX77802_CLOCK_LOW_JITTER_SHIFT 0x3
-
-static struct clk_init_data max77802_clks_init[MAX77802_CLKS_NUM] = {
-	[MAX77802_CLK_32K_AP] = {
-		.name = "32khz_ap",
-		.ops = &max_gen_clk_ops,
-	},
-	[MAX77802_CLK_32K_CP] = {
-		.name = "32khz_cp",
-		.ops = &max_gen_clk_ops,
-	},
-};
-
-static int max77802_clk_probe(struct platform_device *pdev)
-{
-	struct max77686_dev *iodev = dev_get_drvdata(pdev->dev.parent);
-	int ret;
-
-	ret = max_gen_clk_probe(pdev, iodev->regmap, MAX77802_REG_32KHZ,
-				max77802_clks_init, MAX77802_CLKS_NUM);
-
-	if (ret) {
-		dev_err(&pdev->dev, "generic probe failed %d\n", ret);
-		return ret;
-	}
-
-	/* Enable low-jitter mode on the 32khz clocks. */
-	ret = regmap_update_bits(iodev->regmap, MAX77802_REG_32KHZ,
-				 1 << MAX77802_CLOCK_LOW_JITTER_SHIFT,
-				 1 << MAX77802_CLOCK_LOW_JITTER_SHIFT);
-	if (ret < 0)
-		dev_err(&pdev->dev, "failed to enable low-jitter mode\n");
-
-	return ret;
-}
-
-static int max77802_clk_remove(struct platform_device *pdev)
-{
-	return max_gen_clk_remove(pdev, MAX77802_CLKS_NUM);
-}
-
-static const struct platform_device_id max77802_clk_id[] = {
-	{ "max77802-clk", 0},
-	{ },
-};
-MODULE_DEVICE_TABLE(platform, max77802_clk_id);
-
-static struct platform_driver max77802_clk_driver = {
-	.driver = {
-		.name  = "max77802-clk",
-	},
-	.probe = max77802_clk_probe,
-	.remove = max77802_clk_remove,
-	.id_table = max77802_clk_id,
-};
-
-module_platform_driver(max77802_clk_driver);
-
-MODULE_DESCRIPTION("MAXIM 77802 Clock Driver");
-MODULE_AUTHOR("Javier Martinez Canillas <javier@osg.samsung.com");
-MODULE_LICENSE("GPL");
-- 
2.1.4

Re: [PATCH 1/5] clk: max77686: Combine Maxim max77686 and max77802 driver

[Javier Martinez Canillas]

Hello Laxman,

On 06/15/2016 10:13 AM, Laxman Dewangan wrote:
> The clock IP used on the Maxim PMICs max77686 and max77802 are
> same. The configuration of clock register is also same except
> the number of clocks.
> 
> Part of common code utilisation, there is 3 files for these chips
> clock driver, one for common and two files for driver registration.
> 
> Combine both drivers into single file and move common code into
> same common file reduces the 2 files and make max77686 and max77802
> clock driver in single fine. This driver does not depends on the
> parent driver structure. The regmap handle is acquired through
> regmap APIs for the register access.
> 
> This combination of driver helps on adding clock driver for different
> Maxim PMICs which has similar clock IP like MAX77620 and MAX20024.
> 
> Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
> CC: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> CC: Javier Martinez Canillas <javier@dowhile0.org>
> ---

[snip]

> diff --git a/drivers/clk/clk-max77686.c b/drivers/clk/clk-max77686.c
> index 9b6f277..31ba726 100644
> --- a/drivers/clk/clk-max77686.c
> +++ b/drivers/clk/clk-max77686.c
> @@ -1,5 +1,5 @@
>  /*
> - * clk-max77686.c - Clock driver for Maxim 77686
> + * clk-max77686.c - Clock driver for Maxim 77686/MAX77802
>   *
>   * Copyright (C) 2012 Samsung Electornics
>   * Jonghwa Lee <jonghwa3.lee@samsung.com>
> @@ -13,13 +13,7 @@
>   * 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.
> - *
> - * 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
> - *
>   */
> -

I think removing this part of the header is correct but it should be a
separate patch.

[snip]

> +
> +struct max_gen_hw_clk_data {
> +	const char *name;
> +	u32 reg;
> +	u32 mask;
> +	u32 flags;
> +};
> +
> +struct max_gen_clk_data {
> +	struct regmap *regmap;
> +	struct clk_init_data clk_idata;
> +	struct clk_hw hw;
> +	u32 reg;
> +	u32 mask;
> +};
> +

Probably now makes more sense to rename all the "max_gen_foo" structs and
functions to just "max77686_foo" since now are all defined in the max77686
so that naming is more consistent.

Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com>

I also tested on an Exynos5800 Peach Pi Chromebook that has a max77802 IP:

Tested-by: Javier Martinez Canillas <javier@osg.samsung.com>

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

Re: [PATCH 1/5] clk: max77686: Combine Maxim max77686 and max77802 driver

[kbuild test robot]

Hi,

[auto build test WARNING on next-20160615]
[also build test WARNING on v4.7-rc3]
[cannot apply to clk/clk-next robh/for-next v4.7-rc3 v4.7-rc2 v4.7-rc1]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Laxman-Dewangan/clk-Combine-MAX77686-and-MAX77802-driver-and-extend-for-MAX77620/20160615-223243


coccinelle warnings: (new ones prefixed by >>)

>> drivers/clk/clk-max77686.c:177:2-3: Unneeded semicolon

Please review and possibly fold the followup patch.

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[PATCH] clk: max77686: fix semicolon.cocci warnings

[kbuild test robot]

drivers/clk/clk-max77686.c:177:2-3: Unneeded semicolon


 Remove unneeded semicolon.

Generated by: scripts/coccinelle/misc/semicolon.cocci

CC: Laxman Dewangan <ldewangan@nvidia.com>
Signed-off-by: Fengguang Wu <fengguang.wu@intel.com>
---

 clk-max77686.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/drivers/clk/clk-max77686.c
+++ b/drivers/clk/clk-max77686.c
@@ -174,7 +174,7 @@ static int max77686_clk_probe(struct pla
 	default:
 		dev_err(dev, "Unknown Chip ID\n");
 		return -EINVAL;
-	};
+	}
 
 	drv_info->max_clk_data = devm_kcalloc(dev, num_clks,
 					      sizeof(*drv_info->max_clk_data),

Re: [PATCH 1/5] clk: max77686: Combine Maxim max77686 and max77802 driver

[Krzysztof Kozlowski]

Hi,

First, thank for the work. Looks promising! Few comments below.

On 06/15/2016 04:13 PM, Laxman Dewangan wrote:
> The clock IP used on the Maxim PMICs max77686 and max77802 are
> same. The configuration of clock register is also same except
> the number of clocks.
> 
> Part of common code utilisation, there is 3 files for these chips
> clock driver, one for common and two files for driver registration.
> 
> Combine both drivers into single file and move common code into
> same common file reduces the 2 files and make max77686 and max77802
> clock driver in single fine. This driver does not depends on the
> parent driver structure. The regmap handle is acquired through
> regmap APIs for the register access.
> 
> This combination of driver helps on adding clock driver for different
> Maxim PMICs which has similar clock IP like MAX77620 and MAX20024.
> 
> Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
> CC: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> CC: Javier Martinez Canillas <javier@dowhile0.org>
> ---
>  drivers/clk/Kconfig        |  15 +--
>  drivers/clk/Makefile       |   2 -
>  drivers/clk/clk-max-gen.c  | 194 ------------------------------------
>  drivers/clk/clk-max-gen.h  |  32 ------
>  drivers/clk/clk-max77686.c | 241 +++++++++++++++++++++++++++++++++++++++++----
>  drivers/clk/clk-max77802.c |  96 ------------------
>  6 files changed, 225 insertions(+), 355 deletions(-)
>  delete mode 100644 drivers/clk/clk-max-gen.c
>  delete mode 100644 drivers/clk/clk-max-gen.h
>  delete mode 100644 drivers/clk/clk-max77802.c
> 
> diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
> index 98efbfc..6afad74 100644
> --- a/drivers/clk/Kconfig
> +++ b/drivers/clk/Kconfig
> @@ -31,22 +31,11 @@ config COMMON_CLK_WM831X
>  
>  source "drivers/clk/versatile/Kconfig"
>  
> -config COMMON_CLK_MAX_GEN
> -        bool
> -
>  config COMMON_CLK_MAX77686
> -	tristate "Clock driver for Maxim 77686 MFD"
> -	depends on MFD_MAX77686
> -	select COMMON_CLK_MAX_GEN
> -	---help---
> -	  This driver supports Maxim 77686 crystal oscillator clock. 
> -
> -config COMMON_CLK_MAX77802
> -	tristate "Clock driver for Maxim 77802 PMIC"
> +	tristate "Clock driver for Maxim 77686/77802 MFD"
>  	depends on MFD_MAX77686
> -	select COMMON_CLK_MAX_GEN
>  	---help---
> -	  This driver supports Maxim 77802 crystal oscillator clock.
> +	  This driver supports Maxim 77686/77802 crystal oscillator clock.
>  
>  config COMMON_CLK_RK808
>  	tristate "Clock driver for RK808"
> diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
> index dcc5e69..0a3a91b 100644
> --- a/drivers/clk/Makefile
> +++ b/drivers/clk/Makefile
> @@ -26,9 +26,7 @@ obj-$(CONFIG_ARCH_CLPS711X)		+= clk-clps711x.o
>  obj-$(CONFIG_ARCH_EFM32)		+= clk-efm32gg.o
>  obj-$(CONFIG_ARCH_HIGHBANK)		+= clk-highbank.o
>  obj-$(CONFIG_MACH_LOONGSON32)		+= clk-ls1x.o
> -obj-$(CONFIG_COMMON_CLK_MAX_GEN)	+= clk-max-gen.o
>  obj-$(CONFIG_COMMON_CLK_MAX77686)	+= clk-max77686.o
> -obj-$(CONFIG_COMMON_CLK_MAX77802)	+= clk-max77802.o
>  obj-$(CONFIG_ARCH_MB86S7X)		+= clk-mb86s7x.o
>  obj-$(CONFIG_ARCH_MOXART)		+= clk-moxart.o
>  obj-$(CONFIG_ARCH_NOMADIK)		+= clk-nomadik.o
> diff --git a/drivers/clk/clk-max-gen.c b/drivers/clk/clk-max-gen.c
> deleted file mode 100644
> index 35af9cb..0000000
> --- a/drivers/clk/clk-max-gen.c
> +++ /dev/null
> @@ -1,194 +0,0 @@
> -/*
> - * clk-max-gen.c - Generic clock driver for Maxim PMICs clocks
> - *
> - * Copyright (C) 2014 Google, Inc
> - *
> - * Copyright (C) 2012 Samsung Electornics
> - * Jonghwa Lee <jonghwa3.lee@samsung.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;  either version 2 of the  License, or (at your
> - * option) any later version.
> - *
> - * This program is distributed in the hope that 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.
> - *
> - * This driver is based on clk-max77686.c
> - *
> - */
> -
> -#include <linux/kernel.h>
> -#include <linux/slab.h>
> -#include <linux/err.h>
> -#include <linux/regmap.h>
> -#include <linux/platform_device.h>
> -#include <linux/clk-provider.h>
> -#include <linux/mutex.h>
> -#include <linux/clkdev.h>
> -#include <linux/of.h>
> -#include <linux/export.h>
> -
> -#include "clk-max-gen.h"
> -
> -struct max_gen_clk {
> -	struct regmap *regmap;
> -	u32 mask;
> -	u32 reg;
> -	struct clk_hw hw;
> -};
> -
> -static struct max_gen_clk *to_max_gen_clk(struct clk_hw *hw)
> -{
> -	return container_of(hw, struct max_gen_clk, hw);
> -}
> -
> -static int max_gen_clk_prepare(struct clk_hw *hw)
> -{
> -	struct max_gen_clk *max_gen = to_max_gen_clk(hw);
> -
> -	return regmap_update_bits(max_gen->regmap, max_gen->reg,
> -				  max_gen->mask, max_gen->mask);
> -}
> -
> -static void max_gen_clk_unprepare(struct clk_hw *hw)
> -{
> -	struct max_gen_clk *max_gen = to_max_gen_clk(hw);
> -
> -	regmap_update_bits(max_gen->regmap, max_gen->reg,
> -			   max_gen->mask, ~max_gen->mask);
> -}
> -
> -static int max_gen_clk_is_prepared(struct clk_hw *hw)
> -{
> -	struct max_gen_clk *max_gen = to_max_gen_clk(hw);
> -	int ret;
> -	u32 val;
> -
> -	ret = regmap_read(max_gen->regmap, max_gen->reg, &val);
> -
> -	if (ret < 0)
> -		return -EINVAL;
> -
> -	return val & max_gen->mask;
> -}
> -
> -static unsigned long max_gen_recalc_rate(struct clk_hw *hw,
> -					 unsigned long parent_rate)
> -{
> -	return 32768;
> -}
> -
> -struct clk_ops max_gen_clk_ops = {
> -	.prepare	= max_gen_clk_prepare,
> -	.unprepare	= max_gen_clk_unprepare,
> -	.is_prepared	= max_gen_clk_is_prepared,
> -	.recalc_rate	= max_gen_recalc_rate,
> -};
> -EXPORT_SYMBOL_GPL(max_gen_clk_ops);
> -
> -static struct clk *max_gen_clk_register(struct device *dev,
> -					struct max_gen_clk *max_gen)
> -{
> -	struct clk *clk;
> -	struct clk_hw *hw = &max_gen->hw;
> -	int ret;
> -
> -	clk = devm_clk_register(dev, hw);
> -	if (IS_ERR(clk))
> -		return clk;
> -
> -	ret = clk_register_clkdev(clk, hw->init->name, NULL);
> -
> -	if (ret)
> -		return ERR_PTR(ret);
> -
> -	return clk;
> -}
> -
> -int max_gen_clk_probe(struct platform_device *pdev, struct regmap *regmap,
> -		      u32 reg, struct clk_init_data *clks_init, int num_init)
> -{
> -	int i, ret;
> -	struct max_gen_clk *max_gen_clks;
> -	struct clk **clocks;
> -	struct device *dev = pdev->dev.parent;
> -	const char *clk_name;
> -	struct clk_init_data *init;
> -
> -	clocks = devm_kzalloc(dev, sizeof(struct clk *) * num_init, GFP_KERNEL);
> -	if (!clocks)
> -		return -ENOMEM;
> -
> -	max_gen_clks = devm_kzalloc(dev, sizeof(struct max_gen_clk)
> -				    * num_init, GFP_KERNEL);
> -	if (!max_gen_clks)
> -		return -ENOMEM;
> -
> -	for (i = 0; i < num_init; i++) {
> -		max_gen_clks[i].regmap = regmap;
> -		max_gen_clks[i].mask = 1 << i;
> -		max_gen_clks[i].reg = reg;
> -
> -		init = devm_kzalloc(dev, sizeof(*init), GFP_KERNEL);
> -		if (!init)
> -			return -ENOMEM;
> -
> -		if (dev->of_node &&
> -		    !of_property_read_string_index(dev->of_node,
> -						   "clock-output-names",
> -						   i, &clk_name))
> -			init->name = clk_name;
> -		else
> -			init->name = clks_init[i].name;
> -
> -		init->ops = clks_init[i].ops;
> -		init->flags = clks_init[i].flags;
> -
> -		max_gen_clks[i].hw.init = init;
> -
> -		clocks[i] = max_gen_clk_register(dev, &max_gen_clks[i]);
> -		if (IS_ERR(clocks[i])) {
> -			ret = PTR_ERR(clocks[i]);
> -			dev_err(dev, "failed to register %s\n",
> -				max_gen_clks[i].hw.init->name);
> -			return ret;
> -		}
> -	}
> -
> -	platform_set_drvdata(pdev, clocks);
> -
> -	if (dev->of_node) {
> -		struct clk_onecell_data *of_data;
> -
> -		of_data = devm_kzalloc(dev, sizeof(*of_data), GFP_KERNEL);
> -		if (!of_data)
> -			return -ENOMEM;
> -
> -		of_data->clks = clocks;
> -		of_data->clk_num = num_init;
> -		ret = of_clk_add_provider(dev->of_node, of_clk_src_onecell_get,
> -					  of_data);
> -
> -		if (ret) {
> -			dev_err(dev, "failed to register OF clock provider\n");
> -			return ret;
> -		}
> -	}
> -
> -	return 0;
> -}
> -EXPORT_SYMBOL_GPL(max_gen_clk_probe);
> -
> -int max_gen_clk_remove(struct platform_device *pdev, int num_init)
> -{
> -	struct device *dev = pdev->dev.parent;
> -
> -	if (dev->of_node)
> -		of_clk_del_provider(dev->of_node);
> -
> -	return 0;
> -}
> -EXPORT_SYMBOL_GPL(max_gen_clk_remove);
> diff --git a/drivers/clk/clk-max-gen.h b/drivers/clk/clk-max-gen.h
> deleted file mode 100644
> index 997e86f..0000000
> --- a/drivers/clk/clk-max-gen.h
> +++ /dev/null
> @@ -1,32 +0,0 @@
> -/*
> - * clk-max-gen.h - Generic clock driver for Maxim PMICs clocks
> - *
> - * Copyright (C) 2014 Google, Inc
> - *
> - * 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;  either version 2 of the  License, or (at your
> - * option) any later version.
> - *
> - * This program is distributed in the hope that 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.
> - *
> - */
> -
> -#ifndef __CLK_MAX_GEN_H__
> -#define __CLK_MAX_GEN_H__
> -
> -#include <linux/types.h>
> -#include <linux/device.h>
> -#include <linux/clkdev.h>
> -#include <linux/regmap.h>
> -#include <linux/platform_device.h>
> -
> -int max_gen_clk_probe(struct platform_device *pdev, struct regmap *regmap,
> -		      u32 reg, struct clk_init_data *clks_init, int num_init);
> -int max_gen_clk_remove(struct platform_device *pdev, int num_init);
> -extern struct clk_ops max_gen_clk_ops;
> -
> -#endif /* __CLK_MAX_GEN_H__ */
> diff --git a/drivers/clk/clk-max77686.c b/drivers/clk/clk-max77686.c
> index 9b6f277..31ba726 100644
> --- a/drivers/clk/clk-max77686.c
> +++ b/drivers/clk/clk-max77686.c
> @@ -1,5 +1,5 @@
>  /*
> - * clk-max77686.c - Clock driver for Maxim 77686
> + * clk-max77686.c - Clock driver for Maxim 77686/MAX77802
>   *
>   * Copyright (C) 2012 Samsung Electornics
>   * Jonghwa Lee <jonghwa3.lee@samsung.com>
> @@ -13,13 +13,7 @@
>   * 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.
> - *
> - * 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/kernel.h>
>  #include <linux/slab.h>
>  #include <linux/err.h>
> @@ -30,41 +24,252 @@
>  #include <linux/clk-provider.h>
>  #include <linux/mutex.h>
>  #include <linux/clkdev.h>
> +#include <linux/of.h>
> +#include <linux/regmap.h>
> +#include <linux/mutex.h>
>  
>  #include <dt-bindings/clock/maxim,max77686.h>
> -#include "clk-max-gen.h"
> +#include <dt-bindings/clock/maxim,max77802.h>
>  
> -static struct clk_init_data max77686_clks_init[MAX77686_CLKS_NUM] = {
> +#define MAX77802_CLOCK_LOW_JITTER_SHIFT 0x3
> +
> +enum chip_name {
> +	CHIP_MAX77686,
> +	CHIP_MAX77802,
> +};

No need for these. Just use existing enum max77686_types.

> +
> +struct max_gen_hw_clk_data {
> +	const char *name;
> +	u32 reg;
> +	u32 mask;
> +	u32 flags;
> +};
> +
> +struct max_gen_clk_data {
> +	struct regmap *regmap;
> +	struct clk_init_data clk_idata;
> +	struct clk_hw hw;
> +	u32 reg;
> +	u32 mask;

Why do you need reg and mask here? This looks overcomplicated. Both the
names are too similar and the structures duplicate some fields.

For the names, how about:
 - max77686_gen_init_data
 - max77686_clk
?


> +};
> +
> +struct max_gen_clk_driver_info {
> +	enum chip_name chip;
> +	struct clk **clks;
> +	struct max_gen_clk_data *max_clk_data;
> +	struct clk_onecell_data of_data;
> +};
> +
> +static struct max_gen_hw_clk_data max77686_hw_clks_info[MAX77686_CLKS_NUM] = {

This looks const.

>  	[MAX77686_CLK_AP] = {
>  		.name = "32khz_ap",
> -		.ops = &max_gen_clk_ops,
> +		.reg = MAX77686_REG_32KHZ,
> +		.mask = BIT(MAX77686_CLK_AP),
>  	},
>  	[MAX77686_CLK_CP] = {
>  		.name = "32khz_cp",
> -		.ops = &max_gen_clk_ops,
> +		.reg = MAX77686_REG_32KHZ,
> +		.mask = BIT(MAX77686_CLK_CP),
>  	},
>  	[MAX77686_CLK_PMIC] = {
>  		.name = "32khz_pmic",
> -		.ops = &max_gen_clk_ops,
> +		.reg = MAX77686_REG_32KHZ,
> +		.mask = BIT(MAX77686_CLK_PMIC),
> +	},
> +};
> +
> +static struct max_gen_hw_clk_data max77802_hw_clks_info[MAX77802_CLKS_NUM] = {
> +	[MAX77802_CLK_32K_AP] = {
> +		.name = "32khz_ap",
> +		.reg = MAX77802_REG_32KHZ,
> +		.mask = BIT(MAX77802_CLK_32K_AP),
> +	},
> +	[MAX77802_CLK_32K_CP] = {
> +		.name = "32khz_cp",
> +		.reg = MAX77802_REG_32KHZ,
> +		.mask = BIT(MAX77802_CLK_32K_CP),
>  	},
>  };
>  
> +static struct max_gen_clk_data *to_max_gen_clk_data(struct clk_hw *hw)
> +{
> +	return container_of(hw, struct max_gen_clk_data, hw);
> +}
> +
> +static int max_gen_clk_prepare(struct clk_hw *hw)
> +{
> +	struct max_gen_clk_data *max_gen = to_max_gen_clk_data(hw);
> +
> +	return regmap_update_bits(max_gen->regmap, max_gen->reg,
> +				  max_gen->mask, max_gen->mask);
> +}
> +
> +static void max_gen_clk_unprepare(struct clk_hw *hw)
> +{
> +	struct max_gen_clk_data *max_gen = to_max_gen_clk_data(hw);
> +
> +	regmap_update_bits(max_gen->regmap, max_gen->reg,
> +			   max_gen->mask, ~max_gen->mask);
> +}
> +
> +static int max_gen_clk_is_prepared(struct clk_hw *hw)
> +{
> +	struct max_gen_clk_data *max_gen = to_max_gen_clk_data(hw);
> +	int ret;
> +	u32 val;
> +
> +	ret = regmap_read(max_gen->regmap, max_gen->reg, &val);
> +
> +	if (ret < 0)
> +		return -EINVAL;
> +
> +	return val & max_gen->mask;
> +}
> +
> +static unsigned long max_gen_recalc_rate(struct clk_hw *hw,
> +					 unsigned long parent_rate)
> +{
> +	return 32768;
> +}
> +
> +static struct clk_ops max_gen_clk_ops = {
> +	.prepare	= max_gen_clk_prepare,
> +	.unprepare	= max_gen_clk_unprepare,
> +	.is_prepared	= max_gen_clk_is_prepared,
> +	.recalc_rate	= max_gen_recalc_rate,
> +};
> +
>  static int max77686_clk_probe(struct platform_device *pdev)
>  {
> -	struct max77686_dev *iodev = dev_get_drvdata(pdev->dev.parent);
> +	struct device *parent = pdev->dev.parent;

parent = dev->parent;

> +	struct device *dev = &pdev->dev;
> +	const struct platform_device_id *id = platform_get_device_id(pdev);
> +	struct max_gen_clk_driver_info *drv_info;
> +	struct max_gen_clk_data *max_clk_data;

This looks local to for loop, so place it there.

> +	const struct max_gen_hw_clk_data *hw_clks;
> +	struct regmap *regmap;
> +	int i, ret, num_clks;
> +
> +	drv_info = devm_kzalloc(dev, sizeof(*drv_info), GFP_KERNEL);
> +	if (!drv_info)
> +		return -ENOMEM;
> +
> +	regmap = dev_get_regmap(parent, NULL);
> +	if (!regmap) {
> +		dev_err(dev, "Failed to get rtc regmap\n");
> +		return -ENODEV;
> +	}
> +
> +	drv_info->chip = id->driver_data;
> +
> +	switch (drv_info->chip) {
> +	case CHIP_MAX77686:
> +		num_clks = MAX77686_CLKS_NUM;
> +		hw_clks = max77686_hw_clks_info;
> +		break;
> +	case CHIP_MAX77802:
> +		num_clks = MAX77802_CLKS_NUM;
> +		hw_clks = max77802_hw_clks_info;
> +		break;
> +	default:
> +		dev_err(dev, "Unknown Chip ID\n");
> +		return -EINVAL;
> +	};
>  
> -	return max_gen_clk_probe(pdev, iodev->regmap, MAX77686_REG_32KHZ,
> -				 max77686_clks_init, MAX77686_CLKS_NUM);
> +	drv_info->max_clk_data = devm_kcalloc(dev, num_clks,
> +					      sizeof(*drv_info->max_clk_data),
> +					      GFP_KERNEL);
> +	if (!drv_info->max_clk_data)
> +		return -ENOMEM;
> +
> +	drv_info->clks = devm_kcalloc(dev, num_clks,
> +				      sizeof(*drv_info->clks), GFP_KERNEL);
> +	if (!drv_info->clks)
> +		return -ENOMEM;
> +
> +	for (i = 0; i < num_clks; i++) {
> +		struct clk *clk;
> +		const char *clk_name;
> +
> +		max_clk_data = &drv_info->max_clk_data[i];
> +
> +		max_clk_data->regmap = regmap;
> +		max_clk_data->mask = hw_clks[i].mask;
> +		max_clk_data->reg = hw_clks[i].reg;
> +		max_clk_data->clk_idata.flags = hw_clks[i].flags;
> +		max_clk_data->clk_idata.ops = &max_gen_clk_ops;
> +
> +		if (parent->of_node &&
> +		    !of_property_read_string_index(parent->of_node,
> +						   "clock-output-names",
> +						   i, &clk_name))
> +			max_clk_data->clk_idata.name = clk_name;
> +		else
> +			max_clk_data->clk_idata.name = hw_clks[i].name;
> +
> +		max_clk_data->hw.init = &max_clk_data->clk_idata;
> +
> +		clk = devm_clk_register(dev, &max_clk_data->hw);
> +		if (IS_ERR(clk)) {
> +			ret = PTR_ERR(clk);
> +			dev_err(dev, "Failed to clock register: %d\n", ret);
> +			return ret;
> +		}
> +
> +		ret = clk_register_clkdev(clk, max_clk_data->clk_idata.name,
> +					  NULL);
> +		if (ret < 0) {
> +			dev_err(dev, "Failed to clkdev register: %d\n", ret);
> +			return ret;
> +		}
> +		drv_info->clks[i] = clk;
> +	}
> +
> +	platform_set_drvdata(pdev, drv_info);
> +
> +	if (parent->of_node) {
> +		drv_info->of_data.clks = drv_info->clks;
> +		drv_info->of_data.clk_num = num_clks;
> +		ret = of_clk_add_provider(parent->of_node,
> +					  of_clk_src_onecell_get,
> +					  &drv_info->of_data);
> +
> +		if (ret < 0) {
> +			dev_err(dev, "Failed to register OF clock provider: %d\n",
> +				ret);
> +			return ret;
> +		}
> +	}
> +
> +	/* MAX77802: Enable low-jitter mode on the 32khz clocks. */
> +	if (drv_info->chip == CHIP_MAX77802) {
> +		ret = regmap_update_bits(regmap, MAX77802_REG_32KHZ,
> +					 1 << MAX77802_CLOCK_LOW_JITTER_SHIFT,
> +					 1 << MAX77802_CLOCK_LOW_JITTER_SHIFT);
> +		if (ret < 0) {
> +			dev_err(dev, "Failed to set low-jitter mode: %d\n",
> +				ret);

Need a cleanup path:
	of_clk_del_provider(parent->of_node);

> +			return ret;
> +		}
> +	}
> +
> +	return 0;
>  }
>  
>  static int max77686_clk_remove(struct platform_device *pdev)
>  {
> -	return max_gen_clk_remove(pdev, MAX77686_CLKS_NUM);
> +	struct device *parent = pdev->dev.parent;
> +
> +	if (parent->of_node)
> +		of_clk_del_provider(parent->of_node);
> +
> +	return 0;
>  }
>  
>  static const struct platform_device_id max77686_clk_id[] = {
> -	{ "max77686-clk", 0},
> -	{ },
> +	{ "max77686-clk", .driver_data = (kernel_ulong_t)CHIP_MAX77686, },
> +	{ "max77802-clk", .driver_data = (kernel_ulong_t)CHIP_MAX77802, },

You don't need the cast.

Best regards,
Krzysztof

Re: [PATCH 1/5] clk: max77686: Combine Maxim max77686 and max77802 driver

[Krzysztof Kozlowski]

On 06/15/2016 04:13 PM, Laxman Dewangan wrote:
> The clock IP used on the Maxim PMICs max77686 and max77802 are
> same. The configuration of clock register is also same except
> the number of clocks.
> 
> Part of common code utilisation, there is 3 files for these chips
> clock driver, one for common and two files for driver registration.
> 
> Combine both drivers into single file and move common code into
> same common file reduces the 2 files and make max77686 and max77802
> clock driver in single fine. This driver does not depends on the
> parent driver structure. The regmap handle is acquired through
> regmap APIs for the register access.
> 
> This combination of driver helps on adding clock driver for different
> Maxim PMICs which has similar clock IP like MAX77620 and MAX20024.
> 
> Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
> CC: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> CC: Javier Martinez Canillas <javier@dowhile0.org>
> ---
>  drivers/clk/Kconfig        |  15 +--
>  drivers/clk/Makefile       |   2 -
>  drivers/clk/clk-max-gen.c  | 194 ------------------------------------
>  drivers/clk/clk-max-gen.h  |  32 ------
>  drivers/clk/clk-max77686.c | 241 +++++++++++++++++++++++++++++++++++++++++----
>  drivers/clk/clk-max77802.c |  96 ------------------
>  6 files changed, 225 insertions(+), 355 deletions(-)
>  delete mode 100644 drivers/clk/clk-max-gen.c
>  delete mode 100644 drivers/clk/clk-max-gen.h
>  delete mode 100644 drivers/clk/clk-max77802.c

Tested on max77686 and max77802:
Tested-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>

Best regards,
Krzysztof

[PATCH 2/5] clk: Combine DT binding doc for max77686 and max77802

[Laxman Dewangan]

The clock driver for Maxim PMICs max77686 and max77802 are
combined into single file to extend the support for same clock
IP for different PMICs.

Remove the separate DT binding document file for maxim,max77802 and
move all information to maxim,max77686 DT binding document.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
CC: Krzysztof Kozlowski <k.kozlowski@samsung.com>
CC: Javier Martinez Canillas <javier@dowhile0.org>
---
 .../devicetree/bindings/clock/maxim,max77686.txt   | 78 ++++++++++++++++------
 .../devicetree/bindings/clock/maxim,max77802.txt   | 44 ------------
 2 files changed, 58 insertions(+), 64 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/clock/maxim,max77802.txt

diff --git a/Documentation/devicetree/bindings/clock/maxim,max77686.txt b/Documentation/devicetree/bindings/clock/maxim,max77686.txt
index 9c40739..354e5ab 100644
--- a/Documentation/devicetree/bindings/clock/maxim,max77686.txt
+++ b/Documentation/devicetree/bindings/clock/maxim,max77686.txt
@@ -1,11 +1,15 @@
-Binding for Maxim MAX77686 32k clock generator block
+Binding for Maxim MAX77686/MAX77802 32k clock generator block
 
-This is a part of device tree bindings of MAX77686 multi-function device.
-More information can be found in bindings/mfd/max77686.txt file.
+This is a part of device tree bindings of MAX77686/MAX77802 multi-function
+device. More information can be found in bindings/mfd/max77686.txt file for
+MAX77686 and bindings/mfd/max77802.txt for MAX77802.
 
 The MAX77686 contains three 32.768khz clock outputs that can be controlled
 (gated/ungated) over I2C.
 
+The MAX77802 contains two 32.768khz clock outputs that can be controlled
+(gated/ungated) over I2C.
+
 Following properties should be presend in main device node of the MFD chip.
 
 Required properties:
@@ -21,26 +25,60 @@ to specify the clock which they consume. Following indices are allowed:
     - 1: 32khz_cp clock,
     - 2: 32khz_pmic clock.
 
+For MAX77802, 32khz_pmic is not valid.
+
 Clocks are defined as preprocessor macros in dt-bindings/clock/maxim,max77686.h
-header and can be used in device tree sources.
+header for MAX77686 and dt-bindings/clock/maxim,max77802.h for MAX77802 and can
+be used in device tree sources.
+
+Example:
+
+1. With MAX77686:
+
+#include <dt-bindings/clock/maxim,max77686.h>
+:::
+
+	Node of the MFD chip
+		max77686: max77686@09 {
+			compatible = "maxim,max77686";
+			interrupt-parent = <&wakeup_eint>;
+			interrupts = <26 0>;
+			reg = <0x09>;
+			#clock-cells = <1>;
+
+			/* ... */
+		};
+
+	Clock consumer node
+
+		foo@0 {
+			compatible = "bar,foo";
+			/* ... */
+			clock-names = "my-clock";
+			clocks = <&max77686 MAX77686_CLK_PMIC>;
+		};
+
+2. With MAX77802:
 
-Example: Node of the MFD chip
+#include <dt-bindings/clock/maxim,max77802.h>
+:::
 
-	max77686: max77686@09 {
-		compatible = "maxim,max77686";
-		interrupt-parent = <&wakeup_eint>;
-		interrupts = <26 0>;
-		reg = <0x09>;
-		#clock-cells = <1>;
+	Node of the MFD chip
+		max77802: max77802@09 {
+			compatible = "maxim,max77802";
+			interrupt-parent = <&wakeup_eint>;
+			interrupts = <26 0>;
+			reg = <0x09>;
+			#clock-cells = <1>;
 
-		/* ... */
-	};
+			/* ... */
+		};
 
-Example: Clock consumer node
+	Clock consumer node
 
-	foo@0 {
-		compatible = "bar,foo";
-		/* ... */
-		clock-names = "my-clock";
-		clocks = <&max77686 MAX77686_CLK_PMIC>;
-	};
+		foo@0 {
+			compatible = "bar,foo";
+			/* ... */
+			clock-names = "my-clock";
+			clocks = <&max77802 MAX77802_CLK_32K_AP>;
+		};
diff --git a/Documentation/devicetree/bindings/clock/maxim,max77802.txt b/Documentation/devicetree/bindings/clock/maxim,max77802.txt
deleted file mode 100644
index c6dc783..0000000
--- a/Documentation/devicetree/bindings/clock/maxim,max77802.txt
+++ /dev/null
@@ -1,44 +0,0 @@
-Binding for Maxim MAX77802 32k clock generator block
-
-This is a part of device tree bindings of MAX77802 multi-function device.
-More information can be found in bindings/mfd/max77802.txt file.
-
-The MAX77802 contains two 32.768khz clock outputs that can be controlled
-(gated/ungated) over I2C.
-
-Following properties should be present in main device node of the MFD chip.
-
-Required properties:
-- #clock-cells: From common clock binding; shall be set to 1.
-
-Optional properties:
-- clock-output-names: From common clock binding.
-
-Each clock is assigned an identifier and client nodes can use this identifier
-to specify the clock which they consume. Following indices are allowed:
-     - 0: 32khz_ap clock,
-     - 1: 32khz_cp clock.
-
-Clocks are defined as preprocessor macros in dt-bindings/clock/maxim,max77802.h
-header and can be used in device tree sources.
-
-Example: Node of the MFD chip
-
-	max77802: max77802@09 {
-		compatible = "maxim,max77802";
-		interrupt-parent = <&wakeup_eint>;
-		interrupts = <26 0>;
-		reg = <0x09>;
-		#clock-cells = <1>;
-
-		/* ... */
-	};
-
-Example: Clock consumer node
-
-	foo@0 {
-		compatible = "bar,foo";
-		/* ... */
-		clock-names = "my-clock";
-		clocks = <&max77802 MAX77802_CLK_32K_AP>;
-	};
-- 
2.1.4

Re: [PATCH 2/5] clk: Combine DT binding doc for max77686 and max77802

[Javier Martinez Canillas]

Hello Laxman,

On 06/15/2016 10:13 AM, Laxman Dewangan wrote:
> The clock driver for Maxim PMICs max77686 and max77802 are
> combined into single file to extend the support for same clock
> IP for different PMICs.
>

The DT binding docs shouldn't neither refer to drivers nor map the file
structure of the Linux drivers. That's just an implementation detail of
Linux and it could be different in other operating systems, the binding
should only describe the hardware.

So the commit message should explain the benefits of merging the two DT
bindings docs without mentioning the Linux driver implementation.
 
> Remove the separate DT binding document file for maxim,max77802 and
> move all information to maxim,max77686 DT binding document.
> 

Now, I wonder if this is the correct approach. A system integrator is
probably going to search for the bindings of the chip that is present
in their system. For example there are different DT bindings docs for
both the max77686 and max77802 PMIC even when support is implemented
by the same driver (drivers/mfd/max77686.c).

But maybe I'm wrong so I'll let people with more DT experience to say
if merging the bindings docs in a single one is the correct approach.

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

Re: [PATCH 2/5] clk: Combine DT binding doc for max77686 and max77802

[Krzysztof Kozlowski]

On 06/15/2016 05:19 PM, Javier Martinez Canillas wrote:
>> Remove the separate DT binding document file for maxim,max77802 and
>> move all information to maxim,max77686 DT binding document.
>>
> 
> Now, I wonder if this is the correct approach. A system integrator is
> probably going to search for the bindings of the chip that is present
> in their system. For example there are different DT bindings docs for
> both the max77686 and max77802 PMIC even when support is implemented
> by the same driver (drivers/mfd/max77686.c).
> 
> But maybe I'm wrong so I'll let people with more DT experience to say
> if merging the bindings docs in a single one is the correct approach.

I personally prefer having one binding document for all devices because
essentially it is almost the same device. For looking up I use git grep
with compatible string (or device name) so it does not matter, IMHO.

Best regards,
Krzysztof

Re: [PATCH 2/5] clk: Combine DT binding doc for max77686 and max77802

[Krzysztof Kozlowski]

On 06/15/2016 04:13 PM, Laxman Dewangan wrote:
> The clock driver for Maxim PMICs max77686 and max77802 are
> combined into single file to extend the support for same clock
> IP for different PMICs.
> 
> Remove the separate DT binding document file for maxim,max77802 and
> move all information to maxim,max77686 DT binding document.
> 
> Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
> CC: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> CC: Javier Martinez Canillas <javier@dowhile0.org>
> ---
>  .../devicetree/bindings/clock/maxim,max77686.txt   | 78 ++++++++++++++++------
>  .../devicetree/bindings/clock/maxim,max77802.txt   | 44 ------------
>  2 files changed, 58 insertions(+), 64 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/clock/maxim,max77802.txt
> 
> diff --git a/Documentation/devicetree/bindings/clock/maxim,max77686.txt b/Documentation/devicetree/bindings/clock/maxim,max77686.txt
> index 9c40739..354e5ab 100644
> --- a/Documentation/devicetree/bindings/clock/maxim,max77686.txt
> +++ b/Documentation/devicetree/bindings/clock/maxim,max77686.txt
> @@ -1,11 +1,15 @@
> -Binding for Maxim MAX77686 32k clock generator block
> +Binding for Maxim MAX77686/MAX77802 32k clock generator block
>  
> -This is a part of device tree bindings of MAX77686 multi-function device.
> -More information can be found in bindings/mfd/max77686.txt file.
> +This is a part of device tree bindings of MAX77686/MAX77802 multi-function
> +device. More information can be found in bindings/mfd/max77686.txt file for
> +MAX77686 and bindings/mfd/max77802.txt for MAX77802.
>  
>  The MAX77686 contains three 32.768khz clock outputs that can be controlled
>  (gated/ungated) over I2C.
>  
> +The MAX77802 contains two 32.768khz clock outputs that can be controlled
> +(gated/ungated) over I2C.
> +
>  Following properties should be presend in main device node of the MFD chip.
>  
>  Required properties:
> @@ -21,26 +25,60 @@ to specify the clock which they consume. Following indices are allowed:
>      - 1: 32khz_cp clock,
>      - 2: 32khz_pmic clock.
>  
> +For MAX77802, 32khz_pmic is not valid.

It is easier to find for which device which ID is allowed, so maybe:
      - 1: 32khz_cp clock (max77686, max77802),
      - 2: 32khz_pmic clock. (max77686)


> +
>  Clocks are defined as preprocessor macros in dt-bindings/clock/maxim,max77686.h
> -header and can be used in device tree sources.
> +header for MAX77686 and dt-bindings/clock/maxim,max77802.h for MAX77802 and can
> +be used in device tree sources.
> +
> +Example:
> +
> +1. With MAX77686:
> +
> +#include <dt-bindings/clock/maxim,max77686.h>
> +:::

I am not sure what the ":::" means here...

Best regards,
Krzysztof

> +
> +	Node of the MFD chip
> +		max77686: max77686@09 {
> +			compatible = "maxim,max77686";
> +			interrupt-parent = <&wakeup_eint>;
> +			interrupts = <26 0>;
> +			reg = <0x09>;
> +			#clock-cells = <1>;
> +
> +			/* ... */
> +		};
> +
> +	Clock consumer node
> +
> +		foo@0 {
> +			compatible = "bar,foo";
> +			/* ... */
> +			clock-names = "my-clock";
> +			clocks = <&max77686 MAX77686_CLK_PMIC>;
> +		};
> +
> +2. With MAX77802:
>  
> -Example: Node of the MFD chip
> +#include <dt-bindings/clock/maxim,max77802.h>
> +:::
>  
> -	max77686: max77686@09 {
> -		compatible = "maxim,max77686";
> -		interrupt-parent = <&wakeup_eint>;
> -		interrupts = <26 0>;
> -		reg = <0x09>;
> -		#clock-cells = <1>;
> +	Node of the MFD chip
> +		max77802: max77802@09 {
> +			compatible = "maxim,max77802";
> +			interrupt-parent = <&wakeup_eint>;
> +			interrupts = <26 0>;
> +			reg = <0x09>;
> +			#clock-cells = <1>;
>  
> -		/* ... */
> -	};
> +			/* ... */
> +		};
>  
> -Example: Clock consumer node
> +	Clock consumer node
>  
> -	foo@0 {
> -		compatible = "bar,foo";
> -		/* ... */
> -		clock-names = "my-clock";
> -		clocks = <&max77686 MAX77686_CLK_PMIC>;
> -	};
> +		foo@0 {
> +			compatible = "bar,foo";
> +			/* ... */
> +			clock-names = "my-clock";
> +			clocks = <&max77802 MAX77802_CLK_32K_AP>;
> +		};
> diff --git a/Documentation/devicetree/bindings/clock/maxim,max77802.txt b/Documentation/devicetree/bindings/clock/maxim,max77802.txt
> deleted file mode 100644
> index c6dc783..0000000
> --- a/Documentation/devicetree/bindings/clock/maxim,max77802.txt
> +++ /dev/null
> @@ -1,44 +0,0 @@
> -Binding for Maxim MAX77802 32k clock generator block
> -
> -This is a part of device tree bindings of MAX77802 multi-function device.
> -More information can be found in bindings/mfd/max77802.txt file.
> -
> -The MAX77802 contains two 32.768khz clock outputs that can be controlled
> -(gated/ungated) over I2C.
> -
> -Following properties should be present in main device node of the MFD chip.
> -
> -Required properties:
> -- #clock-cells: From common clock binding; shall be set to 1.
> -
> -Optional properties:
> -- clock-output-names: From common clock binding.
> -
> -Each clock is assigned an identifier and client nodes can use this identifier
> -to specify the clock which they consume. Following indices are allowed:
> -     - 0: 32khz_ap clock,
> -     - 1: 32khz_cp clock.
> -
> -Clocks are defined as preprocessor macros in dt-bindings/clock/maxim,max77802.h
> -header and can be used in device tree sources.
> -
> -Example: Node of the MFD chip
> -
> -	max77802: max77802@09 {
> -		compatible = "maxim,max77802";
> -		interrupt-parent = <&wakeup_eint>;
> -		interrupts = <26 0>;
> -		reg = <0x09>;
> -		#clock-cells = <1>;
> -
> -		/* ... */
> -	};
> -
> -Example: Clock consumer node
> -
> -	foo@0 {
> -		compatible = "bar,foo";
> -		/* ... */
> -		clock-names = "my-clock";
> -		clocks = <&max77802 MAX77802_CLK_32K_AP>;
> -	};
> 

[PATCH 3/5] clk: max77686: Add DT binding details for PMIC MAX77620

[Laxman Dewangan]

Maxim has used the same clock IP on multiple PMICs like
MAX77686, MAX77802, MAX77620. Only differences are the
number of clocks from these PMICs.

Add clock binding details and example for the max77620 in
maxim,max77686.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
CC: Krzysztof Kozlowski <k.kozlowski@samsung.com>
CC: Javier Martinez Canillas <javier@dowhile0.org>
---
 .../devicetree/bindings/clock/maxim,max77686.txt   | 38 +++++++++++++++++++---
 include/dt-bindings/mfd/max77620.h                 |  4 +++
 2 files changed, 38 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/clock/maxim,max77686.txt b/Documentation/devicetree/bindings/clock/maxim,max77686.txt
index 354e5ab..24deb19 100644
--- a/Documentation/devicetree/bindings/clock/maxim,max77686.txt
+++ b/Documentation/devicetree/bindings/clock/maxim,max77686.txt
@@ -1,8 +1,11 @@
-Binding for Maxim MAX77686/MAX77802 32k clock generator block
+Binding for Maxim MAX77686/MAX77802/MAX77620 32k clock generator block
 
-This is a part of device tree bindings of MAX77686/MAX77802 multi-function
-device. More information can be found in bindings/mfd/max77686.txt file for
-MAX77686 and bindings/mfd/max77802.txt for MAX77802.
+This is a part of device tree bindings of MAX77686/MAX77802/MAX77620
+multi-function device. More information can be found in MFD DT binding
+doc as follows:
+	bindings/mfd/max77686.txt for MAX77686 and
+	bindings/mfd/max77802.txt for MAX77802 and
+	bindings/mfd/max77620.txt for MAX77620.
 
 The MAX77686 contains three 32.768khz clock outputs that can be controlled
 (gated/ungated) over I2C.
@@ -10,6 +13,9 @@ The MAX77686 contains three 32.768khz clock outputs that can be controlled
 The MAX77802 contains two 32.768khz clock outputs that can be controlled
 (gated/ungated) over I2C.
 
+The MAX77686 contains one 32.768khz clock outputs that can be controlled
+(gated/ungated) over I2C.
+
 Following properties should be presend in main device node of the MFD chip.
 
 Required properties:
@@ -82,3 +88,27 @@ Example:
 			clock-names = "my-clock";
 			clocks = <&max77802 MAX77802_CLK_32K_AP>;
 		};
+
+
+3. With MAX77620:
+
+#include <dt-bindings/mfd/max77620.h>
+:::
+
+	Node of the MFD chip
+		max77620: max77620@3c {
+			compatible = "maxim,max77620";
+			reg = <0x3c>;
+			:::
+			#clock-cells = <1>;
+			:::
+		};
+
+	Clock consumer node
+
+		foo@0 {
+			compatible = "bar,foo";
+			/* ... */
+			clock-names = "my-clock";
+			clocks = <&max77620 MAX77620_CLK_32K_OUT0>;
+		};
diff --git a/include/dt-bindings/mfd/max77620.h b/include/dt-bindings/mfd/max77620.h
index b911a07..e1bd08c 100644
--- a/include/dt-bindings/mfd/max77620.h
+++ b/include/dt-bindings/mfd/max77620.h
@@ -36,4 +36,8 @@
 #define MAX77620_FPS_SRC_NONE			3
 #define MAX77620_FPS_SRC_DEF			4
 
+/* MAX77686 clocks */
+#define MAX77620_CLKS_NUM			1
+#define MAX77620_CLK_32K_OUT0			0
+
 #endif
-- 
2.1.4

Re: [PATCH 3/5] clk: max77686: Add DT binding details for PMIC MAX77620

[Javier Martinez Canillas]

Hello Laxman,

On 06/15/2016 10:13 AM, Laxman Dewangan wrote:
> Maxim has used the same clock IP on multiple PMICs like
> MAX77686, MAX77802, MAX77620. Only differences are the
> number of clocks from these PMICs.
> 
> Add clock binding details and example for the max77620 in
> maxim,max77686.
> 
> Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
> CC: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> CC: Javier Martinez Canillas <javier@dowhile0.org>
> ---
>  .../devicetree/bindings/clock/maxim,max77686.txt   | 38 +++++++++++++++++++---
>  include/dt-bindings/mfd/max77620.h                 |  4 +++

[snip]

> +		};
> diff --git a/include/dt-bindings/mfd/max77620.h b/include/dt-bindings/mfd/max77620.h
> index b911a07..e1bd08c 100644
> --- a/include/dt-bindings/mfd/max77620.h
> +++ b/include/dt-bindings/mfd/max77620.h
> @@ -36,4 +36,8 @@
>  #define MAX77620_FPS_SRC_NONE			3
>  #define MAX77620_FPS_SRC_DEF			4
>  
> +/* MAX77686 clocks */
> +#define MAX77620_CLKS_NUM			1
> +#define MAX77620_CLK_32K_OUT0			0
> +
>  #endif
> 

The clocks for the other Maxim PMICs define their clocks in the clock subdir
instead, so I wonder if include/dt-bindings/clock/maxim,max77620.h wouldn't
be better for consistency?

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

Re: [PATCH 3/5] clk: max77686: Add DT binding details for PMIC MAX77620

[Krzysztof Kozlowski]

On 06/15/2016 04:13 PM, Laxman Dewangan wrote:
> Maxim has used the same clock IP on multiple PMICs like
> MAX77686, MAX77802, MAX77620. Only differences are the
> number of clocks from these PMICs.
> 
> Add clock binding details and example for the max77620 in
> maxim,max77686.
> 
> Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
> CC: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> CC: Javier Martinez Canillas <javier@dowhile0.org>
> ---
>  .../devicetree/bindings/clock/maxim,max77686.txt   | 38 +++++++++++++++++++---
>  include/dt-bindings/mfd/max77620.h                 |  4 +++
>  2 files changed, 38 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/clock/maxim,max77686.txt b/Documentation/devicetree/bindings/clock/maxim,max77686.txt
> index 354e5ab..24deb19 100644
> --- a/Documentation/devicetree/bindings/clock/maxim,max77686.txt
> +++ b/Documentation/devicetree/bindings/clock/maxim,max77686.txt
> @@ -1,8 +1,11 @@
> -Binding for Maxim MAX77686/MAX77802 32k clock generator block
> +Binding for Maxim MAX77686/MAX77802/MAX77620 32k clock generator block
>  
> -This is a part of device tree bindings of MAX77686/MAX77802 multi-function
> -device. More information can be found in bindings/mfd/max77686.txt file for
> -MAX77686 and bindings/mfd/max77802.txt for MAX77802.
> +This is a part of device tree bindings of MAX77686/MAX77802/MAX77620
> +multi-function device. More information can be found in MFD DT binding
> +doc as follows:
> +	bindings/mfd/max77686.txt for MAX77686 and
> +	bindings/mfd/max77802.txt for MAX77802 and
> +	bindings/mfd/max77620.txt for MAX77620.
>  
>  The MAX77686 contains three 32.768khz clock outputs that can be controlled
>  (gated/ungated) over I2C.
> @@ -10,6 +13,9 @@ The MAX77686 contains three 32.768khz clock outputs that can be controlled
>  The MAX77802 contains two 32.768khz clock outputs that can be controlled
>  (gated/ungated) over I2C.
>  
> +The MAX77686 contains one 32.768khz clock outputs that can be controlled
> +(gated/ungated) over I2C.
> +
>  Following properties should be presend in main device node of the MFD chip.

Please update the information about allowed IDs.

>  
>  Required properties:
> @@ -82,3 +88,27 @@ Example:
>  			clock-names = "my-clock";
>  			clocks = <&max77802 MAX77802_CLK_32K_AP>;
>  		};
> +
> +
> +3. With MAX77620:
> +
> +#include <dt-bindings/mfd/max77620.h>
> +:::

Same as before: ':::'

> +
> +	Node of the MFD chip
> +		max77620: max77620@3c {
> +			compatible = "maxim,max77620";
> +			reg = <0x3c>;
> +			:::
> +			#clock-cells = <1>;
> +			:::
> +		};
> +
> +	Clock consumer node
> +
> +		foo@0 {
> +			compatible = "bar,foo";
> +			/* ... */
> +			clock-names = "my-clock";
> +			clocks = <&max77620 MAX77620_CLK_32K_OUT0>;
> +		};
> diff --git a/include/dt-bindings/mfd/max77620.h b/include/dt-bindings/mfd/max77620.h
> index b911a07..e1bd08c 100644
> --- a/include/dt-bindings/mfd/max77620.h
> +++ b/include/dt-bindings/mfd/max77620.h
> @@ -36,4 +36,8 @@
>  #define MAX77620_FPS_SRC_NONE			3
>  #define MAX77620_FPS_SRC_DEF			4
>  
> +/* MAX77686 clocks */

copy&paste error.

> +#define MAX77620_CLKS_NUM			1
> +#define MAX77620_CLK_32K_OUT0			0

First clk ID, then NUM.

Best regards,
Krzysztof


> +
>  #endif
> 

[PATCH 4/5] clk: max77686: Add support for MAX77620 clocks

[Laxman Dewangan]

Maxim Max77620 has one 32KHz clock output and the clock HW
IP used on this PMIC is same as what it is there in the MAX77686.

Add clock driver support for MAX77620 on the MAX77686 driver.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
CC: Krzysztof Kozlowski <k.kozlowski@samsung.com>
CC: Javier Martinez Canillas <javier@dowhile0.org>
---
 drivers/clk/Kconfig        |  7 ++++---
 drivers/clk/clk-max77686.c | 16 ++++++++++++++++
 2 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
index 6afad74..d75f4c5 100644
--- a/drivers/clk/Kconfig
+++ b/drivers/clk/Kconfig
@@ -32,10 +32,11 @@ config COMMON_CLK_WM831X
 source "drivers/clk/versatile/Kconfig"
 
 config COMMON_CLK_MAX77686
-	tristate "Clock driver for Maxim 77686/77802 MFD"
-	depends on MFD_MAX77686
+	tristate "Clock driver for Maxim 77686/77802/MAX77620 MFD"
+	depends on MFD_MAX77686 || MFD_MAX77620
 	---help---
-	  This driver supports Maxim 77686/77802 crystal oscillator clock.
+	  This driver supports Maxim 77686/77802/MAX77620 crystal oscillator
+	  clock.
 
 config COMMON_CLK_RK808
 	tristate "Clock driver for RK808"
diff --git a/drivers/clk/clk-max77686.c b/drivers/clk/clk-max77686.c
index 31ba726..d2be736 100644
--- a/drivers/clk/clk-max77686.c
+++ b/drivers/clk/clk-max77686.c
@@ -19,6 +19,7 @@
 #include <linux/err.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>
+#include <linux/mfd/max77620.h>
 #include <linux/mfd/max77686.h>
 #include <linux/mfd/max77686-private.h>
 #include <linux/clk-provider.h>
@@ -30,12 +31,14 @@
 
 #include <dt-bindings/clock/maxim,max77686.h>
 #include <dt-bindings/clock/maxim,max77802.h>
+#include <dt-bindings/mfd/max77620.h>
 
 #define MAX77802_CLOCK_LOW_JITTER_SHIFT 0x3
 
 enum chip_name {
 	CHIP_MAX77686,
 	CHIP_MAX77802,
+	CHIP_MAX77620,
 };
 
 struct max_gen_hw_clk_data {
@@ -91,6 +94,14 @@ static struct max_gen_hw_clk_data max77802_hw_clks_info[MAX77802_CLKS_NUM] = {
 	},
 };
 
+static struct max_gen_hw_clk_data max77620_hw_clks_info[MAX77620_CLKS_NUM] = {
+	[MAX77620_CLK_32K_OUT0] = {
+		.name = "32khz_pmic",
+		.reg = MAX77620_REG_CNFG1_32K,
+		.mask = MAX77620_CNFG1_32K_OUT0_EN,
+	},
+};
+
 static struct max_gen_clk_data *to_max_gen_clk_data(struct clk_hw *hw)
 {
 	return container_of(hw, struct max_gen_clk_data, hw);
@@ -171,6 +182,10 @@ static int max77686_clk_probe(struct platform_device *pdev)
 		num_clks = MAX77802_CLKS_NUM;
 		hw_clks = max77802_hw_clks_info;
 		break;
+	case CHIP_MAX77620:
+		num_clks = MAX77620_CLKS_NUM;
+		hw_clks = max77620_hw_clks_info;
+		break;
 	default:
 		dev_err(dev, "Unknown Chip ID\n");
 		return -EINVAL;
@@ -269,6 +284,7 @@ static int max77686_clk_remove(struct platform_device *pdev)
 static const struct platform_device_id max77686_clk_id[] = {
 	{ "max77686-clk", .driver_data = (kernel_ulong_t)CHIP_MAX77686, },
 	{ "max77802-clk", .driver_data = (kernel_ulong_t)CHIP_MAX77802, },
+	{ "max77620-clock", .driver_data = (kernel_ulong_t)CHIP_MAX77620, },
 	{},
 };
 MODULE_DEVICE_TABLE(platform, max77686_clk_id);
-- 
2.1.4

Re: [PATCH 4/5] clk: max77686: Add support for MAX77620 clocks

[Javier Martinez Canillas]

Hello Laxman,

On 06/15/2016 10:13 AM, Laxman Dewangan wrote:
> Maxim Max77620 has one 32KHz clock output and the clock HW
> IP used on this PMIC is same as what it is there in the MAX77686.
> 
> Add clock driver support for MAX77620 on the MAX77686 driver.
> 
> Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
> CC: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> CC: Javier Martinez Canillas <javier@dowhile0.org>
> ---
>  drivers/clk/Kconfig        |  7 ++++---
>  drivers/clk/clk-max77686.c | 16 ++++++++++++++++
>  2 files changed, 20 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
> index 6afad74..d75f4c5 100644
> --- a/drivers/clk/Kconfig
> +++ b/drivers/clk/Kconfig
> @@ -32,10 +32,11 @@ config COMMON_CLK_WM831X
>  source "drivers/clk/versatile/Kconfig"
>  
>  config COMMON_CLK_MAX77686
> -	tristate "Clock driver for Maxim 77686/77802 MFD"
> -	depends on MFD_MAX77686
> +	tristate "Clock driver for Maxim 77686/77802/MAX77620 MFD"
> +	depends on MFD_MAX77686 || MFD_MAX77620
>  	---help---
> -	  This driver supports Maxim 77686/77802 crystal oscillator clock.
> +	  This driver supports Maxim 77686/77802/MAX77620 crystal oscillator

s/MAX77620/77620

Besides this minor nit, the patch looks good to me:

Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com>

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

Re: [PATCH 4/5] clk: max77686: Add support for MAX77620 clocks

[Krzysztof Kozlowski]

On 06/15/2016 04:13 PM, Laxman Dewangan wrote:
> Maxim Max77620 has one 32KHz clock output and the clock HW
> IP used on this PMIC is same as what it is there in the MAX77686.
> 
> Add clock driver support for MAX77620 on the MAX77686 driver.
> 
> Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
> CC: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> CC: Javier Martinez Canillas <javier@dowhile0.org>
> ---
>  drivers/clk/Kconfig        |  7 ++++---
>  drivers/clk/clk-max77686.c | 16 ++++++++++++++++
>  2 files changed, 20 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
> index 6afad74..d75f4c5 100644
> --- a/drivers/clk/Kconfig
> +++ b/drivers/clk/Kconfig
> @@ -32,10 +32,11 @@ config COMMON_CLK_WM831X
>  source "drivers/clk/versatile/Kconfig"
>  
>  config COMMON_CLK_MAX77686
> -	tristate "Clock driver for Maxim 77686/77802 MFD"
> -	depends on MFD_MAX77686
> +	tristate "Clock driver for Maxim 77686/77802/MAX77620 MFD"
> +	depends on MFD_MAX77686 || MFD_MAX77620
>  	---help---
> -	  This driver supports Maxim 77686/77802 crystal oscillator clock.
> +	  This driver supports Maxim 77686/77802/MAX77620 crystal oscillator
> +	  clock.
>  
>  config COMMON_CLK_RK808
>  	tristate "Clock driver for RK808"
> diff --git a/drivers/clk/clk-max77686.c b/drivers/clk/clk-max77686.c
> index 31ba726..d2be736 100644
> --- a/drivers/clk/clk-max77686.c
> +++ b/drivers/clk/clk-max77686.c
> @@ -19,6 +19,7 @@
>  #include <linux/err.h>
>  #include <linux/module.h>
>  #include <linux/platform_device.h>
> +#include <linux/mfd/max77620.h>
>  #include <linux/mfd/max77686.h>
>  #include <linux/mfd/max77686-private.h>
>  #include <linux/clk-provider.h>
> @@ -30,12 +31,14 @@
>  
>  #include <dt-bindings/clock/maxim,max77686.h>
>  #include <dt-bindings/clock/maxim,max77802.h>
> +#include <dt-bindings/mfd/max77620.h>
>  
>  #define MAX77802_CLOCK_LOW_JITTER_SHIFT 0x3
>  
>  enum chip_name {
>  	CHIP_MAX77686,
>  	CHIP_MAX77802,
> +	CHIP_MAX77620,

Now it is more obvious why it is here. However I don't like the
duplication of device IDs, under different names and different values:
1. include/linux/mfd/max77686-private.h
2. include/linux/mfd/max77620.h
3. here

I was thinking about way of combining it... but these headers are
different. Unless there will be one header for all three devices.

Anyway, please add the max77686 prefix to the enum.

Best regards,
Krzysztof

>  };
>  
>  struct max_gen_hw_clk_data {
> @@ -91,6 +94,14 @@ static struct max_gen_hw_clk_data max77802_hw_clks_info[MAX77802_CLKS_NUM] = {
>  	},
>  };
>  
> +static struct max_gen_hw_clk_data max77620_hw_clks_info[MAX77620_CLKS_NUM] = {
> +	[MAX77620_CLK_32K_OUT0] = {
> +		.name = "32khz_pmic",
> +		.reg = MAX77620_REG_CNFG1_32K,
> +		.mask = MAX77620_CNFG1_32K_OUT0_EN,
> +	},
> +};
> +
>  static struct max_gen_clk_data *to_max_gen_clk_data(struct clk_hw *hw)
>  {
>  	return container_of(hw, struct max_gen_clk_data, hw);
> @@ -171,6 +182,10 @@ static int max77686_clk_probe(struct platform_device *pdev)
>  		num_clks = MAX77802_CLKS_NUM;
>  		hw_clks = max77802_hw_clks_info;
>  		break;
> +	case CHIP_MAX77620:
> +		num_clks = MAX77620_CLKS_NUM;
> +		hw_clks = max77620_hw_clks_info;
> +		break;
>  	default:
>  		dev_err(dev, "Unknown Chip ID\n");
>  		return -EINVAL;
> @@ -269,6 +284,7 @@ static int max77686_clk_remove(struct platform_device *pdev)
>  static const struct platform_device_id max77686_clk_id[] = {
>  	{ "max77686-clk", .driver_data = (kernel_ulong_t)CHIP_MAX77686, },
>  	{ "max77802-clk", .driver_data = (kernel_ulong_t)CHIP_MAX77802, },
> +	{ "max77620-clock", .driver_data = (kernel_ulong_t)CHIP_MAX77620, },
>  	{},
>  };
>  MODULE_DEVICE_TABLE(platform, max77686_clk_id);
> 

Re: [PATCH 4/5] clk: max77686: Add support for MAX77620 clocks

[Laxman Dewangan]

Hi Krzysztof/Javier,
Thanks for review of the series.

I will post the V2 after taking care of all comment.

One query about the comment.

Thanks,
Laxman

On Thursday 16 June 2016 03:24 PM, Krzysztof Kozlowski wrote:
> On 06/15/2016 04:13 PM, Laxman Dewangan wrote:
>>   enum chip_name {
>>   	CHIP_MAX77686,
>>   	CHIP_MAX77802,
>> +	CHIP_MAX77620,
> Now it is more obvious why it is here. However I don't like the
> duplication of device IDs, under different names and different values:
> 1. include/linux/mfd/max77686-private.h
> 2. include/linux/mfd/max77620.h
> 3. here
>
> I was thinking about way of combining it... but these headers are
> different. Unless there will be one header for all three devices.
>
> Anyway, please add the max77686 prefix to the enum.

so will it be:

enum max77686_chip_name {
     CHIP_MAX77686,
     CHIP_MAX77802,
     CHIP_MAX77620,
};


Will it be fine here?

Re: [PATCH 4/5] clk: max77686: Add support for MAX77620 clocks

[Krzysztof Kozlowski]

On 06/16/2016 11:52 AM, Laxman Dewangan wrote:
> Hi Krzysztof/Javier,
> Thanks for review of the series.
> 
> I will post the V2 after taking care of all comment.
> 
> One query about the comment.
> 
> Thanks,
> Laxman
> 
> On Thursday 16 June 2016 03:24 PM, Krzysztof Kozlowski wrote:
>> On 06/15/2016 04:13 PM, Laxman Dewangan wrote:
>>>   enum chip_name {
>>>       CHIP_MAX77686,
>>>       CHIP_MAX77802,
>>> +    CHIP_MAX77620,
>> Now it is more obvious why it is here. However I don't like the
>> duplication of device IDs, under different names and different values:
>> 1. include/linux/mfd/max77686-private.h
>> 2. include/linux/mfd/max77620.h
>> 3. here
>>
>> I was thinking about way of combining it... but these headers are
>> different. Unless there will be one header for all three devices.
>>
>> Anyway, please add the max77686 prefix to the enum.
> 
> so will it be:
> 
> enum max77686_chip_name {
>     CHIP_MAX77686,
>     CHIP_MAX77802,
>     CHIP_MAX77620,
> };
> 
> 
> Will it be fine here?

Yes, looks fine to me.

Best regards,
Krzysztof

Re: [PATCH 4/5] clk: max77686: Add support for MAX77620 clocks

[Krzysztof Kozlowski]

On 06/15/2016 04:13 PM, Laxman Dewangan wrote:
> Maxim Max77620 has one 32KHz clock output and the clock HW
> IP used on this PMIC is same as what it is there in the MAX77686.
> 
> Add clock driver support for MAX77620 on the MAX77686 driver.
> 
> Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
> CC: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> CC: Javier Martinez Canillas <javier@dowhile0.org>
> ---
>  drivers/clk/Kconfig        |  7 ++++---
>  drivers/clk/clk-max77686.c | 16 ++++++++++++++++
>  2 files changed, 20 insertions(+), 3 deletions(-)

Tested on max77686 and max77802:
Tested-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>

Best regards,
Krzysztof

[PATCH 5/5] ARM: config: Get rid of config COMMON_CLK_MAX77802

[Laxman Dewangan]

The clock driver used by Maxim PMIC MAX77802 is clk-max77686
which can be enabled with config CONFIG_COMMON_CLK_MAX77686.

Hence the config CONFIG_COMMON_CLK_MAX77802 is not required.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
CC: Krzysztof Kozlowski <k.kozlowski@samsung.com>
CC: Javier Martinez Canillas <javier@dowhile0.org>
---
 arch/arm/configs/exynos_defconfig   | 1 -
 arch/arm/configs/multi_v7_defconfig | 1 -
 2 files changed, 2 deletions(-)

diff --git a/arch/arm/configs/exynos_defconfig b/arch/arm/configs/exynos_defconfig
index daf9762..cfd75c7 100644
--- a/arch/arm/configs/exynos_defconfig
+++ b/arch/arm/configs/exynos_defconfig
@@ -216,7 +216,6 @@ CONFIG_DMADEVICES=y
 CONFIG_PL330_DMA=y
 CONFIG_CROS_EC_CHARDEV=y
 CONFIG_COMMON_CLK_MAX77686=y
-CONFIG_COMMON_CLK_MAX77802=y
 CONFIG_COMMON_CLK_S2MPS11=y
 CONFIG_EXTCON=y
 CONFIG_EXTCON_MAX14577=y
diff --git a/arch/arm/configs/multi_v7_defconfig b/arch/arm/configs/multi_v7_defconfig
index ee7c884..7cbf389 100644
--- a/arch/arm/configs/multi_v7_defconfig
+++ b/arch/arm/configs/multi_v7_defconfig
@@ -787,7 +787,6 @@ CONFIG_CHROME_PLATFORMS=y
 CONFIG_STAGING_BOARD=y
 CONFIG_CROS_EC_CHARDEV=m
 CONFIG_COMMON_CLK_MAX77686=y
-CONFIG_COMMON_CLK_MAX77802=m
 CONFIG_COMMON_CLK_RK808=m
 CONFIG_COMMON_CLK_S2MPS11=m
 CONFIG_APQ_MMCC_8084=y
-- 
2.1.4

Re: [PATCH 5/5] ARM: config: Get rid of config COMMON_CLK_MAX77802

[Javier Martinez Canillas]

Hello Laxman,

On 06/15/2016 10:13 AM, Laxman Dewangan wrote:
> The clock driver used by Maxim PMIC MAX77802 is clk-max77686
> which can be enabled with config CONFIG_COMMON_CLK_MAX77686.
> 
> Hence the config CONFIG_COMMON_CLK_MAX77802 is not required.
> 
> Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
> CC: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> CC: Javier Martinez Canillas <javier@dowhile0.org>
> ---
>  arch/arm/configs/exynos_defconfig   | 1 -
>  arch/arm/configs/multi_v7_defconfig | 1 -

Please split these two changes in separate patches, there are just
cleanups needed after 1/5 and they can be picked separately.

The change looks good to me though so feel free to add:

Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com>
Tested-by: Javier Martinez Canillas <javier@osg.samsung.com>

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

Re: [PATCH 5/5] ARM: config: Get rid of config COMMON_CLK_MAX77802

[Krzysztof Kozlowski]

On 06/15/2016 05:39 PM, Javier Martinez Canillas wrote:
> Hello Laxman,
> 
> On 06/15/2016 10:13 AM, Laxman Dewangan wrote:
>> The clock driver used by Maxim PMIC MAX77802 is clk-max77686
>> which can be enabled with config CONFIG_COMMON_CLK_MAX77686.
>>
>> Hence the config CONFIG_COMMON_CLK_MAX77802 is not required.
>>
>> Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
>> CC: Krzysztof Kozlowski <k.kozlowski@samsung.com>
>> CC: Javier Martinez Canillas <javier@dowhile0.org>
>> ---
>>  arch/arm/configs/exynos_defconfig   | 1 -
>>  arch/arm/configs/multi_v7_defconfig | 1 -
> 
> Please split these two changes in separate patches, there are just
> cleanups needed after 1/5 and they can be picked separately.

Not entirely separately. If we do not want to loose max77802 clocks on
defconfigs, then it should go with the same tree or after driver get
merged. I prefer the latter, similar case to 1898994229f8 ("ARM:
multi_v7_defconfig: Switch max77693 to module").

Best regards,
Krzysztof

Re: [PATCH 5/5] ARM: config: Get rid of config COMMON_CLK_MAX77802

[Javier Martinez Canillas]

Hello Krzysztof,

On 06/16/2016 08:25 AM, Krzysztof Kozlowski wrote:
> On 06/15/2016 05:39 PM, Javier Martinez Canillas wrote:
>> Hello Laxman,
>>
>> On 06/15/2016 10:13 AM, Laxman Dewangan wrote:
>>> The clock driver used by Maxim PMIC MAX77802 is clk-max77686
>>> which can be enabled with config CONFIG_COMMON_CLK_MAX77686.
>>>
>>> Hence the config CONFIG_COMMON_CLK_MAX77802 is not required.
>>>
>>> Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
>>> CC: Krzysztof Kozlowski <k.kozlowski@samsung.com>
>>> CC: Javier Martinez Canillas <javier@dowhile0.org>
>>> ---
>>>  arch/arm/configs/exynos_defconfig   | 1 -
>>>  arch/arm/configs/multi_v7_defconfig | 1 -
>>
>> Please split these two changes in separate patches, there are just
>> cleanups needed after 1/5 and they can be picked separately.
> 
> Not entirely separately. If we do not want to loose max77802 clocks on
> defconfigs, then it should go with the same tree or after driver get
> merged. I prefer the latter, similar case to 1898994229f8 ("ARM:
> multi_v7_defconfig: Switch max77693 to module").
>

Agreed. Your latter example is what I tried to say (that can be picked
as cleanups separately once 1/5 hits mainline). I just didn't express
clearly.
 
> Best regards,
> Krzysztof
> 

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

Re: [PATCH 5/5] ARM: config: Get rid of config COMMON_CLK_MAX77802

[Laxman Dewangan]

On Thursday 16 June 2016 06:36 PM, Javier Martinez Canillas wrote:
> Hello Krzysztof,
>
> On 06/16/2016 08:25 AM, Krzysztof Kozlowski wrote:
>> On 06/15/2016 05:39 PM, Javier Martinez Canillas wrote:
>>> Hello Laxman,
>>>
>>> On 06/15/2016 10:13 AM, Laxman Dewangan wrote:
>>>> The clock driver used by Maxim PMIC MAX77802 is clk-max77686
>>>> which can be enabled with config CONFIG_COMMON_CLK_MAX77686.
>>>>
>>>> Hence the config CONFIG_COMMON_CLK_MAX77802 is not required.
>>>>
>>>> Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
>>>> CC: Krzysztof Kozlowski <k.kozlowski@samsung.com>
>>>> CC: Javier Martinez Canillas <javier@dowhile0.org>
>>>> ---
>>>>   arch/arm/configs/exynos_defconfig   | 1 -
>>>>   arch/arm/configs/multi_v7_defconfig | 1 -
>>> Please split these two changes in separate patches, there are just
>>> cleanups needed after 1/5 and they can be picked separately.
>> Not entirely separately. If we do not want to loose max77802 clocks on
>> defconfigs, then it should go with the same tree or after driver get
>> merged. I prefer the latter, similar case to 1898994229f8 ("ARM:
>> multi_v7_defconfig: Switch max77693 to module").
>>
> Agreed. Your latter example is what I tried to say (that can be picked
> as cleanups separately once 1/5 hits mainline). I just didn't express
> clearly.
>   
>

OK, I will drop this from series when will send V2.
The changes can be done later on once clock changes get merged.