[PATCH-v2 0/3] mfd: 88pm800: Add Device tree support

[PATCH-v2 0/3] mfd: 88pm800: Add Device tree support

[Vaibhav Hiremath]

This patch-series adds support for Device tree to 88PM800 mfd driver.
It also enabled configuration of irq clear method through DT.

Testing::
 - Boot tested on PXA1928 based platform.
 - probe of mfd, rtc and regulator function passing successfully.
 - Basic read operations on registers


V1 => V1
=======
Link to V1: http://lkml.iu.edu/hypermail/linux/kernel/1505.3/04386.html

  - Split binding changes from original commit
  - Updated binding info as per Rob's suggestion
  - Dropped PATCH 4/4, as discussed during review
  - Dropped PATCH 3/4, as it is independent RTC code change,
    so will submit it separately to ease merging.
  - Fixed all other minor comments

Attempt has been made to push some of the patches to the list sometime
back in 2013.

Link to previous patch submission:
        https://lkml.org/lkml/2013/8/14/86

Vaibhav Hiremath (3):
  mfd: 88pm800: Add device tree support
  mfd: 88pm800: Allow configuration of interrupt clear method
  mfd: devicetree: bindings: Add new 88pm800 mfd binding

 Documentation/devicetree/bindings/mfd/88pm800.txt | 60 +++++++++++++++++++++++
 drivers/mfd/88pm800.c                             | 40 +++++++++++++--
 include/linux/mfd/88pm80x.h                       |  6 ++-
 3 files changed, 99 insertions(+), 7 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mfd/88pm800.txt

-- 
1.9.1

[PATCH-v2 1/3] mfd: 88pm800: Add device tree support

[Vaibhav Hiremath]

Add DT support to the 88pm800 driver, along with compatible
field for it's sub-devices (rtc, onkey and regulator)

Signed-off-by: Chao Xie <chao.xie-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>
Signed-off-by: Vaibhav Hiremath <vaibhav.hiremath-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 drivers/mfd/88pm800.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/drivers/mfd/88pm800.c b/drivers/mfd/88pm800.c
index 841717a..059f01a 100644
--- a/drivers/mfd/88pm800.c
+++ b/drivers/mfd/88pm800.c
@@ -27,6 +27,7 @@
 #include <linux/mfd/core.h>
 #include <linux/mfd/88pm80x.h>
 #include <linux/slab.h>
+#include <linux/of_device.h>
 
 /* Interrupt Registers */
 #define PM800_INT_STATUS1		(0x05)
@@ -121,6 +122,11 @@ static const struct i2c_device_id pm80x_id_table[] = {
 };
 MODULE_DEVICE_TABLE(i2c, pm80x_id_table);
 
