On Mon, 12 Sep 2022 10:32:02 -0700
Matt Ranostay <matt.ranostay@xxxxxxxxxxxx> wrote:
> Add support for 3x 10-bit ADC and 1x DAC channels registered via the iio
> subsystem.
>
> To prevent breakage and unexpected dependencies this support only is
> only built if CONFIG_IIO is enabled, and is only weakly referenced by
> 'imply IIO' within the respective Kconfig.
>
> Additionally the iio device only gets registered if at least one channel
> is enabled in the power-on configuration read from SRAM.
>
> Signed-off-by: Matt Ranostay <matt.ranostay@xxxxxxxxxxxx>
Hi Matt,
Can we not provide a _scale?
Whilst not technically required in all IIO Drivers, a bare _raw interface
is rarely that much use and here the ADC and DAC clearly have very different
scales.
Otherwise, as seen below, I'd like a comment on why the registration
is kicked off in a delayed work item. Right now that looks like a hack
to ensure something else has happened first. That's fine, but there
doesn't seem to be rescheduling if whatever that 'thing' is hasn't happened yet.
To use this sort of delayed trick, I'd definitely expect a backoff again
to be implemented...
> ---
> drivers/hid/Kconfig | 1 +
> drivers/hid/hid-mcp2221.c | 187 ++++++++++++++++++++++++++++++++++++++
> 2 files changed, 188 insertions(+)
...
> static int mcp2221_probe(struct hid_device *hdev,
> const struct hid_device_id *id)
> {
> @@ -902,6 +1084,11 @@ static int mcp2221_probe(struct hid_device *hdev,
> if (ret)
> goto err_i2c;
>
> +#if IS_REACHABLE(CONFIG_IIO)
> + INIT_DELAYED_WORK(&mcp->init_work, mcp_init_work);
> + schedule_delayed_work(&mcp->init_work, msecs_to_jiffies(500));
Good to have a comment here to say why you are kicking the registration of the
IIO device onto a delayed work path.
> +#endif
> +
> return 0;
>
> err_i2c:
