[RFT 0/4] mfd: regulator: max14577: Remove support for platform data

[RFT 0/4] mfd: regulator: max14577: Remove support for platform data

[Krzysztof Kozlowski]

Hi,

Minor cleanup of max14577 family of drivers. The dependency inside:
1. Patch #3 and #4 depends for safeness on #1 so no one would try to
   run the driver on non-DT platform (such platform does not exist anyway...).
2. Patch #4 depends on patch #3 (compile time dependency).

Unfortunately I do not have the boards using these drivers, so testing
would be appreciated.

Best regards,
Krzysztof

Krzysztof Kozlowski (4):
  mfd: max14577: Explicitly depend on OF
  mfd: max14577: Export OF module alias information
  regulator: max14577: Remove support for platform data
  mfd: max14577: Remove platform data and instantiation from I2C

 drivers/mfd/Kconfig                    |  1 +
 drivers/mfd/max14577.c                 | 23 +----------------------
 drivers/regulator/max14577-regulator.c | 15 +++------------
 include/linux/mfd/max14577.h           | 20 --------------------
 4 files changed, 5 insertions(+), 54 deletions(-)

-- 
2.9.3

[RFT 1/4] mfd: max14577: Explicitly depend on OF

[Krzysztof Kozlowski]

max14577 family of drivers are used only on Exynos-based ARMv7 boards
which all were converted to DeviceTree long time ago.  Driver still
supports board-file initialization but it is going to be removed thus
dependency on OF has to be marked explicitly.

Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
---
 drivers/mfd/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 55ecdfb74d31..ca196bd48ff2 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -572,6 +572,7 @@ config MFD_88PM860X
 config MFD_MAX14577
 	tristate "Maxim Semiconductor MAX14577/77836 MUIC + Charger Support"
 	depends on I2C
+	depends on OF || COMPILE_TEST
 	select MFD_CORE
 	select REGMAP_I2C
 	select REGMAP_IRQ
-- 
2.9.3

[RFT 2/4] mfd: max14577: Export OF module alias information

[Krzysztof Kozlowski]

Driver can be built as module and in such case for DT-based platform the
OF match table will be used for matching.  Export the module alias
information for this.  This will also allow removal of I2C device id
related code.

Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
---
 drivers/mfd/max14577.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/mfd/max14577.c b/drivers/mfd/max14577.c
index 6cbe96b28f42..f6772d64a6a4 100644
--- a/drivers/mfd/max14577.c
+++ b/drivers/mfd/max14577.c
@@ -152,6 +152,7 @@ static const struct of_device_id max14577_dt_match[] = {
 	},
 	{},
 };
+MODULE_DEVICE_TABLE(of, max14577_dt_match);
 
 static bool max14577_muic_volatile_reg(struct device *dev, unsigned int reg)
 {
-- 
2.9.3

[RFT 3/4] regulator: max14577: Remove support for platform data

[Krzysztof Kozlowski]

max14577 family of drivers are used only on Exynos-based ARMv7 boards
which all were converted to DeviceTree long time ago.  Remove the
support for platform data to simplify the driver.

Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
---
 drivers/regulator/max14577-regulator.c | 15 +++------------
 1 file changed, 3 insertions(+), 12 deletions(-)

diff --git a/drivers/regulator/max14577-regulator.c b/drivers/regulator/max14577-regulator.c
index 0db288ce319c..fa9b4b3a2878 100644
--- a/drivers/regulator/max14577-regulator.c
+++ b/drivers/regulator/max14577-regulator.c
@@ -245,7 +245,6 @@ static struct regmap *max14577_get_regmap(struct max14577 *max14577,
 static int max14577_regulator_probe(struct platform_device *pdev)
 {
 	struct max14577 *max14577 = dev_get_drvdata(pdev->dev.parent);
-	struct max14577_platform_data *pdata = dev_get_platdata(max14577->dev);
 	int i, ret = 0;
 	struct regulator_config config = {};
 	const struct regulator_desc *supported_regulators;
@@ -268,17 +267,9 @@ static int max14577_regulator_probe(struct platform_device *pdev)
 
 	for (i = 0; i < supported_regulators_size; i++) {
 		struct regulator_dev *regulator;
-		/*
-		 * Index of supported_regulators[] is also the id and must
-		 * match index of pdata->regulators[].
-		 */
-		if (pdata && pdata->regulators) {
-			config.init_data = pdata->regulators[i].initdata;
-			config.of_node = pdata->regulators[i].of_node;
-		} else {
-			config.init_data = match_init_data(i, dev_type);
-			config.of_node = match_of_node(i, dev_type);
-		}
+
+		config.init_data = match_init_data(i, dev_type);
+		config.of_node = match_of_node(i, dev_type);
 		config.regmap = max14577_get_regmap(max14577,
 				supported_regulators[i].id);
 
-- 
2.9.3

Re: [RFT 3/4] regulator: max14577: Remove support for platform data

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 304 bytes --]

On Fri, Feb 17, 2017 at 10:01:59PM +0200, Krzysztof Kozlowski wrote:
> max14577 family of drivers are used only on Exynos-based ARMv7 boards
> which all were converted to DeviceTree long time ago.  Remove the
> support for platform data to simplify the driver.

Acked-by: Mark Brown <broonie@kernel.org>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[RFT 4/4] mfd: max14577: Remove platform data and instantiation from I2C

[Krzysztof Kozlowski]

max14577 family of drivers can be now instantiated only from DeviceTree
thus we can remove any I2C device id code and platform data.  This
simplifies the driver a little bit.

Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
---
 drivers/mfd/max14577.c       | 22 ----------------------
 include/linux/mfd/max14577.h | 20 --------------------
 2 files changed, 42 deletions(-)

diff --git a/drivers/mfd/max14577.c b/drivers/mfd/max14577.c
index f6772d64a6a4..a6ec1451af94 100644
--- a/drivers/mfd/max14577.c
+++ b/drivers/mfd/max14577.c
@@ -373,7 +373,6 @@ static int max14577_i2c_probe(struct i2c_client *i2c,
 			      const struct i2c_device_id *id)
 {
 	struct max14577 *max14577;
-	struct max14577_platform_data *pdata = dev_get_platdata(&i2c->dev);
 	struct device_node *np = i2c->dev.of_node;
 	int ret = 0;
 	const struct regmap_irq_chip *irq_chip;
@@ -381,18 +380,6 @@ static int max14577_i2c_probe(struct i2c_client *i2c,
 	unsigned int mfd_devs_size;
 	int irq_flags;
 
-	if (np) {
-		pdata = devm_kzalloc(&i2c->dev, sizeof(*pdata), GFP_KERNEL);
-		if (!pdata)
-			return -ENOMEM;
-		i2c->dev.platform_data = pdata;
-	}
-
-	if (!pdata) {
-		dev_err(&i2c->dev, "No platform data found.\n");
-		return -EINVAL;
-	}
-
 	max14577 = devm_kzalloc(&i2c->dev, sizeof(*max14577), GFP_KERNEL);
 	if (!max14577)
 		return -ENOMEM;
@@ -486,13 +473,6 @@ static int max14577_i2c_remove(struct i2c_client *i2c)
 	return 0;
 }
 
-static const struct i2c_device_id max14577_i2c_id[] = {
-	{ "max14577", MAXIM_DEVICE_TYPE_MAX14577, },
-	{ "max77836", MAXIM_DEVICE_TYPE_MAX77836, },
-	{ }
-};
-MODULE_DEVICE_TABLE(i2c, max14577_i2c_id);
-
 #ifdef CONFIG_PM_SLEEP
 static int max14577_suspend(struct device *dev)
 {
@@ -538,12 +518,10 @@ static struct i2c_driver max14577_i2c_driver = {
 	},
 	.probe = max14577_i2c_probe,
 	.remove = max14577_i2c_remove,
-	.id_table = max14577_i2c_id,
 };
 
 static int __init max14577_i2c_init(void)
 {
-	BUILD_BUG_ON(ARRAY_SIZE(max14577_i2c_id) != MAXIM_DEVICE_TYPE_NUM);
 	BUILD_BUG_ON(ARRAY_SIZE(max14577_dt_match) != MAXIM_DEVICE_TYPE_NUM);
 
 	/* Valid charger current values must be provided for each chipset */
diff --git a/include/linux/mfd/max14577.h b/include/linux/mfd/max14577.h
index d81b52bb8bee..a97155662713 100644
--- a/include/linux/mfd/max14577.h
+++ b/include/linux/mfd/max14577.h
@@ -62,26 +62,6 @@ struct max14577_charger_platform_data {
 };
 
 /*
- * MAX14577 MFD platform data
- */
-struct max14577_platform_data {
-	/* IRQ */
-	int irq_base;
-
-	/* current control GPIOs */
-	int gpio_pogo_vbatt_en;
-	int gpio_pogo_vbus_en;
-
-	/* current control GPIO control function */
-	int (*set_gpio_pogo_vbatt_en) (int gpio_val);
-	int (*set_gpio_pogo_vbus_en) (int gpio_val);
-
-	int (*set_gpio_pogo_cb) (int new_dev);
-
-	struct max14577_regulator_platform_data *regulators;
-};
-
-/*
  * Valid limits of current for max14577 and max77836 chargers.
  * They must correspond to MBCICHWRCL and MBCICHWRCH fields in CHGCTRL4
  * register for given chipset.
-- 
2.9.3

Re: [RFT 0/4] mfd: regulator: max14577: Remove support for platform data

[Bartlomiej Zolnierkiewicz]

Hi,

On Friday, February 17, 2017 10:01:56 PM Krzysztof Kozlowski wrote:
> Hi,
> 
> Minor cleanup of max14577 family of drivers. The dependency inside:
> 1. Patch #3 and #4 depends for safeness on #1 so no one would try to
>    run the driver on non-DT platform (such platform does not exist anyway...).
> 2. Patch #4 depends on patch #3 (compile time dependency).
> 
> Unfortunately I do not have the boards using these drivers, so testing
> would be appreciated.

Your patchset looks fine to me but I've noticed that
maxim,max14577 bindings are not used currently in
the mainline kernel.

AFAIK MAX14577 support is needed only for Gear1 board
which doesn't have DTS file in the upstream kernel
(moreover Exynos4212 SoC support has been removed
recently).

It seems that MAX14577 support should also go away
(unless there are some other boards using it that we
support in the mainline kernel).

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

> Best regards,
> Krzysztof
> 
> Krzysztof Kozlowski (4):
>   mfd: max14577: Explicitly depend on OF
>   mfd: max14577: Export OF module alias information
>   regulator: max14577: Remove support for platform data
>   mfd: max14577: Remove platform data and instantiation from I2C
> 
>  drivers/mfd/Kconfig                    |  1 +
>  drivers/mfd/max14577.c                 | 23 +----------------------
>  drivers/regulator/max14577-regulator.c | 15 +++------------
>  include/linux/mfd/max14577.h           | 20 --------------------
>  4 files changed, 5 insertions(+), 54 deletions(-)

Re: [RFT 0/4] mfd: regulator: max14577: Remove support for platform data

[Krzysztof Kozlowski]

On Mon, Feb 27, 2017 at 2:55 PM, Bartlomiej Zolnierkiewicz
<b.zolnierkie@samsung.com> wrote:
>
> Hi,
>
> On Friday, February 17, 2017 10:01:56 PM Krzysztof Kozlowski wrote:
>> Hi,
>>
>> Minor cleanup of max14577 family of drivers. The dependency inside:
>> 1. Patch #3 and #4 depends for safeness on #1 so no one would try to
>>    run the driver on non-DT platform (such platform does not exist anyway...).
>> 2. Patch #4 depends on patch #3 (compile time dependency).
>>
>> Unfortunately I do not have the boards using these drivers, so testing
>> would be appreciated.
>
> Your patchset looks fine to me but I've noticed that
> maxim,max14577 bindings are not used currently in
> the mainline kernel.
>
> AFAIK MAX14577 support is needed only for Gear1 board
> which doesn't have DTS file in the upstream kernel
> (moreover Exynos4212 SoC support has been removed
> recently).
>
> It seems that MAX14577 support should also go away
> (unless there are some other boards using it that we
> support in the mainline kernel).

The driver supports two chipsets: max14577 and max77836. As you noted,
the first one is for Gear 1. The second one is present on Gear2 which
is supported by mainline. Simplifying the driver to support only
max77836 would be a task requiring testing. I think review would be in
such case not enough.

Best regards,
Krzysztof

Re: [RFT 0/4] mfd: regulator: max14577: Remove support for platform data

[Bartlomiej Zolnierkiewicz]

On Monday, February 27, 2017 05:32:37 PM Krzysztof Kozlowski wrote:
> On Mon, Feb 27, 2017 at 2:55 PM, Bartlomiej Zolnierkiewicz
> <b.zolnierkie@samsung.com> wrote:
> >
> > Hi,
> >
> > On Friday, February 17, 2017 10:01:56 PM Krzysztof Kozlowski wrote:
> >> Hi,
> >>
> >> Minor cleanup of max14577 family of drivers. The dependency inside:
> >> 1. Patch #3 and #4 depends for safeness on #1 so no one would try to
> >>    run the driver on non-DT platform (such platform does not exist anyway...).
> >> 2. Patch #4 depends on patch #3 (compile time dependency).
> >>
> >> Unfortunately I do not have the boards using these drivers, so testing
> >> would be appreciated.
> >
> > Your patchset looks fine to me but I've noticed that
> > maxim,max14577 bindings are not used currently in
> > the mainline kernel.
> >
> > AFAIK MAX14577 support is needed only for Gear1 board
> > which doesn't have DTS file in the upstream kernel
> > (moreover Exynos4212 SoC support has been removed
> > recently).
> >
> > It seems that MAX14577 support should also go away
> > (unless there are some other boards using it that we
> > support in the mainline kernel).
> 
> The driver supports two chipsets: max14577 and max77836. As you noted,
> the first one is for Gear 1. The second one is present on Gear2 which
> is supported by mainline. Simplifying the driver to support only
> max77836 would be a task requiring testing. I think review would be in
> such case not enough.

I think that this shouldn't be a big problem (if no one
else volunteers we can do the testing on Gear2 device).

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

Re: [RFT 0/4] mfd: regulator: max14577: Remove support for platform data

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 855 bytes --]

On Mon, Feb 27, 2017 at 04:52:36PM +0100, Bartlomiej Zolnierkiewicz wrote:
> On Monday, February 27, 2017 05:32:37 PM Krzysztof Kozlowski wrote:

> > The driver supports two chipsets: max14577 and max77836. As you noted,
> > the first one is for Gear 1. The second one is present on Gear2 which
> > is supported by mainline. Simplifying the driver to support only
> > max77836 would be a task requiring testing. I think review would be in
> > such case not enough.

> I think that this shouldn't be a big problem (if no one
> else volunteers we can do the testing on Gear2 device).

OTOH is it really gaining us much to remove the functionality vs the
effort for testing?  Removing the entire driver is fairly
straightforward but if it's just some extra device support that's
sitting there not bothering anyone perhaps it's not really a good use of
time.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [RFT 0/4] mfd: regulator: max14577: Remove support for platform data

[Bartlomiej Zolnierkiewicz]

Hi,

On Tuesday, February 28, 2017 09:47:40 AM Mark Brown wrote:
> On Mon, Feb 27, 2017 at 04:52:36PM +0100, Bartlomiej Zolnierkiewicz wrote:
> > On Monday, February 27, 2017 05:32:37 PM Krzysztof Kozlowski wrote:
> 
> > > The driver supports two chipsets: max14577 and max77836. As you noted,
> > > the first one is for Gear 1. The second one is present on Gear2 which
> > > is supported by mainline. Simplifying the driver to support only
> > > max77836 would be a task requiring testing. I think review would be in
> > > such case not enough.
> 
> > I think that this shouldn't be a big problem (if no one
> > else volunteers we can do the testing on Gear2 device).
> 
> OTOH is it really gaining us much to remove the functionality vs the
> effort for testing?  Removing the entire driver is fairly
> straightforward but if it's just some extra device support that's
> sitting there not bothering anyone perhaps it's not really a good use of
> time.

We have to test the device from time to time anyway just
to check that upstream support is still working so it
shouldn't be much extra effort.

Leaving the code as it is for now is also okay for me
(till it starts bothering somebody, i.e. people doing
automatic DT bindings coverage/validity checking).

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics