On Wed, Mar 8, 2023 at 5:58 PM Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> wrote:
>
> 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() :-(
>
Would wrapping i2c_a/detach_adapter() in a notifier callback work? So
that SH can call it directly while notifiers would call it indirectly
through the wrapper?
Bart
