[PATCH v1 0/2] mfd: lm3533: Get rid of legacy GPIO APIs

[PATCH v1 0/2] mfd: lm3533: Get rid of legacy GPIO APIs

[Andy Shevchenko]

Driver is quite outdated from Linux kernel internal APIs perspective.
Update the GPIO part in this miniseries.

The first patch to make sure noone is going to use legacy platform data.
An alternative approach is to remove completely these family of drivers.

Andy Shevchenko (2):
  mfd: lm3533: Hide legacy platform data in the driver
  mfd: lm3533: Move to new GPIO descriptor-based APIs

 drivers/mfd/lm3533-core.c  | 45 +++++++++++++++++++-------------------
 include/linux/mfd/lm3533.h | 18 ++-------------
 2 files changed, 25 insertions(+), 38 deletions(-)

-- 
2.43.0.rc1.1336.g36b5255a03ac

[PATCH v1 1/2] mfd: lm3533: Hide legacy platform data in the driver

[Andy Shevchenko]

First of all, there is no user for the platform data in the kernel.
Second, it needs a lot of updates to follow the modern standards
of the kernel, including proper Device Tree bindings and device
property handling.

For now, just hide the legacy platform data in the driver's code.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/mfd/lm3533-core.c  | 23 ++++++++++++++++++-----
 include/linux/mfd/lm3533.h | 15 ---------------
 2 files changed, 18 insertions(+), 20 deletions(-)

diff --git a/drivers/mfd/lm3533-core.c b/drivers/mfd/lm3533-core.c
index c211183cecb2..515a6c3b3244 100644
--- a/drivers/mfd/lm3533-core.c
+++ b/drivers/mfd/lm3533-core.c
@@ -21,6 +21,20 @@
 
 #include <linux/mfd/lm3533.h>
 
+struct lm3533_platform_data {
+	int gpio_hwen;
+
+	enum lm3533_boost_ovp boost_ovp;
+	enum lm3533_boost_freq boost_freq;
+
+	struct lm3533_als_platform_data *als;
+
+	struct lm3533_bl_platform_data *backlights;
+	int num_backlights;
+
+	struct lm3533_led_platform_data *leds;
+	int num_leds;
+};
 
 #define LM3533_BOOST_OVP_MASK		0x06
 #define LM3533_BOOST_OVP_SHIFT		1
