[PATCH-v2 0/5] clk: 88pm800: Add new clk provider driver for 88PM800 MFD

[PATCH-v2 0/5] clk: 88pm800: Add new clk provider driver for 88PM800 MFD

[Vaibhav Hiremath]

88PM800 family of devices provides multiple buffered 32.768 KHz clock
output.
88PM800 : CLK32k_1, CLK32k_2 and CLK32k_3
88PM860 : CLK32k_1 and CLK32k_2

This patch-series adds new clock provider driver for enabling/disabling
buffered 32Khz clock output from 88PM800 family of device. Adds clock device to
MFD and updates the binding documentation.

The PATCH [1/4], created separately, as it would be easy between maintainers to
queue it.


Note that, I have access to only 88PM800 and 88PM860 datasheet,
so enabling support for only above two devices.

Testing:
  - Boot tested on 88PM860 based platform
  - Clock enable/disable
  - Non PM860, to make sure that it won't lead to any issues

V1 => V2:
========
Link to  V1: https://lkml.org/lkml/2015/7/21/298

  - Added new patch to the series [1/5], fixing tabbing issue in the file
  - Removed unnecessary brackets around macro value
  - Used devm_kcalloc variants for array allocation instead of kzalloc.
  - Fixed all other trivial issues.

Vaibhav Hiremath (5):
  mfd: 88pm800: Fix tabbing issue
  mfd: 88pm800: Update the header file with 32K clk related macros
  mfd: devicetree: bindings: Add clock subdevice node information
  clk: 88pm800: Add clk provider driver for 88pm800 family of devices
  mfd: 88pm800: Add support for clk subdevice

 Documentation/devicetree/bindings/mfd/88pm800.txt |  27 ++
 drivers/clk/Kconfig                               |   8 +
 drivers/clk/Makefile                              |   1 +
 drivers/clk/clk-88pm800.c                         | 341 ++++++++++++++++++++++
 drivers/mfd/88pm800.c                             | 150 ++++++----
 include/linux/mfd/88pm80x.h                       |  12 +
 6 files changed, 475 insertions(+), 64 deletions(-)
 create mode 100644 drivers/clk/clk-88pm800.c

-- 
1.9.1

[PATCH-v2 1/5] mfd: 88pm800: Fix tabbing issue

[Vaibhav Hiremath]

Driver had coding style issues where spaces were used instead
of tabs. This patch fixes them all.

Signed-off-by: Vaibhav Hiremath <vaibhav.hiremath@linaro.org>
---
 drivers/mfd/88pm800.c | 125 ++++++++++++++++++++++++--------------------------
 1 file changed, 61 insertions(+), 64 deletions(-)

diff --git a/drivers/mfd/88pm800.c b/drivers/mfd/88pm800.c
index f104a32..c4e097d 100644
--- a/drivers/mfd/88pm800.c
+++ b/drivers/mfd/88pm800.c
@@ -30,55 +30,55 @@
 #include <linux/of_device.h>
 
 /* Interrupt Registers */
-#define PM800_INT_STATUS1		(0x05)
+#define PM800_INT_STATUS1		0x05
 #define PM800_ONKEY_INT_STS1		(1 << 0)
 #define PM800_EXTON_INT_STS1		(1 << 1)
-#define PM800_CHG_INT_STS1			(1 << 2)
-#define PM800_BAT_INT_STS1			(1 << 3)
-#define PM800_RTC_INT_STS1			(1 << 4)
+#define PM800_CHG_INT_STS1		(1 << 2)
+#define PM800_BAT_INT_STS1		(1 << 3)
+#define PM800_RTC_INT_STS1		(1 << 4)
 #define PM800_CLASSD_OC_INT_STS1	(1 << 5)
 
-#define PM800_INT_STATUS2		(0x06)
+#define PM800_INT_STATUS2		0x06
 #define PM800_VBAT_INT_STS2		(1 << 0)
 #define PM800_VSYS_INT_STS2		(1 << 1)
 #define PM800_VCHG_INT_STS2		(1 << 2)
 #define PM800_TINT_INT_STS2		(1 << 3)
-#define PM800_GPADC0_INT_STS2	(1 << 4)
+#define PM800_GPADC0_INT_STS2		(1 << 4)
 #define PM800_TBAT_INT_STS2		(1 << 5)
-#define PM800_GPADC2_INT_STS2	(1 << 6)
-#define PM800_GPADC3_INT_STS2	(1 << 7)
+#define PM800_GPADC2_INT_STS2		(1 << 6)
+#define PM800_GPADC3_INT_STS2		(1 << 7)
 
-#define PM800_INT_STATUS3		(0x07)
+#define PM800_INT_STATUS3		0x07
 
-#define PM800_INT_STATUS4		(0x08)
+#define PM800_INT_STATUS4		0x08
 #define PM800_GPIO0_INT_STS4		(1 << 0)
 #define PM800_GPIO1_INT_STS4		(1 << 1)
 #define PM800_GPIO2_INT_STS4		(1 << 2)
 #define PM800_GPIO3_INT_STS4		(1 << 3)
 #define PM800_GPIO4_INT_STS4		(1 << 4)
 
-#define PM800_INT_ENA_1		(0x09)
+#define PM800_INT_ENA_1			0x09
 #define PM800_ONKEY_INT_ENA1		(1 << 0)
 #define PM800_EXTON_INT_ENA1		(1 << 1)
-#define PM800_CHG_INT_ENA1			(1 << 2)
-#define PM800_BAT_INT_ENA1			(1 << 3)
-#define PM800_RTC_INT_ENA1			(1 << 4)
+#define PM800_CHG_INT_ENA1		(1 << 2)
+#define PM800_BAT_INT_ENA1		(1 << 3)
+#define PM800_RTC_INT_ENA1		(1 << 4)
 #define PM800_CLASSD_OC_INT_ENA1	(1 << 5)
 
-#define PM800_INT_ENA_2		(0x0A)
+#define PM800_INT_ENA_2			0x0A
 #define PM800_VBAT_INT_ENA2		(1 << 0)
 #define PM800_VSYS_INT_ENA2		(1 << 1)
 #define PM800_VCHG_INT_ENA2		(1 << 2)
 #define PM800_TINT_INT_ENA2		(1 << 3)
 
-#define PM800_INT_ENA_3		(0x0B)
+#define PM800_INT_ENA_3			0x0B
 #define PM800_GPADC0_INT_ENA3		(1 << 0)
 #define PM800_GPADC1_INT_ENA3		(1 << 1)
 #define PM800_GPADC2_INT_ENA3		(1 << 2)
 #define PM800_GPADC3_INT_ENA3		(1 << 3)
 #define PM800_GPADC4_INT_ENA3		(1 << 4)
 
-#define PM800_INT_ENA_4		(0x0C)
+#define PM800_INT_ENA_4			0x0C
 #define PM800_GPIO0_INT_ENA4		(1 << 0)
 #define PM800_GPIO1_INT_ENA4		(1 << 1)
 #define PM800_GPIO2_INT_ENA4		(1 << 2)
@@ -86,7 +86,7 @@
 #define PM800_GPIO4_INT_ENA4		(1 << 4)
 
 /* number of INT_ENA & INT_STATUS regs */
-#define PM800_INT_REG_NUM			(4)
+#define PM800_INT_REG_NUM		4
 
 /* Interrupt Number in 88PM800 */
 enum {
@@ -114,7 +114,7 @@ enum {
 };
 
 /* PM800: generation identification number */
-#define PM800_CHIP_GEN_ID_NUM	0x3
+#define PM800_CHIP_GEN_ID_NUM		0x3
 
 static const struct i2c_device_id pm80x_id_table[] = {
 	{"88PM800", 0},
@@ -129,47 +129,47 @@ static const struct of_device_id pm80x_of_match_table[] = {
 
 static struct resource rtc_resources[] = {
 	{
-	 .name = "88pm80x-rtc",
-	 .start = PM800_IRQ_RTC,
-	 .end = PM800_IRQ_RTC,
-	 .flags = IORESOURCE_IRQ,
-	 },
+		.name = "88pm80x-rtc",
+		.start = PM800_IRQ_RTC,
+		.end = PM800_IRQ_RTC,
+		.flags = IORESOURCE_IRQ,
+	},
 };
 
 static struct mfd_cell rtc_devs[] = {
 	{
-	 .name = "88pm80x-rtc",
-	 .of_compatible = "marvell,88pm80x-rtc",
-	 .num_resources = ARRAY_SIZE(rtc_resources),
-	 .resources = &rtc_resources[0],
-	 .id = -1,
-	 },
+		.name = "88pm80x-rtc",
+		.of_compatible = "marvell,88pm80x-rtc",
+		.num_resources = ARRAY_SIZE(rtc_resources),
+		.resources = &rtc_resources[0],
+		.id = -1,
+	},
 };
 
 static struct resource onkey_resources[] = {
 	{
-	 .name = "88pm80x-onkey",
-	 .start = PM800_IRQ_ONKEY,
-	 .end = PM800_IRQ_ONKEY,
-	 .flags = IORESOURCE_IRQ,
-	 },
+		.name = "88pm80x-onkey",
+		.start = PM800_IRQ_ONKEY,
+		.end = PM800_IRQ_ONKEY,
+		.flags = IORESOURCE_IRQ,
+	},
 };
 
 static const struct mfd_cell onkey_devs[] = {
 	{
-	 .name = "88pm80x-onkey",
-	 .of_compatible = "marvell,88pm80x-onkey",
-	 .num_resources = 1,
-	 .resources = &onkey_resources[0],
-	 .id = -1,
-	 },
+		.name = "88pm80x-onkey",
+		.of_compatible = "marvell,88pm80x-onkey",
+		.num_resources = 1,
+		.resources = &onkey_resources[0],
+		.id = -1,
+	},
 };
 
 static const struct mfd_cell regulator_devs[] = {
 	{
-	 .name = "88pm80x-regulator",
-	 .of_compatible = "marvell,88pm80x-regulator",
-	 .id = -1,
+		.name = "88pm80x-regulator",
+		.of_compatible = "marvell,88pm80x-regulator",
+		.id = -1,
 	},
 };
 
@@ -323,8 +323,8 @@ out:
 static int device_onkey_init(struct pm80x_chip *chip)
 {
 	return mfd_add_devices(chip->dev, 0, &onkey_devs[0],
-			      ARRAY_SIZE(onkey_devs), &onkey_resources[0], 0,
-			      NULL);
+			ARRAY_SIZE(onkey_devs), &onkey_resources[0], 0,
+			NULL);
 }
 
 static int device_rtc_init(struct pm80x_chip *chip)
@@ -335,13 +335,13 @@ static int device_rtc_init(struct pm80x_chip *chip)
 	rtc_devs[0].pdata_size = pdata->rtc ? sizeof(struct pm80x_rtc_pdata) : 0;
 
 	return mfd_add_devices(chip->dev, 0, &rtc_devs[0],
-			      ARRAY_SIZE(rtc_devs), NULL, 0, NULL);
+			ARRAY_SIZE(rtc_devs), NULL, 0, NULL);
 }
 
 static int device_regulator_init(struct pm80x_chip *chip)
 {
 	return mfd_add_devices(chip->dev, 0, &regulator_devs[0],
-			      ARRAY_SIZE(regulator_devs), NULL, 0, NULL);
+			ARRAY_SIZE(regulator_devs), NULL, 0, NULL);
 }
 
 static int device_irq_init_800(struct pm80x_chip *chip)
@@ -358,11 +358,10 @@ static int device_irq_init_800(struct pm80x_chip *chip)
 
 	/*
 	 * irq_clr_on_wr defines the way of clearing interrupt by
-	 * read/write(0/1).  It's read-clear by default.
+	 * read/write(0/1). It's read-clear by default.
 	 */
-	mask =
-	    PM800_WAKEUP2_INV_INT | PM800_WAKEUP2_INT_CLEAR |
-	    PM800_WAKEUP2_INT_MASK;
+	mask = PM800_WAKEUP2_INV_INT | PM800_WAKEUP2_INT_CLEAR |
+		PM800_WAKEUP2_INT_MASK;
 
 	irq_clr_mode = pdata->irq_clr_method == PM800_IRQ_CLR_ON_WRITE ?
 		PM800_WAKEUP2_INT_WRITE_CLEAR : PM800_WAKEUP2_INT_READ_CLEAR;
@@ -371,9 +370,8 @@ static int device_irq_init_800(struct pm80x_chip *chip)
 	if (ret < 0)
 		goto out;
 
-	ret =
-	    regmap_add_irq_chip(chip->regmap, chip->irq, flags, -1,
-				chip->regmap_irq_chip, &chip->irq_data);
+	ret = regmap_add_irq_chip(chip->regmap, chip->irq, flags, -1,
+			chip->regmap_irq_chip, &chip->irq_data);
 
 out:
 	return ret;
@@ -409,18 +407,18 @@ static int pm800_pages_init(struct pm80x_chip *chip)
 
 	/* PM800 block power page */
 	subchip->power_page = i2c_new_dummy(client->adapter,
-					    subchip->power_page_addr);
+					subchip->power_page_addr);
 	if (subchip->power_page == NULL) {
 		ret = -ENODEV;
 		goto out;
 	}
 
 	subchip->regmap_power = devm_regmap_init_i2c(subchip->power_page,
-						     &pm80x_regmap_config);
+				&pm80x_regmap_config);
 	if (IS_ERR(subchip->regmap_power)) {
 		ret = PTR_ERR(subchip->regmap_power);
 		dev_err(chip->dev,
-			"Failed to allocate regmap_power: %d\n", ret);
+				"Failed to allocate regmap_power: %d\n", ret);
 		goto out;
 	}
 
@@ -428,18 +426,18 @@ static int pm800_pages_init(struct pm80x_chip *chip)
 
 	/* PM800 block GPADC */
 	subchip->gpadc_page = i2c_new_dummy(client->adapter,
-					    subchip->gpadc_page_addr);
+				subchip->gpadc_page_addr);
 	if (subchip->gpadc_page == NULL) {
 		ret = -ENODEV;
 		goto out;
 	}
 
 	subchip->regmap_gpadc = devm_regmap_init_i2c(subchip->gpadc_page,
-						     &pm80x_regmap_config);
+				&pm80x_regmap_config);
 	if (IS_ERR(subchip->regmap_gpadc)) {
 		ret = PTR_ERR(subchip->regmap_gpadc);
 		dev_err(chip->dev,
-			"Failed to allocate regmap_gpadc: %d\n", ret);
+				"Failed to allocate regmap_gpadc: %d\n", ret);
 		goto out;
 	}
 	i2c_set_clientdata(subchip->gpadc_page, chip);
