diff mbox series

[v6,2/3] HID: mcp2221: change 'select GPIOLIB' to imply

Message ID 20221001005208.8010-3-matt.ranostay@konsulko.com
State Accepted
Commit ea418b35103a98329cb019927cc3668c3759b9eb
Headers show
Series HID: mcp2221: iio support and device resource management | expand

Commit Message

Matt Ranostay Oct. 1, 2022, 12:52 a.m. UTC
To avoid recursive dependencies on GPIOLIB when 'imply IIO' is requested
with other drivers we should switch GPIOLIB to an imply.

This isn't the most ideal solution but avoids modifiying the Kconfig for
other drivers, and only requires a singular IS_REACHABLE(CONFIG_GPIOLIB)
check.

Signed-off-by: Matt Ranostay <matt.ranostay@konsulko.com>
---
 drivers/hid/Kconfig       | 2 +-
 drivers/hid/hid-mcp2221.c | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

Comments

Benjamin Tissoires Oct. 21, 2022, 11:50 a.m. UTC | #1
On Sat, Oct 1, 2022 at 2:52 AM Matt Ranostay <matt.ranostay@konsulko.com> wrote:
>
> To avoid recursive dependencies on GPIOLIB when 'imply IIO' is requested
> with other drivers we should switch GPIOLIB to an imply.
>
> This isn't the most ideal solution but avoids modifiying the Kconfig for
> other drivers, and only requires a singular IS_REACHABLE(CONFIG_GPIOLIB)
> check.
>
> Signed-off-by: Matt Ranostay <matt.ranostay@konsulko.com>
> ---
>  drivers/hid/Kconfig       | 2 +-
>  drivers/hid/hid-mcp2221.c | 2 ++
>  2 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> index 185a077d59cd..745fc38794ad 100644
> --- a/drivers/hid/Kconfig
> +++ b/drivers/hid/Kconfig
> @@ -1252,7 +1252,7 @@ config HID_ALPS
>  config HID_MCP2221
>         tristate "Microchip MCP2221 HID USB-to-I2C/SMbus host support"
>         depends on USB_HID && I2C
> -       depends on GPIOLIB
> +       imply GPIOLIB
>         help
>         Provides I2C and SMBUS host adapter functionality over USB-HID
>         through MCP2221 device.
> diff --git a/drivers/hid/hid-mcp2221.c b/drivers/hid/hid-mcp2221.c
> index 4d10a24e3e13..fb54f1c6fd9c 100644
> --- a/drivers/hid/hid-mcp2221.c
> +++ b/drivers/hid/hid-mcp2221.c
> @@ -915,9 +915,11 @@ static int mcp2221_probe(struct hid_device *hdev,
>         mcp->gc->can_sleep = 1;
>         mcp->gc->parent = &hdev->dev;
>
> +#if IS_REACHABLE(CONFIG_GPIOLIB)
>         ret = devm_gpiochip_add_data(&hdev->dev, mcp->gc, mcp);
>         if (ret)
>                 return ret;
> +#endif

Hi Matt,

This patch actually breaks my CI because devm_gpiochip_add_data() is
not the only one function that should be protected against
CONFIG_GPIOLIB.

I am getting:
---
ERROR: modpost: "gpiochip_get_data" [drivers/hid/hid-mcp2221.ko] undefined!
---

Can you also protect gpiochip_get_data() and make sure that the driver
is not completely buggy after? I assume a simple #if around all of the
calls will be worse than the current non compiling situation.

Cheers,
Benjamin

>
>         return 0;
>  }
> --
> 2.37.2
>
Jiri Kosina Oct. 21, 2022, 12:12 p.m. UTC | #2
On Fri, 21 Oct 2022, Benjamin Tissoires wrote:

> > To avoid recursive dependencies on GPIOLIB when 'imply IIO' is requested
> > with other drivers we should switch GPIOLIB to an imply.
> >
> > This isn't the most ideal solution but avoids modifiying the Kconfig for
> > other drivers, and only requires a singular IS_REACHABLE(CONFIG_GPIOLIB)
> > check.
> >
> > Signed-off-by: Matt Ranostay <matt.ranostay@konsulko.com>
> > ---
> >  drivers/hid/Kconfig       | 2 +-
> >  drivers/hid/hid-mcp2221.c | 2 ++
> >  2 files changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> > index 185a077d59cd..745fc38794ad 100644
> > --- a/drivers/hid/Kconfig
> > +++ b/drivers/hid/Kconfig
> > @@ -1252,7 +1252,7 @@ config HID_ALPS
> >  config HID_MCP2221
> >         tristate "Microchip MCP2221 HID USB-to-I2C/SMbus host support"
> >         depends on USB_HID && I2C
> > -       depends on GPIOLIB
> > +       imply GPIOLIB
> >         help
> >         Provides I2C and SMBUS host adapter functionality over USB-HID
> >         through MCP2221 device.
> > diff --git a/drivers/hid/hid-mcp2221.c b/drivers/hid/hid-mcp2221.c
> > index 4d10a24e3e13..fb54f1c6fd9c 100644
> > --- a/drivers/hid/hid-mcp2221.c
> > +++ b/drivers/hid/hid-mcp2221.c
> > @@ -915,9 +915,11 @@ static int mcp2221_probe(struct hid_device *hdev,
> >         mcp->gc->can_sleep = 1;
> >         mcp->gc->parent = &hdev->dev;
> >
> > +#if IS_REACHABLE(CONFIG_GPIOLIB)
> >         ret = devm_gpiochip_add_data(&hdev->dev, mcp->gc, mcp);
> >         if (ret)
> >                 return ret;
> > +#endif
> 
> Hi Matt,
> 
> This patch actually breaks my CI because devm_gpiochip_add_data() is
> not the only one function that should be protected against
> CONFIG_GPIOLIB.
> 
> I am getting:
> ---
> ERROR: modpost: "gpiochip_get_data" [drivers/hid/hid-mcp2221.ko] undefined!
> ---
> 
> Can you also protect gpiochip_get_data() and make sure that the driver
> is not completely buggy after? I assume a simple #if around all of the
> calls will be worse than the current non compiling situation.

Benjamin,

this should be fixed in hid.git via 3d74c9eca1a2bda. If you still see 
issues with that applied, please speak up :)