+static const struct of_device_id pm80x_of_match_table[] = {
+	{ .compatible = "marvell,88pm800", },
+	{},
+};
+
 static struct resource rtc_resources[] = {
 	{
 	 .name = "88pm80x-rtc",
@@ -133,6 +139,7 @@ static struct resource rtc_resources[] = {
 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,
@@ -151,6 +158,7 @@ static struct resource onkey_resources[] = {
 static const struct mfd_cell onkey_devs[] = {
 	{
 	 .name = "88pm80x-onkey",
+	 .of_compatible = "marvell,88pm80x-onkey",
 	 .num_resources = 1,
 	 .resources = &onkey_resources[0],
 	 .id = -1,
@@ -160,6 +168,7 @@ static const struct mfd_cell onkey_devs[] = {
 static const struct mfd_cell regulator_devs[] = {
 	{
 	 .name = "88pm80x-regulator",
+	 .of_compatible = "marvell,88pm80x-regulator",
 	 .id = -1,
 	},
 };
@@ -544,8 +553,23 @@ static int pm800_probe(struct i2c_client *client,
 	int ret = 0;
 	struct pm80x_chip *chip;
 	struct pm80x_platform_data *pdata = dev_get_platdata(&client->dev);
+	struct device_node *np = client->dev.of_node;
 	struct pm80x_subchip *subchip;
 
+	if (!pdata && !np) {
+		dev_err(&client->dev,
+			"pm80x requires platform data or of_node\n");
+		return -EINVAL;
+	}
+
+	if (!pdata) {
+		pdata = devm_kzalloc(&client->dev, sizeof(*pdata), GFP_KERNEL);
+		if (!pdata) {
+			dev_err(&client->dev, "failed to allocaate memory\n");
+			return -ENOMEM;
+		}
+	}
+
 	ret = pm80x_init(client);
 	if (ret) {
 		dev_err(&client->dev, "pm800_init fail\n");
@@ -611,6 +635,7 @@ static struct i2c_driver pm800_driver = {
 		.name = "88PM800",
 		.owner = THIS_MODULE,
 		.pm = &pm80x_pm_ops,
+		.of_match_table	= pm80x_of_match_table,
 		},
 	.probe = pm800_probe,
 	.remove = pm800_remove,
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH-v2 2/3] mfd: 88pm800: Allow configuration of interrupt clear method

[Vaibhav Hiremath]

As per the spec, bit 1 (INT_CLEAR_MODE) of reg addr 0xe
(page 0) controls the method of clearing interrupt
status of 88pm800 family of devices;

  0: clear on read
  1: clear on write

This patch allows to configure this field, through DT.

Also, as suggested by "Lee Jones" renaming DT property and variable
field to appropriate name.

Signed-off-by: Zhao Ye <zhaoy-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>
Signed-off-by: Vaibhav Hiremath <vaibhav.hiremath-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 drivers/mfd/88pm800.c       | 15 ++++++++++-----
 include/linux/mfd/88pm80x.h |  6 ++++--
 2 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/drivers/mfd/88pm800.c b/drivers/mfd/88pm800.c
index 059f01a..c1a6306 100644
--- a/drivers/mfd/88pm800.c
+++ b/drivers/mfd/88pm800.c
@@ -376,7 +376,7 @@ static int device_irq_init_800(struct pm80x_chip *chip)
 {
 	struct regmap *map = chip->regmap;
 	unsigned long flags = IRQF_ONESHOT;
-	int data, mask, ret = -EINVAL;
+	int irq_clr_mode, mask, ret = -EINVAL;
 
 	if (!map || !chip->irq) {
 		dev_err(chip->dev, "incorrect parameters\n");
@@ -384,15 +384,16 @@ static int device_irq_init_800(struct pm80x_chip *chip)
 	}
 
 	/*
-	 * irq_mode defines the way of clearing interrupt. it's read-clear by
-	 * default.
+	 * irq_clr_on_wr defines the way of clearing interrupt by
+	 * read/write(0/1).  It's read-clear by default.
 	 */
 	mask =
 	    PM800_WAKEUP2_INV_INT | PM800_WAKEUP2_INT_CLEAR |
 	    PM800_WAKEUP2_INT_MASK;
 
-	data = PM800_WAKEUP2_INT_CLEAR;
-	ret = regmap_update_bits(map, PM800_WAKEUP2, mask, data);
+	irq_clr_mode = (chip->irq_clr_on_wr) ?
+		PM800_WAKEUP2_INT_WRITE_CLEAR : PM800_WAKEUP2_INT_READ_CLEAR;
+	ret = regmap_update_bits(map, PM800_WAKEUP2, mask, irq_clr_mode);
 
 	if (ret < 0)
 		goto out;
@@ -514,6 +515,7 @@ static int device_800_init(struct pm80x_chip *chip,
 	}
 
 	chip->regmap_irq_chip = &pm800_irq_chip;
+	chip->irq_clr_on_wr = pdata->irq_clr_on_wr;
 
 	ret = device_irq_init_800(chip);
 	if (ret < 0) {
@@ -568,6 +570,9 @@ static int pm800_probe(struct i2c_client *client,
 			dev_err(&client->dev, "failed to allocaate memory\n");
 			return -ENOMEM;
 		}
+
+		pdata->irq_clr_on_wr = of_property_read_bool(np,
+					"marvell,irq-clr-on-write");
 	}
 
 	ret = pm80x_init(client);
diff --git a/include/linux/mfd/88pm80x.h b/include/linux/mfd/88pm80x.h
index 97cb283..94b3dcd 100644
--- a/include/linux/mfd/88pm80x.h
+++ b/include/linux/mfd/88pm80x.h
@@ -77,6 +77,8 @@ enum {
 #define PM800_WAKEUP2		(0x0E)
 #define PM800_WAKEUP2_INV_INT		(1 << 0)
 #define PM800_WAKEUP2_INT_CLEAR		(1 << 1)
+#define PM800_WAKEUP2_INT_READ_CLEAR		(0 << 1)
+#define PM800_WAKEUP2_INT_WRITE_CLEAR		(1 << 1)
 #define PM800_WAKEUP2_INT_MASK		(1 << 2)
 
 #define PM800_POWER_UP_LOG	(0x10)
@@ -300,7 +302,7 @@ struct pm80x_chip {
 	struct regmap_irq_chip_data *irq_data;
 	int type;
 	int irq;
-	int irq_mode;
+	int irq_clr_on_wr;	/* '1': Clear on write, '0': Clear on read*/
 	unsigned long wu_flag;
 	spinlock_t lock;
 };
@@ -315,7 +317,7 @@ struct pm80x_platform_data {
 	 */
 	struct regulator_init_data *regulators[PM800_ID_RG_MAX];
 	unsigned int num_regulators;
-	int irq_mode;		/* Clear interrupt by read/write(0/1) */
+	int irq_clr_on_wr;		/* Clear interrupt by read/write(0/1) */
 	int batt_det;		/* enable/disable */
 	int (*plat_config)(struct pm80x_chip *chip,
 				struct pm80x_platform_data *pdata);
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH-v2 3/3] mfd: devicetree: bindings: Add new 88pm800 mfd binding

[Vaibhav Hiremath]

With addition of DT support to 88pm800 mfd driver, this patch
adds new DT binding documentation along with respective properties.

Signed-off-by: Vaibhav Hiremath <vaibhav.hiremath-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 Documentation/devicetree/bindings/mfd/88pm800.txt | 60 +++++++++++++++++++++++
 1 file changed, 60 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/88pm800.txt

diff --git a/Documentation/devicetree/bindings/mfd/88pm800.txt b/Documentation/devicetree/bindings/mfd/88pm800.txt
new file mode 100644
index 0000000..b8e72df
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/88pm800.txt
@@ -0,0 +1,60 @@
+* Marvell 88PM8xx Power Management IC
+
+Required parent device properties:
+- compatible : "marvell,88pm800", "marvell,88pm805", "marvell,88pm860"
+- reg : the I2C slave address for the 88pm8xx chip
+- interrupts : IRQ line for the 88pm8xx chip
+- interrupt-controller: describes the 88pm8xx as an interrupt controller
+- #interrupt-cells : should be 1.
+		- The cell is the 88pm8xx local IRQ number
+
+Optional parent device properties:
+- marvell,irq-clr-on-write: indicates whether interrupt status is cleared
+  by write or read.
+  If enabled, interrupt is cleared by write else just read would do.
+
+88pm8xx family of devices consists of varied group of sub-devices:
+
+Device		 	Supply Names	 Description
+------		 	------------	 -----------
+88pm80x-onkey		:		: On key
+88pm80x-rtc		:		: RTC
+88pm80x-regulator	:		: Regulators
+
+Note: More device list will follow
+
+Example:
+
+	pmic: 88pm800@30 {
+		compatible = "marvell,88pm800";
+		reg = <0x30>;
+		interrupts = <GIC_SPI 77 IRQ_TYPE_LEVEL_HIGH>;
+		interrupt-parent = <&gic>;
+		interrupt-controller;
+		#interrupt-cells = <1>;
+
+		marvell,irq-clr-on-write;
+
+		regulators {
+			compatible = "marvell,88pm80x-regulator";
+
+			buck1a: BUCK1A {
+				regulator-compatible = "88PM800-BUCK1A";
+				regulator-min-microvolt = <600000>;
+				regulator-max-microvolt = <1800000>;
+				regulator-boot-on;
+				regulator-always-on;
+			};
+			ldo1: LDO1 {
+				regulator-compatible = "88PM800-LDO1";
+				regulator-min-microvolt = <1700000>;
+				regulator-max-microvolt = <3300000>;
+				regulator-boot-on;
+				regulator-always-on;
+			};
+		};
+
+		rtc {
+			compatible = "marvell,88pm80x-rtc";
+		};
+	};
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH-v2 3/3] mfd: devicetree: bindings: Add new 88pm800 mfd binding

[Rob Herring]

On Wed, Jun 17, 2015 at 1:58 PM, Vaibhav Hiremath
<vaibhav.hiremath-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> With addition of DT support to 88pm800 mfd driver, this patch
> adds new DT binding documentation along with respective properties.
>
> Signed-off-by: Vaibhav Hiremath <vaibhav.hiremath-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

> ---
>  Documentation/devicetree/bindings/mfd/88pm800.txt | 60 +++++++++++++++++++++++
>  1 file changed, 60 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mfd/88pm800.txt
>
> diff --git a/Documentation/devicetree/bindings/mfd/88pm800.txt b/Documentation/devicetree/bindings/mfd/88pm800.txt
> new file mode 100644
> index 0000000..b8e72df
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/88pm800.txt
> @@ -0,0 +1,60 @@
> +* Marvell 88PM8xx Power Management IC
> +
> +Required parent device properties:
> +- compatible : "marvell,88pm800", "marvell,88pm805", "marvell,88pm860"
> +- reg : the I2C slave address for the 88pm8xx chip
> +- interrupts : IRQ line for the 88pm8xx chip
> +- interrupt-controller: describes the 88pm8xx as an interrupt controller
> +- #interrupt-cells : should be 1.
> +               - The cell is the 88pm8xx local IRQ number
> +
> +Optional parent device properties:
> +- marvell,irq-clr-on-write: indicates whether interrupt status is cleared
> +  by write or read.
> +  If enabled, interrupt is cleared by write else just read would do.
> +
> +88pm8xx family of devices consists of varied group of sub-devices:
> +
> +Device                 Supply Names     Description
> +------                 ------------     -----------
> +88pm80x-onkey          :               : On key
> +88pm80x-rtc            :               : RTC
> +88pm80x-regulator      :               : Regulators
> +
> +Note: More device list will follow
> +
> +Example:
> +
> +       pmic: 88pm800@30 {
> +               compatible = "marvell,88pm800";
> +               reg = <0x30>;
> +               interrupts = <GIC_SPI 77 IRQ_TYPE_LEVEL_HIGH>;
> +               interrupt-parent = <&gic>;
> +               interrupt-controller;
> +               #interrupt-cells = <1>;
> +
> +               marvell,irq-clr-on-write;
> +
> +               regulators {
> +                       compatible = "marvell,88pm80x-regulator";
> +
> +                       buck1a: BUCK1A {
> +                               regulator-compatible = "88PM800-BUCK1A";
> +                               regulator-min-microvolt = <600000>;
> +                               regulator-max-microvolt = <1800000>;
> +                               regulator-boot-on;
> +                               regulator-always-on;
> +                       };
> +                       ldo1: LDO1 {
> +                               regulator-compatible = "88PM800-LDO1";
> +                               regulator-min-microvolt = <1700000>;
> +                               regulator-max-microvolt = <3300000>;
> +                               regulator-boot-on;
> +                               regulator-always-on;
> +                       };
> +               };
> +
> +               rtc {
> +                       compatible = "marvell,88pm80x-rtc";
> +               };
> +       };
> --
> 1.9.1
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH-v2 3/3] mfd: devicetree: bindings: Add new 88pm800 mfd binding

[Vaibhav Hiremath]

On Tuesday 23 June 2015 09:07 PM, Rob Herring wrote:
> On Wed, Jun 17, 2015 at 1:58 PM, Vaibhav Hiremath
> <vaibhav.hiremath@linaro.org> wrote:
>> With addition of DT support to 88pm800 mfd driver, this patch
>> adds new DT binding documentation along with respective properties.
>>
>> Signed-off-by: Vaibhav Hiremath <vaibhav.hiremath@linaro.org>
>
> Acked-by: Rob Herring <robh@kernel.org>
>

Thanks for your review.

Thanks,
Vaibhav

Re: [PATCH-v2 3/3] mfd: devicetree: bindings: Add new 88pm800 mfd binding

[Krzysztof Kozlowski]

2015-06-18 3:58 GMT+09:00 Vaibhav Hiremath <vaibhav.hiremath-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>:
> With addition of DT support to 88pm800 mfd driver, this patch
> adds new DT binding documentation along with respective properties.
>
> Signed-off-by: Vaibhav Hiremath <vaibhav.hiremath-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
>  Documentation/devicetree/bindings/mfd/88pm800.txt | 60 +++++++++++++++++++++++
>  1 file changed, 60 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mfd/88pm800.txt
>
> diff --git a/Documentation/devicetree/bindings/mfd/88pm800.txt b/Documentation/devicetree/bindings/mfd/88pm800.txt
> new file mode 100644
> index 0000000..b8e72df
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/88pm800.txt
> @@ -0,0 +1,60 @@
> +* Marvell 88PM8xx Power Management IC
> +
> +Required parent device properties:
> +- compatible : "marvell,88pm800", "marvell,88pm805", "marvell,88pm860"
> +- reg : the I2C slave address for the 88pm8xx chip
> +- interrupts : IRQ line for the 88pm8xx chip
> +- interrupt-controller: describes the 88pm8xx as an interrupt controller
> +- #interrupt-cells : should be 1.
> +               - The cell is the 88pm8xx local IRQ number
> +
> +Optional parent device properties:
> +- marvell,irq-clr-on-write: indicates whether interrupt status is cleared
> +  by write or read.
> +  If enabled, interrupt is cleared by write else just read would do.
> +
> +88pm8xx family of devices consists of varied group of sub-devices:
> +
> +Device                 Supply Names     Description
> +------                 ------------     -----------
> +88pm80x-onkey          :               : On key
> +88pm80x-rtc            :               : RTC
> +88pm80x-regulator      :               : Regulators
> +
> +Note: More device list will follow
> +
> +Example:
> +
> +       pmic: 88pm800@30 {
> +               compatible = "marvell,88pm800";
> +               reg = <0x30>;
> +               interrupts = <GIC_SPI 77 IRQ_TYPE_LEVEL_HIGH>;
> +               interrupt-parent = <&gic>;
> +               interrupt-controller;
> +               #interrupt-cells = <1>;
> +
> +               marvell,irq-clr-on-write;
> +
> +               regulators {
> +                       compatible = "marvell,88pm80x-regulator";
> +
> +                       buck1a: BUCK1A {
> +                               regulator-compatible = "88PM800-BUCK1A";

That's a deprecated property.
Documentation/devicetree/bindings/regulator/regulator.txt

Best regards,
Krzysztof
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH-v2 3/3] mfd: devicetree: bindings: Add new 88pm800 mfd binding

[Vaibhav Hiremath]

On Wednesday 24 June 2015 05:59 AM, Krzysztof Kozlowski wrote:
> 2015-06-18 3:58 GMT+09:00 Vaibhav Hiremath <vaibhav.hiremath-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>:
>> With addition of DT support to 88pm800 mfd driver, this patch
>> adds new DT binding documentation along with respective properties.
>>
>> Signed-off-by: Vaibhav Hiremath <vaibhav.hiremath-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>> ---
>>   Documentation/devicetree/bindings/mfd/88pm800.txt | 60 +++++++++++++++++++++++
>>   1 file changed, 60 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/mfd/88pm800.txt
>>
>> diff --git a/Documentation/devicetree/bindings/mfd/88pm800.txt b/Documentation/devicetree/bindings/mfd/88pm800.txt
>> new file mode 100644
>> index 0000000..b8e72df
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mfd/88pm800.txt
>> @@ -0,0 +1,60 @@
>> +* Marvell 88PM8xx Power Management IC
>> +
>> +Required parent device properties:
>> +- compatible : "marvell,88pm800", "marvell,88pm805", "marvell,88pm860"
>> +- reg : the I2C slave address for the 88pm8xx chip
>> +- interrupts : IRQ line for the 88pm8xx chip
>> +- interrupt-controller: describes the 88pm8xx as an interrupt controller
>> +- #interrupt-cells : should be 1.
>> +               - The cell is the 88pm8xx local IRQ number
>> +
>> +Optional parent device properties:
>> +- marvell,irq-clr-on-write: indicates whether interrupt status is cleared
>> +  by write or read.
>> +  If enabled, interrupt is cleared by write else just read would do.
>> +
>> +88pm8xx family of devices consists of varied group of sub-devices:
>> +
>> +Device                 Supply Names     Description
>> +------                 ------------     -----------
>> +88pm80x-onkey          :               : On key
>> +88pm80x-rtc            :               : RTC
>> +88pm80x-regulator      :               : Regulators
>> +
>> +Note: More device list will follow
>> +
>> +Example:
>> +
>> +       pmic: 88pm800@30 {
>> +               compatible = "marvell,88pm800";
>> +               reg = <0x30>;
>> +               interrupts = <GIC_SPI 77 IRQ_TYPE_LEVEL_HIGH>;
>> +               interrupt-parent = <&gic>;
>> +               interrupt-controller;
>> +               #interrupt-cells = <1>;
>> +
>> +               marvell,irq-clr-on-write;
>> +
>> +               regulators {
>> +                       compatible = "marvell,88pm80x-regulator";
>> +
>> +                       buck1a: BUCK1A {
>> +                               regulator-compatible = "88PM800-BUCK1A";
>
> That's a deprecated property.
> Documentation/devicetree/bindings/regulator/regulator.txt
>

Yes,

Will replace with regulator-name and resubmit shortly with Rob's
Acked-by.



Thanks,
Vaibhav
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH-v2] rtc: 88pm80x: add device tree support

[Vaibhav Hiremath]

Along with DT support, this patch also cleans up the unnecessary
code around 'rtc_wakeup' initialization.

Signed-off-by: Chao Xie <chao.xie@marvell.com>
Signed-off-by: Vaibhav Hiremath <vaibhav.hiremath@linaro.org>
---
Link to V1: https://lkml.org/lkml/2015/5/29/757

 drivers/rtc/rtc-88pm80x.c | 28 +++++++++++++++++-----------
 1 file changed, 17 insertions(+), 11 deletions(-)

diff --git a/drivers/rtc/rtc-88pm80x.c b/drivers/rtc/rtc-88pm80x.c
index 7df0579..8f66519 100644
--- a/drivers/rtc/rtc-88pm80x.c
+++ b/drivers/rtc/rtc-88pm80x.c
@@ -251,17 +251,26 @@ static SIMPLE_DEV_PM_OPS(pm80x_rtc_pm_ops, pm80x_rtc_suspend, pm80x_rtc_resume);
 static int pm80x_rtc_probe(struct platform_device *pdev)
 {
 	struct pm80x_chip *chip = dev_get_drvdata(pdev->dev.parent);
-	struct pm80x_platform_data *pm80x_pdata =
-				dev_get_platdata(pdev->dev.parent);
-	struct pm80x_rtc_pdata *pdata = NULL;
+	struct pm80x_rtc_pdata *pdata = dev_get_platdata(&pdev->dev);
 	struct pm80x_rtc_info *info;
+	struct device_node *node = pdev->dev.of_node;
 	struct rtc_time tm;
 	unsigned long ticks = 0;
 	int ret;
 
-	pdata = dev_get_platdata(&pdev->dev);
-	if (pdata == NULL)
-		dev_warn(&pdev->dev, "No platform data!\n");
+	if (!pdata && !node) {
+		dev_err(&pdev->dev,
+			"pm80x-rtc requires platform data or of_node\n");
+		return -EINVAL;
+	}
+
+	if (!pdata) {
+		pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
+		if (!pdata) {
+			dev_err(&pdev->dev, "failed to allocate memory\n");
+			return -ENOMEM;
+		}
+	}
 
 	info =
 	    devm_kzalloc(&pdev->dev, sizeof(struct pm80x_rtc_info), GFP_KERNEL);
@@ -327,11 +336,8 @@ static int pm80x_rtc_probe(struct platform_device *pdev)
 	regmap_update_bits(info->map, PM800_RTC_CONTROL, PM800_RTC1_USE_XO,
 			   PM800_RTC1_USE_XO);
 
-	if (pm80x_pdata) {
-		pdata = pm80x_pdata->rtc;
-		if (pdata)
-			info->rtc_dev->dev.platform_data = &pdata->rtc_wakeup;
-	}
+	/* remeber whether this power up is caused by PMIC RTC or not */
+	info->rtc_dev->dev.platform_data = &pdata->rtc_wakeup;
 
 	device_init_wakeup(&pdev->dev, 1);
 
-- 
1.9.1

Re: [rtc-linux] [PATCH-v2] rtc: 88pm80x: add device tree support

[Alexandre Belloni]

Hi,

On 18/06/2015 at 00:28:06 +0530, Vaibhav Hiremath wrote :
> -	pdata = dev_get_platdata(&pdev->dev);
> -	if (pdata == NULL)
> -		dev_warn(&pdev->dev, "No platform data!\n");
> +	if (!pdata && !node) {
> +		dev_err(&pdev->dev,
> +			"pm80x-rtc requires platform data or of_node\n");
> +		return -EINVAL;
> +	}
> +
> +	if (!pdata) {
> +		pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);

I had troubles to follow the rtc_wakeup initialization cleanup but it
seems OK. However, I'm wondering why you are adding DT support as this
will always be probed from the MFD driver which pass the platform_data
and avoids that allocation.

> +		if (!pdata) {
> +			dev_err(&pdev->dev, "failed to allocate memory\n");
> +			return -ENOMEM;
> +		}
> +	}
>  
>  	info =
>  	    devm_kzalloc(&pdev->dev, sizeof(struct pm80x_rtc_info), GFP_KERNEL);
> @@ -327,11 +336,8 @@ static int pm80x_rtc_probe(struct platform_device *pdev)
>  	regmap_update_bits(info->map, PM800_RTC_CONTROL, PM800_RTC1_USE_XO,
>  			   PM800_RTC1_USE_XO);
>  
> -	if (pm80x_pdata) {
> -		pdata = pm80x_pdata->rtc;
> -		if (pdata)
> -			info->rtc_dev->dev.platform_data = &pdata->rtc_wakeup;
> -	}
> +	/* remeber whether this power up is caused by PMIC RTC or not */
remember -^


-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: [rtc-linux] [PATCH-v2] rtc: 88pm80x: add device tree support

[Vaibhav Hiremath]

On Saturday 20 June 2015 06:13 AM, Alexandre Belloni wrote:
> Hi,
>

Sorry I missed this email.
Just wanted to send reminder email and saw this :)

> On 18/06/2015 at 00:28:06 +0530, Vaibhav Hiremath wrote :
>> -	pdata = dev_get_platdata(&pdev->dev);
>> -	if (pdata == NULL)
>> -		dev_warn(&pdev->dev, "No platform data!\n");
>> +	if (!pdata && !node) {
>> +		dev_err(&pdev->dev,
>> +			"pm80x-rtc requires platform data or of_node\n");
>> +		return -EINVAL;
>> +	}
>> +
>> +	if (!pdata) {
>> +		pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
>
> I had troubles to follow the rtc_wakeup initialization cleanup but it
> seems OK.

Probably, once we have complete PM support ready then it would be
easier to understand. I think as of now lets have this minimal code for
rtc_wakeup.

> However, I'm wondering why you are adding DT support as this
> will always be probed from the MFD driver which pass the platform_data
> and avoids that allocation.
>

You are right.

Originally, it was cleanup patch,

  - remove pm80x_pdata
  - Add check for pdata and np
  - and around rtc_wakeup

While doing that I added this nice to have allocation.

Thanks,
Vaibhav
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [rtc-linux] [PATCH-v2] rtc: 88pm80x: add device tree support

[Vaibhav Hiremath]

On Thursday 25 June 2015 01:16 PM, Vaibhav Hiremath wrote:
>
>
> On Saturday 20 June 2015 06:13 AM, Alexandre Belloni wrote:
>> Hi,
>>
>
> Sorry I missed this email.
> Just wanted to send reminder email and saw this :)
>
>> On 18/06/2015 at 00:28:06 +0530, Vaibhav Hiremath wrote :
>>> -    pdata = dev_get_platdata(&pdev->dev);
>>> -    if (pdata == NULL)
>>> -        dev_warn(&pdev->dev, "No platform data!\n");
>>> +    if (!pdata && !node) {
>>> +        dev_err(&pdev->dev,
>>> +            "pm80x-rtc requires platform data or of_node\n");
>>> +        return -EINVAL;
>>> +    }
>>> +
>>> +    if (!pdata) {
>>> +        pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
>>
>> I had troubles to follow the rtc_wakeup initialization cleanup but it
>> seems OK.
>
> Probably, once we have complete PM support ready then it would be
> easier to understand. I think as of now lets have this minimal code for
> rtc_wakeup.
>
>> However, I'm wondering why you are adding DT support as this
>> will always be probed from the MFD driver which pass the platform_data
>> and avoids that allocation.
>>
>
> You are right.
>
> Originally, it was cleanup patch,
>
>   - remove pm80x_pdata
>   - Add check for pdata and np
>   - and around rtc_wakeup
>
> While doing that I added this nice to have allocation.
>

Any update/feedback on this?

Thanks,
Vaibhav
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH-v2 0/3] mfd: 88pm800: Add Device tree support

[Vaibhav Hiremath]

On Thursday 18 June 2015 12:28 AM, Vaibhav Hiremath wrote:
> This patch-series adds support for Device tree to 88PM800 mfd driver.
> It also enabled configuration of irq clear method through DT.
>
> Testing::
>   - Boot tested on PXA1928 based platform.
>   - probe of mfd, rtc and regulator function passing successfully.
>   - Basic read operations on registers
>
>
> V1 => V1
> =======
> Link to V1: http://lkml.iu.edu/hypermail/linux/kernel/1505.3/04386.html
>
>    - Split binding changes from original commit
>    - Updated binding info as per Rob's suggestion
>    - Dropped PATCH 4/4, as discussed during review
>    - Dropped PATCH 3/4, as it is independent RTC code change,
>      so will submit it separately to ease merging.
>    - Fixed all other minor comments
>
> Attempt has been made to push some of the patches to the list sometime
> back in 2013.
>
> Link to previous patch submission:
>          https://lkml.org/lkml/2013/8/14/86
>
> Vaibhav Hiremath (3):
>    mfd: 88pm800: Add device tree support
>    mfd: 88pm800: Allow configuration of interrupt clear method
>    mfd: devicetree: bindings: Add new 88pm800 mfd binding
>
>   Documentation/devicetree/bindings/mfd/88pm800.txt | 60 +++++++++++++++++++++++
>   drivers/mfd/88pm800.c                             | 40 +++++++++++++--
>   include/linux/mfd/88pm80x.h                       |  6 ++-
>   3 files changed, 99 insertions(+), 7 deletions(-)
>   create mode 100644 Documentation/devicetree/bindings/mfd/88pm800.txt
>


Any update on this?

If there are no review comments, then can it be queued ?

Thanks,
Vaibhav