@@ -620,8 +618,7 @@ static int pm800_probe(struct i2c_client *client,
 	chip = i2c_get_clientdata(client);
 
 	/* init subchip for PM800 */
-	subchip =
-	    devm_kzalloc(&client->dev, sizeof(struct pm80x_subchip),
+	subchip = devm_kzalloc(&client->dev, sizeof(struct pm80x_subchip),
 			 GFP_KERNEL);
 	if (!subchip) {
 		ret = -ENOMEM;
@@ -684,7 +681,7 @@ static struct i2c_driver pm800_driver = {
 		.owner = THIS_MODULE,
 		.pm = &pm80x_pm_ops,
 		.of_match_table	= pm80x_of_match_table,
-		},
+	},
 	.probe = pm800_probe,
 	.remove = pm800_remove,
 	.id_table = pm80x_id_table,
-- 
1.9.1

[PATCH-v2 1/5] mfd: 88pm800: Fix tabbing issue

[Lee Jones]

On Tue, 25 Aug 2015, Vaibhav Hiremath wrote:

> Driver had coding style issues where spaces were used instead
> of tabs. This patch fixes them all.

That's not all it's doing though is it?

Please describe all of your changes.

> Signed-off-by: Vaibhav Hiremath <vaibhav.hiremath@linaro.org>
> ---
>  drivers/mfd/88pm800.c | 125 ++++++++++++++++++++++++--------------------------
>  1 file changed, 61 insertions(+), 64 deletions(-)
> 
> diff --git a/drivers/mfd/88pm800.c b/drivers/mfd/88pm800.c
> index f104a32..c4e097d 100644
> --- a/drivers/mfd/88pm800.c
> +++ b/drivers/mfd/88pm800.c
> @@ -30,55 +30,55 @@
>  #include <linux/of_device.h>
>  
>  /* Interrupt Registers */
> -#define PM800_INT_STATUS1		(0x05)
> +#define PM800_INT_STATUS1		0x05
>  #define PM800_ONKEY_INT_STS1		(1 << 0)
>  #define PM800_EXTON_INT_STS1		(1 << 1)
> -#define PM800_CHG_INT_STS1			(1 << 2)
> -#define PM800_BAT_INT_STS1			(1 << 3)
> -#define PM800_RTC_INT_STS1			(1 << 4)
> +#define PM800_CHG_INT_STS1		(1 << 2)
> +#define PM800_BAT_INT_STS1		(1 << 3)
> +#define PM800_RTC_INT_STS1		(1 << 4)
>  #define PM800_CLASSD_OC_INT_STS1	(1 << 5)

These should all be replaced by BIT().

Please fix the whole file.

> -#define PM800_INT_STATUS2		(0x06)
> +#define PM800_INT_STATUS2		0x06
>  #define PM800_VBAT_INT_STS2		(1 << 0)
>  #define PM800_VSYS_INT_STS2		(1 << 1)
>  #define PM800_VCHG_INT_STS2		(1 << 2)
>  #define PM800_TINT_INT_STS2		(1 << 3)
> -#define PM800_GPADC0_INT_STS2	(1 << 4)
> +#define PM800_GPADC0_INT_STS2		(1 << 4)
>  #define PM800_TBAT_INT_STS2		(1 << 5)
> -#define PM800_GPADC2_INT_STS2	(1 << 6)
> -#define PM800_GPADC3_INT_STS2	(1 << 7)
> +#define PM800_GPADC2_INT_STS2		(1 << 6)
> +#define PM800_GPADC3_INT_STS2		(1 << 7)
>  
> -#define PM800_INT_STATUS3		(0x07)
> +#define PM800_INT_STATUS3		0x07
>  
> -#define PM800_INT_STATUS4		(0x08)
> +#define PM800_INT_STATUS4		0x08
>  #define PM800_GPIO0_INT_STS4		(1 << 0)
>  #define PM800_GPIO1_INT_STS4		(1 << 1)
>  #define PM800_GPIO2_INT_STS4		(1 << 2)
>  #define PM800_GPIO3_INT_STS4		(1 << 3)
>  #define PM800_GPIO4_INT_STS4		(1 << 4)
>  
> -#define PM800_INT_ENA_1		(0x09)
> +#define PM800_INT_ENA_1			0x09
>  #define PM800_ONKEY_INT_ENA1		(1 << 0)
>  #define PM800_EXTON_INT_ENA1		(1 << 1)
> -#define PM800_CHG_INT_ENA1			(1 << 2)
> -#define PM800_BAT_INT_ENA1			(1 << 3)
> -#define PM800_RTC_INT_ENA1			(1 << 4)
> +#define PM800_CHG_INT_ENA1		(1 << 2)
> +#define PM800_BAT_INT_ENA1		(1 << 3)
> +#define PM800_RTC_INT_ENA1		(1 << 4)
>  #define PM800_CLASSD_OC_INT_ENA1	(1 << 5)
>  
> -#define PM800_INT_ENA_2		(0x0A)
> +#define PM800_INT_ENA_2			0x0A
>  #define PM800_VBAT_INT_ENA2		(1 << 0)
>  #define PM800_VSYS_INT_ENA2		(1 << 1)
>  #define PM800_VCHG_INT_ENA2		(1 << 2)
>  #define PM800_TINT_INT_ENA2		(1 << 3)
>  
> -#define PM800_INT_ENA_3		(0x0B)
> +#define PM800_INT_ENA_3			0x0B
>  #define PM800_GPADC0_INT_ENA3		(1 << 0)
>  #define PM800_GPADC1_INT_ENA3		(1 << 1)
>  #define PM800_GPADC2_INT_ENA3		(1 << 2)
>  #define PM800_GPADC3_INT_ENA3		(1 << 3)
>  #define PM800_GPADC4_INT_ENA3		(1 << 4)
>  
> -#define PM800_INT_ENA_4		(0x0C)
> +#define PM800_INT_ENA_4			0x0C
>  #define PM800_GPIO0_INT_ENA4		(1 << 0)
>  #define PM800_GPIO1_INT_ENA4		(1 << 1)
>  #define PM800_GPIO2_INT_ENA4		(1 << 2)
> @@ -86,7 +86,7 @@
>  #define PM800_GPIO4_INT_ENA4		(1 << 4)
>  
>  /* number of INT_ENA & INT_STATUS regs */
> -#define PM800_INT_REG_NUM			(4)
> +#define PM800_INT_REG_NUM		4
>  
>  /* Interrupt Number in 88PM800 */
>  enum {
> @@ -114,7 +114,7 @@ enum {
>  };
>  
>  /* PM800: generation identification number */
> -#define PM800_CHIP_GEN_ID_NUM	0x3
> +#define PM800_CHIP_GEN_ID_NUM		0x3
>  
>  static const struct i2c_device_id pm80x_id_table[] = {
>  	{"88PM800", 0},
> @@ -129,47 +129,47 @@ static const struct of_device_id pm80x_of_match_table[] = {
>  
>  static struct resource rtc_resources[] = {
>  	{
> -	 .name = "88pm80x-rtc",
> -	 .start = PM800_IRQ_RTC,
> -	 .end = PM800_IRQ_RTC,
> -	 .flags = IORESOURCE_IRQ,
> -	 },
> +		.name = "88pm80x-rtc",
> +		.start = PM800_IRQ_RTC,
> +		.end = PM800_IRQ_RTC,
> +		.flags = IORESOURCE_IRQ,
> +	},
>  };
>  
>  static struct mfd_cell rtc_devs[] = {
>  	{
> -	 .name = "88pm80x-rtc",
> -	 .of_compatible = "marvell,88pm80x-rtc",
> -	 .num_resources = ARRAY_SIZE(rtc_resources),
> -	 .resources = &rtc_resources[0],
> -	 .id = -1,
> -	 },
> +		.name = "88pm80x-rtc",
> +		.of_compatible = "marvell,88pm80x-rtc",
> +		.num_resources = ARRAY_SIZE(rtc_resources),
> +		.resources = &rtc_resources[0],
> +		.id = -1,
> +	},
>  };
>  
>  static struct resource onkey_resources[] = {
>  	{
> -	 .name = "88pm80x-onkey",
> -	 .start = PM800_IRQ_ONKEY,
> -	 .end = PM800_IRQ_ONKEY,
> -	 .flags = IORESOURCE_IRQ,
> -	 },
> +		.name = "88pm80x-onkey",
> +		.start = PM800_IRQ_ONKEY,
> +		.end = PM800_IRQ_ONKEY,
> +		.flags = IORESOURCE_IRQ,
> +	},
>  };
>  
>  static const struct mfd_cell onkey_devs[] = {
>  	{
> -	 .name = "88pm80x-onkey",
> -	 .of_compatible = "marvell,88pm80x-onkey",
> -	 .num_resources = 1,
> -	 .resources = &onkey_resources[0],
> -	 .id = -1,
> -	 },
> +		.name = "88pm80x-onkey",
> +		.of_compatible = "marvell,88pm80x-onkey",
> +		.num_resources = 1,
> +		.resources = &onkey_resources[0],
> +		.id = -1,
> +	},
>  };
>  
>  static const struct mfd_cell regulator_devs[] = {
>  	{
> -	 .name = "88pm80x-regulator",
> -	 .of_compatible = "marvell,88pm80x-regulator",
> -	 .id = -1,
> +		.name = "88pm80x-regulator",
> +		.of_compatible = "marvell,88pm80x-regulator",
> +		.id = -1,
>  	},
>  };
>  
> @@ -323,8 +323,8 @@ out:
>  static int device_onkey_init(struct pm80x_chip *chip)
>  {
>  	return mfd_add_devices(chip->dev, 0, &onkey_devs[0],
> -			      ARRAY_SIZE(onkey_devs), &onkey_resources[0], 0,
> -			      NULL);
> +			ARRAY_SIZE(onkey_devs), &onkey_resources[0], 0,
> +			NULL);
>  }
>  
>  static int device_rtc_init(struct pm80x_chip *chip)
> @@ -335,13 +335,13 @@ static int device_rtc_init(struct pm80x_chip *chip)
>  	rtc_devs[0].pdata_size = pdata->rtc ? sizeof(struct pm80x_rtc_pdata) : 0;
>  
>  	return mfd_add_devices(chip->dev, 0, &rtc_devs[0],
> -			      ARRAY_SIZE(rtc_devs), NULL, 0, NULL);
> +			ARRAY_SIZE(rtc_devs), NULL, 0, NULL);
>  }
>  
>  static int device_regulator_init(struct pm80x_chip *chip)
>  {
>  	return mfd_add_devices(chip->dev, 0, &regulator_devs[0],
> -			      ARRAY_SIZE(regulator_devs), NULL, 0, NULL);
> +			ARRAY_SIZE(regulator_devs), NULL, 0, NULL);
>  }
>  
>  static int device_irq_init_800(struct pm80x_chip *chip)
> @@ -358,11 +358,10 @@ static int device_irq_init_800(struct pm80x_chip *chip)
>  
>  	/*
>  	 * irq_clr_on_wr defines the way of clearing interrupt by
> -	 * read/write(0/1).  It's read-clear by default.
> +	 * read/write(0/1). It's read-clear by default.
>  	 */
> -	mask =
> -	    PM800_WAKEUP2_INV_INT | PM800_WAKEUP2_INT_CLEAR |
> -	    PM800_WAKEUP2_INT_MASK;
> +	mask = PM800_WAKEUP2_INV_INT | PM800_WAKEUP2_INT_CLEAR |
> +		PM800_WAKEUP2_INT_MASK;
>  
>  	irq_clr_mode = pdata->irq_clr_method == PM800_IRQ_CLR_ON_WRITE ?
>  		PM800_WAKEUP2_INT_WRITE_CLEAR : PM800_WAKEUP2_INT_READ_CLEAR;
> @@ -371,9 +370,8 @@ static int device_irq_init_800(struct pm80x_chip *chip)
>  	if (ret < 0)
>  		goto out;
>  
> -	ret =
> -	    regmap_add_irq_chip(chip->regmap, chip->irq, flags, -1,
> -				chip->regmap_irq_chip, &chip->irq_data);
> +	ret = regmap_add_irq_chip(chip->regmap, chip->irq, flags, -1,
> +			chip->regmap_irq_chip, &chip->irq_data);
>  
>  out:
>  	return ret;
> @@ -409,18 +407,18 @@ static int pm800_pages_init(struct pm80x_chip *chip)
>  
>  	/* PM800 block power page */
>  	subchip->power_page = i2c_new_dummy(client->adapter,
> -					    subchip->power_page_addr);
> +					subchip->power_page_addr);
>  	if (subchip->power_page == NULL) {
>  		ret = -ENODEV;
>  		goto out;
>  	}
>  
>  	subchip->regmap_power = devm_regmap_init_i2c(subchip->power_page,
> -						     &pm80x_regmap_config);
> +				&pm80x_regmap_config);
>  	if (IS_ERR(subchip->regmap_power)) {
>  		ret = PTR_ERR(subchip->regmap_power);
>  		dev_err(chip->dev,
> -			"Failed to allocate regmap_power: %d\n", ret);
> +				"Failed to allocate regmap_power: %d\n", ret);
>  		goto out;
>  	}
>  
> @@ -428,18 +426,18 @@ static int pm800_pages_init(struct pm80x_chip *chip)
>  
>  	/* PM800 block GPADC */
>  	subchip->gpadc_page = i2c_new_dummy(client->adapter,
> -					    subchip->gpadc_page_addr);
> +				subchip->gpadc_page_addr);
>  	if (subchip->gpadc_page == NULL) {
>  		ret = -ENODEV;
>  		goto out;
>  	}
>  
>  	subchip->regmap_gpadc = devm_regmap_init_i2c(subchip->gpadc_page,
> -						     &pm80x_regmap_config);
> +				&pm80x_regmap_config);
>  	if (IS_ERR(subchip->regmap_gpadc)) {
>  		ret = PTR_ERR(subchip->regmap_gpadc);
>  		dev_err(chip->dev,
> -			"Failed to allocate regmap_gpadc: %d\n", ret);
> +				"Failed to allocate regmap_gpadc: %d\n", ret);
>  		goto out;
>  	}
>  	i2c_set_clientdata(subchip->gpadc_page, chip);
> @@ -620,8 +618,7 @@ static int pm800_probe(struct i2c_client *client,
>  	chip = i2c_get_clientdata(client);
>  
>  	/* init subchip for PM800 */
> -	subchip =
> -	    devm_kzalloc(&client->dev, sizeof(struct pm80x_subchip),
> +	subchip = devm_kzalloc(&client->dev, sizeof(struct pm80x_subchip),
>  			 GFP_KERNEL);
>  	if (!subchip) {
>  		ret = -ENOMEM;
> @@ -684,7 +681,7 @@ static struct i2c_driver pm800_driver = {
>  		.owner = THIS_MODULE,
>  		.pm = &pm80x_pm_ops,
>  		.of_match_table	= pm80x_of_match_table,
> -		},
> +	},
>  	.probe = pm800_probe,
>  	.remove = pm800_remove,
>  	.id_table = pm80x_id_table,

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

[PATCH-v2 1/5] mfd: 88pm800: Fix tabbing issue

[Vaibhav Hiremath]

On Tuesday 25 August 2015 01:21 PM, Lee Jones wrote:
> On Tue, 25 Aug 2015, Vaibhav Hiremath wrote:
>
>> Driver had coding style issues where spaces were used instead
>> of tabs. This patch fixes them all.
>
> That's not all it's doing though is it?
>

Yes, its just tabbing related fixes.


> Please describe all of your changes.
>
>> Signed-off-by: Vaibhav Hiremath <vaibhav.hiremath@linaro.org>
>> ---
>>   drivers/mfd/88pm800.c | 125 ++++++++++++++++++++++++--------------------------
>>   1 file changed, 61 insertions(+), 64 deletions(-)
>>
>> diff --git a/drivers/mfd/88pm800.c b/drivers/mfd/88pm800.c
>> index f104a32..c4e097d 100644
>> --- a/drivers/mfd/88pm800.c
>> +++ b/drivers/mfd/88pm800.c
>> @@ -30,55 +30,55 @@
>>   #include <linux/of_device.h>
>>
>>   /* Interrupt Registers */
>> -#define PM800_INT_STATUS1		(0x05)
>> +#define PM800_INT_STATUS1		0x05
>>   #define PM800_ONKEY_INT_STS1		(1 << 0)
>>   #define PM800_EXTON_INT_STS1		(1 << 1)
>> -#define PM800_CHG_INT_STS1			(1 << 2)
>> -#define PM800_BAT_INT_STS1			(1 << 3)
>> -#define PM800_RTC_INT_STS1			(1 << 4)
>> +#define PM800_CHG_INT_STS1		(1 << 2)
>> +#define PM800_BAT_INT_STS1		(1 << 3)
>> +#define PM800_RTC_INT_STS1		(1 << 4)
>>   #define PM800_CLASSD_OC_INT_STS1	(1 << 5)
>
> These should all be replaced by BIT().
>
> Please fix the whole file.

Will do.

Thanks,
Vaibhav

[PATCH-v2 1/5] mfd: 88pm800: Fix tabbing issue

[Lee Jones]

On Tue, 25 Aug 2015, Vaibhav Hiremath wrote:
> On Tuesday 25 August 2015 01:21 PM, Lee Jones wrote:
> >On Tue, 25 Aug 2015, Vaibhav Hiremath wrote:
> >
> >>Driver had coding style issues where spaces were used instead
> >>of tabs. This patch fixes them all.
> >
> >That's not all it's doing though is it?
> >
> 
> Yes, its just tabbing related fixes.

No, it's really not.

> >Please describe all of your changes.

... we're also taking the opportunity to remove the unnecessary
protection surrounding some of the defined values.

> >>Signed-off-by: Vaibhav Hiremath <vaibhav.hiremath@linaro.org>
> >>---
> >>  drivers/mfd/88pm800.c | 125 ++++++++++++++++++++++++--------------------------
> >>  1 file changed, 61 insertions(+), 64 deletions(-)
> >>
> >>diff --git a/drivers/mfd/88pm800.c b/drivers/mfd/88pm800.c
> >>index f104a32..c4e097d 100644
> >>--- a/drivers/mfd/88pm800.c
> >>+++ b/drivers/mfd/88pm800.c
> >>@@ -30,55 +30,55 @@
> >>  #include <linux/of_device.h>
> >>
> >>  /* Interrupt Registers */
> >>-#define PM800_INT_STATUS1		(0x05)
> >>+#define PM800_INT_STATUS1		0x05
> >>  #define PM800_ONKEY_INT_STS1		(1 << 0)
> >>  #define PM800_EXTON_INT_STS1		(1 << 1)
> >>-#define PM800_CHG_INT_STS1			(1 << 2)
> >>-#define PM800_BAT_INT_STS1			(1 << 3)
> >>-#define PM800_RTC_INT_STS1			(1 << 4)
> >>+#define PM800_CHG_INT_STS1		(1 << 2)
> >>+#define PM800_BAT_INT_STS1		(1 << 3)
> >>+#define PM800_RTC_INT_STS1		(1 << 4)
> >>  #define PM800_CLASSD_OC_INT_STS1	(1 << 5)
> >
> >These should all be replaced by BIT().
> >
> >Please fix the whole file.
> 
> Will do.
> 
> Thanks,
> Vaibhav

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

[PATCH-v2 1/5] mfd: 88pm800: Fix tabbing issue

[Vaibhav Hiremath]

On Tuesday 25 August 2015 04:00 PM, Lee Jones wrote:
> On Tue, 25 Aug 2015, Vaibhav Hiremath wrote:
>> On Tuesday 25 August 2015 01:21 PM, Lee Jones wrote:
>>> On Tue, 25 Aug 2015, Vaibhav Hiremath wrote:
>>>
>>>> Driver had coding style issues where spaces were used instead
>>>> of tabs. This patch fixes them all.
>>>
>>> That's not all it's doing though is it?
>>>
>>
>> Yes, its just tabbing related fixes.
>
> No, it's really not.
>
>>> Please describe all of your changes.
>
> ... we're also taking the opportunity to remove the unnecessary
> protection surrounding some of the defined values.
>

Ohhh ok. Forgot about brackets :)

Anyway,
Based on the comments you provided, I think both header file and
source file needs some more cleanup.
I will try to incorporate all changes in next version.

I may also submit separate series for cleanup version (depends on
patches I produce).

Thanks,
Vaibhav

[PATCH-v2 1/5] mfd: 88pm800: Fix tabbing issue

[Vaibhav Hiremath]

Driver had coding style issues where spaces were used instead
of tabs. This patch fixes them all.

Signed-off-by: Vaibhav Hiremath <vaibhav.hiremath@linaro.org>
---
 drivers/mfd/88pm800.c | 125 ++++++++++++++++++++++++--------------------------
 1 file changed, 61 insertions(+), 64 deletions(-)

diff --git a/drivers/mfd/88pm800.c b/drivers/mfd/88pm800.c
index f104a32..c4e097d 100644
--- a/drivers/mfd/88pm800.c
+++ b/drivers/mfd/88pm800.c
@@ -30,55 +30,55 @@
 #include <linux/of_device.h>
 
 /* Interrupt Registers */
-#define PM800_INT_STATUS1		(0x05)
+#define PM800_INT_STATUS1		0x05
 #define PM800_ONKEY_INT_STS1		(1 << 0)
 #define PM800_EXTON_INT_STS1		(1 << 1)
-#define PM800_CHG_INT_STS1			(1 << 2)
-#define PM800_BAT_INT_STS1			(1 << 3)
-#define PM800_RTC_INT_STS1			(1 << 4)
+#define PM800_CHG_INT_STS1		(1 << 2)
+#define PM800_BAT_INT_STS1		(1 << 3)
+#define PM800_RTC_INT_STS1		(1 << 4)
 #define PM800_CLASSD_OC_INT_STS1	(1 << 5)
 
-#define PM800_INT_STATUS2		(0x06)
+#define PM800_INT_STATUS2		0x06
 #define PM800_VBAT_INT_STS2		(1 << 0)
 #define PM800_VSYS_INT_STS2		(1 << 1)
 #define PM800_VCHG_INT_STS2		(1 << 2)
 #define PM800_TINT_INT_STS2		(1 << 3)
-#define PM800_GPADC0_INT_STS2	(1 << 4)
+#define PM800_GPADC0_INT_STS2		(1 << 4)
 #define PM800_TBAT_INT_STS2		(1 << 5)
-#define PM800_GPADC2_INT_STS2	(1 << 6)
-#define PM800_GPADC3_INT_STS2	(1 << 7)
+#define PM800_GPADC2_INT_STS2		(1 << 6)
+#define PM800_GPADC3_INT_STS2		(1 << 7)
 
-#define PM800_INT_STATUS3		(0x07)
+#define PM800_INT_STATUS3		0x07
 
-#define PM800_INT_STATUS4		(0x08)
+#define PM800_INT_STATUS4		0x08
 #define PM800_GPIO0_INT_STS4		(1 << 0)
 #define PM800_GPIO1_INT_STS4		(1 << 1)
 #define PM800_GPIO2_INT_STS4		(1 << 2)
 #define PM800_GPIO3_INT_STS4		(1 << 3)
 #define PM800_GPIO4_INT_STS4		(1 << 4)
 
-#define PM800_INT_ENA_1		(0x09)
+#define PM800_INT_ENA_1			0x09
 #define PM800_ONKEY_INT_ENA1		(1 << 0)
 #define PM800_EXTON_INT_ENA1		(1 << 1)
-#define PM800_CHG_INT_ENA1			(1 << 2)
-#define PM800_BAT_INT_ENA1			(1 << 3)
-#define PM800_RTC_INT_ENA1			(1 << 4)
+#define PM800_CHG_INT_ENA1		(1 << 2)
+#define PM800_BAT_INT_ENA1		(1 << 3)
+#define PM800_RTC_INT_ENA1		(1 << 4)
 #define PM800_CLASSD_OC_INT_ENA1	(1 << 5)
 
-#define PM800_INT_ENA_2		(0x0A)
+#define PM800_INT_ENA_2			0x0A
 #define PM800_VBAT_INT_ENA2		(1 << 0)
 #define PM800_VSYS_INT_ENA2		(1 << 1)
 #define PM800_VCHG_INT_ENA2		(1 << 2)
 #define PM800_TINT_INT_ENA2		(1 << 3)
 
-#define PM800_INT_ENA_3		(0x0B)
+#define PM800_INT_ENA_3			0x0B
 #define PM800_GPADC0_INT_ENA3		(1 << 0)
 #define PM800_GPADC1_INT_ENA3		(1 << 1)
 #define PM800_GPADC2_INT_ENA3		(1 << 2)
 #define PM800_GPADC3_INT_ENA3		(1 << 3)
 #define PM800_GPADC4_INT_ENA3		(1 << 4)
 
-#define PM800_INT_ENA_4		(0x0C)
+#define PM800_INT_ENA_4			0x0C
 #define PM800_GPIO0_INT_ENA4		(1 << 0)
 #define PM800_GPIO1_INT_ENA4		(1 << 1)
 #define PM800_GPIO2_INT_ENA4		(1 << 2)
@@ -86,7 +86,7 @@
 #define PM800_GPIO4_INT_ENA4		(1 << 4)
 
 /* number of INT_ENA & INT_STATUS regs */
-#define PM800_INT_REG_NUM			(4)
+#define PM800_INT_REG_NUM		4
 
 /* Interrupt Number in 88PM800 */
 enum {
@@ -114,7 +114,7 @@ enum {
 };
 
 /* PM800: generation identification number */
-#define PM800_CHIP_GEN_ID_NUM	0x3
+#define PM800_CHIP_GEN_ID_NUM		0x3
 
 static const struct i2c_device_id pm80x_id_table[] = {
 	{"88PM800", 0},
@@ -129,47 +129,47 @@ static const struct of_device_id pm80x_of_match_table[] = {
 
 static struct resource rtc_resources[] = {
 	{
-	 .name = "88pm80x-rtc",
-	 .start = PM800_IRQ_RTC,
-	 .end = PM800_IRQ_RTC,
-	 .flags = IORESOURCE_IRQ,
-	 },
+		.name = "88pm80x-rtc",
+		.start = PM800_IRQ_RTC,
+		.end = PM800_IRQ_RTC,
+		.flags = IORESOURCE_IRQ,
+	},
 };
 
 static struct mfd_cell rtc_devs[] = {
 	{
-	 .name = "88pm80x-rtc",
-	 .of_compatible = "marvell,88pm80x-rtc",
-	 .num_resources = ARRAY_SIZE(rtc_resources),
-	 .resources = &rtc_resources[0],
-	 .id = -1,
-	 },
+		.name = "88pm80x-rtc",
+		.of_compatible = "marvell,88pm80x-rtc",
+		.num_resources = ARRAY_SIZE(rtc_resources),
+		.resources = &rtc_resources[0],
+		.id = -1,
+	},
 };
 
 static struct resource onkey_resources[] = {
 	{
-	 .name = "88pm80x-onkey",
-	 .start = PM800_IRQ_ONKEY,
-	 .end = PM800_IRQ_ONKEY,
-	 .flags = IORESOURCE_IRQ,
-	 },
+		.name = "88pm80x-onkey",
+		.start = PM800_IRQ_ONKEY,
+		.end = PM800_IRQ_ONKEY,
+		.flags = IORESOURCE_IRQ,
+	},
 };
 
 static const struct mfd_cell onkey_devs[] = {
 	{
-	 .name = "88pm80x-onkey",
-	 .of_compatible = "marvell,88pm80x-onkey",
-	 .num_resources = 1,
-	 .resources = &onkey_resources[0],
-	 .id = -1,
-	 },
+		.name = "88pm80x-onkey",
+		.of_compatible = "marvell,88pm80x-onkey",
+		.num_resources = 1,
+		.resources = &onkey_resources[0],
+		.id = -1,
+	},
 };
 
 static const struct mfd_cell regulator_devs[] = {
 	{
-	 .name = "88pm80x-regulator",
-	 .of_compatible = "marvell,88pm80x-regulator",
-	 .id = -1,
+		.name = "88pm80x-regulator",
+		.of_compatible = "marvell,88pm80x-regulator",
+		.id = -1,
 	},
 };
 
@@ -323,8 +323,8 @@ out:
 static int device_onkey_init(struct pm80x_chip *chip)
 {
 	return mfd_add_devices(chip->dev, 0, &onkey_devs[0],
-			      ARRAY_SIZE(onkey_devs), &onkey_resources[0], 0,
-			      NULL);
+			ARRAY_SIZE(onkey_devs), &onkey_resources[0], 0,
+			NULL);
 }
 
 static int device_rtc_init(struct pm80x_chip *chip)
@@ -335,13 +335,13 @@ static int device_rtc_init(struct pm80x_chip *chip)
 	rtc_devs[0].pdata_size = pdata->rtc ? sizeof(struct pm80x_rtc_pdata) : 0;
 
 	return mfd_add_devices(chip->dev, 0, &rtc_devs[0],
-			      ARRAY_SIZE(rtc_devs), NULL, 0, NULL);
+			ARRAY_SIZE(rtc_devs), NULL, 0, NULL);
 }
 
 static int device_regulator_init(struct pm80x_chip *chip)
 {
 	return mfd_add_devices(chip->dev, 0, &regulator_devs[0],
-			      ARRAY_SIZE(regulator_devs), NULL, 0, NULL);
+			ARRAY_SIZE(regulator_devs), NULL, 0, NULL);
 }
 
 static int device_irq_init_800(struct pm80x_chip *chip)
@@ -358,11 +358,10 @@ static int device_irq_init_800(struct pm80x_chip *chip)
 
 	/*
 	 * irq_clr_on_wr defines the way of clearing interrupt by
-	 * read/write(0/1).  It's read-clear by default.
+	 * read/write(0/1). It's read-clear by default.
 	 */
-	mask =
-	    PM800_WAKEUP2_INV_INT | PM800_WAKEUP2_INT_CLEAR |
-	    PM800_WAKEUP2_INT_MASK;
+	mask = PM800_WAKEUP2_INV_INT | PM800_WAKEUP2_INT_CLEAR |
+		PM800_WAKEUP2_INT_MASK;
 
 	irq_clr_mode = pdata->irq_clr_method == PM800_IRQ_CLR_ON_WRITE ?
 		PM800_WAKEUP2_INT_WRITE_CLEAR : PM800_WAKEUP2_INT_READ_CLEAR;
@@ -371,9 +370,8 @@ static int device_irq_init_800(struct pm80x_chip *chip)
 	if (ret < 0)
 		goto out;
 
-	ret =
-	    regmap_add_irq_chip(chip->regmap, chip->irq, flags, -1,
-				chip->regmap_irq_chip, &chip->irq_data);
+	ret = regmap_add_irq_chip(chip->regmap, chip->irq, flags, -1,
+			chip->regmap_irq_chip, &chip->irq_data);
 
 out:
 	return ret;
@@ -409,18 +407,18 @@ static int pm800_pages_init(struct pm80x_chip *chip)
 
 	/* PM800 block power page */
 	subchip->power_page = i2c_new_dummy(client->adapter,
-					    subchip->power_page_addr);
+					subchip->power_page_addr);
 	if (subchip->power_page == NULL) {
 		ret = -ENODEV;
 		goto out;
 	}
 
 	subchip->regmap_power = devm_regmap_init_i2c(subchip->power_page,
-						     &pm80x_regmap_config);
+				&pm80x_regmap_config);
 	if (IS_ERR(subchip->regmap_power)) {
 		ret = PTR_ERR(subchip->regmap_power);
 		dev_err(chip->dev,
-			"Failed to allocate regmap_power: %d\n", ret);
+				"Failed to allocate regmap_power: %d\n", ret);
 		goto out;
 	}
 
@@ -428,18 +426,18 @@ static int pm800_pages_init(struct pm80x_chip *chip)
 
 	/* PM800 block GPADC */
 	subchip->gpadc_page = i2c_new_dummy(client->adapter,
-					    subchip->gpadc_page_addr);
+				subchip->gpadc_page_addr);
 	if (subchip->gpadc_page == NULL) {
 		ret = -ENODEV;
 		goto out;
 	}
 
 	subchip->regmap_gpadc = devm_regmap_init_i2c(subchip->gpadc_page,
-						     &pm80x_regmap_config);
+				&pm80x_regmap_config);
 	if (IS_ERR(subchip->regmap_gpadc)) {
 		ret = PTR_ERR(subchip->regmap_gpadc);
 		dev_err(chip->dev,
-			"Failed to allocate regmap_gpadc: %d\n", ret);
+				"Failed to allocate regmap_gpadc: %d\n", ret);
 		goto out;
 	}
 	i2c_set_clientdata(subchip->gpadc_page, chip);
