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