@@ -473,15 +487,14 @@ static int lm3533_device_setup(struct lm3533 *lm3533,
 
 static int lm3533_device_init(struct lm3533 *lm3533)
 {
-	struct lm3533_platform_data *pdata = dev_get_platdata(lm3533->dev);
+	struct lm3533_platform_data *pdata;
 	int ret;
 
 	dev_dbg(lm3533->dev, "%s\n", __func__);
 
-	if (!pdata) {
-		dev_err(lm3533->dev, "no platform data\n");
-		return -EINVAL;
-	}
+	pdata = devm_kzalloc(lm3533->dev, sizeof(*pdata), GFP_KERNEL);
+	if (!pdata)
+		return -ENOMEM;
 
 	lm3533->gpio_hwen = pdata->gpio_hwen;
 
diff --git a/include/linux/mfd/lm3533.h b/include/linux/mfd/lm3533.h
index 77092f6363ad..ce42f0737768 100644
--- a/include/linux/mfd/lm3533.h
+++ b/include/linux/mfd/lm3533.h
@@ -68,21 +68,6 @@ enum lm3533_boost_ovp {
 	LM3533_BOOST_OVP_40V,
 };
 
-struct lm3533_platform_data {
-	int gpio_hwen;
-
-	enum lm3533_boost_ovp boost_ovp;
-	enum lm3533_boost_freq boost_freq;
-
-	struct lm3533_als_platform_data *als;
-
-	struct lm3533_bl_platform_data *backlights;
-	int num_backlights;
-
-	struct lm3533_led_platform_data *leds;
-	int num_leds;
-};
-
 extern int lm3533_ctrlbank_enable(struct lm3533_ctrlbank *cb);
 extern int lm3533_ctrlbank_disable(struct lm3533_ctrlbank *cb);
 
-- 
2.43.0.rc1.1336.g36b5255a03ac

Re: [PATCH v1 1/2] mfd: lm3533: Hide legacy platform data in the driver

[Linus Walleij]

On Wed, May 8, 2024 at 12:48 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:

> First of all, there is no user for the platform data in the kernel.
> Second, it needs a lot of updates to follow the modern standards
> of the kernel, including proper Device Tree bindings and device
> property handling.
>
> For now, just hide the legacy platform data in the driver's code.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

Re: [PATCH v1 1/2] mfd: lm3533: Hide legacy platform data in the driver

[Lee Jones]

On Wed, 08 May 2024, Andy Shevchenko wrote:

> First of all, there is no user for the platform data in the kernel.
> Second, it needs a lot of updates to follow the modern standards
> of the kernel, including proper Device Tree bindings and device
> property handling.
> 
> For now, just hide the legacy platform data in the driver's code.

Why not just rip it out entirely?

-- 
Lee Jones [李琼斯]

Re: [PATCH v1 1/2] mfd: lm3533: Hide legacy platform data in the driver

[Andy Shevchenko]

On Fri, May 31, 2024 at 04:00:48PM +0100, Lee Jones wrote:
> On Wed, 08 May 2024, Andy Shevchenko wrote:
> 
> > First of all, there is no user for the platform data in the kernel.
> > Second, it needs a lot of updates to follow the modern standards
> > of the kernel, including proper Device Tree bindings and device
> > property handling.
> > 
> > For now, just hide the legacy platform data in the driver's code.
> 
> Why not just rip it out entirely?

You mean the driver?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 1/2] mfd: lm3533: Hide legacy platform data in the driver

[Lee Jones]

On Fri, 31 May 2024, Andy Shevchenko wrote:

> On Fri, May 31, 2024 at 04:00:48PM +0100, Lee Jones wrote:
> > On Wed, 08 May 2024, Andy Shevchenko wrote:
> > 
> > > First of all, there is no user for the platform data in the kernel.
> > > Second, it needs a lot of updates to follow the modern standards
> > > of the kernel, including proper Device Tree bindings and device
> > > property handling.
> > > 
> > > For now, just hide the legacy platform data in the driver's code.
> > 
> > Why not just rip it out entirely?
> 
> You mean the driver?

The unused platform data.

-- 
Lee Jones [李琼斯]

Re: [PATCH v1 1/2] mfd: lm3533: Hide legacy platform data in the driver

[Andy Shevchenko]

On Fri, May 31, 2024 at 04:54:45PM +0100, Lee Jones wrote:
> On Fri, 31 May 2024, Andy Shevchenko wrote:
> > On Fri, May 31, 2024 at 04:00:48PM +0100, Lee Jones wrote:
> > > On Wed, 08 May 2024, Andy Shevchenko wrote:
> > > 
> > > > First of all, there is no user for the platform data in the kernel.
> > > > Second, it needs a lot of updates to follow the modern standards
> > > > of the kernel, including proper Device Tree bindings and device
> > > > property handling.
> > > > 
> > > > For now, just hide the legacy platform data in the driver's code.
> > > 
> > > Why not just rip it out entirely?
> > 
> > You mean the driver?
> 
> The unused platform data.

Good question. In any case these drivers are non-functional anyway without OOT
board code. If we rip out the main platform data completely, the logical following
question arises: why do we need the per-device platform data? If we rip that out,
we basically make non-functional driver a 100% dead code. Hence what you propose
mostly equals to ripping out the drivers completely.

TL;DR: with the main platform data being ripped out the driver code will be in
inconsistent state.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 1/2] mfd: lm3533: Hide legacy platform data in the driver

[Lee Jones]

On Fri, 31 May 2024, Andy Shevchenko wrote:

> On Fri, May 31, 2024 at 04:54:45PM +0100, Lee Jones wrote:
> > On Fri, 31 May 2024, Andy Shevchenko wrote:
> > > On Fri, May 31, 2024 at 04:00:48PM +0100, Lee Jones wrote:
> > > > On Wed, 08 May 2024, Andy Shevchenko wrote:
> > > > 
> > > > > First of all, there is no user for the platform data in the kernel.
> > > > > Second, it needs a lot of updates to follow the modern standards
> > > > > of the kernel, including proper Device Tree bindings and device
> > > > > property handling.
> > > > > 
> > > > > For now, just hide the legacy platform data in the driver's code.
> > > > 
> > > > Why not just rip it out entirely?
> > > 
> > > You mean the driver?
> > 
> > The unused platform data.
> 
> Good question. In any case these drivers are non-functional anyway without OOT
> board code. If we rip out the main platform data completely, the logical following
> question arises: why do we need the per-device platform data? If we rip that out,
> we basically make non-functional driver a 100% dead code. Hence what you propose
> mostly equals to ripping out the drivers completely.
> 
> TL;DR: with the main platform data being ripped out the driver code will be in
> inconsistent state.

