Hi,
On 27-11-18 20:27, Andy Shevchenko wrote:
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.
Yes it would change behaviour, for the better, all the errors
from i2c_new_device() (*) will not go away when we retry later,
so responding with probe-deferring to them is not useful.
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.
I'm aware and you've my ack for this change.
Regards,
Hans
*) with exception of -ENOMEM which should never happen
