Re: [PATCH can-next v2] can: mcp251xfd: ACPI support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Il giorno mar 10 dic 2024 alle ore 15:31 Marc Kleine-Budde
<mkl@xxxxxxxxxxxxxx> ha scritto:
>
> On 10.12.2024 15:04:05, Stefano Offredi wrote:
> > > 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.
>
> No problem.
>
> > 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
>
> Why not use "microchip,rx-int" for ACPI, too?

yes I'll test it.

>
> > 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).
>
> Just to clarify: from the Linux point of view "rx-int" is a GPIO, not an
> interrupt. It's connected to the "nINT1/GPIO1" pin of the mcp251xfd and
> this is optional. The IRQ which is available under spi->irq is connected
> to the "nINT" pin of the mcp251xfd.
>
> > 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.
>
> I suggest to first remove all "rx-int" related stuff from your ACPI
> table and concentrate that "spi->irq" has a proper value.
>
> The relevant code is in acpi_status acpi_register_spi_device():
>
> | https://elixir.bootlin.com/linux/v5.15/source/drivers/spi/spi.c#L2294
>

Ok thanks I will investigate on it.

Just to be sure, will the driver, if rx-int and interrupt logic is not
used, perform
 a polling on the spi bus to check for messages incoming on the can controller?
So I should see in this operating mode continuous spi communication on
the bus, right?

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   |





[Index of Archives]     [Automotive Discussions]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]     [CAN Bus]

  Powered by Linux