[PATCH] media: em28xx: Drop abuse of gpiolib

[PATCH] media: em28xx: Drop abuse of gpiolib

[Linus Walleij]

The driver is issueing calls to the legacy gpio API from
<linux/gpio.h> to pull a LNA gpio line low or high.

The code as it stands can not work and does not make sense
since the GPIO number assigned to dvb->lna_gpio is only
in scope in this file and never assigned any valid GPIO
number, the driver has no way of asking for a proper GPIO
and will likely ask for GPIO 0, which will likely be wrong.

In one execution path dvb->lna_gpio is assigned some constants
to the local GPIO block which is not using gpiolib, adding
to the confusion.

Delete all use of gpiolib as it can't work. Leave the custom
(local) gpio handling around, as this is likely the only thing
that can actually work.

My guess is that this driver only worked on platforms that
for some reason does not enable CONFIG_GPIOLIB. It was likely
causing a bug on any platform enabling CONFIG_GPIOLIB.

If anyone knows how to fix this driver properly then tell
me.

Cc: linux-gpio@vger.kernel.org
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/media/usb/em28xx/em28xx-dvb.c | 32 ---------------------------
 1 file changed, 32 deletions(-)

diff --git a/drivers/media/usb/em28xx/em28xx-dvb.c b/drivers/media/usb/em28xx/em28xx-dvb.c
index 9fce59979e3b..57598e825135 100644
--- a/drivers/media/usb/em28xx/em28xx-dvb.c
+++ b/drivers/media/usb/em28xx/em28xx-dvb.c
@@ -29,7 +29,6 @@
 #include <media/dmxdev.h>
 #include <media/tuner.h>
 #include "tuner-simple.h"
-#include <linux/gpio.h>
 
 #include "lgdt330x.h"
 #include "lgdt3305.h"
@@ -727,28 +726,10 @@ static int em28xx_pctv_290e_set_lna(struct dvb_frontend *fe)
 	struct dtv_frontend_properties *c = &fe->dtv_property_cache;
 	struct em28xx_i2c_bus *i2c_bus = fe->dvb->priv;
 	struct em28xx *dev = i2c_bus->dev;
-#ifdef CONFIG_GPIOLIB
-	struct em28xx_dvb *dvb = dev->dvb;
-	int ret;
-	unsigned long flags;
-
-	if (c->lna == 1)
-		flags = GPIOF_OUT_INIT_HIGH; /* enable LNA */
-	else
-		flags = GPIOF_OUT_INIT_LOW; /* disable LNA */
 
-	ret = gpio_request_one(dvb->lna_gpio, flags, NULL);
-	if (ret)
-		dev_err(&dev->intf->dev, "gpio request failed %d\n", ret);
-	else
-		gpio_free(dvb->lna_gpio);
-
-	return ret;
-#else
 	dev_warn(&dev->intf->dev, "%s: LNA control is disabled (lna=%u)\n",
 		 KBUILD_MODNAME, c->lna);
 	return 0;
-#endif
 }
 
 static int em28xx_pctv_292e_set_lna(struct dvb_frontend *fe)
@@ -1705,19 +1686,6 @@ static int em28xx_dvb_init(struct em28xx *dev)
 				goto out_free;
 			}
 
-#ifdef CONFIG_GPIOLIB
-			/* enable LNA for DVB-T, DVB-T2 and DVB-C */
-			result = gpio_request_one(dvb->lna_gpio,
-						  GPIOF_OUT_INIT_LOW, NULL);
-			if (result)
-				dev_err(&dev->intf->dev,
-					"gpio request failed %d\n",
-					result);
-			else
-				gpio_free(dvb->lna_gpio);
-
-			result = 0; /* continue even set LNA fails */
-#endif
 			dvb->fe[0]->ops.set_lna = em28xx_pctv_290e_set_lna;
 		}
 
-- 
2.34.1

Re: [PATCH] media: em28xx: Drop abuse of gpiolib

[Dmitry Torokhov]

Hi Linus,

On Wed, Jan 11, 2023 at 02:58:01PM +0100, Linus Walleij wrote:
> The driver is issueing calls to the legacy gpio API from
> <linux/gpio.h> to pull a LNA gpio line low or high.
> 
> The code as it stands can not work and does not make sense
> since the GPIO number assigned to dvb->lna_gpio is only
> in scope in this file and never assigned any valid GPIO
> number, the driver has no way of asking for a proper GPIO
> and will likely ask for GPIO 0, which will likely be wrong.
> 
> In one execution path dvb->lna_gpio is assigned some constants
> to the local GPIO block which is not using gpiolib, adding
> to the confusion.

The dvb->lna_gpio gets reassigned in the call to

	dvb->fe[0] = dvb_attach(cxd2820r_attach, ...);

which calls cxd2820r_attach() which ends up calling cxd2820r_probe()
which creates a gpiochip and passes back the first gpio number. It all
seems very fragile and will break if dependencies are not linked "just
right" and I have no idea if this all still actually works...

Thanks.

-- 
Dmitry