Thanks,
Benjamin Tissoires Oct. 21, 2022, 12:29 p.m. UTC | #3
On Fri, Oct 21, 2022 at 2:13 PM Jiri Kosina <jikos@kernel.org> wrote:
>
> On Fri, 21 Oct 2022, Benjamin Tissoires wrote:
>
> > > To avoid recursive dependencies on GPIOLIB when 'imply IIO' is requested
> > > with other drivers we should switch GPIOLIB to an imply.
> > >
> > > This isn't the most ideal solution but avoids modifiying the Kconfig for
> > > other drivers, and only requires a singular IS_REACHABLE(CONFIG_GPIOLIB)
> > > check.
> > >
> > > Signed-off-by: Matt Ranostay <matt.ranostay@konsulko.com>
> > > ---
> > >  drivers/hid/Kconfig       | 2 +-
> > >  drivers/hid/hid-mcp2221.c | 2 ++
> > >  2 files changed, 3 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> > > index 185a077d59cd..745fc38794ad 100644
> > > --- a/drivers/hid/Kconfig
> > > +++ b/drivers/hid/Kconfig
> > > @@ -1252,7 +1252,7 @@ config HID_ALPS
> > >  config HID_MCP2221
> > >         tristate "Microchip MCP2221 HID USB-to-I2C/SMbus host support"
> > >         depends on USB_HID && I2C
> > > -       depends on GPIOLIB
> > > +       imply GPIOLIB
> > >         help
> > >         Provides I2C and SMBUS host adapter functionality over USB-HID
> > >         through MCP2221 device.
> > > diff --git a/drivers/hid/hid-mcp2221.c b/drivers/hid/hid-mcp2221.c
> > > index 4d10a24e3e13..fb54f1c6fd9c 100644
> > > --- a/drivers/hid/hid-mcp2221.c
> > > +++ b/drivers/hid/hid-mcp2221.c
> > > @@ -915,9 +915,11 @@ static int mcp2221_probe(struct hid_device *hdev,
> > >         mcp->gc->can_sleep = 1;
> > >         mcp->gc->parent = &hdev->dev;
> > >
> > > +#if IS_REACHABLE(CONFIG_GPIOLIB)
> > >         ret = devm_gpiochip_add_data(&hdev->dev, mcp->gc, mcp);
> > >         if (ret)
> > >                 return ret;
> > > +#endif
> >
> > Hi Matt,
> >
> > This patch actually breaks my CI because devm_gpiochip_add_data() is
> > not the only one function that should be protected against
> > CONFIG_GPIOLIB.
> >
> > I am getting:
> > ---
> > ERROR: modpost: "gpiochip_get_data" [drivers/hid/hid-mcp2221.ko] undefined!
> > ---
> >
> > Can you also protect gpiochip_get_data() and make sure that the driver
> > is not completely buggy after? I assume a simple #if around all of the
> > calls will be worse than the current non compiling situation.
>
> Benjamin,
>
> this should be fixed in hid.git via 3d74c9eca1a2bda. If you still see
> issues with that applied, please speak up :)
>

Oh, I wasn't Cc-ed on that series, and my bot still hasn't updated my CI branch.

Thanks for the fix Matt :)

Cheers,
Benjamin
diff mbox series

Patch

diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 185a077d59cd..745fc38794ad 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -1252,7 +1252,7 @@  config HID_ALPS
 config HID_MCP2221
 	tristate "Microchip MCP2221 HID USB-to-I2C/SMbus host support"
 	depends on USB_HID && I2C
-	depends on GPIOLIB
+	imply GPIOLIB
 	help
 	Provides I2C and SMBUS host adapter functionality over USB-HID
 	through MCP2221 device.
diff --git a/drivers/hid/hid-mcp2221.c b/drivers/hid/hid-mcp2221.c
index 4d10a24e3e13..fb54f1c6fd9c 100644
--- a/drivers/hid/hid-mcp2221.c
+++ b/drivers/hid/hid-mcp2221.c
@@ -915,9 +915,11 @@  static int mcp2221_probe(struct hid_device *hdev,
 	mcp->gc->can_sleep = 1;
 	mcp->gc->parent = &hdev->dev;
 
+#if IS_REACHABLE(CONFIG_GPIOLIB)
 	ret = devm_gpiochip_add_data(&hdev->dev, mcp->gc, mcp);
 	if (ret)
 		return ret;
+#endif
 
 	return 0;
 }