mfd: lm3533: Get rid of legacy GPIO APIs

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

Posted by Andy Shevchenko 1 year, 7 months ago

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

Posted by Lee Jones 1 year, 6 months ago

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

Posted by Andy Shevchenko 1 year, 6 months ago

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

Posted by Lee Jones 1 year, 6 months ago

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

Posted by Andy Shevchenko 1 year, 6 months ago

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

Posted by Lee Jones 1 year, 6 months ago

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

Posted by Johan Hovold 1 year, 6 months ago

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

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

Posted by Andy Shevchenko 1 year, 6 months ago

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

Posted by Linus Walleij 1 year, 6 months ago

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