On Tue, Nov 27, 2018 at 05:14:06PM +0100, Hans de Goede wrote: > On 27-11-18 16:37, Andy Shevchenko wrote: > > The caller would like to know the reason why the i2c_acpi_new_device() fails. > > For example, if adapter is not available, it might be in the future and we > > would like to re-probe the clients again. But at the same time we would like to > > bail out if the error seems unrecoverable, such as invalid argument supplied. > > To achieve this, return error pointer in some cases. > > acpi_dev_free_resource_list(&resource_list); > > - if (ret < 0 || !info->addr) > > - return NULL; > > + if (!info->addr) > > + return ERR_PTR(-EADDRNOTAVAIL); > > adapter = i2c_acpi_find_adapter_by_handle(lookup.adapter_handle); > > if (!adapter) > > - return NULL; > > + return ERR_PTR(-ENODEV); > Why not simply return -EPROBE_DEFER here (and simplify the callers a lot). > This is the only case where we really want to defer. > > + client = i2c_new_device(adapter, info); > > + if (!client) > > + return ERR_PTR(-ENODEV); > > If you look at i2c_new_device, it can fail because it is > out of memory, the i2c slave address is invalid, or > their already is an i2c slave with the same address, > iow if this were to return an ERR_PTR itself, this > would return -ENOMEM, -EINVAL or -EBUSY and never > -EPROBE_DEFER. It would change the behaviour. In any case, it's only two users and both written by you, so, just to be sure you aware of this change and bless it. -- With Best Regards, Andy Shevchenko
