| Message ID | 20240418011353.1764672-1-wsadowski@marvell.com |
|---|---|
| Headers | show |
| Series | Marvell HW overlay support for Cadence xSPI | expand |
On 18/04/2024 03:13, Witold Sadowski wrote: > From: Piyush Malgujar <pmalgujar@marvell.com> > > These changes enables to read the configs from ACPI tables as required > for successful probing in ACPI uefi environment. > In case of ACPI disabled/dts based environment, it will continue to > read configs from dts as before > ... > } > @@ -924,6 +989,21 @@ static int cdns_xspi_probe(struct platform_device *pdev) > return 0; > } > > +#ifdef CONFIG_ACPI > +static const struct acpi_device_id cdns_xspi_acpi_match[] = { > + { > + .id = "cdns,xspi-nor", > + .driver_data = (kernel_ulong_t) &cdns_driver_data, > + }, > + { > + .id = "mrvl,xspi-nor", > + .driver_data = (kernel_ulong_t) &mrvl_driver_data, UEFI provides compatibles for ACPI? I think that's first such format in the kernel. Best regards, Krzysztof
> On 18/04/2024 03:13, Witold Sadowski wrote: > > From: Piyush Malgujar <pmalgujar@marvell.com> > > > > These changes enables to read the configs from ACPI tables as required > > for successful probing in ACPI uefi environment. > > In case of ACPI disabled/dts based environment, it will continue to > > read configs from dts as before > > > > ... > > > } > > @@ -924,6 +989,21 @@ static int cdns_xspi_probe(struct platform_device > *pdev) > > return 0; > > } > > > > +#ifdef CONFIG_ACPI > > +static const struct acpi_device_id cdns_xspi_acpi_match[] = { > > + { > > + .id = "cdns,xspi-nor", > > + .driver_data = (kernel_ulong_t) &cdns_driver_data, > > + }, > > + { > > + .id = "mrvl,xspi-nor", > > + .driver_data = (kernel_ulong_t) &mrvl_driver_data, > > UEFI provides compatibles for ACPI? I think that's first such format in > the kernel. Yes, that code is not doing what was expected. Current usage scenario in ACPI mode is: xSPI block with HID PRP0001, and additional compatible package set to correct compatible string With that approach only issue(in ACPI mode) is with matching device with data field from of_device_id. It looks like there are functions to match that when DTB is used, but in ACPI mode it fails. I believe solution is to traverse dev->driver->of_match_table manually To match device name with correct compatible data structure. That will be included in next patchset. > > Best regards, > Krzysztof Regards Witek
On 29/04/2024 16:30, Witold Sadowski wrote: >>> >>> +#ifdef CONFIG_ACPI >>> +static const struct acpi_device_id cdns_xspi_acpi_match[] = { >>> + { >>> + .id = "cdns,xspi-nor", >>> + .driver_data = (kernel_ulong_t) &cdns_driver_data, >>> + }, >>> + { >>> + .id = "mrvl,xspi-nor", >>> + .driver_data = (kernel_ulong_t) &mrvl_driver_data, >> >> UEFI provides compatibles for ACPI? I think that's first such format in >> the kernel. > > Yes, that code is not doing what was expected. > Current usage scenario in ACPI mode is: > xSPI block with HID PRP0001, and additional compatible package set to > correct compatible string > With that approach only issue(in ACPI mode) is with matching device > with data field from of_device_id. It looks like there are functions > to match that when DTB is used, but in ACPI mode it fails. > I believe solution is to traverse dev->driver->of_match_table manually > To match device name with correct compatible data structure. > That will be included in next patchset. PRP0001 should be handled by regular of_device_id table, of course assuming your kernel has build-in CONFIG_OF. Best regards, Krzysztof
> On 29/04/2024 16:30, Witold Sadowski wrote: > >>> > >>> +#ifdef CONFIG_ACPI > >>> +static const struct acpi_device_id cdns_xspi_acpi_match[] = { > >>> + { > >>> + .id = "cdns,xspi-nor", > >>> + .driver_data = (kernel_ulong_t) &cdns_driver_data, > >>> + }, > >>> + { > >>> + .id = "mrvl,xspi-nor", > >>> + .driver_data = (kernel_ulong_t) &mrvl_driver_data, > >> > >> UEFI provides compatibles for ACPI? I think that's first such format > >> in the kernel. > > > > Yes, that code is not doing what was expected. > > Current usage scenario in ACPI mode is: > > xSPI block with HID PRP0001, and additional compatible package set to > > correct compatible string With that approach only issue(in ACPI mode) > > is with matching device with data field from of_device_id. It looks > > like there are functions to match that when DTB is used, but in ACPI > > mode it fails. > > I believe solution is to traverse dev->driver->of_match_table manually > > To match device name with correct compatible data structure. > > That will be included in next patchset. > > PRP0001 should be handled by regular of_device_id table, of course > assuming your kernel has build-in CONFIG_OF. And it is correctly matched by id, but functions to retrieve data fails. I'm referring to of_device_get_match_data - there is no of node in ACPI case. I have come up with solution, as I wasn't able to find similar function that will work with ACPI and dtb on the same time: static const void * cdns_xspi_get_data(struct device *dev) { const struct of_device_id *matches = dev->driver->of_match_table; for (; matches->name[0] || matches->type[0] || matches->compatible[0]; matches++) { if (device_is_compatible(dev, matches->compatible)) return matches->data; } return NULL; } Is there a better way to handle that? > > Best regards, > Krzysztof
On Wed, May 08, 2024 at 08:04:49AM +0000, Witold Sadowski wrote: > > I have come up with solution, as I wasn't able to find similar function that > will work with ACPI and dtb on the same time: The usual thing would just be to try both an ACPI match and an OF match.
> On Wed, May 08, 2024 at 08:04:49AM +0000, Witold Sadowski wrote: > > > > > I have come up with solution, as I wasn't able to find similar > > function that will work with ACPI and dtb on the same time: > > The usual thing would just be to try both an ACPI match and an OF match. Ok, thanks. I thought it can be done in single step.