Re: [PATCH] spi: spi-fsl-dspi: Add ACPI support

Re: [PATCH] spi: spi-fsl-dspi: Add ACPI support

[Andy Shevchenko]

On Sat, Aug 22, 2020 at 9:37 PM Vladimir Oltean <olteanv@gmail.com> wrote:
> On Fri, Aug 21, 2020 at 06:40:29PM +0530, kuldip dwivedi wrote:

> Just noticed this now.
> So for device tree, spi-fsl-dspi supports the following compatibles:
>
> fsl,vf610-dspi
> fsl,ls1021a-v1.0-dspi
> fsl,ls1012a-dspi
> fsl,ls1028a-dspi
> fsl,ls1043a-dspi
> fsl,ls1046a-dspi
> fsl,ls2080a-dspi
> fsl,ls2085a-dspi
> fsl,lx2160a-dspi
>
> Depending on the compatible string, the driver knows whether to use DMA
> or XSPI mode, and what the FIFO size is.
>
> Now, of course not all of the above SoCs are going to support ACPI, but
> it is reasonable to expect that more than one will. And in that case,
> the driver should still be able to know on what SoC it's running,
> because for example LS1043A doesn't support DMA, and LS2085A doesn't
> support XSPI.
>
> How is this dealt with in ACPI?

Theoretically you may declare your HID in the same / similar way as
PRP0001 and use same compatible strings and all other DT properties
(when they make sense and not duplicate ACPI functionality).
But better if ACPI people can tell you (I Cc'ed Rafael and ACPI
mailing list) if it is gonna work.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] spi: spi-fsl-dspi: Add ACPI support

[Andy Shevchenko]

On Wed, Aug 26, 2020 at 10:34 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Sat, Aug 22, 2020 at 9:37 PM Vladimir Oltean <olteanv@gmail.com> wrote:
> > On Fri, Aug 21, 2020 at 06:40:29PM +0530, kuldip dwivedi wrote:
>
> > Just noticed this now.
> > So for device tree, spi-fsl-dspi supports the following compatibles:
> >
> > fsl,vf610-dspi
> > fsl,ls1021a-v1.0-dspi
> > fsl,ls1012a-dspi
> > fsl,ls1028a-dspi
> > fsl,ls1043a-dspi
> > fsl,ls1046a-dspi
> > fsl,ls2080a-dspi
> > fsl,ls2085a-dspi
> > fsl,lx2160a-dspi
> >
> > Depending on the compatible string, the driver knows whether to use DMA
> > or XSPI mode, and what the FIFO size is.

FIFO size can be read from the property (or better if you can derive
it directly from HW, like DesignWare does).
DMA is just defined by FixedDMA resources (your platform with DMA
provides them, otherwise no such resources).

> > Now, of course not all of the above SoCs are going to support ACPI, but
> > it is reasonable to expect that more than one will. And in that case,
> > the driver should still be able to know on what SoC it's running,
> > because for example LS1043A doesn't support DMA, and LS2085A doesn't
> > support XSPI.
> >
> > How is this dealt with in ACPI?
>
> Theoretically you may declare your HID in the same / similar way as
> PRP0001 and use same compatible strings and all other DT properties
> (when they make sense and not duplicate ACPI functionality).
> But better if ACPI people can tell you (I Cc'ed Rafael and ACPI
> mailing list) if it is gonna work.


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] spi: spi-fsl-dspi: Add ACPI support

[Vladimir Oltean]

