| Message ID | 20241104200848.58693-2-hdegoede@redhat.com (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers | show |
| Series | platform/x86: x86-android-tablets: Add support for Vexia EDU ATLA 10 tablet | expand |
On Mon, Nov 4, 2024 at 10:09 PM Hans de Goede <hdegoede@redhat.com> wrote: > > Add get_i2c_adap_by_handle() helper function, this is a preparation patch > for adding support for getting i2c_adapter-s by PCI parent devname(). Reviewed-by: Andy Shevchenko <andy@kernel.org>
On Mon, 4 Nov 2024, Hans de Goede wrote: > Add get_i2c_adap_by_handle() helper function, this is a preparation patch > for adding support for getting i2c_adapter-s by PCI parent devname(). > > Suggested-by: Andy Shevchenko <andy@kernel.org> > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > --- > Changes in v2: > - New patch in v2 of this series > --- > .../platform/x86/x86-android-tablets/core.c | 25 ++++++++++++------- > 1 file changed, 16 insertions(+), 9 deletions(-) > > diff --git a/drivers/platform/x86/x86-android-tablets/core.c b/drivers/platform/x86/x86-android-tablets/core.c > index ef572b90e06b..4154395c60bb 100644 > --- a/drivers/platform/x86/x86-android-tablets/core.c > +++ b/drivers/platform/x86/x86-android-tablets/core.c > @@ -155,26 +155,33 @@ static struct gpiod_lookup_table * const *gpiod_lookup_tables; > static const struct software_node *bat_swnode; > static void (*exit_handler)(void); > > +static struct i2c_adapter * > +get_i2c_adap_by_handle(const struct x86_i2c_client_info *client_info) > +{ > + acpi_handle handle; > + acpi_status status; > + > + status = acpi_get_handle(NULL, client_info->adapter_path, &handle); > + if (ACPI_FAILURE(status)) { > + pr_err("Error could not get %s handle\n", client_info->adapter_path); > + return NULL; > + } > + > + return i2c_acpi_find_adapter_by_handle(handle); > +} > + > static __init int x86_instantiate_i2c_client(const struct x86_dev_info *dev_info, > int idx) > { > const struct x86_i2c_client_info *client_info = &dev_info->i2c_client_info[idx]; > struct i2c_board_info board_info = client_info->board_info; > struct i2c_adapter *adap; > - acpi_handle handle; > - acpi_status status; > > board_info.irq = x86_acpi_irq_helper_get(&client_info->irq_data); > if (board_info.irq < 0) > return board_info.irq; > > - status = acpi_get_handle(NULL, client_info->adapter_path, &handle); > - if (ACPI_FAILURE(status)) { > - pr_err("Error could not get %s handle\n", client_info->adapter_path); > - return -ENODEV; > - } > - > - adap = i2c_acpi_find_adapter_by_handle(handle); > + adap = get_i2c_adap_by_handle(client_info); > if (!adap) { > pr_err("error could not get %s adapter\n", client_info->adapter_path); > return -ENODEV; Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> Not a big deal, but you might want to consider if printing both error messages is fine or if the error printing should be somehow modified when that other print moves into the inner function.
Hi Ilpo, On 5-Nov-24 10:37 AM, Ilpo Järvinen wrote: > On Mon, 4 Nov 2024, Hans de Goede wrote: > >> Add get_i2c_adap_by_handle() helper function, this is a preparation patch >> for adding support for getting i2c_adapter-s by PCI parent devname(). >> >> Suggested-by: Andy Shevchenko <andy@kernel.org> >> Signed-off-by: Hans de Goede <hdegoede@redhat.com> >> --- >> Changes in v2: >> - New patch in v2 of this series >> --- >> .../platform/x86/x86-android-tablets/core.c | 25 ++++++++++++------- >> 1 file changed, 16 insertions(+), 9 deletions(-) >> >> diff --git a/drivers/platform/x86/x86-android-tablets/core.c b/drivers/platform/x86/x86-android-tablets/core.c >> index ef572b90e06b..4154395c60bb 100644 >> --- a/drivers/platform/x86/x86-android-tablets/core.c >> +++ b/drivers/platform/x86/x86-android-tablets/core.c >> @@ -155,26 +155,33 @@ static struct gpiod_lookup_table * const *gpiod_lookup_tables; >> static const struct software_node *bat_swnode; >> static void (*exit_handler)(void); >> >> +static struct i2c_adapter * >> +get_i2c_adap_by_handle(const struct x86_i2c_client_info *client_info) >> +{ >> + acpi_handle handle; >> + acpi_status status; >> + >> + status = acpi_get_handle(NULL, client_info->adapter_path, &handle); >> + if (ACPI_FAILURE(status)) { >> + pr_err("Error could not get %s handle\n", client_info->adapter_path); >> + return NULL; >> + } >> + >> + return i2c_acpi_find_adapter_by_handle(handle); >> +} >> + >> static __init int x86_instantiate_i2c_client(const struct x86_dev_info *dev_info, >> int idx) >> { >> const struct x86_i2c_client_info *client_info = &dev_info->i2c_client_info[idx]; >> struct i2c_board_info board_info = client_info->board_info; >> struct i2c_adapter *adap; >> - acpi_handle handle; >> - acpi_status status; >> >> board_info.irq = x86_acpi_irq_helper_get(&client_info->irq_data); >> if (board_info.irq < 0) >> return board_info.irq; >> >> - status = acpi_get_handle(NULL, client_info->adapter_path, &handle); >> - if (ACPI_FAILURE(status)) { >> - pr_err("Error could not get %s handle\n", client_info->adapter_path); >> - return -ENODEV; >> - } >> - >> - adap = i2c_acpi_find_adapter_by_handle(handle); >> + adap = get_i2c_adap_by_handle(client_info); >> if (!adap) { >> pr_err("error could not get %s adapter\n", client_info->adapter_path); >> return -ENODEV; > > Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> > > Not a big deal, but you might want to consider if printing both error > messages is fine or if the error printing should be somehow modified when > that other print moves into the inner function. I already considered this and since this should never happen printing both messages if it does happen is fine IMHO. Basically the idea is if the acpi_get_handle() fails print both messages, if the i2c_acpi_find_adapter_by_handle() fails only print the latter message. This way one can tell one scenario from the other while keeping the logic simple. The same applies to the get_i2c_adap_by_pci_parent() helper added in patch 2. Regards, Hans
diff --git a/drivers/platform/x86/x86-android-tablets/core.c b/drivers/platform/x86/x86-android-tablets/core.c index ef572b90e06b..4154395c60bb 100644 --- a/drivers/platform/x86/x86-android-tablets/core.c +++ b/drivers/platform/x86/x86-android-tablets/core.c @@ -155,26 +155,33 @@ static struct gpiod_lookup_table * const *gpiod_lookup_tables; static const struct software_node *bat_swnode; static void (*exit_handler)(void); +static struct i2c_adapter * +get_i2c_adap_by_handle(const struct x86_i2c_client_info *client_info) +{ + acpi_handle handle; + acpi_status status; + + status = acpi_get_handle(NULL, client_info->adapter_path, &handle); + if (ACPI_FAILURE(status)) { + pr_err("Error could not get %s handle\n", client_info->adapter_path); + return NULL; + } + + return i2c_acpi_find_adapter_by_handle(handle); +} + static __init int x86_instantiate_i2c_client(const struct x86_dev_info *dev_info, int idx) { const struct x86_i2c_client_info *client_info = &dev_info->i2c_client_info[idx]; struct i2c_board_info board_info = client_info->board_info; struct i2c_adapter *adap; - acpi_handle handle; - acpi_status status; board_info.irq = x86_acpi_irq_helper_get(&client_info->irq_data); if (board_info.irq < 0) return board_info.irq; - status = acpi_get_handle(NULL, client_info->adapter_path, &handle); - if (ACPI_FAILURE(status)) { - pr_err("Error could not get %s handle\n", client_info->adapter_path); - return -ENODEV; - } - - adap = i2c_acpi_find_adapter_by_handle(handle); + adap = get_i2c_adap_by_handle(client_info); if (!adap) { pr_err("error could not get %s adapter\n", client_info->adapter_path); return -ENODEV;
Add get_i2c_adap_by_handle() helper function, this is a preparation patch for adding support for getting i2c_adapter-s by PCI parent devname(). Suggested-by: Andy Shevchenko <andy@kernel.org> Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- Changes in v2: - New patch in v2 of this series --- .../platform/x86/x86-android-tablets/core.c | 25 ++++++++++++------- 1 file changed, 16 insertions(+), 9 deletions(-)