Re: [PATCH v6 03/15] i2c: acpi: Add i2c_acpi_new_device_by_fwnode() function

Hans de Goede
2021-11-26T11:21:08Z

Hi,

On 11/25/21 18:22, Andy Shevchenko wrote:
> On Thu, Nov 25, 2021 at 6:54 PM Hans de Goede <hdegoede@redhat.com> wrote:
>>
>> Change i2c_acpi_new_device() into i2c_acpi_new_device_by_fwnode() and
>> add a static inline wrapper providing the old i2c_acpi_new_device()
>> behavior.
>>
>> This is necessary because in some cases we may only have access
>> to the fwnode / acpi_device and not to the matching physical-node
>> struct device *.
>
> One nit-pick below.
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

Thank you, I've fixed the nit-pick in my local tree, either for v7 of
the series, of for in the immutable branch which I plan to create for
this.

Regards,

Hans

>> Suggested-by: Andy Shevchenko <andy.shevchenko@gmail.com>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>> Changes in v6:
>> - New patch in v6 of this patch series
>> ---
>> drivers/i2c/i2c-core-acpi.c | 18 ++++++++++++------
>> include/linux/i2c.h | 17 +++++++++++++----
>> 2 files changed, 25 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/i2c/i2c-core-acpi.c b/drivers/i2c/i2c-core-acpi.c
>> index 04338cbd08a9..1db3cc5fc47f 100644
>> --- a/drivers/i2c/i2c-core-acpi.c
>> +++ b/drivers/i2c/i2c-core-acpi.c
>> @@ -476,8 +476,9 @@ struct notifier_block i2c_acpi_notifier = {
>> };
>>
>> /**
>> - * i2c_acpi_new_device - Create i2c-client for the Nth I2cSerialBus resource
>> - * @dev: Device owning the ACPI resources to get the client from
>
>> + * i2c_acpi_new_device_by_fwnode - Create i2c-client for the Nth I2cSerialBus
>> + * resource
>
> Can be on one line.
>
>> + * @fwnode: fwnode with the ACPI resources to get the client from
>> * @index: Index of ACPI resource to get
>> * @info: describes the I2C device; note this is modified (addr gets set)
>> * Context: can sleep
>> @@ -493,15 +494,20 @@ struct notifier_block i2c_acpi_notifier = {
>> * Returns a pointer to the new i2c-client, or error pointer in case of failure.
>> * Specifically, -EPROBE_DEFER is returned if the adapter is not found.
>> */
>> -struct i2c_client *i2c_acpi_new_device(struct device *dev, int index,
>> - struct i2c_board_info *info)
>> +struct i2c_client *i2c_acpi_new_device_by_fwnode(struct fwnode_handle *fwnode,
>> + int index,
>> + struct i2c_board_info *info)
>> {
>> - struct acpi_device *adev = ACPI_COMPANION(dev);
>> struct i2c_acpi_lookup lookup;
>> struct i2c_adapter *adapter;
>> + struct acpi_device *adev;
>> LIST_HEAD(resource_list);
>> int ret;
>>
>> + adev = to_acpi_device_node(fwnode);
>> + if (!adev)
>> + return ERR_PTR(-ENODEV);
>> +
>> memset(&lookup, 0, sizeof(lookup));
>> lookup.info = info;
>> lookup.device_handle = acpi_device_handle(adev);
>> @@ -523,7 +529,7 @@ struct i2c_client *i2c_acpi_new_device(struct device *dev, int index,
>>
>> return i2c_new_client_device(adapter, info);
>> }
>> -EXPORT_SYMBOL_GPL(i2c_acpi_new_device);
>> +EXPORT_SYMBOL_GPL(i2c_acpi_new_device_by_fwnode);
>>
>> bool i2c_acpi_waive_d0_probe(struct device *dev)
>> {
>> diff --git a/include/linux/i2c.h b/include/linux/i2c.h
>> index 16119ac1aa97..7d4f52ceb7b5 100644
>> --- a/include/linux/i2c.h
>> +++ b/include/linux/i2c.h
>> @@ -1025,8 +1025,9 @@ bool i2c_acpi_get_i2c_resource(struct acpi_resource *ares,
>> struct acpi_resource_i2c_serialbus **i2c);
>> int i2c_acpi_client_count(struct acpi_device *adev);
>> u32 i2c_acpi_find_bus_speed(struct device *dev);
>> -struct i2c_client *i2c_acpi_new_device(struct device *dev, int index,
>> - struct i2c_board_info *info);
>> +struct i2c_client *i2c_acpi_new_device_by_fwnode(struct fwnode_handle *fwnode,
>> + int index,
>> + struct i2c_board_info *info);
>> struct i2c_adapter *i2c_acpi_find_adapter_by_handle(acpi_handle handle);
>> bool i2c_acpi_waive_d0_probe(struct device *dev);
>> #else
>> @@ -1043,8 +1044,9 @@ static inline u32 i2c_acpi_find_bus_speed(struct device *dev)
>> {
>> return 0;
>> }
>> -static inline struct i2c_client *i2c_acpi_new_device(struct device *dev,
>> - int index, struct i2c_board_info *info)
>> +static inline struct i2c_client *i2c_acpi_new_device_by_fwnode(
>> + struct fwnode_handle *fwnode, int index,
>> + struct i2c_board_info *info)
>> {
>> return ERR_PTR(-ENODEV);
>> }
>> @@ -1058,4 +1060,11 @@ static inline bool i2c_acpi_waive_d0_probe(struct device *dev)
>> }
>> #endif /* CONFIG_ACPI */
>>
>> +static inline struct i2c_client *i2c_acpi_new_device(struct device *dev,
>> + int index,
>> + struct i2c_board_info *info)
>> +{
>> + return i2c_acpi_new_device_by_fwnode(dev_fwnode(dev), index, info);
>> +}
>> +
>> #endif /* _LINUX_I2C_H */
>> --
>> 2.33.1
>>
>
>