| Message ID | CABe79T7EeyzKS97zeQ5akoSaZX0=hMgHxw4XkLCQW15eRR=14Q@mail.gmail.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On Wed, Jul 4, 2018 at 6:37 AM, Srinath Mannam <srinath.mannam@broadcom.com> 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/linux-kernel@vger.kernel.org/msg1724366.html (Unfortunately I don't see the original patch there by unknown reason)
On 04/07/18 10:32, Andy Shevchenko wrote: > On Wed, Jul 4, 2018 at 6:37 AM, Srinath Mannam > <srinath.mannam@broadcom.com> 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.
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 <sudeep.holla@arm.com> wrote: > > > On 04/07/18 10:32, Andy Shevchenko wrote: >> On Wed, Jul 4, 2018 at 6:37 AM, Srinath Mannam >> <srinath.mannam@broadcom.com> 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 -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 >> <srinath.mannam@broadcom.com> 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 -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jul 4, 2018 at 1:17 PM, Nikolaus Voss <nikolaus.voss@loewensteinmedical.de> 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 >>> <srinath.mannam@broadcom.com> 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.
On Wed, Jul 4, 2018 at 12:24 PM, Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > On Wed, Jul 4, 2018 at 1:17 PM, Nikolaus Voss > <nikolaus.voss@loewensteinmedical.de> 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 >>>> <srinath.mannam@broadcom.com> 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 -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 4 Jul 2018, Andy Shevchenko wrote: > On Wed, Jul 4, 2018 at 1:17 PM, Nikolaus Voss > <nikolaus.voss@loewensteinmedical.de> 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 >>>> <srinath.mannam@broadcom.com> 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 -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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));