Hi Bartosz,
On Thu, Dec 29, 2022 at 5:12 PM Bartosz Golaszewski <brgl@xxxxxxxx> wrote:
> From: Bartosz Golaszewski <bartosz.golaszewski@xxxxxxxxxx>
>
> We have a set of return values that notifier callbacks can return. They
> should not return 0, error codes or anything other than those predefined
> values. Make the i2c character device's callback return NOTIFY_DONE or
> NOTIFY_OK depending on the situation.
>
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@xxxxxxxxxx>
Thanks for your patch, which is now commit cddf70d0bce71c2a ("i2c:
dev: fix notifier return values") in v6.3-rc1.
On SH/R-Mobile platforms, this leads to missing /dev/i2c-* entries.
On R-Car Gen4, they are still present, as all I2C adapters are
initialized after i2cdev.
> --- a/drivers/i2c/i2c-dev.c
> +++ b/drivers/i2c/i2c-dev.c
> @@ -653,12 +653,12 @@ static int i2cdev_attach_adapter(struct device *dev, void *dummy)
> int res;
>
> if (dev->type != &i2c_adapter_type)
> - return 0;
> + return NOTIFY_DONE;
> adap = to_i2c_adapter(dev);
>
> i2c_dev = get_free_i2c_dev(adap);
> if (IS_ERR(i2c_dev))
> - return PTR_ERR(i2c_dev);
> + return NOTIFY_DONE;
>
> cdev_init(&i2c_dev->cdev, &i2cdev_fops);
> i2c_dev->cdev.owner = THIS_MODULE;
> @@ -678,11 +678,11 @@ static int i2cdev_attach_adapter(struct device *dev, void *dummy)
> goto err_put_i2c_dev;
>
> pr_debug("adapter [%s] registered as minor %d\n", adap->name, adap->nr);
> - return 0;
> + return NOTIFY_OK;
Unfortunately i2cdev_{at,de}tach_adapter() are not only used as
notifiers (called from i2cdev_notifier_call()), but also called from
i2c_dev_init():
/* Bind to already existing adapters right away */
i2c_for_each_dev(NULL, i2cdev_attach_adapter);
and i2c_dev_exit():
i2c_for_each_dev(NULL, i2cdev_detach_adapter);
As soon i2c_dev_{at,de}tach_adapter() returns a non-zero
value (e.g. NOTIFY_OK), {i2c,bus}_for_each_dev() aborts
processing.
In i2c_dev_init(), this leads to a failure in registering any
already existing i2c adapters after the first one, causing missing
/dev/i2c-* entries.
In i2c_dev_exit(), this leads to a failure unregistering any but the
first i2c adapter.
As there is no one-to-one mapping from error codes to notify codes,
I think this cannot just be handled inside i2cdev_notifier_call() :-(
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