What do you think Johan?  Do you see any reason to keep it around?

-- 
Lee Jones [李琼斯]

Re: [PATCH v1 1/2] mfd: lm3533: Hide legacy platform data in the driver

[Andy Shevchenko]

On Fri, May 31, 2024 at 05:58:34PM +0100, Lee Jones wrote:
> On Fri, 31 May 2024, Andy Shevchenko wrote:
> 
> > On Fri, May 31, 2024 at 04:54:45PM +0100, Lee Jones wrote:
> > > On Fri, 31 May 2024, Andy Shevchenko wrote:
> > > > On Fri, May 31, 2024 at 04:00:48PM +0100, Lee Jones wrote:
> > > > > On Wed, 08 May 2024, Andy Shevchenko wrote:
> > > > > 
> > > > > > First of all, there is no user for the platform data in the kernel.
> > > > > > Second, it needs a lot of updates to follow the modern standards
> > > > > > of the kernel, including proper Device Tree bindings and device
> > > > > > property handling.
> > > > > > 
> > > > > > For now, just hide the legacy platform data in the driver's code.
> > > > > 
> > > > > Why not just rip it out entirely?
> > > > 
> > > > You mean the driver?
> > > 
> > > The unused platform data.
> > 
> > Good question. In any case these drivers are non-functional anyway without OOT
> > board code. If we rip out the main platform data completely, the logical following
> > question arises: why do we need the per-device platform data? If we rip that out,
> > we basically make non-functional driver a 100% dead code. Hence what you propose
> > mostly equals to ripping out the drivers completely.
> > 
> > TL;DR: with the main platform data being ripped out the driver code will be in
> > inconsistent state.
> 
> What do you think Johan?  Do you see any reason to keep it around?

FWIW, I just have sent a removal. My main objective here is to get rid of
legacy GPIO APIs. Other than that I don't care if driver will stay or go.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 1/2] mfd: lm3533: Hide legacy platform data in the driver

[Johan Hovold]

On Fri, May 31, 2024 at 05:58:34PM +0100, Lee Jones wrote:
> On Fri, 31 May 2024, Andy Shevchenko wrote:
> 
> > On Fri, May 31, 2024 at 04:54:45PM +0100, Lee Jones wrote:
> > > On Fri, 31 May 2024, Andy Shevchenko wrote:
> > > > On Fri, May 31, 2024 at 04:00:48PM +0100, Lee Jones wrote:
> > > > > On Wed, 08 May 2024, Andy Shevchenko wrote:
> > > > > 
> > > > > > First of all, there is no user for the platform data in the kernel.
> > > > > > Second, it needs a lot of updates to follow the modern standards
> > > > > > of the kernel, including proper Device Tree bindings and device
> > > > > > property handling.
> > > > > > 
> > > > > > For now, just hide the legacy platform data in the driver's code.
> > > > > 
> > > > > Why not just rip it out entirely?
> > > > 
> > > > You mean the driver?
> > > 
> > > The unused platform data.
> > 
> > Good question. In any case these drivers are non-functional anyway without OOT
> > board code. If we rip out the main platform data completely, the logical following
> > question arises: why do we need the per-device platform data? If we rip that out,
> > we basically make non-functional driver a 100% dead code. Hence what you propose
> > mostly equals to ripping out the drivers completely.
> > 
> > TL;DR: with the main platform data being ripped out the driver code will be in
> > inconsistent state.
> 
> What do you think Johan?  Do you see any reason to keep it around?

Yeah, I'd prefer to keep it around. This device is used in a bunch of
Sony phones and Bjorn A posted a series adding devicetree bindings a few
years ago which I believe was more or less acked and ready go.

I'll try to find some time to look at that myself as I think I may
favour a less verbose binding (e.g. similar to pm8008 that I'm working
on).

For now I suggest keeping the platform data where it is and just convert
the single gpio lookup to look for a "hwen" gpio that can be provided by
lookup tables and soon devicetree.

Johan

[PATCH v1 2/2] mfd: lm3533: Move to new GPIO descriptor-based APIs

[Andy Shevchenko]

Legacy GPIO APIs are subject to remove. Convert the driver to new APIs.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/mfd/lm3533-core.c  | 26 +++++++-------------------
 include/linux/mfd/lm3533.h |  3 ++-
 2 files changed, 9 insertions(+), 20 deletions(-)

diff --git a/drivers/mfd/lm3533-core.c b/drivers/mfd/lm3533-core.c
index 515a6c3b3244..e135142a158d 100644
--- a/drivers/mfd/lm3533-core.c
+++ b/drivers/mfd/lm3533-core.c
@@ -11,7 +11,7 @@
 #include <linux/init.h>
 #include <linux/kernel.h>
 #include <linux/err.h>
-#include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
 #include <linux/i2c.h>
 #include <linux/mfd/core.h>
 #include <linux/regmap.h>
@@ -22,8 +22,6 @@
 #include <linux/mfd/lm3533.h>
 
 struct lm3533_platform_data {
-	int gpio_hwen;
-
 	enum lm3533_boost_ovp boost_ovp;
 	enum lm3533_boost_freq boost_freq;
 
@@ -239,14 +237,12 @@ static int lm3533_set_lvled_config(struct lm3533 *lm3533, u8 lvled, u8 led)
 
 static void lm3533_enable(struct lm3533 *lm3533)
 {
-	if (gpio_is_valid(lm3533->gpio_hwen))
-		gpio_set_value(lm3533->gpio_hwen, 1);
+	gpiod_set_value(lm3533->hwen, 1);
 }
 
 static void lm3533_disable(struct lm3533 *lm3533)
 {
-	if (gpio_is_valid(lm3533->gpio_hwen))
-		gpio_set_value(lm3533->gpio_hwen, 0);
+	gpiod_set_value(lm3533->hwen, 0);
 }
 
 enum lm3533_attribute_type {
@@ -496,18 +492,10 @@ static int lm3533_device_init(struct lm3533 *lm3533)
 	if (!pdata)
 		return -ENOMEM;
 
-	lm3533->gpio_hwen = pdata->gpio_hwen;
-
-	if (gpio_is_valid(lm3533->gpio_hwen)) {
-		ret = devm_gpio_request_one(lm3533->dev, lm3533->gpio_hwen,
-					GPIOF_OUT_INIT_LOW, "lm3533-hwen");
-		if (ret < 0) {
-			dev_err(lm3533->dev,
-				"failed to request HWEN GPIO %d\n",
-				lm3533->gpio_hwen);
-			return ret;
-		}
-	}
+	lm3533->hwen = devm_gpiod_get(lm3533->dev, NULL, GPIOD_OUT_LOW);
+	if (IS_ERR(lm3533->hwen))
+		return dev_err_probe(lm3533->dev, PTR_ERR(lm3533->hwen), "failed to request HWEN GPIO\n");
+	gpiod_set_consumer_name(lm3533->hwen, "lm3533-hwen");
 
 	lm3533_enable(lm3533);
 
diff --git a/include/linux/mfd/lm3533.h b/include/linux/mfd/lm3533.h
index ce42f0737768..d30c70c2a5f6 100644
--- a/include/linux/mfd/lm3533.h
+++ b/include/linux/mfd/lm3533.h
@@ -16,6 +16,7 @@
 	DEVICE_ATTR(_name, S_IRUGO | S_IWUSR , show_##_name, store_##_name)
 
 struct device;
+struct gpio_desc;
 struct regmap;
 
 struct lm3533 {
@@ -23,7 +24,7 @@ struct lm3533 {
 
 	struct regmap *regmap;
 
-	int gpio_hwen;
+	struct gpio_desc *hwen;
 	int irq;
 
 	unsigned have_als:1;
-- 
2.43.0.rc1.1336.g36b5255a03ac

Re: [PATCH v1 2/2] mfd: lm3533: Move to new GPIO descriptor-based APIs

[Linus Walleij]

On Wed, May 8, 2024 at 12:48 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:

> Legacy GPIO APIs are subject to remove. Convert the driver to new APIs.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Apparently only out-of-tree code uses this driver, since no board files
need to be changed. They need to adapt. Perhaps the driver should even
be deleted, but that is for another day.
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij