2019-06-20 13:36:45

by Charles Keepax

[permalink] [raw]
Subject: [PATCH v5 4/7] i2c: core: Make i2c_acpi_get_irq available to the rest of the I2C core

In preparation for more refactoring make i2c_acpi_get_irq available
outside i2c-core-acpi.c.

Reviewed-by: Andy Shevchenko <[email protected]>
Reviewed-by: Mika Westerberg <[email protected]>
Signed-off-by: Charles Keepax <[email protected]>
---

Changes since v4:
- Leave i2c_acpi_get_irq accepting an acpi_device, this should
avoid the NULL pointer issue we had with i2c_acpi_find_client_by_adev

Thanks,
Charles

drivers/i2c/i2c-core-acpi.c | 10 +++++++++-
drivers/i2c/i2c-core.h | 9 +++++++++
2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/i2c-core-acpi.c b/drivers/i2c/i2c-core-acpi.c
index 7d4d66ba752d4..0ddfcca6091e1 100644
--- a/drivers/i2c/i2c-core-acpi.c
+++ b/drivers/i2c/i2c-core-acpi.c
@@ -144,7 +144,15 @@ static int i2c_acpi_add_resource(struct acpi_resource *ares, void *data)
return 1; /* No need to add resource to the list */
}

-static int i2c_acpi_get_irq(struct acpi_device *adev)
+/**
+ * i2c_acpi_get_irq - get device IRQ number from ACPI
+ * @client: Pointer to the I2C client device
+ *
+ * Find the IRQ number used by a specific client device.
+ *
+ * Return: The IRQ number or an error code.
+ */
+int i2c_acpi_get_irq(struct acpi_device *adev)
{
struct list_head resource_list;
int irq = -ENOENT;
diff --git a/drivers/i2c/i2c-core.h b/drivers/i2c/i2c-core.h
index 2a3b28bf826b1..4fbe0a0bcc4c4 100644
--- a/drivers/i2c/i2c-core.h
+++ b/drivers/i2c/i2c-core.h
@@ -58,11 +58,15 @@ static inline int __i2c_check_suspended(struct i2c_adapter *adap)
return 0;
}

+struct acpi_device;
+
#ifdef CONFIG_ACPI
const struct acpi_device_id *
i2c_acpi_match_device(const struct acpi_device_id *matches,
struct i2c_client *client);
void i2c_acpi_register_devices(struct i2c_adapter *adap);
+
+int i2c_acpi_get_irq(struct acpi_device *adev);
#else /* CONFIG_ACPI */
static inline void i2c_acpi_register_devices(struct i2c_adapter *adap) { }
static inline const struct acpi_device_id *
@@ -71,6 +75,11 @@ i2c_acpi_match_device(const struct acpi_device_id *matches,
{
return NULL;
}
+
+static inline int i2c_acpi_get_irq(struct acpi_device *adev)
+{
+ return 0;
+}
#endif /* CONFIG_ACPI */
extern struct notifier_block i2c_acpi_notifier;

--
2.11.0


2019-06-20 15:00:15

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v5 4/7] i2c: core: Make i2c_acpi_get_irq available to the rest of the I2C core

On Thu, Jun 20, 2019 at 02:34:17PM +0100, Charles Keepax wrote:
> In preparation for more refactoring make i2c_acpi_get_irq available
> outside i2c-core-acpi.c.

> Reviewed-by: Andy Shevchenko <[email protected]>
> Reviewed-by: Mika Westerberg <[email protected]>

I'm not sure Rb tags are suitable for patches 4 and 5 since the changes made.

> +struct acpi_device;

Hmm... Doesn't acpi.h define that for !ACPI case?

> #ifdef CONFIG_ACPI
> const struct acpi_device_id *
> i2c_acpi_match_device(const struct acpi_device_id *matches,
> struct i2c_client *client);
> void i2c_acpi_register_devices(struct i2c_adapter *adap);
> +
> +int i2c_acpi_get_irq(struct acpi_device *adev);

Since you call this afterwards with struct device from which companion is
derived, can't we directly use struct device as a parameter?

Yes, in case of adev call, it might be &adev->dev I suppose?

--
With Best Regards,
Andy Shevchenko


2019-06-20 15:15:50

by Charles Keepax

[permalink] [raw]
Subject: Re: [PATCH v5 4/7] i2c: core: Make i2c_acpi_get_irq available to the rest of the I2C core

On Thu, Jun 20, 2019 at 05:59:50PM +0300, Andy Shevchenko wrote:
> On Thu, Jun 20, 2019 at 02:34:17PM +0100, Charles Keepax wrote:
> > In preparation for more refactoring make i2c_acpi_get_irq available
> > outside i2c-core-acpi.c.
>
> > Reviewed-by: Andy Shevchenko <[email protected]>
> > Reviewed-by: Mika Westerberg <[email protected]>
>
> I'm not sure Rb tags are suitable for patches 4 and 5 since the changes made.
>

Apologies I figured the changes were small enough will remove for
any changes in future.

> > +struct acpi_device;
>
> Hmm... Doesn't acpi.h define that for !ACPI case?
>

Pretty sure I was getting a build error in that case.

> > #ifdef CONFIG_ACPI
> > const struct acpi_device_id *
> > i2c_acpi_match_device(const struct acpi_device_id *matches,
> > struct i2c_client *client);
> > void i2c_acpi_register_devices(struct i2c_adapter *adap);
> > +
> > +int i2c_acpi_get_irq(struct acpi_device *adev);
>
> Since you call this afterwards with struct device from which companion is
> derived, can't we directly use struct device as a parameter?
>
> Yes, in case of adev call, it might be &adev->dev I suppose?
>

A good idea I will investigate and do a respin taking in the
other comments too.

Thanks,
Charles