On Wed, Aug 26, 2020 at 10:36:15PM +0300, Andy Shevchenko wrote:
> On Wed, Aug 26, 2020 at 10:34 PM Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
> > On Sat, Aug 22, 2020 at 9:37 PM Vladimir Oltean <olteanv@gmail.com> wrote:
> > > On Fri, Aug 21, 2020 at 06:40:29PM +0530, kuldip dwivedi wrote:
> >
> > > Just noticed this now.
> > > So for device tree, spi-fsl-dspi supports the following compatibles:
> > >
> > > fsl,vf610-dspi
> > > fsl,ls1021a-v1.0-dspi
> > > fsl,ls1012a-dspi
> > > fsl,ls1028a-dspi
> > > fsl,ls1043a-dspi
> > > fsl,ls1046a-dspi
> > > fsl,ls2080a-dspi
> > > fsl,ls2085a-dspi
> > > fsl,lx2160a-dspi
> > >
> > > Depending on the compatible string, the driver knows whether to use DMA
> > > or XSPI mode, and what the FIFO size is.
>
> FIFO size can be read from the property

My personal preference is for the driver to hold the expert information
about the hardware parameters, and not the device tree. Today you need
to know only about this set of parameters, tomorrow you need something
new, but you also need to work with old DT blobs... It's a mess.

> (or better if you can derive it directly from HW, like DesignWare
> does).

Nope, can't do that. This IP block doesn't even have an ID or revision
register.

> DMA is just defined by FixedDMA resources (your platform with DMA
> provides them, otherwise no such resources).
>

This is a case of knowing that tomatoes are fruit, but being wise enough
to not put them in a fruit salad.

I would not be happy to see this driver make the decision to use DMA
based just on the presence of DMA channels in the firmware blob. On some
platforms, there are DMA channels but due to an erratum they don't work.
While on other platforms, using the DMA channels would simply cause a
performance downgrade.
Same thing about interrupts, in fact. The firmware blob tells the driver
what the interrupt line is, but it doesn't mean that the driver has to
use it.

Thanks,
-Vladimir

Re: [PATCH] spi: spi-fsl-dspi: Add ACPI support

[Vladimir Oltean]

On Wed, Aug 26, 2020 at 10:34:04PM +0300, Andy Shevchenko wrote:
> On Sat, Aug 22, 2020 at 9:37 PM Vladimir Oltean <olteanv@gmail.com> wrote:
> > On Fri, Aug 21, 2020 at 06:40:29PM +0530, kuldip dwivedi wrote:
>
> > Just noticed this now.
> > So for device tree, spi-fsl-dspi supports the following compatibles:
> >
> > fsl,vf610-dspi
> > fsl,ls1021a-v1.0-dspi
> > fsl,ls1012a-dspi
> > fsl,ls1028a-dspi
> > fsl,ls1043a-dspi
> > fsl,ls1046a-dspi
> > fsl,ls2080a-dspi
> > fsl,ls2085a-dspi
> > fsl,lx2160a-dspi
> >
> > Depending on the compatible string, the driver knows whether to use DMA
> > or XSPI mode, and what the FIFO size is.
> >
> > Now, of course not all of the above SoCs are going to support ACPI, but
> > it is reasonable to expect that more than one will. And in that case,
> > the driver should still be able to know on what SoC it's running,
> > because for example LS1043A doesn't support DMA, and LS2085A doesn't
> > support XSPI.
> >
> > How is this dealt with in ACPI?
>
> Theoretically you may declare your HID in the same / similar way as
> PRP0001 and use same compatible strings and all other DT properties
> (when they make sense and not duplicate ACPI functionality).
> But better if ACPI people can tell you (I Cc'ed Rafael and ACPI
> mailing list) if it is gonna work.
>

Something doesn't look right about PRP0001, what's the catch?

Re: [PATCH] spi: spi-fsl-dspi: Add ACPI support

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 687 bytes --]

On Wed, Aug 26, 2020 at 10:56:49PM +0300, Vladimir Oltean wrote:
> On Wed, Aug 26, 2020 at 10:36:15PM +0300, Andy Shevchenko wrote:

> > FIFO size can be read from the property

> My personal preference is for the driver to hold the expert information
> about the hardware parameters, and not the device tree. Today you need
> to know only about this set of parameters, tomorrow you need something
> new, but you also need to work with old DT blobs... It's a mess.

