Hi,
The first patch cleans up the driver from unneccesary
wakeup handling.
The second patch adds device tree support to the driver.
Regards
Andrzej
Andrzej Hajda (2):
max77693: remove device wakeup from driver
max77693: added device tree support
drivers/mfd/max77693.c | 18 ++++++++----------
include/linux/mfd/max77693-private.h | 1 -
include/linux/mfd/max77693.h | 2 --
3 files changed, 8 insertions(+), 13 deletions(-)
--
1.8.1.2
The patch removes wakeup related code from
the driver and plaftorm data - it is already
handled by i2c core using I2C_CLIENT_WAKE flag
from struct i2c_board_info. As a result MFD
itself do not requires platform data.
Signed-off-by: Andrzej Hajda <[email protected]>
Signed-off-by: Kyungmin Park <[email protected]>
---
drivers/mfd/max77693.c | 10 ----------
include/linux/mfd/max77693-private.h | 1 -
include/linux/mfd/max77693.h | 2 --
3 files changed, 13 deletions(-)
diff --git a/drivers/mfd/max77693.c b/drivers/mfd/max77693.c
index 9e60fed..27f5da3 100644
--- a/drivers/mfd/max77693.c
+++ b/drivers/mfd/max77693.c
@@ -110,15 +110,9 @@ static int max77693_i2c_probe(struct i2c_client *i2c,
const struct i2c_device_id *id)
{
struct max77693_dev *max77693;
- struct max77693_platform_data *pdata = i2c->dev.platform_data;
u8 reg_data;
int ret = 0;
- if (!pdata) {
- dev_err(&i2c->dev, "No platform data found.\n");
- return -EINVAL;
- }
-
max77693 = devm_kzalloc(&i2c->dev,
sizeof(struct max77693_dev), GFP_KERNEL);
if (max77693 == NULL)
@@ -138,8 +132,6 @@ static int max77693_i2c_probe(struct i2c_client *i2c,
return ret;
}
- max77693->wakeup = pdata->wakeup;
-
ret = max77693_read_reg(max77693->regmap, MAX77693_PMIC_REG_PMIC_ID2,
®_data);
if (ret < 0) {
@@ -179,8 +171,6 @@ static int max77693_i2c_probe(struct i2c_client *i2c,
if (ret < 0)
goto err_mfd;
- device_init_wakeup(max77693->dev, pdata->wakeup);
-
return ret;
err_mfd:
diff --git a/include/linux/mfd/max77693-private.h b/include/linux/mfd/max77693-private.h
index 244fb0d..3e050b9 100644
--- a/include/linux/mfd/max77693-private.h
+++ b/include/linux/mfd/max77693-private.h
@@ -323,7 +323,6 @@ struct max77693_dev {
int irq;
int irq_gpio;
- bool wakeup;
struct mutex irqlock;
int irq_masks_cur[MAX77693_IRQ_GROUP_NR];
int irq_masks_cache[MAX77693_IRQ_GROUP_NR];
diff --git a/include/linux/mfd/max77693.h b/include/linux/mfd/max77693.h
index 676f0f3..3f3dc45 100644
--- a/include/linux/mfd/max77693.h
+++ b/include/linux/mfd/max77693.h
@@ -64,8 +64,6 @@ struct max77693_muic_platform_data {
};
struct max77693_platform_data {
- int wakeup;
-
/* regulator data */
struct max77693_regulator_data *regulators;
int num_regulators;
--
1.8.1.2
This patch adds only of_match_table.
There are no device specific properties.
Signed-off-by: Andrzej Hajda <[email protected]>
Reviewed-by: Sylwester Nawrocki <[email protected]>
Signed-off-by: Kyungmin Park <[email protected]>
---
drivers/mfd/max77693.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/mfd/max77693.c b/drivers/mfd/max77693.c
index 27f5da3..e0b11a9 100644
--- a/drivers/mfd/max77693.c
+++ b/drivers/mfd/max77693.c
@@ -225,11 +225,19 @@ static const struct dev_pm_ops max77693_pm = {
.resume = max77693_resume,
};
+#ifdef CONFIG_OF
+static struct of_device_id max77693_dt_match[] = {
+ {.compatible = "maxim,max77693"},
+ {},
+};
+#endif
+
static struct i2c_driver max77693_i2c_driver = {
.driver = {
.name = "max77693",
.owner = THIS_MODULE,
.pm = &max77693_pm,
+ .of_match_table = of_match_ptr(max77693_dt_match),
},
.probe = max77693_i2c_probe,
.remove = max77693_i2c_remove,
--
1.8.1.2
On Wed, Aug 21, 2013 at 05:53:34PM +0100, Andrzej Hajda wrote:
> This patch adds only of_match_table.
> There are no device specific properties.
Could you clarify what functionality this enables and what it doesn't,
please?
This doesn't seem to enable support for the regulators described in the
binding [1] (which from the looks of it needs proof-reading and possibly
rework).
Are there any changes we might need in future to either support new
functionality or to generalise the binding. e.g. do we need a regulator
for the LED?
Given the binding has never been supported, are we happy now that it
best represents the hardware, or are there avenues of improvement
*before* it becomes ABI?
Thanks,
Mark.
[1] Documentation/devicetree/bindings/mfd/max77693.txt
>
> Signed-off-by: Andrzej Hajda <[email protected]>
> Reviewed-by: Sylwester Nawrocki <[email protected]>
> Signed-off-by: Kyungmin Park <[email protected]>
> ---
> drivers/mfd/max77693.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/drivers/mfd/max77693.c b/drivers/mfd/max77693.c
> index 27f5da3..e0b11a9 100644
> --- a/drivers/mfd/max77693.c
> +++ b/drivers/mfd/max77693.c
> @@ -225,11 +225,19 @@ static const struct dev_pm_ops max77693_pm = {
> .resume = max77693_resume,
> };
>
> +#ifdef CONFIG_OF
> +static struct of_device_id max77693_dt_match[] = {
> + {.compatible = "maxim,max77693"},
> + {},
> +};
> +#endif
> +
> static struct i2c_driver max77693_i2c_driver = {
> .driver = {
> .name = "max77693",
> .owner = THIS_MODULE,
> .pm = &max77693_pm,
> + .of_match_table = of_match_ptr(max77693_dt_match),
> },
> .probe = max77693_i2c_probe,
> .remove = max77693_i2c_remove,
> --
> 1.8.1.2
>
>
Hi Andrzej,
On Wednesday 21 of August 2013 18:53:34 Andrzej Hajda wrote:
> This patch adds only of_match_table.
> There are no device specific properties.
>
> Signed-off-by: Andrzej Hajda <[email protected]>
> Reviewed-by: Sylwester Nawrocki <[email protected]>
> Signed-off-by: Kyungmin Park <[email protected]>
> ---
> drivers/mfd/max77693.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/drivers/mfd/max77693.c b/drivers/mfd/max77693.c
> index 27f5da3..e0b11a9 100644
> --- a/drivers/mfd/max77693.c
> +++ b/drivers/mfd/max77693.c
> @@ -225,11 +225,19 @@ static const struct dev_pm_ops max77693_pm = {
> .resume = max77693_resume,
> };
>
> +#ifdef CONFIG_OF
> +static struct of_device_id max77693_dt_match[] = {
> + {.compatible = "maxim,max77693"},
> + {},
> +};
> +#endif
> +
> static struct i2c_driver max77693_i2c_driver = {
> .driver = {
> .name = "max77693",
> .owner = THIS_MODULE,
> .pm = &max77693_pm,
> + .of_match_table = of_match_ptr(max77693_dt_match),
As far as I'm aware of, you don't need explicit OF match table for I2C
devices, because the I2C OF core can use the array of struct i2c_device_id
pointed by .id_table field of struct i2c_driver.
I'm not sure if a separate OF table isn't preferred, though, so your patch
might be fine.
Best regards,
Tomasz
Hi Mark,
On Friday 23 of August 2013 15:14:33 Mark Rutland wrote:
> On Wed, Aug 21, 2013 at 05:53:34PM +0100, Andrzej Hajda wrote:
> > This patch adds only of_match_table.
> > There are no device specific properties.
>
> Could you clarify what functionality this enables and what it doesn't,
> please?
This patch simply adds explicit OF match table for this device. Before,
the driver could be matched only by a fallback to i2c_device_id table.
> This doesn't seem to enable support for the regulators described in the
> binding [1] (which from the looks of it needs proof-reading and possibly
> rework).
>
> Are there any changes we might need in future to either support new
> functionality or to generalise the binding. e.g. do we need a regulator
> for the LED?
>
> Given the binding has never been supported, are we happy now that it
> best represents the hardware, or are there avenues of improvement
> *before* it becomes ABI?
Well, documentation of the binding has been present in kernel tree since
June, but I too think that we should review it and make sure it makes
sense. I'll try to get some information on this chip at work, on Monday.
Best regards,
Tomasz
On Fri, Aug 23, 2013 at 10:08:39PM +0200, Tomasz Figa wrote:
> On Wednesday 21 of August 2013 18:53:34 Andrzej Hajda wrote:
> > +#ifdef CONFIG_OF
> > +static struct of_device_id max77693_dt_match[] = {
> > + {.compatible = "maxim,max77693"},
> > + {},
> > +};
> > +#endif
> As far as I'm aware of, you don't need explicit OF match table for I2C
> devices, because the I2C OF core can use the array of struct i2c_device_id
> pointed by .id_table field of struct i2c_driver.
> I'm not sure if a separate OF table isn't preferred, though, so your patch
> might be fine.
It's still good practice to explicitly define a binding since that
gives a vendor prefix and there are overlaps out there in chip vendor
namings - for example both Wolfson and Wondermedia use "WMxxxx".
On 08/21/2013 06:53 PM, Andrzej Hajda wrote:
> The patch removes wakeup related code from
> the driver and plaftorm data - it is already
> handled by i2c core using I2C_CLIENT_WAKE flag
> from struct i2c_board_info. As a result MFD
> itself do not requires platform data.
>
> Signed-off-by: Andrzej Hajda <[email protected]>
> Signed-off-by: Kyungmin Park <[email protected]>
> ---
> drivers/mfd/max77693.c | 10 ----------
> include/linux/mfd/max77693-private.h | 1 -
> include/linux/mfd/max77693.h | 2 --
> 3 files changed, 13 deletions(-)
>
> diff --git a/drivers/mfd/max77693.c b/drivers/mfd/max77693.c
> index 9e60fed..27f5da3 100644
> --- a/drivers/mfd/max77693.c
> +++ b/drivers/mfd/max77693.c
> @@ -110,15 +110,9 @@ static int max77693_i2c_probe(struct i2c_client *i2c,
> const struct i2c_device_id *id)
> {
> struct max77693_dev *max77693;
> - struct max77693_platform_data *pdata = i2c->dev.platform_data;
> u8 reg_data;
> int ret = 0;
>
> - if (!pdata) {
> - dev_err(&i2c->dev, "No platform data found.\n");
> - return -EINVAL;
> - }
> -
> max77693 = devm_kzalloc(&i2c->dev,
> sizeof(struct max77693_dev), GFP_KERNEL);
> if (max77693 == NULL)
> @@ -138,8 +132,6 @@ static int max77693_i2c_probe(struct i2c_client *i2c,
> return ret;
> }
>
> - max77693->wakeup = pdata->wakeup;
> -
> ret = max77693_read_reg(max77693->regmap, MAX77693_PMIC_REG_PMIC_ID2,
> ®_data);
> if (ret < 0) {
> @@ -179,8 +171,6 @@ static int max77693_i2c_probe(struct i2c_client *i2c,
> if (ret < 0)
> goto err_mfd;
>
> - device_init_wakeup(max77693->dev, pdata->wakeup);
> -
> return ret;
>
> err_mfd:
> diff --git a/include/linux/mfd/max77693-private.h b/include/linux/mfd/max77693-private.h
> index 244fb0d..3e050b9 100644
> --- a/include/linux/mfd/max77693-private.h
> +++ b/include/linux/mfd/max77693-private.h
> @@ -323,7 +323,6 @@ struct max77693_dev {
>
> int irq;
> int irq_gpio;
> - bool wakeup;
> struct mutex irqlock;
> int irq_masks_cur[MAX77693_IRQ_GROUP_NR];
> int irq_masks_cache[MAX77693_IRQ_GROUP_NR];
> diff --git a/include/linux/mfd/max77693.h b/include/linux/mfd/max77693.h
> index 676f0f3..3f3dc45 100644
> --- a/include/linux/mfd/max77693.h
> +++ b/include/linux/mfd/max77693.h
> @@ -64,8 +64,6 @@ struct max77693_muic_platform_data {
> };
>
> struct max77693_platform_data {
> - int wakeup;
> -
> /* regulator data */
> struct max77693_regulator_data *regulators;
> int num_regulators;
Ping.
Regards
Andrzej
On 08/21/2013 06:53 PM, Andrzej Hajda wrote:
> This patch adds only of_match_table.
> There are no device specific properties.
>
> Signed-off-by: Andrzej Hajda <[email protected]>
> Reviewed-by: Sylwester Nawrocki <[email protected]>
> Signed-off-by: Kyungmin Park <[email protected]>
> ---
> drivers/mfd/max77693.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/drivers/mfd/max77693.c b/drivers/mfd/max77693.c
> index 27f5da3..e0b11a9 100644
> --- a/drivers/mfd/max77693.c
> +++ b/drivers/mfd/max77693.c
> @@ -225,11 +225,19 @@ static const struct dev_pm_ops max77693_pm = {
> .resume = max77693_resume,
> };
>
> +#ifdef CONFIG_OF
> +static struct of_device_id max77693_dt_match[] = {
> + {.compatible = "maxim,max77693"},
> + {},
> +};
> +#endif
> +
> static struct i2c_driver max77693_i2c_driver = {
> .driver = {
> .name = "max77693",
> .owner = THIS_MODULE,
> .pm = &max77693_pm,
> + .of_match_table = of_match_ptr(max77693_dt_match),
> },
> .probe = max77693_i2c_probe,
> .remove = max77693_i2c_remove,
Ping.
Regards
Andrzej
On Wed, 21 Aug 2013, Andrzej Hajda wrote:
> This patch adds only of_match_table.
> There are no device specific properties.
>
> Signed-off-by: Andrzej Hajda <[email protected]>
> Reviewed-by: Sylwester Nawrocki <[email protected]>
> Signed-off-by: Kyungmin Park <[email protected]>
> ---
> drivers/mfd/max77693.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/drivers/mfd/max77693.c b/drivers/mfd/max77693.c
> index 27f5da3..e0b11a9 100644
> --- a/drivers/mfd/max77693.c
> +++ b/drivers/mfd/max77693.c
> @@ -225,11 +225,19 @@ static const struct dev_pm_ops max77693_pm = {
> .resume = max77693_resume,
> };
>
> +#ifdef CONFIG_OF
> +static struct of_device_id max77693_dt_match[] = {
> + {.compatible = "maxim,max77693"},
Please add spaces before ".comp.." and after "...693""
> + {},
> +};
> +#endif
> +
> static struct i2c_driver max77693_i2c_driver = {
> .driver = {
> .name = "max77693",
> .owner = THIS_MODULE,
> .pm = &max77693_pm,
> + .of_match_table = of_match_ptr(max77693_dt_match),
> },
> .probe = max77693_i2c_probe,
> .remove = max77693_i2c_remove,
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
This patch adds only of_match_table.
There are no device specific properties.
Signed-off-by: Andrzej Hajda <[email protected]>
Reviewed-by: Sylwester Nawrocki <[email protected]>
Signed-off-by: Kyungmin Park <[email protected]>
---
Hi,
Spaces added.
Regards
Andrzej
---
drivers/mfd/max77693.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/mfd/max77693.c b/drivers/mfd/max77693.c
index 27f5da3..e0b11a9 100644
--- a/drivers/mfd/max77693.c
+++ b/drivers/mfd/max77693.c
@@ -225,11 +225,19 @@ static const struct dev_pm_ops max77693_pm = {
.resume = max77693_resume,
};
+#ifdef CONFIG_OF
+static struct of_device_id max77693_dt_match[] = {
+ { .compatible = "maxim,max77693" },
+ {},
+};
+#endif
+
static struct i2c_driver max77693_i2c_driver = {
.driver = {
.name = "max77693",
.owner = THIS_MODULE,
.pm = &max77693_pm,
+ .of_match_table = of_match_ptr(max77693_dt_match),
},
.probe = max77693_i2c_probe,
.remove = max77693_i2c_remove,
--
1.8.1.2
> The patch removes wakeup related code from
> the driver and plaftorm data - it is already
> handled by i2c core using I2C_CLIENT_WAKE flag
> from struct i2c_board_info. As a result MFD
> itself do not requires platform data.
I have expanded this to use more than 46 chars of the line
buffer. Please also do this for future submissions.
> Signed-off-by: Andrzej Hajda <[email protected]>
> Signed-off-by: Kyungmin Park <[email protected]>
> ---
> drivers/mfd/max77693.c | 10 ----------
> include/linux/mfd/max77693-private.h | 1 -
> include/linux/mfd/max77693.h | 2 --
> 3 files changed, 13 deletions(-)
Applied, thanks.
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
On Fri, 27 Sep 2013, Andrzej Hajda wrote:
> This patch adds only of_match_table.
> There are no device specific properties.
>
> Signed-off-by: Andrzej Hajda <[email protected]>
> Reviewed-by: Sylwester Nawrocki <[email protected]>
> Signed-off-by: Kyungmin Park <[email protected]>
> ---
> Hi,
>
> Spaces added.
>
> Regards
> Andrzej
> ---
> drivers/mfd/max77693.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
Applied, thanks.
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog