2015-06-17 19:01:08

by Vaibhav Hiremath

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

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


2015-06-17 19:01:19

by Vaibhav Hiremath

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

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 <[email protected]>
Signed-off-by: Vaibhav Hiremath <[email protected]>
---
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

2015-06-17 19:01:32

by Vaibhav Hiremath

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

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

Signed-off-by: Chao Xie <[email protected]>
Signed-off-by: Vaibhav Hiremath <[email protected]>
---
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

2015-06-17 19:02:05

by Vaibhav Hiremath

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

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 <[email protected]>
Signed-off-by: Vaibhav Hiremath <[email protected]>
---
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

2015-06-17 19:01:44

by Vaibhav Hiremath

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

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 <[email protected]>
---
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

2015-06-20 00:44:04

by Alexandre Belloni

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

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

2015-06-23 05:07:33

by Vaibhav Hiremath

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



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

2015-06-23 15:38:27

by Rob Herring

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

On Wed, Jun 17, 2015 at 1:58 PM, Vaibhav Hiremath
<[email protected]> 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 <[email protected]>

Acked-by: Rob Herring <[email protected]>

> ---
> 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
>

2015-06-23 17:02:46

by Vaibhav Hiremath

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



On Tuesday 23 June 2015 09:07 PM, Rob Herring wrote:
> On Wed, Jun 17, 2015 at 1:58 PM, Vaibhav Hiremath
> <[email protected]> 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 <[email protected]>
>
> Acked-by: Rob Herring <[email protected]>
>

Thanks for your review.

Thanks,
Vaibhav

2015-06-24 00:29:09

by Krzysztof Kozlowski

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

2015-06-18 3:58 GMT+09:00 Vaibhav Hiremath <[email protected]>:
> 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 <[email protected]>
> ---
> 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

2015-06-24 05:20:01

by Vaibhav Hiremath

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



On Wednesday 24 June 2015 05:59 AM, Krzysztof Kozlowski wrote:
> 2015-06-18 3:58 GMT+09:00 Vaibhav Hiremath <[email protected]>:
>> 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 <[email protected]>
>> ---
>> 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

2015-06-25 07:46:25

by Vaibhav Hiremath

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



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

2015-07-07 06:37:33

by Vaibhav Hiremath

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



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