I strongly agree, it is much better to just know what the hardware is
and set any properties based off that so we don't need to update
firmwares to take advantage of additional features or fix quirks that
are discovered.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 484 bytes --]

Re: [PATCH] spi: spi-fsl-dspi: Add ACPI support

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 910 bytes --]

On Wed, Aug 26, 2020 at 11:41:08PM +0300, Vladimir Oltean wrote:
> On Wed, Aug 26, 2020 at 10:34:04PM +0300, Andy Shevchenko wrote:

> > Theoretically you may declare your HID in the same / similar way as
> > PRP0001 and use same compatible strings and all other DT properties
> > (when they make sense and not duplicate ACPI functionality).
> > But better if ACPI people can tell you (I Cc'ed Rafael and ACPI
> > mailing list) if it is gonna work.

> Something doesn't look right about PRP0001, what's the catch?

Microsoft decided not to implement support for it in Windows, it's
essentially there for embedded style x86 platforms running Linux so they
don't need to reimplement so many wheels and can just reuse existing DT
bindings but it causes problems if you want to run Windows (and possibly
some of the enterprise Linux distros, I can't remember if any of them
had concerns about it) on the platform.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] spi: spi-fsl-dspi: Add ACPI support

[Vladimir Oltean]

On Wed, Aug 26, 2020 at 09:45:47PM +0100, Mark Brown wrote:
> On Wed, Aug 26, 2020 at 11:41:08PM +0300, Vladimir Oltean wrote:
> > On Wed, Aug 26, 2020 at 10:34:04PM +0300, Andy Shevchenko wrote:
>
> > > Theoretically you may declare your HID in the same / similar way as
> > > PRP0001 and use same compatible strings and all other DT properties
> > > (when they make sense and not duplicate ACPI functionality).
> > > But better if ACPI people can tell you (I Cc'ed Rafael and ACPI
> > > mailing list) if it is gonna work.
>
> > Something doesn't look right about PRP0001, what's the catch?
>
> Microsoft decided not to implement support for it in Windows, it's
> essentially there for embedded style x86 platforms running Linux so they
> don't need to reimplement so many wheels and can just reuse existing DT
> bindings but it causes problems if you want to run Windows (and possibly
> some of the enterprise Linux distros, I can't remember if any of them
> had concerns about it) on the platform.

So if a silicon vendor doesn't care about Windows, what incentive does
it have to even register an official ACPI/PNP ID for its devices?

Re: [PATCH] spi: spi-fsl-dspi: Add ACPI support

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1326 bytes --]

On Thu, Aug 27, 2020 at 12:06:57AM +0300, Vladimir Oltean wrote:
> On Wed, Aug 26, 2020 at 09:45:47PM +0100, Mark Brown wrote:
> > On Wed, Aug 26, 2020 at 11:41:08PM +0300, Vladimir Oltean wrote:

> > > Something doesn't look right about PRP0001, what's the catch?

> > Microsoft decided not to implement support for it in Windows, it's
> > essentially there for embedded style x86 platforms running Linux so they
> > don't need to reimplement so many wheels and can just reuse existing DT
> > bindings but it causes problems if you want to run Windows (and possibly
> > some of the enterprise Linux distros, I can't remember if any of them
> > had concerns about it) on the platform.

> So if a silicon vendor doesn't care about Windows, what incentive does
> it have to even register an official ACPI/PNP ID for its devices?

Not that there's any registration process or anything, there's some
namespacing but that's it, but the main thing would just be keeping the
ACPI bindings and DT bindings separate.  ACPI has some strong opinions
on how systems are built and described so while you can use the PRP0001
stuff to parse DT bindings on an ACPI system it doesn't alway fit well,
and there are some things where you just plain shouldn't use PRP0001
since the ACPI and DT models for that sort of device diverge so strongly.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]