Il giorno mar 10 dic 2024 alle ore 13:09 Marc Kleine-Budde
<mkl@xxxxxxxxxxxxxx> ha scritto:
>
> On 10.12.2024 12:52:57, Stefano Offredi wrote:
> > > > +#ifdef CONFIG_ACPI
> > > > +static const struct acpi_device_id mcp251xfd_acpi_id_table[] = {
> > > > + { "MCP2517", .driver_data = (kernel_ulong_t)&mcp251xfd_devtype_data_mcp2517fd, },
> > > > + { "MCP2518", .driver_data = (kernel_ulong_t)&mcp251xfd_devtype_data_mcp2518fd, },
> > > > + { "MCP251X", .driver_data = (kernel_ulong_t)&mcp251xfd_devtype_data_mcp251xfd, },
> > > > + {}
> > > > +};
> > > > +MODULE_DEVICE_TABLE(acpi, mcp251xfd_acpi_id_table);
> > > > +
> > > > +static const struct acpi_gpio_params rx_int_gpios = { 1, 0, false };
> > > > +
> > > > +static const struct acpi_gpio_mapping acpi_mcp251xfd_gpios[] = {
> > > > + { "rx-int-gpios", &rx_int_gpios, 1 },
> > >
> > > The devm_gpiod_get_optional() uses "microchip,rx-int". How does it find
> ^^^^^^^^^^
>
> > > the "rx-int-gpios" here? Does the ACPI matching code remove the
> > > "microchip," prefix?
> > >
> > It should use the devm_acpi_dev_add_driver_gpios() function to get
> > rx-int-gpios using names listed in acpi_mcp251xfd_gpios[] (rx-int-gpios).
> > Then watching at other drivers using gpios loading, after calling
> > devm_acpi_dev_add_driver_gpios(),
> > they call devm_gpiod_get_optional(), with the same name but without "-gpios".
> > So in our case it is exactly "rx-int".
>
> No! Please re-read the code, devm_gpiod_get_optional() uses
> "microchip,rx-int", not "rx-int":
>
> | rx_int = devm_gpiod_get_optional(&spi->dev, "microchip,rx-int",
> ^^^^^^^^^^
> | GPIOD_IN);
>
very sorry Marc, in my test environment I had modified this line but I
have not reported it in the patch.
Be patient, it's the first time I post a patch to the kernel. Any hint
is very appreciated.
To keep compatibility with DTS loading I propose the following:
#ifdef CONFIG_ACPI
ret = devm_acpi_dev_add_driver_gpios(&spi->dev, acpi_mcp251xfd_gpios);
if (ret) {
dev_dbg(&spi->dev, "failed to add gpios mapping table\n");
return ret;
}
rx_int = devm_gpiod_get_optional(&spi->dev, "rx-int", GPIOD_IN);
#else
rx_int = devm_gpiod_get_optional(&spi->dev,
"microchip,rx-int", GPIOD_IN);
#endif
Even with those lines of code, If I print spi->irq before the call
to request_threaded_irq() it keeps different error codes
values (-19, -22, depending on whether I set the gpio controller
values in ACPI table).
Even If I remove in ACPI table all the gpio references listed
here below:
- GpioInt (Edge, ActiveLow, ExclusiveAndWake, PullDefault...
- Package () {"rx-int-gpios", Package () { ^CAN0, 0, 0, 0 } }
the spi->irq keeps error value -2.
>
> > The chain starts from the acpi table here:
> > Package () {"rx-int-gpios", Package () { ^CAN0, 0, 0, 0 } }.
> >
> > For example you can watch at st33zp24/spi.c:237 driver, or st-nci/spi.c:235
>
> This is the case for st33zp24/spi.c, but not your code:
>
> | static const struct acpi_gpio_mapping acpi_st33zp24_gpios[] = {
> | { "lpcpd-gpios", &lpcpd_gpios, 1 },
> ^^^^^^^^^^^
> | { },
> | };
>
> | tpm_dev->io_lpcpd = devm_gpiod_get_optional(dev, "lpcpd",
> ^^^^^
> | GPIOD_OUT_HIGH);
>
> FYI:
> Without a reference to a kernel version this line file+line number is
> not of much value. Can you post a link to https://elixir.bootlin.com
> which includes the kernel version?
>
thanks again, here my kernel developing version:
https://elixir.bootlin.com/linux/v5.15/source
regards,
Stefano
> regards,
> Marc
>
> --
> Pengutronix e.K. | Marc Kleine-Budde |
> Embedded Linux | https://www.pengutronix.de |
> Vertretung Nürnberg | Phone: +49-5121-206917-129 |
> Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-9 |
