This patch provides a function, to get of_device_id after
matching with ACPI device _DSD object compatible property
in the case driver does not contain acpi_device_id list
and driver probe called for ACPI device ID PRP0001 with
compatible property match with of_device_id compatible.
Signed-off-by: Srinath Mannam <[email protected]>
---
drivers/acpi/bus.c | 23 +++++++++++++++++++++++
include/linux/acpi.h | 10 ++++++++++
2 files changed, 33 insertions(+)
diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
index 84b4a62..e676bf7 100644
--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c
@@ -830,6 +830,29 @@ const struct acpi_device_id *acpi_match_device(const struct acpi_device_id *ids,
}
EXPORT_SYMBOL_GPL(acpi_match_device);
+/**
+ * acpi_match_of_device_id - Match a struct device in given of_device_id list
+ * @ids: Array of struct of_device_id object to match against.
+ * @dev: The device structure to match.
+ *
+ * Check if @dev has a valid ACPI handle and if there is a struct acpi_device
+ * object for that handle and use that object to match against a given list of
+ * device IDs.
+ *
+ * Return a pointer to the first matching ID on success or %NULL on failure.
+ */
+const
+struct of_device_id *acpi_match_of_device_id(const struct of_device_id *ids,
+ const struct device *dev)
+{
+ const struct of_device_id *id = NULL;
+
+ __acpi_match_device(acpi_companion_match(dev), NULL, ids, NULL, &id);
+ return id;
+}
+EXPORT_SYMBOL_GPL(acpi_match_of_device_id);
+
+
const void *acpi_device_get_match_data(const struct device *dev)
{
const struct acpi_device_id *match;
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 4b35a66..2f24800 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -589,6 +589,10 @@ extern int acpi_nvs_for_each_region(int (*func)(__u64, __u64, void *),
const struct acpi_device_id *acpi_match_device(const struct acpi_device_id *ids,
const struct device *dev);
+const
+struct of_device_id *acpi_match_of_device_id(const struct of_device_id *ids,
+ const struct device *dev);
+
const void *acpi_device_get_match_data(const struct device *dev);
extern bool acpi_driver_match_device(struct device *dev,
const struct device_driver *drv);
@@ -775,6 +779,12 @@ static inline const struct acpi_device_id *acpi_match_device(
return NULL;
}
+static inline const struct of_device_id *acpi_match_of_device_id(
+ const struct of_device_id *ids, const struct device *dev)
+{
+ return NULL;
+}
+
static inline const void *acpi_device_get_match_data(const struct device *dev)
{
return NULL;
--
2.7.4
On Tue, Jul 03, 2018 at 02:52:40PM +0530, Srinath Mannam wrote:
> This patch provides a function, to get of_device_id after
> matching with ACPI device _DSD object compatible property
> in the case driver does not contain acpi_device_id list
> and driver probe called for ACPI device ID PRP0001 with
> compatible property match with of_device_id compatible.
>
So, IIUC you are adding this to get of_device_id and then fetch data ptr
from it. Can we see the user in some driver ?
--
Regards,
Sudeep
On Tue, Jul 3, 2018 at 12:22 PM, Srinath Mannam
<[email protected]> wrote:
> This patch provides a function, to get of_device_id after
> matching with ACPI device _DSD object compatible property
> in the case driver does not contain acpi_device_id list
> and driver probe called for ACPI device ID PRP0001 with
> compatible property match with of_device_id compatible.
I don't see any usefulness of this function. Care to provide a real use case?
--
With Best Regards,
Andy Shevchenko
Hi Sudeep, Andy,
Yes, This patch is to get of_device_id and then fetch data pointer.
To add ACPI support in multiple drivers which are device-tree based
and has list of of_device_ids, by using this function
very minimal changes and can avoid acpi_device_id list in the driver.
I will send driver changes where this function used to add ACPI
support in following patches.
Below are the changes added to add ACPI support in sdhci iproc driver
using this function.
diff --git a/drivers/mmc/host/sdhci-iproc.c b/drivers/mmc/host/sdhci-iproc.c
index db40218..f1ecac97 100644
--- a/drivers/mmc/host/sdhci-iproc.c
+++ b/drivers/mmc/host/sdhci-iproc.c
@@ -15,6 +15,7 @@
* iProc SDHCI platform driver
*/
+#include <linux/acpi.h>
#include <linux/delay.h>
#include <linux/module.h>
#include <linux/mmc/host.h>
@@ -267,8 +268,13 @@ static int sdhci_iproc_probe(struct platform_device *pdev)
int ret;
match = of_match_device(sdhci_iproc_of_match, &pdev->dev);
- if (!match)
- return -EINVAL;
+ if (!match) {
+ match = acpi_match_of_device_id(sdhci_iproc_of_match,
+ &pdev->dev);
+ if (!match)
+ return -EINVAL;
+ }
+
iproc_data = match->data;
host = sdhci_pltfm_init(pdev, iproc_data->pdata, sizeof(*iproc_host));
Regards,
Srinath.
On Tue, Jul 3, 2018 at 11:11 PM, Andy Shevchenko
<[email protected]> wrote:
> On Tue, Jul 3, 2018 at 12:22 PM, Srinath Mannam
> <[email protected]> wrote:
>> This patch provides a function, to get of_device_id after
>> matching with ACPI device _DSD object compatible property
>> in the case driver does not contain acpi_device_id list
>> and driver probe called for ACPI device ID PRP0001 with
>> compatible property match with of_device_id compatible.
>
> I don't see any usefulness of this function. Care to provide a real use case?
>
> --
> With Best Regards,
> Andy Shevchenko
On Wed, Jul 4, 2018 at 6:37 AM, Srinath Mannam
<[email protected]> wrote:
> Hi Sudeep, Andy,
>
> Yes, This patch is to get of_device_id and then fetch data pointer.
>
> To add ACPI support in multiple drivers which are device-tree based
> and has list of of_device_ids, by using this function
> very minimal changes and can avoid acpi_device_id list in the driver.
> I will send driver changes where this function used to add ACPI
> support in following patches.
>
> Below are the changes added to add ACPI support in sdhci iproc driver
> using this function.
So, did you get an ACPI ID for it?
That's how proper ACPI support should be done.
P.S. What you are trying to do is being discussed with Nikolaus in [1].
I have to NAK your approach in any case. Sorry.
[1]:
https://www.mail-archive.com/[email protected]/msg1724366.html
(Unfortunately I don't see the original patch there by unknown reason)
--
With Best Regards,
Andy Shevchenko
On 04/07/18 10:32, Andy Shevchenko wrote:
> On Wed, Jul 4, 2018 at 6:37 AM, Srinath Mannam
> <[email protected]> wrote:
>> Hi Sudeep, Andy,
>>
>> Yes, This patch is to get of_device_id and then fetch data pointer.
>>
>> To add ACPI support in multiple drivers which are device-tree based
>> and has list of of_device_ids, by using this function
>> very minimal changes and can avoid acpi_device_id list in the driver.
>> I will send driver changes where this function used to add ACPI
>> support in following patches.
>>
>> Below are the changes added to add ACPI support in sdhci iproc driver
>> using this function.
>
> So, did you get an ACPI ID for it?
> That's how proper ACPI support should be done.
>
> P.S. What you are trying to do is being discussed with Nikolaus in [1].
> I have to NAK your approach in any case. Sorry.
>
+1 on NACK for this and anything else that abuse PRP0001 as a short cut
approach.
--
Regards,
Sudeep
Hi Sudeep, Andy,
Thank you for all the valuable information and knowledge.
Regards,
Srinath.
On Wed, Jul 4, 2018 at 3:08 PM, Sudeep Holla <[email protected]> wrote:
>
>
> On 04/07/18 10:32, Andy Shevchenko wrote:
>> On Wed, Jul 4, 2018 at 6:37 AM, Srinath Mannam
>> <[email protected]> wrote:
>>> Hi Sudeep, Andy,
>>>
>>> Yes, This patch is to get of_device_id and then fetch data pointer.
>>>
>>> To add ACPI support in multiple drivers which are device-tree based
>>> and has list of of_device_ids, by using this function
>>> very minimal changes and can avoid acpi_device_id list in the driver.
>>> I will send driver changes where this function used to add ACPI
>>> support in following patches.
>>>
>>> Below are the changes added to add ACPI support in sdhci iproc driver
>>> using this function.
>>
>> So, did you get an ACPI ID for it?
>> That's how proper ACPI support should be done.
>>
>> P.S. What you are trying to do is being discussed with Nikolaus in [1].
>> I have to NAK your approach in any case. Sorry.
>>
>
> +1 on NACK for this and anything else that abuse PRP0001 as a short cut
> approach.
>
> --
> Regards,
> Sudeep
On Wed, 4 Jul 2018, Sudeep Holla wrote:
> On 04/07/18 10:32, Andy Shevchenko wrote:
>> On Wed, Jul 4, 2018 at 6:37 AM, Srinath Mannam
>> <[email protected]> wrote:
>>> Hi Sudeep, Andy,
>>>
>>> Yes, This patch is to get of_device_id and then fetch data pointer.
>>>
>>> To add ACPI support in multiple drivers which are device-tree based
>>> and has list of of_device_ids, by using this function
>>> very minimal changes and can avoid acpi_device_id list in the driver.
>>> I will send driver changes where this function used to add ACPI
>>> support in following patches.
>>>
>>> Below are the changes added to add ACPI support in sdhci iproc driver
>>> using this function.
>>
>> So, did you get an ACPI ID for it?
>> That's how proper ACPI support should be done.
>>
>> P.S. What you are trying to do is being discussed with Nikolaus in [1].
>> I have to NAK your approach in any case. Sorry.
>>
>
> +1 on NACK for this and anything else that abuse PRP0001 as a short cut
> approach.
This is no abuse but exactly what PRP0001 is meant for. The basic idea of
PRP0001 is to reuse DT "compatible" strings in ACPI namespace, see
Documentation/acpi/enumeration.txt. Reusing also means getting
access to the of_device_id.
Allocating an ACPI id for an already existing DT driver is redundant, isn't it?
Niko
On Wed, Jul 4, 2018 at 1:17 PM, Nikolaus Voss
<[email protected]> wrote:
> On Wed, 4 Jul 2018, Sudeep Holla wrote:
>> On 04/07/18 10:32, Andy Shevchenko wrote:
>>> On Wed, Jul 4, 2018 at 6:37 AM, Srinath Mannam
>>> <[email protected]> wrote:
>> +1 on NACK for this and anything else that abuse PRP0001 as a short cut
>> approach.
> This is no abuse but exactly what PRP0001 is meant for. The basic idea of
> PRP0001 is to reuse DT "compatible" strings in ACPI namespace, see
> Documentation/acpi/enumeration.txt. Reusing also means getting access to the
> of_device_id.
The idea was for almost DIY and / or manufacturer to develop a
prototype without modifying match code and faking ACPI IDs.
That's why the target of PRP0001 is almost sensors connected to I2C and SPI.
That's why I agreed on your patch to help with this. But! The proper
solution for the devices (device manufacturer) is to allocate an ACPI
ID and provide a corresponding table to the driver.
This is my understanding of that exercise. Rafael can correct me.
> Allocating an ACPI id for an already existing DT driver is redundant, isn't
> it?
It is not.
--
With Best Regards,
Andy Shevchenko
On Wed, Jul 4, 2018 at 12:24 PM, Andy Shevchenko
<[email protected]> wrote:
> On Wed, Jul 4, 2018 at 1:17 PM, Nikolaus Voss
> <[email protected]> wrote:
>> On Wed, 4 Jul 2018, Sudeep Holla wrote:
>>> On 04/07/18 10:32, Andy Shevchenko wrote:
>>>> On Wed, Jul 4, 2018 at 6:37 AM, Srinath Mannam
>>>> <[email protected]> wrote:
>
>>> +1 on NACK for this and anything else that abuse PRP0001 as a short cut
>>> approach.
>> This is no abuse but exactly what PRP0001 is meant for. The basic idea of
>> PRP0001 is to reuse DT "compatible" strings in ACPI namespace, see
>> Documentation/acpi/enumeration.txt. Reusing also means getting access to the
>> of_device_id.
>
> The idea was for almost DIY and / or manufacturer to develop a
> prototype without modifying match code and faking ACPI IDs.
> That's why the target of PRP0001 is almost sensors connected to I2C and SPI.
>
> That's why I agreed on your patch to help with this. But! The proper
> solution for the devices (device manufacturer) is to allocate an ACPI
> ID and provide a corresponding table to the driver.
>
> This is my understanding of that exercise. Rafael can correct me.
You are right.
>> Allocating an ACPI id for an already existing DT driver is redundant, isn't
>> it?
>
> It is not.
Again, right.
Thanks,
Rafael
On Wed, 4 Jul 2018, Andy Shevchenko wrote:
> On Wed, Jul 4, 2018 at 1:17 PM, Nikolaus Voss
> <[email protected]> wrote:
>> On Wed, 4 Jul 2018, Sudeep Holla wrote:
>>> On 04/07/18 10:32, Andy Shevchenko wrote:
>>>> On Wed, Jul 4, 2018 at 6:37 AM, Srinath Mannam
>>>> <[email protected]> wrote:
>
>>> +1 on NACK for this and anything else that abuse PRP0001 as a short cut
>>> approach.
>> This is no abuse but exactly what PRP0001 is meant for. The basic idea of
>> PRP0001 is to reuse DT "compatible" strings in ACPI namespace, see
>> Documentation/acpi/enumeration.txt. Reusing also means getting access to the
>> of_device_id.
>
> The idea was for almost DIY and / or manufacturer to develop a
> prototype without modifying match code and faking ACPI IDs.
> That's why the target of PRP0001 is almost sensors connected to I2C and SPI.
>
> That's why I agreed on your patch to help with this. But! The proper
> solution for the devices (device manufacturer) is to allocate an ACPI
> ID and provide a corresponding table to the driver.
>
> This is my understanding of that exercise. Rafael can correct me.
This is not meant as a short cut for device manufacturers. The patch is meant to
make PRP0001 work when access to specific driver_data is needed. I see
nothing bad with it.
>> Allocating an ACPI id for an already existing DT driver is redundant, isn't
>> it?
>
> It is not.
The driver won't work any better with it. The driver code gets another
table as big as of_device_id table. Can you give me a hint what the added
value is?
Niko
On Wed, Jul 04, 2018 at 12:17:20PM +0200, Nikolaus Voss wrote:
> On Wed, 4 Jul 2018, Sudeep Holla wrote:
> >
[...]
> >+1 on NACK for this and anything else that abuse PRP0001 as a short cut
> >approach.
>
> This is no abuse but exactly what PRP0001 is meant for. The basic idea of
> PRP0001 is to reuse DT "compatible" strings in ACPI namespace, see
> Documentation/acpi/enumeration.txt. Reusing also means getting access to the
> of_device_id.
>
Sorry for not being descriptive. It has been discussed a lot in past and
I assume someone would had gone through them, so gave no information in
my response.
> Allocating an ACPI id for an already existing DT driver is redundant, isn't it?
>
I think Andy had provided the summary and the intentions. Rafael has also
confirmed, I have nothing else to add.
--
Regards,
Sudeep