Re: [PATCH] HID: mcp2221: add ADC support

[Matt Ranostay <matt.ranostay@konsulko.com>]

On Fri, Nov 20, 2020 at 11:54 AM Lars-Peter Clausen <lars@metafoo.de> wrote:
>
> On 11/20/20 8:17 PM, Matt Ranostay wrote:
> > On Fri, Nov 20, 2020 at 5:15 AM rishi gupta <gupt21@gmail.com> wrote:
> >> On Fri, Nov 20, 2020 at 8:31 AM Matt Ranostay
> >> <matt.ranostay@konsulko.com> wrote:
> >>> Add support for the three 10-bit ADC channels registered via
> >>> the IIO subsystem.
> >>>
> >>> Cc: linux-input@vger.kernel.org
> >>> Cc: linux-iio@vger.kernel.org
> >>> CC: Rishi Gupta <gupt21@gmail.com>
> >>> Signed-off-by: Matt Ranostay <matt.ranostay@konsulko.com>
> >>> ---
> >>>   drivers/hid/Kconfig       |  1 +
> >>>   drivers/hid/hid-mcp2221.c | 65 ++++++++++++++++++++++++++++++++++++++-
> >>>   2 files changed, 65 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> >>> index 05315b434276..4795744d9979 100644
> >>> --- a/drivers/hid/Kconfig
> >>> +++ b/drivers/hid/Kconfig
> >>> @@ -1157,6 +1157,7 @@ config HID_MCP2221
> >>>          tristate "Microchip MCP2221 HID USB-to-I2C/SMbus host support"
> >>>          depends on USB_HID && I2C
> >>>          depends on GPIOLIB
> >>> +       depends on IIO
> >> I am wondering what will happen on systems which do not enable IIO.
> >> This driver can not be used there.
> >> Is my understanding correct?
> > Actually yeah this should be "select IIO" to avoid that issue.
>
> No, we should not have a individual driver select a framework. This will
> cause all kinds of issues with reverse dependencies.
>
> It might be worth splitting this driver into a MFD driver, then the MFD
> cells could have their own module that depend on the subsystem and if
> not enabled the functionality will not be provided.

Would it make sense to use IS_REACHABLE(CONFIG_IIO) for the iio blocks?

Guessing the weak reference "imply IIO" would still be bad for the
driver selecting a framework?

Thanks,

Matt

>
> - Lars
>