@@ -620,8 +618,7 @@ static int pm800_probe(struct i2c_client *client,
 	chip = i2c_get_clientdata(client);
 
 	/* init subchip for PM800 */
-	subchip =
-	    devm_kzalloc(&client->dev, sizeof(struct pm80x_subchip),
+	subchip = devm_kzalloc(&client->dev, sizeof(struct pm80x_subchip),
 			 GFP_KERNEL);
 	if (!subchip) {
 		ret = -ENOMEM;
@@ -684,7 +681,7 @@ static struct i2c_driver pm800_driver = {
 		.owner = THIS_MODULE,
 		.pm = &pm80x_pm_ops,
 		.of_match_table	= pm80x_of_match_table,
-		},
+	},
 	.probe = pm800_probe,
 	.remove = pm800_remove,
 	.id_table = pm80x_id_table,
-- 
1.9.1

[PATCH-v2 1/5] mfd: 88pm800: Fix tabbing issue

[Lee Jones]

On Tue, 25 Aug 2015, Vaibhav Hiremath wrote:

> Driver had coding style issues where spaces were used instead
> of tabs. This patch fixes them all.
> 
> Signed-off-by: Vaibhav Hiremath <vaibhav.hiremath@linaro.org>
> ---
>  drivers/mfd/88pm800.c | 125 ++++++++++++++++++++++++--------------------------
>  1 file changed, 61 insertions(+), 64 deletions(-)

Same comment as before.

How similar is this file to the other one?

Can you reduce the size by having shared register defines?

> diff --git a/drivers/mfd/88pm800.c b/drivers/mfd/88pm800.c
> index f104a32..c4e097d 100644
> --- a/drivers/mfd/88pm800.c
> +++ b/drivers/mfd/88pm800.c
> @@ -30,55 +30,55 @@
>  #include <linux/of_device.h>
>  
>  /* Interrupt Registers */
> -#define PM800_INT_STATUS1		(0x05)
> +#define PM800_INT_STATUS1		0x05
>  #define PM800_ONKEY_INT_STS1		(1 << 0)
>  #define PM800_EXTON_INT_STS1		(1 << 1)
> -#define PM800_CHG_INT_STS1			(1 << 2)
> -#define PM800_BAT_INT_STS1			(1 << 3)
> -#define PM800_RTC_INT_STS1			(1 << 4)
> +#define PM800_CHG_INT_STS1		(1 << 2)
> +#define PM800_BAT_INT_STS1		(1 << 3)
> +#define PM800_RTC_INT_STS1		(1 << 4)
>  #define PM800_CLASSD_OC_INT_STS1	(1 << 5)
>  
> -#define PM800_INT_STATUS2		(0x06)
> +#define PM800_INT_STATUS2		0x06
>  #define PM800_VBAT_INT_STS2		(1 << 0)
>  #define PM800_VSYS_INT_STS2		(1 << 1)
>  #define PM800_VCHG_INT_STS2		(1 << 2)
>  #define PM800_TINT_INT_STS2		(1 << 3)
> -#define PM800_GPADC0_INT_STS2	(1 << 4)
> +#define PM800_GPADC0_INT_STS2		(1 << 4)
>  #define PM800_TBAT_INT_STS2		(1 << 5)
> -#define PM800_GPADC2_INT_STS2	(1 << 6)
> -#define PM800_GPADC3_INT_STS2	(1 << 7)
> +#define PM800_GPADC2_INT_STS2		(1 << 6)
> +#define PM800_GPADC3_INT_STS2		(1 << 7)
>  
> -#define PM800_INT_STATUS3		(0x07)
> +#define PM800_INT_STATUS3		0x07
>  
> -#define PM800_INT_STATUS4		(0x08)
> +#define PM800_INT_STATUS4		0x08
>  #define PM800_GPIO0_INT_STS4		(1 << 0)
>  #define PM800_GPIO1_INT_STS4		(1 << 1)
>  #define PM800_GPIO2_INT_STS4		(1 << 2)
>  #define PM800_GPIO3_INT_STS4		(1 << 3)
>  #define PM800_GPIO4_INT_STS4		(1 << 4)
>  
> -#define PM800_INT_ENA_1		(0x09)
> +#define PM800_INT_ENA_1			0x09
>  #define PM800_ONKEY_INT_ENA1		(1 << 0)
>  #define PM800_EXTON_INT_ENA1		(1 << 1)
> -#define PM800_CHG_INT_ENA1			(1 << 2)
> -#define PM800_BAT_INT_ENA1			(1 << 3)
> -#define PM800_RTC_INT_ENA1			(1 << 4)
> +#define PM800_CHG_INT_ENA1		(1 << 2)
> +#define PM800_BAT_INT_ENA1		(1 << 3)
> +#define PM800_RTC_INT_ENA1		(1 << 4)
>  #define PM800_CLASSD_OC_INT_ENA1	(1 << 5)
>  
> -#define PM800_INT_ENA_2		(0x0A)
> +#define PM800_INT_ENA_2			0x0A
>  #define PM800_VBAT_INT_ENA2		(1 << 0)
>  #define PM800_VSYS_INT_ENA2		(1 << 1)
>  #define PM800_VCHG_INT_ENA2		(1 << 2)
>  #define PM800_TINT_INT_ENA2		(1 << 3)
>  
> -#define PM800_INT_ENA_3		(0x0B)
> +#define PM800_INT_ENA_3			0x0B
>  #define PM800_GPADC0_INT_ENA3		(1 << 0)
>  #define PM800_GPADC1_INT_ENA3		(1 << 1)
>  #define PM800_GPADC2_INT_ENA3		(1 << 2)
>  #define PM800_GPADC3_INT_ENA3		(1 << 3)
>  #define PM800_GPADC4_INT_ENA3		(1 << 4)
>  
> -#define PM800_INT_ENA_4		(0x0C)
> +#define PM800_INT_ENA_4			0x0C
>  #define PM800_GPIO0_INT_ENA4		(1 << 0)
>  #define PM800_GPIO1_INT_ENA4		(1 << 1)
>  #define PM800_GPIO2_INT_ENA4		(1 << 2)
> @@ -86,7 +86,7 @@
>  #define PM800_GPIO4_INT_ENA4		(1 << 4)
>  
>  /* number of INT_ENA & INT_STATUS regs */
> -#define PM800_INT_REG_NUM			(4)
> +#define PM800_INT_REG_NUM		4
>  
>  /* Interrupt Number in 88PM800 */
>  enum {
> @@ -114,7 +114,7 @@ enum {
>  };
>  
>  /* PM800: generation identification number */
> -#define PM800_CHIP_GEN_ID_NUM	0x3
> +#define PM800_CHIP_GEN_ID_NUM		0x3
>  
>  static const struct i2c_device_id pm80x_id_table[] = {
>  	{"88PM800", 0},
> @@ -129,47 +129,47 @@ static const struct of_device_id pm80x_of_match_table[] = {
>  
>  static struct resource rtc_resources[] = {
>  	{
> -	 .name = "88pm80x-rtc",
> -	 .start = PM800_IRQ_RTC,
> -	 .end = PM800_IRQ_RTC,
> -	 .flags = IORESOURCE_IRQ,
> -	 },
> +		.name = "88pm80x-rtc",
> +		.start = PM800_IRQ_RTC,
> +		.end = PM800_IRQ_RTC,
> +		.flags = IORESOURCE_IRQ,
> +	},
>  };
>  
>  static struct mfd_cell rtc_devs[] = {
>  	{
> -	 .name = "88pm80x-rtc",
> -	 .of_compatible = "marvell,88pm80x-rtc",
> -	 .num_resources = ARRAY_SIZE(rtc_resources),
> -	 .resources = &rtc_resources[0],
> -	 .id = -1,
> -	 },
> +		.name = "88pm80x-rtc",
> +		.of_compatible = "marvell,88pm80x-rtc",
> +		.num_resources = ARRAY_SIZE(rtc_resources),
> +		.resources = &rtc_resources[0],
> +		.id = -1,
> +	},
>  };
>  
>  static struct resource onkey_resources[] = {
>  	{
> -	 .name = "88pm80x-onkey",
> -	 .start = PM800_IRQ_ONKEY,
> -	 .end = PM800_IRQ_ONKEY,
> -	 .flags = IORESOURCE_IRQ,
> -	 },
> +		.name = "88pm80x-onkey",
> +		.start = PM800_IRQ_ONKEY,
> +		.end = PM800_IRQ_ONKEY,
> +		.flags = IORESOURCE_IRQ,
> +	},
>  };
>  
>  static const struct mfd_cell onkey_devs[] = {
>  	{
> -	 .name = "88pm80x-onkey",
> -	 .of_compatible = "marvell,88pm80x-onkey",
> -	 .num_resources = 1,
> -	 .resources = &onkey_resources[0],
> -	 .id = -1,
> -	 },
> +		.name = "88pm80x-onkey",
> +		.of_compatible = "marvell,88pm80x-onkey",
> +		.num_resources = 1,
> +		.resources = &onkey_resources[0],
> +		.id = -1,
> +	},
>  };
>  
>  static const struct mfd_cell regulator_devs[] = {
>  	{
> -	 .name = "88pm80x-regulator",
> -	 .of_compatible = "marvell,88pm80x-regulator",
> -	 .id = -1,
> +		.name = "88pm80x-regulator",
> +		.of_compatible = "marvell,88pm80x-regulator",
> +		.id = -1,
>  	},
>  };
>  
> @@ -323,8 +323,8 @@ out:
>  static int device_onkey_init(struct pm80x_chip *chip)
>  {
>  	return mfd_add_devices(chip->dev, 0, &onkey_devs[0],
> -			      ARRAY_SIZE(onkey_devs), &onkey_resources[0], 0,
> -			      NULL);
> +			ARRAY_SIZE(onkey_devs), &onkey_resources[0], 0,
> +			NULL);
>  }
>  
>  static int device_rtc_init(struct pm80x_chip *chip)
> @@ -335,13 +335,13 @@ static int device_rtc_init(struct pm80x_chip *chip)
>  	rtc_devs[0].pdata_size = pdata->rtc ? sizeof(struct pm80x_rtc_pdata) : 0;
>  
>  	return mfd_add_devices(chip->dev, 0, &rtc_devs[0],
> -			      ARRAY_SIZE(rtc_devs), NULL, 0, NULL);
> +			ARRAY_SIZE(rtc_devs), NULL, 0, NULL);
>  }
>  
>  static int device_regulator_init(struct pm80x_chip *chip)
>  {
>  	return mfd_add_devices(chip->dev, 0, &regulator_devs[0],
> -			      ARRAY_SIZE(regulator_devs), NULL, 0, NULL);
> +			ARRAY_SIZE(regulator_devs), NULL, 0, NULL);
>  }
>  
>  static int device_irq_init_800(struct pm80x_chip *chip)
> @@ -358,11 +358,10 @@ static int device_irq_init_800(struct pm80x_chip *chip)
>  
>  	/*
>  	 * irq_clr_on_wr defines the way of clearing interrupt by
> -	 * read/write(0/1).  It's read-clear by default.
> +	 * read/write(0/1). It's read-clear by default.
>  	 */
> -	mask =
> -	    PM800_WAKEUP2_INV_INT | PM800_WAKEUP2_INT_CLEAR |
> -	    PM800_WAKEUP2_INT_MASK;
> +	mask = PM800_WAKEUP2_INV_INT | PM800_WAKEUP2_INT_CLEAR |
> +		PM800_WAKEUP2_INT_MASK;
>  
>  	irq_clr_mode = pdata->irq_clr_method == PM800_IRQ_CLR_ON_WRITE ?
>  		PM800_WAKEUP2_INT_WRITE_CLEAR : PM800_WAKEUP2_INT_READ_CLEAR;
> @@ -371,9 +370,8 @@ static int device_irq_init_800(struct pm80x_chip *chip)
>  	if (ret < 0)
>  		goto out;
>  
> -	ret =
> -	    regmap_add_irq_chip(chip->regmap, chip->irq, flags, -1,
> -				chip->regmap_irq_chip, &chip->irq_data);
> +	ret = regmap_add_irq_chip(chip->regmap, chip->irq, flags, -1,
> +			chip->regmap_irq_chip, &chip->irq_data);
>  
>  out:
>  	return ret;
> @@ -409,18 +407,18 @@ static int pm800_pages_init(struct pm80x_chip *chip)
>  
>  	/* PM800 block power page */
>  	subchip->power_page = i2c_new_dummy(client->adapter,
> -					    subchip->power_page_addr);
> +					subchip->power_page_addr);
>  	if (subchip->power_page == NULL) {
>  		ret = -ENODEV;
>  		goto out;
>  	}
>  
>  	subchip->regmap_power = devm_regmap_init_i2c(subchip->power_page,
> -						     &pm80x_regmap_config);
> +				&pm80x_regmap_config);
>  	if (IS_ERR(subchip->regmap_power)) {
>  		ret = PTR_ERR(subchip->regmap_power);
>  		dev_err(chip->dev,
> -			"Failed to allocate regmap_power: %d\n", ret);
> +				"Failed to allocate regmap_power: %d\n", ret);
>  		goto out;
>  	}
>  
> @@ -428,18 +426,18 @@ static int pm800_pages_init(struct pm80x_chip *chip)
>  
>  	/* PM800 block GPADC */
>  	subchip->gpadc_page = i2c_new_dummy(client->adapter,
> -					    subchip->gpadc_page_addr);
> +				subchip->gpadc_page_addr);
>  	if (subchip->gpadc_page == NULL) {
>  		ret = -ENODEV;
>  		goto out;
>  	}
>  
>  	subchip->regmap_gpadc = devm_regmap_init_i2c(subchip->gpadc_page,
> -						     &pm80x_regmap_config);
> +				&pm80x_regmap_config);
>  	if (IS_ERR(subchip->regmap_gpadc)) {
>  		ret = PTR_ERR(subchip->regmap_gpadc);
>  		dev_err(chip->dev,
> -			"Failed to allocate regmap_gpadc: %d\n", ret);
> +				"Failed to allocate regmap_gpadc: %d\n", ret);
>  		goto out;
>  	}
>  	i2c_set_clientdata(subchip->gpadc_page, chip);
> @@ -620,8 +618,7 @@ static int pm800_probe(struct i2c_client *client,
>  	chip = i2c_get_clientdata(client);
>  
>  	/* init subchip for PM800 */
> -	subchip =
> -	    devm_kzalloc(&client->dev, sizeof(struct pm80x_subchip),
> +	subchip = devm_kzalloc(&client->dev, sizeof(struct pm80x_subchip),
>  			 GFP_KERNEL);
>  	if (!subchip) {
>  		ret = -ENOMEM;
> @@ -684,7 +681,7 @@ static struct i2c_driver pm800_driver = {
>  		.owner = THIS_MODULE,
>  		.pm = &pm80x_pm_ops,
>  		.of_match_table	= pm80x_of_match_table,
> -		},
> +	},
>  	.probe = pm800_probe,
>  	.remove = pm800_remove,
>  	.id_table = pm80x_id_table,

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

[PATCH-v2 1/5] mfd: 88pm800: Fix tabbing issue

[Vaibhav Hiremath]

On Tuesday 25 August 2015 01:22 PM, Lee Jones wrote:
> On Tue, 25 Aug 2015, Vaibhav Hiremath wrote:
>
>> Driver had coding style issues where spaces were used instead
>> of tabs. This patch fixes them all.
>>
>> Signed-off-by: Vaibhav Hiremath <vaibhav.hiremath@linaro.org>
>> ---
>>   drivers/mfd/88pm800.c | 125 ++++++++++++++++++++++++--------------------------
>>   1 file changed, 61 insertions(+), 64 deletions(-)
>
> Same comment as before.
>
> How similar is this file to the other one?

I believe you are referring to include/linux/mfd/88pm80x.h

>
> Can you reduce the size by having shared register defines?
>

Not sure whether I understand your comment here,
They are common defines.

PM860 only defines are named as

PM860_XXXX

Similarly,
PM805 only defines are named as

PM805_XXXX


and PM800_xxx should be common.

Thanks,
Vaibhav

[PATCH-v2 1/5] mfd: 88pm800: Fix tabbing issue

[Lee Jones]

On Tue, 25 Aug 2015, Vaibhav Hiremath wrote:

> 
> 
> On Tuesday 25 August 2015 01:22 PM, Lee Jones wrote:
> >On Tue, 25 Aug 2015, Vaibhav Hiremath wrote:
> >
> >>Driver had coding style issues where spaces were used instead
> >>of tabs. This patch fixes them all.
> >>
> >>Signed-off-by: Vaibhav Hiremath <vaibhav.hiremath@linaro.org>
> >>---
> >>  drivers/mfd/88pm800.c | 125 ++++++++++++++++++++++++--------------------------
> >>  1 file changed, 61 insertions(+), 64 deletions(-)
> >
> >Same comment as before.
> >
> >How similar is this file to the other one?
> 
> I believe you are referring to include/linux/mfd/88pm80x.h
> 
> >
> >Can you reduce the size by having shared register defines?
> >
> 
> Not sure whether I understand your comment here,
> They are common defines.

I guess the question was a little convoluted.

How about; why are the extra defines in the driver?  Why aren't all of
them in the header file?  What is the difference between the defines
in the C file compared with the ones defined in the header?

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

[PATCH-v2 1/5] mfd: 88pm800: Fix tabbing issue

[Vaibhav Hiremath]

On Tuesday 25 August 2015 04:05 PM, Lee Jones wrote:
> On Tue, 25 Aug 2015, Vaibhav Hiremath wrote:
>
>>
>>
>> On Tuesday 25 August 2015 01:22 PM, Lee Jones wrote:
>>> On Tue, 25 Aug 2015, Vaibhav Hiremath wrote:
>>>
>>>> Driver had coding style issues where spaces were used instead
>>>> of tabs. This patch fixes them all.
>>>>
>>>> Signed-off-by: Vaibhav Hiremath <vaibhav.hiremath@linaro.org>
>>>> ---
>>>>   drivers/mfd/88pm800.c | 125 ++++++++++++++++++++++++--------------------------
>>>>   1 file changed, 61 insertions(+), 64 deletions(-)
>>>
>>> Same comment as before.
>>>
>>> How similar is this file to the other one?
>>
>> I believe you are referring to include/linux/mfd/88pm80x.h
>>
>>>
>>> Can you reduce the size by having shared register defines?
>>>
>>
>> Not sure whether I understand your comment here,
>> They are common defines.
>
> I guess the question was a little convoluted.
>
> How about; why are the extra defines in the driver?  Why aren't all of
> them in the header file?  What is the difference between the defines
> in the C file compared with the ones defined in the header?
>


I do not see any difference and need of having defines separately in C
file.
That's the cleanup I was referring to in another email.

I will walk through the whole driver code and try to cleanup all such
coding related issues. Probably expect separate series here.

Thanks,
Vaibhav

[PATCH-v2 2/5] mfd: 88pm800: Update the header file with 32K clk related macros

[Vaibhav Hiremath]

Update header file with required macros for 32KHz buffered clock
output of 88PM800 family of device.
These macros will be used in clk provider driver.

Signed-off-by: Vaibhav Hiremath <vaibhav.hiremath@linaro.org>
---
 include/linux/mfd/88pm80x.h | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/include/linux/mfd/88pm80x.h b/include/linux/mfd/88pm80x.h
index 122cfd2..0215d5f 100644
--- a/include/linux/mfd/88pm80x.h
+++ b/include/linux/mfd/88pm80x.h
@@ -91,6 +91,7 @@ enum {
 /* Referance and low power registers */
 #define PM800_LOW_POWER1		(0x20)
 #define PM800_LOW_POWER2		(0x21)
+#define PM800_LOW_POWER2_XO_LJ_EN	BIT(5)
 
 #define PM800_LOW_POWER_CONFIG3		(0x22)
 #define PM800_LDOBK_FREEZE		BIT(7)
@@ -138,6 +139,13 @@ enum {
 #define PM800_ALARM			BIT(5)
 #define PM800_RTC1_USE_XO		BIT(7)
 
+#define PM800_32K_OUTX_SEL_MASK		0x3
+/* 32KHz clk output sel mode */
+#define PM800_32K_OUTX_SEL_ZERO		0x0
+#define PM800_32K_OUTX_SEL_INT_32KHZ	0x1
+#define PM800_32K_OUTX_SEL_XO_32KHZ	0x2
+#define PM800_32K_OUTX_SEL_HIZ		0x3
+
 /* Regulator Control Registers: BUCK1,BUCK5,LDO1 have DVC */
 
 /* buck registers */
@@ -208,6 +216,10 @@ enum {
 #define PM800_PMOD_MEAS1		0x52
 #define PM800_PMOD_MEAS2		0x53
 
+/* Oscillator control */
+#define PM800_OSC_CNTRL1		0x50
+#define PM800_OSC_CNTRL1_OSC_FREERUN_EN	BIT(1)
+
 #define PM800_GPADC0_MEAS1		0x54
 #define PM800_GPADC0_MEAS2		0x55
 #define PM800_GPADC1_MEAS1		0x56
-- 
1.9.1

[PATCH-v2 2/5] mfd: 88pm800: Update the header file with 32K clk related macros

[Lee Jones]

On Tue, 25 Aug 2015, Vaibhav Hiremath wrote:

> Update header file with required macros for 32KHz buffered clock
> output of 88PM800 family of device.
> These macros will be used in clk provider driver.
> 
> Signed-off-by: Vaibhav Hiremath <vaibhav.hiremath@linaro.org>
> ---
>  include/linux/mfd/88pm80x.h | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/include/linux/mfd/88pm80x.h b/include/linux/mfd/88pm80x.h
> index 122cfd2..0215d5f 100644
> --- a/include/linux/mfd/88pm80x.h
> +++ b/include/linux/mfd/88pm80x.h
> @@ -91,6 +91,7 @@ enum {
>  /* Referance and low power registers */
>  #define PM800_LOW_POWER1		(0x20)
>  #define PM800_LOW_POWER2		(0x21)
> +#define PM800_LOW_POWER2_XO_LJ_EN	BIT(5)

Some people add an extra space for register bits, which I quite like.

So:

#define SOME_REGISTER_ADDRESS	0x123
#define  SOME_BIT_VALUE		BIT(4)

Feel free to use it, or not.

>  #define PM800_LOW_POWER_CONFIG3		(0x22)
>  #define PM800_LDOBK_FREEZE		BIT(7)
> @@ -138,6 +139,13 @@ enum {
>  #define PM800_ALARM			BIT(5)
>  #define PM800_RTC1_USE_XO		BIT(7)
>  
> +#define PM800_32K_OUTX_SEL_MASK		0x3
> +/* 32KHz clk output sel mode */
> +#define PM800_32K_OUTX_SEL_ZERO		0x0
> +#define PM800_32K_OUTX_SEL_INT_32KHZ	0x1
> +#define PM800_32K_OUTX_SEL_XO_32KHZ	0x2
> +#define PM800_32K_OUTX_SEL_HIZ		0x3
> +
>  /* Regulator Control Registers: BUCK1,BUCK5,LDO1 have DVC */
>  
>  /* buck registers */
> @@ -208,6 +216,10 @@ enum {
>  #define PM800_PMOD_MEAS1		0x52
>  #define PM800_PMOD_MEAS2		0x53
>  
> +/* Oscillator control */
> +#define PM800_OSC_CNTRL1		0x50
> +#define PM800_OSC_CNTRL1_OSC_FREERUN_EN	BIT(1)

0x50 goes before 0x52 (and 0x51 if it's there).

>  #define PM800_GPADC0_MEAS1		0x54
>  #define PM800_GPADC0_MEAS2		0x55
>  #define PM800_GPADC1_MEAS1		0x56

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

[PATCH-v2 2/5] mfd: 88pm800: Update the header file with 32K clk related macros

[Vaibhav Hiremath]

On Tuesday 25 August 2015 01:25 PM, Lee Jones wrote:
> On Tue, 25 Aug 2015, Vaibhav Hiremath wrote:
>
>> Update header file with required macros for 32KHz buffered clock
>> output of 88PM800 family of device.
>> These macros will be used in clk provider driver.
>>
>> Signed-off-by: Vaibhav Hiremath <vaibhav.hiremath@linaro.org>
>> ---
>>   include/linux/mfd/88pm80x.h | 12 ++++++++++++
>>   1 file changed, 12 insertions(+)
>>
>> diff --git a/include/linux/mfd/88pm80x.h b/include/linux/mfd/88pm80x.h
>> index 122cfd2..0215d5f 100644
>> --- a/include/linux/mfd/88pm80x.h
>> +++ b/include/linux/mfd/88pm80x.h
>> @@ -91,6 +91,7 @@ enum {
>>   /* Referance and low power registers */
>>   #define PM800_LOW_POWER1		(0x20)
>>   #define PM800_LOW_POWER2		(0x21)
>> +#define PM800_LOW_POWER2_XO_LJ_EN	BIT(5)
>
> Some people add an extra space for register bits, which I quite like.
>
> So:
>
> #define SOME_REGISTER_ADDRESS	0x123
> #define  SOME_BIT_VALUE		BIT(4)
>
> Feel free to use it, or not.
>
>>   #define PM800_LOW_POWER_CONFIG3		(0x22)
>>   #define PM800_LDOBK_FREEZE		BIT(7)
>> @@ -138,6 +139,13 @@ enum {
>>   #define PM800_ALARM			BIT(5)
>>   #define PM800_RTC1_USE_XO		BIT(7)
>>
>> +#define PM800_32K_OUTX_SEL_MASK		0x3
>> +/* 32KHz clk output sel mode */
>> +#define PM800_32K_OUTX_SEL_ZERO		0x0
>> +#define PM800_32K_OUTX_SEL_INT_32KHZ	0x1
>> +#define PM800_32K_OUTX_SEL_XO_32KHZ	0x2
>> +#define PM800_32K_OUTX_SEL_HIZ		0x3
>> +
>>   /* Regulator Control Registers: BUCK1,BUCK5,LDO1 have DVC */
>>
>>   /* buck registers */
>> @@ -208,6 +216,10 @@ enum {
>>   #define PM800_PMOD_MEAS1		0x52
>>   #define PM800_PMOD_MEAS2		0x53
>>
>> +/* Oscillator control */
>> +#define PM800_OSC_CNTRL1		0x50
>> +#define PM800_OSC_CNTRL1_OSC_FREERUN_EN	BIT(1)
>
> 0x50 goes before 0x52 (and 0x51 if it's there).

Will reorder in next version.

Thanks,
Vaibhav

[PATCH-v2 3/5] mfd: devicetree: bindings: Add clock subdevice node information

[Vaibhav Hiremath]

This patch updates the binding documentation for optional
clocks node and related information for buffered 32KHz clock.

Signed-off-by: Vaibhav Hiremath <vaibhav.hiremath@linaro.org>
Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Acked-by: Rob Herring <robh@kernel.org>
---
 Documentation/devicetree/bindings/mfd/88pm800.txt | 27 +++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/Documentation/devicetree/bindings/mfd/88pm800.txt b/Documentation/devicetree/bindings/mfd/88pm800.txt
index 2c82fcb..bc6cb02 100644
--- a/Documentation/devicetree/bindings/mfd/88pm800.txt
+++ b/Documentation/devicetree/bindings/mfd/88pm800.txt
@@ -15,6 +15,26 @@ Optional properties :
   Without this both BUCK1A and BUCK1B operates independently with 3A capacity.
   (This property is only applicable to 88PM860)
 
+Optional nodes:
+- clocks: 88pm800 family of devices provide multiple buffered 32.768
+  KHz outputs, so to register these as clocks with common clock framework
+  instantiate a sub-node named "clocks". It uses the common clock binding
+  documented in :
+  [Documentation/devicetree/bindings/clock/clock-bindings.txt]
+
+  - #clock-cells: should be 1.
+
+  - The following is the list of clocks generated by the controller. Each clock
+    is assigned an identifier and client nodes use this identifier to specify
+    the clock which they consume.
+    Clock		ID           	Devices
+    ----------------------------------------------------------
+    pm800_clk32k_1	0		88PM800 and 88PM860
+    pm800_clk32k_2	1		88PM800 and 88PM860
+    pm800_clk32k_3	2		88PM800
+
+  - compatible: Should be : "marvell,88pm800-clk"
+
 88pm80x family of devices consists of varied group of sub-devices:
 
 Device		 	Supply Names	 Description
@@ -22,6 +42,7 @@ Device		 	Supply Names	 Description
 88pm80x-onkey		:		: On key
 88pm80x-rtc		:		: RTC
 88pm80x-regulator	:		: Regulators
+88pm80x-clk		:		: 32KHz Clk provider
 
 Example:
 
@@ -33,6 +54,12 @@ Example:
 		interrupt-controller;
 		#interrupt-cells = <1>;
 
+		pm800clk: clocks {
+			compatible = "marvell,88pm800-clk";
+			#clock-cells = <1>;
+			clock-output-names = "xx", "yy", "zz";
+		};
+
 		regulators {
 			compatible = "marvell,88pm80x-regulator";
 
-- 
1.9.1

[PATCH-v2 3/5] mfd: devicetree: bindings: Add clock subdevice node information

[Lee Jones]

On Tue, 25 Aug 2015, Vaibhav Hiremath wrote:

> This patch updates the binding documentation for optional
> clocks node and related information for buffered 32KHz clock.
> 
> Signed-off-by: Vaibhav Hiremath <vaibhav.hiremath@linaro.org>
> Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> Acked-by: Rob Herring <robh@kernel.org>
> ---
>  Documentation/devicetree/bindings/mfd/88pm800.txt | 27 +++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/mfd/88pm800.txt b/Documentation/devicetree/bindings/mfd/88pm800.txt
> index 2c82fcb..bc6cb02 100644
> --- a/Documentation/devicetree/bindings/mfd/88pm800.txt
> +++ b/Documentation/devicetree/bindings/mfd/88pm800.txt
> @@ -15,6 +15,26 @@ Optional properties :
>    Without this both BUCK1A and BUCK1B operates independently with 3A capacity.
>    (This property is only applicable to 88PM860)
>  
> +Optional nodes:
> +- clocks: 88pm800 family of devices provide multiple buffered 32.768
> +  KHz outputs, so to register these as clocks with common clock framework
> +  instantiate a sub-node named "clocks". It uses the common clock binding
> +  documented in :
> +  [Documentation/devicetree/bindings/clock/clock-bindings.txt]

I tend to prefer relative paths.  Firstly because they're shorter and
more succinct and secondly because if we do eventually move DT out of
the kernel, it will be less hassle to rename each of them.

> +  - #clock-cells: should be 1.
> +
> +  - The following is the list of clocks generated by the controller. Each clock
> +    is assigned an identifier and client nodes use this identifier to specify
> +    the clock which they consume.
> +    Clock		ID           	Devices
> +    ----------------------------------------------------------
> +    pm800_clk32k_1	0		88PM800 and 88PM860
> +    pm800_clk32k_2	1		88PM800 and 88PM860
> +    pm800_clk32k_3	2		88PM800
> +
> +  - compatible: Should be : "marvell,88pm800-clk"
> +
>  88pm80x family of devices consists of varied group of sub-devices:
>  
>  Device		 	Supply Names	 Description
> @@ -22,6 +42,7 @@ Device		 	Supply Names	 Description
>  88pm80x-onkey		:		: On key
>  88pm80x-rtc		:		: RTC
>  88pm80x-regulator	:		: Regulators
> +88pm80x-clk		:		: 32KHz Clk provider

Nit: Alphabetical?

>  Example:
>  
> @@ -33,6 +54,12 @@ Example:
>  		interrupt-controller;
>  		#interrupt-cells = <1>;
>  
> +		pm800clk: clocks {
> +			compatible = "marvell,88pm800-clk";
> +			#clock-cells = <1>;
> +			clock-output-names = "xx", "yy", "zz";
> +		};
> +
>  		regulators {
>  			compatible = "marvell,88pm80x-regulator";
>  

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

[PATCH-v2 3/5] mfd: devicetree: bindings: Add clock subdevice node information

[Vaibhav Hiremath]

On Tuesday 25 August 2015 01:28 PM, Lee Jones wrote:
> On Tue, 25 Aug 2015, Vaibhav Hiremath wrote:
>
>> This patch updates the binding documentation for optional
>> clocks node and related information for buffered 32KHz clock.
>>
>> Signed-off-by: Vaibhav Hiremath <vaibhav.hiremath@linaro.org>
>> Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
>> Acked-by: Rob Herring <robh@kernel.org>
>> ---
>>   Documentation/devicetree/bindings/mfd/88pm800.txt | 27 +++++++++++++++++++++++
>>   1 file changed, 27 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/mfd/88pm800.txt b/Documentation/devicetree/bindings/mfd/88pm800.txt
>> index 2c82fcb..bc6cb02 100644
>> --- a/Documentation/devicetree/bindings/mfd/88pm800.txt
>> +++ b/Documentation/devicetree/bindings/mfd/88pm800.txt
>> @@ -15,6 +15,26 @@ Optional properties :
>>     Without this both BUCK1A and BUCK1B operates independently with 3A capacity.
>>     (This property is only applicable to 88PM860)
>>
>> +Optional nodes:
>> +- clocks: 88pm800 family of devices provide multiple buffered 32.768
>> +  KHz outputs, so to register these as clocks with common clock framework
>> +  instantiate a sub-node named "clocks". It uses the common clock binding
>> +  documented in :
>> +  [Documentation/devicetree/bindings/clock/clock-bindings.txt]
>
> I tend to prefer relative paths.  Firstly because they're shorter and
> more succinct and secondly because if we do eventually move DT out of
> the kernel, it will be less hassle to rename each of them.
>
>> +  - #clock-cells: should be 1.
>> +
>> +  - The following is the list of clocks generated by the controller. Each clock
>> +    is assigned an identifier and client nodes use this identifier to specify
>> +    the clock which they consume.
>> +    Clock		ID           	Devices
>> +    ----------------------------------------------------------
>> +    pm800_clk32k_1	0		88PM800 and 88PM860
>> +    pm800_clk32k_2	1		88PM800 and 88PM860
>> +    pm800_clk32k_3	2		88PM800
>> +
>> +  - compatible: Should be : "marvell,88pm800-clk"
>> +
>>   88pm80x family of devices consists of varied group of sub-devices:
>>
>>   Device		 	Supply Names	 Description
>> @@ -22,6 +42,7 @@ Device		 	Supply Names	 Description
>>   88pm80x-onkey		:		: On key
>>   88pm80x-rtc		:		: RTC
>>   88pm80x-regulator	:		: Regulators
>> +88pm80x-clk		:		: 32KHz Clk provider
>
> Nit: Alphabetical?
>

Will correct both comments.

Thanks,
Vaibhav

[PATCH-v2 4/5] clk: 88pm800: Add clk provider driver for 88pm800 family of devices

[Vaibhav Hiremath]

88PM800 family of devices supports buffered 32KHz clock output,
for example,

88PM800: CLK32k_1, CLK32k_2 and CLK32k_3
88PM860: CLK32K_1 and CLK32K_2

This patch adds new clk provider driver to support enable/disable
of the 32KHz clock output from 88PM800 family of devices.

Signed-off-by: Vaibhav Hiremath <vaibhav.hiremath@linaro.org>
---
 drivers/clk/Kconfig       |   8 ++
 drivers/clk/Makefile      |   1 +
 drivers/clk/clk-88pm800.c | 341 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 350 insertions(+)
 create mode 100644 drivers/clk/clk-88pm800.c

diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
index 42f7120..c34c14d 100644
--- a/drivers/clk/Kconfig
+++ b/drivers/clk/Kconfig
@@ -167,6 +167,14 @@ config COMMON_CLK_CDCE706
 	---help---
 	  This driver supports TI CDCE706 programmable 3-PLL clock synthesizer.
 
+config COMMON_CLK_88PM800
+	tristate "Clock driver for 88PM800 MFD"
+	depends on MFD_88PM800
+	---help---
+	  This driver supports 88PM800 & 88PM860 crystal oscillator
+	  clock. These multi-function devices have two (88PM860) or three
+	  (88PM800) fixed-rate output, clocked at 32KHz each.
+
 source "drivers/clk/bcm/Kconfig"
 source "drivers/clk/hisilicon/Kconfig"
 source "drivers/clk/qcom/Kconfig"
diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index c4cf075..5248cd3 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -80,3 +80,4 @@ obj-$(CONFIG_X86)			+= x86/
 obj-$(CONFIG_ARCH_ZX)			+= zte/
 obj-$(CONFIG_ARCH_ZYNQ)			+= zynq/
 obj-$(CONFIG_H8300)		+= h8300/
+obj-$(CONFIG_COMMON_CLK_88PM800)	+= clk-88pm800.o
diff --git a/drivers/clk/clk-88pm800.c b/drivers/clk/clk-88pm800.c
new file mode 100644
index 0000000..4c045e1
--- /dev/null
+++ b/drivers/clk/clk-88pm800.c
@@ -0,0 +1,341 @@
+/*
+ * clk-88pm800.c - Clock driver for 88PM800 family of devices
+ *
+ * This driver is created based on clk-s2mps11.c
+ *
+ * Copyright (C) 2015 Linaro Ltd.
+ *
+ * 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.
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/err.h>
+#include <linux/of.h>
+#include <linux/clkdev.h>
+#include <linux/regmap.h>
+#include <linux/clk-provider.h>
+#include <linux/platform_device.h>
+#include <linux/mfd/88pm80x.h>
+
+/* Number of clocks output from 88pm800 family of device */
+enum {
+	PM800_CLK32K_1 = 0,
+	PM800_CLK32K_2,
+	PM800_CLK32K_3,
+	PM800_CLKS_NUM,
+};
+
+struct pm800_clk {
+	struct pm80x_chip *chip;
+	struct device_node *clk_np;
+	struct clk_hw hw;
+	struct clk *clk;
+	struct clk_lookup *lookup;
+	u32 mask;
+	u32 shift;
+	unsigned int reg;
+};
+
+static struct pm800_clk *to_pm800_clk(struct clk_hw *hw)
+{
+	return container_of(hw, struct pm800_clk, hw);
+}
+
+static int pm800_clk_prepare(struct clk_hw *hw)
+{
+	struct pm800_clk *pm800 = to_pm800_clk(hw);
+
+	/* We always want to use XO clock */
+	return regmap_update_bits(pm800->chip->regmap,
+				 pm800->reg,
+				 pm800->mask,
+				 PM800_32K_OUTX_SEL_XO_32KHZ << pm800->shift);
+}
+
+static void pm800_clk_unprepare(struct clk_hw *hw)
+{
+	struct pm800_clk *pm800 = to_pm800_clk(hw);
+
+	regmap_update_bits(pm800->chip->regmap, pm800->reg,
+				pm800->mask, ~pm800->mask);
+}
+
+static int pm800_clk_is_prepared(struct clk_hw *hw)
+{
+	int ret;
+	u32 val;
+	struct pm800_clk *pm800 = to_pm800_clk(hw);
+
+	ret = regmap_read(pm800->chip->regmap,
+				pm800->reg, &val);
+	if (ret < 0)
+		return -EINVAL;
+
+	return (val & pm800->mask) >> pm800->shift;
+}
+
+static unsigned long pm800_clk_recalc_rate(struct clk_hw *hw,
+					     unsigned long parent_rate)
+{
+	return 32768;
+}
+
+static const struct clk_ops pm800_clk_ops = {
+	.prepare	= pm800_clk_prepare,
+	.unprepare	= pm800_clk_unprepare,
+	.is_prepared	= pm800_clk_is_prepared,
+	.recalc_rate	= pm800_clk_recalc_rate,
+};
+
+static struct clk_init_data pm800_clks_init[PM800_CLKS_NUM] = {
+	[PM800_CLK32K_1] = {
+		.name	= "pm800_clk32k_1",
+		.ops	= &pm800_clk_ops,
+		.flags	= CLK_IS_ROOT,
+	},
+	[PM800_CLK32K_2] = {
+		.name	= "pm800_clk32k_2",
+		.ops	= &pm800_clk_ops,
+		.flags	= CLK_IS_ROOT,
+	},
+	[PM800_CLK32K_3] = {
+		.name	= "pm800_clk32k_3",
+		.ops	= &pm800_clk_ops,
+		.flags	= CLK_IS_ROOT,
+	},
+};
+
+static struct clk_init_data pm860_clks_init[PM800_CLKS_NUM] = {
+	[PM800_CLK32K_1] = {
+		.name	= "pm800_clk32k_1",
+		.ops	= &pm800_clk_ops,
+		.flags	= CLK_IS_ROOT,
+	},
+	[PM800_CLK32K_2] = {
+		.name	= "pm800_clk32k_2",
+		.ops	= &pm800_clk_ops,
+		.flags	= CLK_IS_ROOT,
+	},
+};
+
+static int pm800_init_clk(struct pm800_clk *pm800_clks)
+{
+	int ret;
+
+	/* Enable XO_LJ : Low jitter clock of 32KHz from XO */
+	ret = regmap_update_bits(pm800_clks->chip->regmap, PM800_LOW_POWER2,
+			PM800_LOW_POWER2_XO_LJ_EN, PM800_LOW_POWER2_XO_LJ_EN);
+	if (ret)
+		return ret;
+
+	/* Enable USE_XO : Use XO clock for all internal timing reference */
+	ret = regmap_update_bits(pm800_clks->chip->regmap, PM800_RTC_CONTROL,
+			PM800_RTC1_USE_XO, PM800_RTC1_USE_XO);
+	if (ret)
+		return ret;
+
+	/* OSC_FREERUN: Enable Osc free running mode by clearing the bit */
+	ret = regmap_update_bits(pm800_clks->chip->regmap, PM800_OSC_CNTRL1,
+			PM800_OSC_CNTRL1_OSC_FREERUN_EN, 0);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static struct device_node *pm800_clk_parse_dt(struct platform_device *pdev,
+		struct clk_init_data *clks_init)
+{
+	struct pm80x_chip *chip = dev_get_drvdata(pdev->dev.parent);
+	struct device_node *clk_np;
+	int i;
+
+	if (!chip->dev->of_node)
+		return ERR_PTR(-EINVAL);
+
+	clk_np = of_get_child_by_name(chip->dev->of_node, "clocks");
+	if (!clk_np) {
+		dev_err(&pdev->dev, "could not find clock sub-node\n");
+		return ERR_PTR(-EINVAL);
+	}
+
+	for (i = 0; i < PM800_CLKS_NUM; i++) {
+		if (!clks_init[i].name)
+			continue; /* Skip clocks not present in some devices */
+
+		of_property_read_string_index(clk_np, "clock-output-names", i,
+				&clks_init[i].name);
+	}
+
+	return clk_np;
+}
+
+static int pm800_clk_probe(struct platform_device *pdev)
+{
+	struct pm80x_chip *chip = dev_get_drvdata(pdev->dev.parent);
+	struct pm800_clk *pm800_clks;
+	struct clk_init_data *clks_init;
+	static struct clk **clk_table;
+	static struct clk_onecell_data *of_clk_data;
+	int i, ret;
+
+	pm800_clks = devm_kcalloc(&pdev->dev, PM800_CLKS_NUM,
+				sizeof(*pm800_clks), GFP_KERNEL);
+	if (!pm800_clks)
+		return -ENOMEM;
+
+	clk_table = devm_kcalloc(&pdev->dev, PM800_CLKS_NUM,
+				sizeof(struct clk *), GFP_KERNEL);
+	if (!clk_table)
+		return -ENOMEM;
+
+	switch (platform_get_device_id(pdev)->driver_data) {
+	case CHIP_PM800:
+		clks_init = pm800_clks_init;
+		break;
+	case CHIP_PM860:
+		clks_init = pm860_clks_init;
+		break;
+	default:
+		dev_err(&pdev->dev, "Invalid device type\n");
+		return -EINVAL;
+	}
+
+	/* Store clocks of_node in first element of pm800_clks array */
+	pm800_clks->clk_np = pm800_clk_parse_dt(pdev, clks_init);
+	if (IS_ERR(pm800_clks->clk_np))
+		return PTR_ERR(pm800_clks->clk_np);
+
+	of_clk_data = devm_kzalloc(&pdev->dev, sizeof(*of_clk_data), GFP_KERNEL);
+	if (!of_clk_data) {
+		ret = -ENOMEM;
+		goto err_clk_np;
+	}
+
+	for (i = 0; i < PM800_CLKS_NUM; i++) {
+		if (!clks_init[i].name)
+			continue; /* Skip clocks not present in some devices */
+
+		pm800_clks[i].chip = chip;
+		pm800_clks[i].hw.init = &clks_init[i];
+		/*
+		 * As of now, mask and shift formula below works for both
+		 * 88PM800 and it's family of devices,
+		 *
+		 *  PM800_RTC_MISC2:
+		 *      1:0	= CK_32K_OUT1_SEL
+		 *      3:2	= CK_32K_OUT2_SEL
+		 *      5:4	= CK_32K_OUT3_SEL
+		 */
+		pm800_clks[i].shift = i * 2;
+		pm800_clks[i].mask = PM800_32K_OUTX_SEL_MASK << pm800_clks[i].shift;
+		pm800_clks[i].reg = PM800_RTC_MISC2;
+
+		pm800_clks[i].clk = devm_clk_register(&pdev->dev,
+							&pm800_clks[i].hw);
+		if (IS_ERR(pm800_clks[i].clk)) {
+			dev_err(&pdev->dev, "Fail to register : %s\n",
+						clks_init[i].name);
+			ret = PTR_ERR(pm800_clks[i].clk);
+			goto err;
+		}
+
+		pm800_clks[i].lookup = clkdev_create(pm800_clks[i].clk,
+					clks_init[i].name, NULL);
+		if (!pm800_clks[i].lookup) {
+			ret = -ENOMEM;
+			goto err;
+		}
+
+		clk_table[i] = pm800_clks[i].clk;
+	}
+
+	of_clk_data->clks = clk_table;
+	of_clk_data->clk_num = PM800_CLKS_NUM;
+	ret = of_clk_add_provider(pm800_clks->clk_np, of_clk_src_onecell_get,
+			of_clk_data);
+	if (ret) {
+		dev_err(&pdev->dev, "Fail to add OF clk provider : %d\n", ret);
+		goto err;
+	}
+
+	/* Common for all 32KHz clock output */
+	ret = pm800_init_clk(&pm800_clks[0]);
+	if (ret) {
+		dev_err(&pdev->dev, "Failed to initialize clk : %d\n", ret);
+		goto err;
+	}
+
+	platform_set_drvdata(pdev, pm800_clks);
+
+	return 0;
+
+err:
+	for (i = 0; i < PM800_CLKS_NUM; i++) {
+		if (pm800_clks[i].lookup)
+			clkdev_drop(pm800_clks[i].lookup);
+	}
+err_clk_np:
+	/* Drop the reference obtained in pm800_clk_parse_dt */
+	of_node_put(pm800_clks[0].clk_np);
+
+	return ret;
+}
+
+static int pm800_clk_remove(struct platform_device *pdev)
+{
+	struct pm800_clk *pm800_clks = platform_get_drvdata(pdev);
+	int i;
+
+	of_clk_del_provider(pm800_clks[0].clk_np);
+	/* Drop the reference obtained in pm800_clk_parse_dt */
+	of_node_put(pm800_clks[0].clk_np);
+
+	for (i = 0; i < PM800_CLKS_NUM; i++) {
+		if (pm800_clks[i].lookup)
+			clkdev_drop(pm800_clks[i].lookup);
+	}
+
+	return 0;
+}
+
+static const struct platform_device_id pm800_clk_id[] = {
+	{ "88pm800-clk", CHIP_PM800},
+	{ "88pm860-clk", CHIP_PM860},
+	{ },
+};
+MODULE_DEVICE_TABLE(platform, pm800_clk_id);
+
+static struct platform_driver pm800_clk_driver = {
+	.driver = {
+		.name  = "88pm80x-clk",
+	},
+	.probe = pm800_clk_probe,
+	.remove = pm800_clk_remove,
+	.id_table = pm800_clk_id,
+};
+
+static int __init pm800_clk_init(void)
+{
+	return platform_driver_register(&pm800_clk_driver);
+}
+subsys_initcall(pm800_clk_init);
+
+static void __exit pm800_clk_cleanup(void)
+{
+	platform_driver_unregister(&pm800_clk_driver);
+}
+module_exit(pm800_clk_cleanup);
+
+MODULE_DESCRIPTION("88PM800 Clock Driver");
+MODULE_AUTHOR("Vaibhav Hiremath <vaibhav.hiremath@linaro.org>");
+MODULE_LICENSE("GPL");
-- 
1.9.1

[PATCH-v2 4/5] clk: 88pm800: Add clk provider driver for 88pm800 family of devices

[Krzysztof Kozlowski]

On 25.08.2015 03:56, Vaibhav Hiremath wrote:
> 88PM800 family of devices supports buffered 32KHz clock output,
> for example,
> 
> 88PM800: CLK32k_1, CLK32k_2 and CLK32k_3
> 88PM860: CLK32K_1 and CLK32K_2
> 
> This patch adds new clk provider driver to support enable/disable
> of the 32KHz clock output from 88PM800 family of devices.
> 
> Signed-off-by: Vaibhav Hiremath <vaibhav.hiremath@linaro.org>
> ---
>  drivers/clk/Kconfig       |   8 ++
>  drivers/clk/Makefile      |   1 +
>  drivers/clk/clk-88pm800.c | 341 ++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 350 insertions(+)
>  create mode 100644 drivers/clk/clk-88pm800.c
> 
> diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
> index 42f7120..c34c14d 100644
> --- a/drivers/clk/Kconfig
> +++ b/drivers/clk/Kconfig
> @@ -167,6 +167,14 @@ config COMMON_CLK_CDCE706
>  	---help---
>  	  This driver supports TI CDCE706 programmable 3-PLL clock synthesizer.
>  
> +config COMMON_CLK_88PM800
> +	tristate "Clock driver for 88PM800 MFD"
> +	depends on MFD_88PM800
> +	---help---
> +	  This driver supports 88PM800 & 88PM860 crystal oscillator
> +	  clock. These multi-function devices have two (88PM860) or three
> +	  (88PM800) fixed-rate output, clocked at 32KHz each.
> +
>  source "drivers/clk/bcm/Kconfig"
>  source "drivers/clk/hisilicon/Kconfig"
>  source "drivers/clk/qcom/Kconfig"
> diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
> index c4cf075..5248cd3 100644
> --- a/drivers/clk/Makefile
> +++ b/drivers/clk/Makefile
> @@ -80,3 +80,4 @@ obj-$(CONFIG_X86)			+= x86/
>  obj-$(CONFIG_ARCH_ZX)			+= zte/
>  obj-$(CONFIG_ARCH_ZYNQ)			+= zynq/
>  obj-$(CONFIG_H8300)		+= h8300/
> +obj-$(CONFIG_COMMON_CLK_88PM800)	+= clk-88pm800.o
> diff --git a/drivers/clk/clk-88pm800.c b/drivers/clk/clk-88pm800.c
> new file mode 100644
> index 0000000..4c045e1
> --- /dev/null
> +++ b/drivers/clk/clk-88pm800.c
> @@ -0,0 +1,341 @@
> +/*
> + * clk-88pm800.c - Clock driver for 88PM800 family of devices
> + *
> + * This driver is created based on clk-s2mps11.c
> + *
> + * Copyright (C) 2015 Linaro Ltd.
> + *
> + * 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.
> + *
> + */
> +
> +#include <linux/module.h>
> +#include <linux/err.h>
> +#include <linux/of.h>
> +#include <linux/clkdev.h>
> +#include <linux/regmap.h>
> +#include <linux/clk-provider.h>
> +#include <linux/platform_device.h>
> +#include <linux/mfd/88pm80x.h>
> +
> +/* Number of clocks output from 88pm800 family of device */
> +enum {
> +	PM800_CLK32K_1 = 0,
> +	PM800_CLK32K_2,
> +	PM800_CLK32K_3,
> +	PM800_CLKS_NUM,
> +};
> +
> +struct pm800_clk {
> +	struct pm80x_chip *chip;
> +	struct device_node *clk_np;
> +	struct clk_hw hw;
> +	struct clk *clk;
> +	struct clk_lookup *lookup;
> +	u32 mask;
> +	u32 shift;
> +	unsigned int reg;
> +};
> +
> +static struct pm800_clk *to_pm800_clk(struct clk_hw *hw)
> +{
> +	return container_of(hw, struct pm800_clk, hw);
> +}
> +
> +static int pm800_clk_prepare(struct clk_hw *hw)
> +{
> +	struct pm800_clk *pm800 = to_pm800_clk(hw);
> +
> +	/* We always want to use XO clock */
> +	return regmap_update_bits(pm800->chip->regmap,
> +				 pm800->reg,
> +				 pm800->mask,
> +				 PM800_32K_OUTX_SEL_XO_32KHZ << pm800->shift);
> +}
> +
> +static void pm800_clk_unprepare(struct clk_hw *hw)
> +{
> +	struct pm800_clk *pm800 = to_pm800_clk(hw);
> +
> +	regmap_update_bits(pm800->chip->regmap, pm800->reg,
> +				pm800->mask, ~pm800->mask);
> +}
> +
> +static int pm800_clk_is_prepared(struct clk_hw *hw)
> +{
> +	int ret;
> +	u32 val;
> +	struct pm800_clk *pm800 = to_pm800_clk(hw);
> +
> +	ret = regmap_read(pm800->chip->regmap,
> +				pm800->reg, &val);
> +	if (ret < 0)
> +		return -EINVAL;
> +
> +	return (val & pm800->mask) >> pm800->shift;
> +}
> +
> +static unsigned long pm800_clk_recalc_rate(struct clk_hw *hw,
> +					     unsigned long parent_rate)
> +{
> +	return 32768;
> +}
> +
> +static const struct clk_ops pm800_clk_ops = {
> +	.prepare	= pm800_clk_prepare,
> +	.unprepare	= pm800_clk_unprepare,
> +	.is_prepared	= pm800_clk_is_prepared,
> +	.recalc_rate	= pm800_clk_recalc_rate,
> +};
> +
> +static struct clk_init_data pm800_clks_init[PM800_CLKS_NUM] = {
> +	[PM800_CLK32K_1] = {
> +		.name	= "pm800_clk32k_1",
> +		.ops	= &pm800_clk_ops,
> +		.flags	= CLK_IS_ROOT,
> +	},
> +	[PM800_CLK32K_2] = {
> +		.name	= "pm800_clk32k_2",
> +		.ops	= &pm800_clk_ops,
> +		.flags	= CLK_IS_ROOT,
> +	},
> +	[PM800_CLK32K_3] = {
> +		.name	= "pm800_clk32k_3",
> +		.ops	= &pm800_clk_ops,
> +		.flags	= CLK_IS_ROOT,
> +	},
> +};
> +
> +static struct clk_init_data pm860_clks_init[PM800_CLKS_NUM] = {
> +	[PM800_CLK32K_1] = {
> +		.name	= "pm800_clk32k_1",
> +		.ops	= &pm800_clk_ops,
> +		.flags	= CLK_IS_ROOT,
> +	},
> +	[PM800_CLK32K_2] = {
> +		.name	= "pm800_clk32k_2",
> +		.ops	= &pm800_clk_ops,
> +		.flags	= CLK_IS_ROOT,
> +	},
> +};
> +
> +static int pm800_init_clk(struct pm800_clk *pm800_clks)
> +{
> +	int ret;
> +
> +	/* Enable XO_LJ : Low jitter clock of 32KHz from XO */
> +	ret = regmap_update_bits(pm800_clks->chip->regmap, PM800_LOW_POWER2,
> +			PM800_LOW_POWER2_XO_LJ_EN, PM800_LOW_POWER2_XO_LJ_EN);
> +	if (ret)
> +		return ret;
> +
> +	/* Enable USE_XO : Use XO clock for all internal timing reference */
> +	ret = regmap_update_bits(pm800_clks->chip->regmap, PM800_RTC_CONTROL,
> +			PM800_RTC1_USE_XO, PM800_RTC1_USE_XO);
> +	if (ret)
> +		return ret;
> +
> +	/* OSC_FREERUN: Enable Osc free running mode by clearing the bit */
> +	ret = regmap_update_bits(pm800_clks->chip->regmap, PM800_OSC_CNTRL1,
> +			PM800_OSC_CNTRL1_OSC_FREERUN_EN, 0);
> +	if (ret)
> +		return ret;
> +
> +	return 0;
> +}
> +
> +static struct device_node *pm800_clk_parse_dt(struct platform_device *pdev,
> +		struct clk_init_data *clks_init)
> +{
> +	struct pm80x_chip *chip = dev_get_drvdata(pdev->dev.parent);
> +	struct device_node *clk_np;
> +	int i;
> +
> +	if (!chip->dev->of_node)
> +		return ERR_PTR(-EINVAL);
> +
> +	clk_np = of_get_child_by_name(chip->dev->of_node, "clocks");
> +	if (!clk_np) {
> +		dev_err(&pdev->dev, "could not find clock sub-node\n");
> +		return ERR_PTR(-EINVAL);
> +	}
> +
> +	for (i = 0; i < PM800_CLKS_NUM; i++) {
> +		if (!clks_init[i].name)
> +			continue; /* Skip clocks not present in some devices */
> +
> +		of_property_read_string_index(clk_np, "clock-output-names", i,
> +				&clks_init[i].name);
> +	}
> +
> +	return clk_np;
> +}
> +
> +static int pm800_clk_probe(struct platform_device *pdev)
> +{
> +	struct pm80x_chip *chip = dev_get_drvdata(pdev->dev.parent);
> +	struct pm800_clk *pm800_clks;
> +	struct clk_init_data *clks_init;
> +	static struct clk **clk_table;
> +	static struct clk_onecell_data *of_clk_data;
> +	int i, ret;
> +
> +	pm800_clks = devm_kcalloc(&pdev->dev, PM800_CLKS_NUM,
> +				sizeof(*pm800_clks), GFP_KERNEL);
> +	if (!pm800_clks)
> +		return -ENOMEM;
> +
> +	clk_table = devm_kcalloc(&pdev->dev, PM800_CLKS_NUM,
> +				sizeof(struct clk *), GFP_KERNEL);
> +	if (!clk_table)
> +		return -ENOMEM;
> +
> +	switch (platform_get_device_id(pdev)->driver_data) {
> +	case CHIP_PM800:
> +		clks_init = pm800_clks_init;
> +		break;
> +	case CHIP_PM860:
> +		clks_init = pm860_clks_init;
> +		break;
> +	default:
> +		dev_err(&pdev->dev, "Invalid device type\n");
> +		return -EINVAL;
> +	}
> +
> +	/* Store clocks of_node in first element of pm800_clks array */
> +	pm800_clks->clk_np = pm800_clk_parse_dt(pdev, clks_init);
> +	if (IS_ERR(pm800_clks->clk_np))
> +		return PTR_ERR(pm800_clks->clk_np);
> +
> +	of_clk_data = devm_kzalloc(&pdev->dev, sizeof(*of_clk_data), GFP_KERNEL);
> +	if (!of_clk_data) {
> +		ret = -ENOMEM;
> +		goto err_clk_np;
> +	}
> +
> +	for (i = 0; i < PM800_CLKS_NUM; i++) {
> +		if (!clks_init[i].name)
> +			continue; /* Skip clocks not present in some devices */
> +
> +		pm800_clks[i].chip = chip;
> +		pm800_clks[i].hw.init = &clks_init[i];
> +		/*
> +		 * As of now, mask and shift formula below works for both
> +		 * 88PM800 and it's family of devices,
> +		 *
> +		 *  PM800_RTC_MISC2:
> +		 *      1:0	= CK_32K_OUT1_SEL
> +		 *      3:2	= CK_32K_OUT2_SEL
> +		 *      5:4	= CK_32K_OUT3_SEL
> +		 */
> +		pm800_clks[i].shift = i * 2;
> +		pm800_clks[i].mask = PM800_32K_OUTX_SEL_MASK << pm800_clks[i].shift;
> +		pm800_clks[i].reg = PM800_RTC_MISC2;
> +
> +		pm800_clks[i].clk = devm_clk_register(&pdev->dev,
> +							&pm800_clks[i].hw);
> +		if (IS_ERR(pm800_clks[i].clk)) {
> +			dev_err(&pdev->dev, "Fail to register : %s\n",
> +						clks_init[i].name);
> +			ret = PTR_ERR(pm800_clks[i].clk);
> +			goto err;
> +		}
> +
> +		pm800_clks[i].lookup = clkdev_create(pm800_clks[i].clk,
> +					clks_init[i].name, NULL);
> +		if (!pm800_clks[i].lookup) {
> +			ret = -ENOMEM;
> +			goto err;
> +		}
> +
> +		clk_table[i] = pm800_clks[i].clk;
> +	}
> +
> +	of_clk_data->clks = clk_table;
> +	of_clk_data->clk_num = PM800_CLKS_NUM;
> +	ret = of_clk_add_provider(pm800_clks->clk_np, of_clk_src_onecell_get,
> +			of_clk_data);
> +	if (ret) {
> +		dev_err(&pdev->dev, "Fail to add OF clk provider : %d\n", ret);
> +		goto err;
> +	}
> +
> +	/* Common for all 32KHz clock output */
> +	ret = pm800_init_clk(&pm800_clks[0]);
> +	if (ret) {
> +		dev_err(&pdev->dev, "Failed to initialize clk : %d\n", ret);
> +		goto err;

You're missing here of_clk_del_provider().

> +	}
> +
> +	platform_set_drvdata(pdev, pm800_clks);
> +
> +	return 0;
> +
> +err:
> +	for (i = 0; i < PM800_CLKS_NUM; i++) {
> +		if (pm800_clks[i].lookup)
> +			clkdev_drop(pm800_clks[i].lookup);
> +	}
> +err_clk_np:
> +	/* Drop the reference obtained in pm800_clk_parse_dt */
> +	of_node_put(pm800_clks[0].clk_np);
> +
> +	return ret;
> +}
> +
> +static int pm800_clk_remove(struct platform_device *pdev)
> +{
> +	struct pm800_clk *pm800_clks = platform_get_drvdata(pdev);
> +	int i;
> +
> +	of_clk_del_provider(pm800_clks[0].clk_np);
> +	/* Drop the reference obtained in pm800_clk_parse_dt */
> +	of_node_put(pm800_clks[0].clk_np);
> +
> +	for (i = 0; i < PM800_CLKS_NUM; i++) {
> +		if (pm800_clks[i].lookup)
> +			clkdev_drop(pm800_clks[i].lookup);
> +	}
> +
> +	return 0;
> +}
> +
> +static const struct platform_device_id pm800_clk_id[] = {
> +	{ "88pm800-clk", CHIP_PM800},
> +	{ "88pm860-clk", CHIP_PM860},
> +	{ },
> +};
> +MODULE_DEVICE_TABLE(platform, pm800_clk_id);
> +
> +static struct platform_driver pm800_clk_driver = {
> +	.driver = {
> +		.name  = "88pm80x-clk",
> +	},
> +	.probe = pm800_clk_probe,
> +	.remove = pm800_clk_remove,
> +	.id_table = pm800_clk_id,
> +};
> +
> +static int __init pm800_clk_init(void)
> +{
> +	return platform_driver_register(&pm800_clk_driver);
> +}
> +subsys_initcall(pm800_clk_init);

Does it have to be subsys_initcall instead of regular
module_platform_driver? Strict ordering of modules should be avoided. Do
you require it?

Rest looks fine (actually Stephen pointed some issue during previous
review so I had easier job).

Best regards,
Krzysztof

[PATCH-v2 5/5] mfd: 88pm800: Add support for clk subdevice

[Vaibhav Hiremath]

This patch adds mfd_cell/clk-subdevice for 88PM800 MFD
(and family of devices).

Signed-off-by: Vaibhav Hiremath <vaibhav.hiremath@linaro.org>
---
 drivers/mfd/88pm800.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/drivers/mfd/88pm800.c b/drivers/mfd/88pm800.c
index c4e097d..a928eed 100644
--- a/drivers/mfd/88pm800.c
+++ b/drivers/mfd/88pm800.c
@@ -173,6 +173,14 @@ static const struct mfd_cell regulator_devs[] = {
 	},
 };
 
+static struct mfd_cell clk_devs[] = {
+	{
+		.name = "88pm80x-clk",
+		.of_compatible = "marvell,88pm800-clk",
+		.id = -1,
+	},
+};
+
 static const struct regmap_irq pm800_irqs[] = {
 	/* INT0 */
 	[PM800_IRQ_ONKEY] = {
@@ -344,6 +352,17 @@ static int device_regulator_init(struct pm80x_chip *chip)
 			ARRAY_SIZE(regulator_devs), NULL, 0, NULL);
 }
 
+static int device_clk_init(struct pm80x_chip *chip)
+{
+	if (chip->type == CHIP_PM800)
+		clk_devs[0].name = "88pm800-clk";
+	else if (chip->type == CHIP_PM860)
+		clk_devs[0].name = "88pm860-clk";
+
+	return mfd_add_devices(chip->dev, 0, &clk_devs[0],
+			ARRAY_SIZE(clk_devs), NULL, 0, NULL);
+}
+
 static int device_irq_init_800(struct pm80x_chip *chip)
 {
 	struct regmap *map = chip->regmap;
@@ -511,6 +530,12 @@ static int device_800_init(struct pm80x_chip *chip)
 		goto out;
 	}
 
+	ret = device_clk_init(chip);
+	if (ret) {
+		dev_err(chip->dev, "Failed to add clk subdev\n");
+		goto out;
+	}
+
 	return 0;
 out_dev:
 	mfd_remove_devices(chip->dev);
-- 
1.9.1

[PATCH-v2 5/5] mfd: 88pm800: Add support for clk subdevice

[Krzysztof Kozlowski]

On 25.08.2015 03:56, Vaibhav Hiremath wrote:
> This patch adds mfd_cell/clk-subdevice for 88PM800 MFD
> (and family of devices).
> 
> Signed-off-by: Vaibhav Hiremath <vaibhav.hiremath@linaro.org>
> ---
>  drivers/mfd/88pm800.c | 25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
> 
> diff --git a/drivers/mfd/88pm800.c b/drivers/mfd/88pm800.c
> index c4e097d..a928eed 100644
> --- a/drivers/mfd/88pm800.c
> +++ b/drivers/mfd/88pm800.c
> @@ -173,6 +173,14 @@ static const struct mfd_cell regulator_devs[] = {
>  	},
>  };
>  
> +static struct mfd_cell clk_devs[] = {
> +	{
> +		.name = "88pm80x-clk",
> +		.of_compatible = "marvell,88pm800-clk",
> +		.id = -1,

The "-1" here stands for PLATFORM_DEVID_NONE?

Best regards,
Krzysztof

> +	},
> +};
> +
>  static const struct regmap_irq pm800_irqs[] = {
>  	/* INT0 */
>  	[PM800_IRQ_ONKEY] = {
> @@ -344,6 +352,17 @@ static int device_regulator_init(struct pm80x_chip *chip)
>  			ARRAY_SIZE(regulator_devs), NULL, 0, NULL);
>  }
>  
> +static int device_clk_init(struct pm80x_chip *chip)
> +{
> +	if (chip->type == CHIP_PM800)
> +		clk_devs[0].name = "88pm800-clk";
> +	else if (chip->type == CHIP_PM860)
> +		clk_devs[0].name = "88pm860-clk";
> +
> +	return mfd_add_devices(chip->dev, 0, &clk_devs[0],
> +			ARRAY_SIZE(clk_devs), NULL, 0, NULL);
> +}
> +
>  static int device_irq_init_800(struct pm80x_chip *chip)
>  {
>  	struct regmap *map = chip->regmap;
> @@ -511,6 +530,12 @@ static int device_800_init(struct pm80x_chip *chip)
>  		goto out;
>  	}
>  
> +	ret = device_clk_init(chip);
> +	if (ret) {
> +		dev_err(chip->dev, "Failed to add clk subdev\n");
> +		goto out;
> +	}
> +
>  	return 0;
>  out_dev:
>  	mfd_remove_devices(chip->dev);
> 

[PATCH-v2 5/5] mfd: 88pm800: Add support for clk subdevice

[Lee Jones]

On Tue, 25 Aug 2015, Krzysztof Kozlowski wrote:

> On 25.08.2015 03:56, Vaibhav Hiremath wrote:
> > This patch adds mfd_cell/clk-subdevice for 88PM800 MFD
> > (and family of devices).
> > 
> > Signed-off-by: Vaibhav Hiremath <vaibhav.hiremath@linaro.org>
> > ---
> >  drivers/mfd/88pm800.c | 25 +++++++++++++++++++++++++
> >  1 file changed, 25 insertions(+)
> > 
> > diff --git a/drivers/mfd/88pm800.c b/drivers/mfd/88pm800.c
> > index c4e097d..a928eed 100644
> > --- a/drivers/mfd/88pm800.c
> > +++ b/drivers/mfd/88pm800.c
> > @@ -173,6 +173,14 @@ static const struct mfd_cell regulator_devs[] = {
> >  	},
> >  };
> >  
> > +static struct mfd_cell clk_devs[] = {
> > +	{
> > +		.name = "88pm80x-clk",
> > +		.of_compatible = "marvell,88pm800-clk",
> > +		.id = -1,
> 
> The "-1" here stands for PLATFORM_DEVID_NONE?

Right.  Please use the define, but use it in the mfd_add_devices()
call, rather than individual cells.

> > +	},
> > +};
> > +
> >  static const struct regmap_irq pm800_irqs[] = {
> >  	/* INT0 */
> >  	[PM800_IRQ_ONKEY] = {
> > @@ -344,6 +352,17 @@ static int device_regulator_init(struct pm80x_chip *chip)
> >  			ARRAY_SIZE(regulator_devs), NULL, 0, NULL);
> >  }
> >  
> > +static int device_clk_init(struct pm80x_chip *chip)
> > +{
> > +	if (chip->type == CHIP_PM800)
> > +		clk_devs[0].name = "88pm800-clk";
> > +	else if (chip->type == CHIP_PM860)
> > +		clk_devs[0].name = "88pm860-clk";
> > +
> > +	return mfd_add_devices(chip->dev, 0, &clk_devs[0],
> > +			ARRAY_SIZE(clk_devs), NULL, 0, NULL);
> > +}
> > +
> >  static int device_irq_init_800(struct pm80x_chip *chip)
> >  {
> >  	struct regmap *map = chip->regmap;
> > @@ -511,6 +530,12 @@ static int device_800_init(struct pm80x_chip *chip)
> >  		goto out;
> >  	}
> >  
> > +	ret = device_clk_init(chip);
> > +	if (ret) {
> > +		dev_err(chip->dev, "Failed to add clk subdev\n");
> > +		goto out;
> > +	}
> > +
> >  	return 0;
> >  out_dev:
> >  	mfd_remove_devices(chip->dev);
> > 
> 

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

[PATCH-v2 5/5] mfd: 88pm800: Add support for clk subdevice

[Vaibhav Hiremath]

On Tuesday 25 August 2015 01:19 PM, Lee Jones wrote:
> On Tue, 25 Aug 2015, Krzysztof Kozlowski wrote:
>
>> On 25.08.2015 03:56, Vaibhav Hiremath wrote:
>>> This patch adds mfd_cell/clk-subdevice for 88PM800 MFD
>>> (and family of devices).
>>>
>>> Signed-off-by: Vaibhav Hiremath <vaibhav.hiremath@linaro.org>
>>> ---
>>>   drivers/mfd/88pm800.c | 25 +++++++++++++++++++++++++
>>>   1 file changed, 25 insertions(+)
>>>
>>> diff --git a/drivers/mfd/88pm800.c b/drivers/mfd/88pm800.c
>>> index c4e097d..a928eed 100644
>>> --- a/drivers/mfd/88pm800.c
>>> +++ b/drivers/mfd/88pm800.c
>>> @@ -173,6 +173,14 @@ static const struct mfd_cell regulator_devs[] = {
>>>   	},
>>>   };
>>>
>>> +static struct mfd_cell clk_devs[] = {
>>> +	{
>>> +		.name = "88pm80x-clk",
>>> +		.of_compatible = "marvell,88pm800-clk",
>>> +		.id = -1,
>>
>> The "-1" here stands for PLATFORM_DEVID_NONE?
>
> Right.  Please use the define, but use it in the mfd_add_devices()
> call, rather than individual cells.
>

Ok,

Thanks,
Vaibhav

[PATCH-v2 5/5] mfd: 88pm800: Add support for clk subdevice

[Vaibhav Hiremath]

On Tuesday 25 August 2015 10:55 AM, Krzysztof Kozlowski wrote:
> On 25.08.2015 03:56, Vaibhav Hiremath wrote:
>> This patch adds mfd_cell/clk-subdevice for 88PM800 MFD
>> (and family of devices).
>>
>> Signed-off-by: Vaibhav Hiremath <vaibhav.hiremath@linaro.org>
>> ---
>>   drivers/mfd/88pm800.c | 25 +++++++++++++++++++++++++
>>   1 file changed, 25 insertions(+)
>>
>> diff --git a/drivers/mfd/88pm800.c b/drivers/mfd/88pm800.c
>> index c4e097d..a928eed 100644
>> --- a/drivers/mfd/88pm800.c
>> +++ b/drivers/mfd/88pm800.c
>> @@ -173,6 +173,14 @@ static const struct mfd_cell regulator_devs[] = {
>>   	},
>>   };
>>
>> +static struct mfd_cell clk_devs[] = {
>> +	{
>> +		.name = "88pm80x-clk",
>> +		.of_compatible = "marvell,88pm800-clk",
>> +		.id = -1,
>
> The "-1" here stands for PLATFORM_DEVID_NONE?
>

Yes,

But it is rarely used in the kernel. so was reluctant to use and
decided to follow existing implementation.

Thanks,
Vaibhav