| Message ID | 20190621100815.12417-5-ckeepax@opensource.cirrus.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | I2C IRQ Probe Improvements | expand |
On Fri, Jun 21, 2019 at 11:08:12AM +0100, Charles Keepax wrote: > In preparation for more refactoring make i2c_acpi_get_irq available > outside i2c-core-acpi.c. > > Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com> > --- > > Changes since v5: > - Pass a struct device rather than acpi_device to i2c_acpi_get_irq, > note this is more awkward than I would have liked as I am very > unconvinced that adev->dev can actually be passed to > ACPI_COMPANION. If anyone can answer that for sure that would be > very helpful. Sounds you are right. I stand corrected. We can not use ACPI_COMPANION() against &adev->dev.
On Fri, Jun 21, 2019 at 11:08:12AM +0100, Charles Keepax wrote: > In preparation for more refactoring make i2c_acpi_get_irq available > outside i2c-core-acpi.c. > > Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com> > --- > > Changes since v5: > - Pass a struct device rather than acpi_device to i2c_acpi_get_irq, > note this is more awkward than I would have liked as I am very > unconvinced that adev->dev can actually be passed to > ACPI_COMPANION. If anyone can answer that for sure that would be > very helpful. I don't think you can do that. I probably missed some previous discussion but what's wrong passing struct i2c_client instead and use ACPI_COMPANION() for that? > > Thanks, > Charles > > drivers/i2c/i2c-core-acpi.c | 13 +++++++++++-- > drivers/i2c/i2c-core.h | 7 +++++++ > 2 files changed, 18 insertions(+), 2 deletions(-) > > diff --git a/drivers/i2c/i2c-core-acpi.c b/drivers/i2c/i2c-core-acpi.c > index c91492eaacd93..37bf80b35365f 100644 > --- a/drivers/i2c/i2c-core-acpi.c > +++ b/drivers/i2c/i2c-core-acpi.c > @@ -145,8 +145,17 @@ static int i2c_acpi_add_resource(struct acpi_resource *ares, void *data) > return 1; > } > > -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 I think this should be @dev now. > + * > + * Find the IRQ number used by a specific client device. > + * > + * Return: The IRQ number or an error code. > + */ > +int i2c_acpi_get_irq(struct device *dev) > { > + struct acpi_device *adev = container_of(dev, struct acpi_device, dev); > struct list_head resource_list; > int irq = -ENOENT; > int ret; > @@ -199,7 +208,7 @@ static int i2c_acpi_get_info(struct acpi_device *adev, > *adapter_handle = lookup.adapter_handle; > > /* Then fill IRQ number if any */ > - info->irq = i2c_acpi_get_irq(adev); > + info->irq = i2c_acpi_get_irq(&adev->dev); > if (info->irq < 0) > return info->irq; > > diff --git a/drivers/i2c/i2c-core.h b/drivers/i2c/i2c-core.h > index 2a3b28bf826b1..1735ac17a957a 100644 > --- a/drivers/i2c/i2c-core.h > +++ b/drivers/i2c/i2c-core.h > @@ -63,6 +63,8 @@ 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 device *dev); > #else /* CONFIG_ACPI */ > static inline void i2c_acpi_register_devices(struct i2c_adapter *adap) { } > static inline const struct acpi_device_id * > @@ -71,6 +73,11 @@ i2c_acpi_match_device(const struct acpi_device_id *matches, > { > return NULL; > } > + > +static inline int i2c_acpi_get_irq(struct device *dev) > +{ > + return 0; > +} > #endif /* CONFIG_ACPI */ > extern struct notifier_block i2c_acpi_notifier; > > -- > 2.11.0
On Tue, Jun 25, 2019 at 02:50:11PM +0300, Mika Westerberg wrote: > On Fri, Jun 21, 2019 at 11:08:12AM +0100, Charles Keepax wrote: > > In preparation for more refactoring make i2c_acpi_get_irq available > > outside i2c-core-acpi.c. > > > > Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com> > > --- > > > > Changes since v5: > > - Pass a struct device rather than acpi_device to i2c_acpi_get_irq, > > note this is more awkward than I would have liked as I am very > > unconvinced that adev->dev can actually be passed to > > ACPI_COMPANION. If anyone can answer that for sure that would be > > very helpful. > > I don't think you can do that. > Yeah I think we are pretty sure that is not possible, although not what is done in the patch, was just responding to on an earlier comment. > I probably missed some previous discussion but what's wrong passing > struct i2c_client instead and use ACPI_COMPANION() for that? > Really this is all about the splitting out the original patch into two patches, one to export the function and one to move its use to probe time. There isn't really any nice way to do it as two patches and still pass the i2c_client struct. Hence we ended up on this system with struct device. I would be happy to squash the two patches, and go back to the i2c_client approach, if that was preferred and as long as Andy doesn't mind. > > > > Thanks, > > Charles > > > > drivers/i2c/i2c-core-acpi.c | 13 +++++++++++-- > > drivers/i2c/i2c-core.h | 7 +++++++ > > 2 files changed, 18 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/i2c/i2c-core-acpi.c b/drivers/i2c/i2c-core-acpi.c > > index c91492eaacd93..37bf80b35365f 100644 > > --- a/drivers/i2c/i2c-core-acpi.c > > +++ b/drivers/i2c/i2c-core-acpi.c > > @@ -145,8 +145,17 @@ static int i2c_acpi_add_resource(struct acpi_resource *ares, void *data) > > return 1; > > } > > > > -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 > > I think this should be @dev now. > Yes it should, sorry will fix that. Thanks, Charles
On Tue, Jun 25, 2019 at 01:42:45PM +0100, Charles Keepax wrote: > On Tue, Jun 25, 2019 at 02:50:11PM +0300, Mika Westerberg wrote: > I would be happy to squash the two patches, and go back to the > i2c_client approach, if that was preferred and as long as Andy > doesn't mind. I don't.
On Tue, Jun 25, 2019 at 04:25:15PM +0300, Andy Shevchenko wrote: > On Tue, Jun 25, 2019 at 01:42:45PM +0100, Charles Keepax wrote: > > On Tue, Jun 25, 2019 at 02:50:11PM +0300, Mika Westerberg wrote: > > > I would be happy to squash the two patches, and go back to the > > i2c_client approach, if that was preferred and as long as Andy > > doesn't mind. > > I don't. > Groovy alright think I will squash these two patches and return to the i2c_client interface. Thanks, Charles
diff --git a/drivers/i2c/i2c-core-acpi.c b/drivers/i2c/i2c-core-acpi.c index c91492eaacd93..37bf80b35365f 100644 --- a/drivers/i2c/i2c-core-acpi.c +++ b/drivers/i2c/i2c-core-acpi.c @@ -145,8 +145,17 @@ static int i2c_acpi_add_resource(struct acpi_resource *ares, void *data) return 1; } -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 device *dev) { + struct acpi_device *adev = container_of(dev, struct acpi_device, dev); struct list_head resource_list; int irq = -ENOENT; int ret; @@ -199,7 +208,7 @@ static int i2c_acpi_get_info(struct acpi_device *adev, *adapter_handle = lookup.adapter_handle; /* Then fill IRQ number if any */ - info->irq = i2c_acpi_get_irq(adev); + info->irq = i2c_acpi_get_irq(&adev->dev); if (info->irq < 0) return info->irq; diff --git a/drivers/i2c/i2c-core.h b/drivers/i2c/i2c-core.h index 2a3b28bf826b1..1735ac17a957a 100644 --- a/drivers/i2c/i2c-core.h +++ b/drivers/i2c/i2c-core.h @@ -63,6 +63,8 @@ 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 device *dev); #else /* CONFIG_ACPI */ static inline void i2c_acpi_register_devices(struct i2c_adapter *adap) { } static inline const struct acpi_device_id * @@ -71,6 +73,11 @@ i2c_acpi_match_device(const struct acpi_device_id *matches, { return NULL; } + +static inline int i2c_acpi_get_irq(struct device *dev) +{ + return 0; +} #endif /* CONFIG_ACPI */ extern struct notifier_block i2c_acpi_notifier;
In preparation for more refactoring make i2c_acpi_get_irq available outside i2c-core-acpi.c. Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com> --- Changes since v5: - Pass a struct device rather than acpi_device to i2c_acpi_get_irq, note this is more awkward than I would have liked as I am very unconvinced that adev->dev can actually be passed to ACPI_COMPANION. If anyone can answer that for sure that would be very helpful. Thanks, Charles drivers/i2c/i2c-core-acpi.c | 13 +++++++++++-- drivers/i2c/i2c-core.h | 7 +++++++ 2 files changed, 18 insertions(+), 2 deletions(-)