| Message ID | 1467027248-6191-1-git-send-email-mika.westerberg@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Jun 27, 2016 at 02:34:08PM +0300, Mika Westerberg wrote: > Allow the same devices to be used in ACPI based systems if they provide > proper DT compatible string. Please allow a reasonable time for review, especially for an invasive change like this which needs a bunch of research to figure out how sensible the infrastructure to shove DT into ACPI is.
On Mon, Jun 27, 2016 at 01:24:48PM +0100, Mark Brown wrote: > On Mon, Jun 27, 2016 at 02:34:08PM +0300, Mika Westerberg wrote: > > > Allow the same devices to be used in ACPI based systems if they provide > > proper DT compatible string. > > Please allow a reasonable time for review, especially for an invasive > change like this which needs a bunch of research to figure out how > sensible the infrastructure to shove DT into ACPI is. Sure. Can I ask your opinion regarding an alternative approach to this? Instead of adding these "pseudo", Linux specific devices to DT or ACPI to support raw SPI access, what if we amend spidev a bit: - Introduce CONFIG_SPI_SPIDEV_MASTER which is part of spidev.c and can be selected separately. - If that option is set we create /dev/spi-<bus_num> for each SPI master. - Introduce two new ioctls SPI_IOC_RD/WR_CS which allow setting of chip select. The /dev/spi-0 then can be used analogous to /dev/i2c-0. You first need to program wanted chip select using those new ioctls. Then you can use the device as normal spidev (all existing file operations and ioctls still work). You can pick another chip select as needed. If there is an actual SPI device bound to a chip select, that cannot be used through /dev/spi-0. -- To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Jun 28, 2016 at 12:22:34PM +0300, Mika Westerberg wrote: > On Mon, Jun 27, 2016 at 01:24:48PM +0100, Mark Brown wrote: > > Please allow a reasonable time for review, especially for an invasive > > change like this which needs a bunch of research to figure out how > > sensible the infrastructure to shove DT into ACPI is. > Sure. The big thing I need to check out here is if this breaks the check for directly listing spidev in DT so we end up with the same abstraction failures that plague DT since people refuse to describe their hardware. > The /dev/spi-0 then can be used analogous to /dev/i2c-0. You first need > to program wanted chip select using those new ioctls. Then you can use > the device as normal spidev (all existing file operations and ioctls > still work). You can pick another chip select as needed. If there is an > actual SPI device bound to a chip select, that cannot be used through > /dev/spi-0. The trouble with this is that unlike I2C using SPI devices involves toggling signals that we don't definitively know are connected to the bus. It's therefore much less safe than the equivalent thing for DT. If we went the extra step and required that a slave be listed in DT then that is much less of a concern.
On Tue, Jun 28, 2016 at 12:47:42PM +0100, Mark Brown wrote: > On Tue, Jun 28, 2016 at 12:22:34PM +0300, Mika Westerberg wrote: > > On Mon, Jun 27, 2016 at 01:24:48PM +0100, Mark Brown wrote: > > > > Please allow a reasonable time for review, especially for an invasive > > > change like this which needs a bunch of research to figure out how > > > sensible the infrastructure to shove DT into ACPI is. > > > Sure. > > The big thing I need to check out here is if this breaks the check for > directly listing spidev in DT so we end up with the same abstraction > failures that plague DT since people refuse to describe their hardware. You are talking about this check, right? if (spi->dev.of_node && !of_match_device(spidev_dt_ids, &spi->dev)) { dev_err(&spi->dev, "buggy DT: spidev listed directly in DT\n"); WARN_ON(spi->dev.of_node && !of_match_device(spidev_dt_ids, &spi->dev)); } As far as I can tell it should work, with the patch applied, just as it worked before. For ACPI there is no possibility to list the node directly in ASL code as we always require DT compatible string in _DSD. In other words we never match using name of the node. > > The /dev/spi-0 then can be used analogous to /dev/i2c-0. You first need > > to program wanted chip select using those new ioctls. Then you can use > > the device as normal spidev (all existing file operations and ioctls > > still work). You can pick another chip select as needed. If there is an > > actual SPI device bound to a chip select, that cannot be used through > > /dev/spi-0. > > The trouble with this is that unlike I2C using SPI devices involves > toggling signals that we don't definitively know are connected to the > bus. It's therefore much less safe than the equivalent thing for DT. > If we went the extra step and required that a slave be listed in DT then > that is much less of a concern. OK, I see. -- To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Jun 28, 2016 at 05:35:28PM +0300, Mika Westerberg wrote: > On Tue, Jun 28, 2016 at 12:47:42PM +0100, Mark Brown wrote: > > The big thing I need to check out here is if this breaks the check for > > directly listing spidev in DT so we end up with the same abstraction > > failures that plague DT since people refuse to describe their hardware. > You are talking about this check, right? > if (spi->dev.of_node && !of_match_device(spidev_dt_ids, &spi->dev)) { > dev_err(&spi->dev, "buggy DT: spidev listed directly in DT\n"); > WARN_ON(spi->dev.of_node && > !of_match_device(spidev_dt_ids, &spi->dev)); > } > As far as I can tell it should work, with the patch applied, just as it worked > before. For ACPI there is no possibility to list the node directly in ASL code > as we always require DT compatible string in _DSD. Yes, but does of_node get set and does of_match_device() DTRT if it's a DT into ACPI device? > In other words we never match using name of the node. If that's the case then it's definitely broken - we either always warn or never warn, neither is good.
On Tue, Jun 28, 2016 at 04:07:52PM +0100, Mark Brown wrote: > On Tue, Jun 28, 2016 at 05:35:28PM +0300, Mika Westerberg wrote: > > On Tue, Jun 28, 2016 at 12:47:42PM +0100, Mark Brown wrote: > > > > The big thing I need to check out here is if this breaks the check for > > > directly listing spidev in DT so we end up with the same abstraction > > > failures that plague DT since people refuse to describe their hardware. > > > You are talking about this check, right? > > > if (spi->dev.of_node && !of_match_device(spidev_dt_ids, &spi->dev)) { > > dev_err(&spi->dev, "buggy DT: spidev listed directly in DT\n"); > > WARN_ON(spi->dev.of_node && > > !of_match_device(spidev_dt_ids, &spi->dev)); > > } > > > As far as I can tell it should work, with the patch applied, just as it worked > > before. For ACPI there is no possibility to list the node directly in ASL code > > as we always require DT compatible string in _DSD. > > Yes, but does of_node get set and does of_match_device() DTRT if it's a > DT into ACPI device? of_node will be NULL and of_match_device() will not match (returns NULL as well). > > In other words we never match using name of the node. > > If that's the case then it's definitely broken - we either always warn > or never warn, neither is good. For DT we warn if the device is used directly (by the node name) as previously. For ACPI we never warn because it is not possible to use the node name directly. We always require use of a proper compatible string. I don't see how that is broken. -- To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Jun 28, 2016 at 06:23:38PM +0300, Mika Westerberg wrote: > On Tue, Jun 28, 2016 at 04:07:52PM +0100, Mark Brown wrote: > > If that's the case then it's definitely broken - we either always warn > > or never warn, neither is good. > For DT we warn if the device is used directly (by the node name) as > previously. For ACPI we never warn because it is not possible to use the > node name directly. We always require use of a proper compatible string. > I don't see how that is broken. So you're saying that it's not possible for ACPI to use anything *except* an explicitly listed compatible string to bind? What we want to avoid is any ACPI tables that explicitly list spidev since that's a total abstraction failure.
On Tue, Jun 28, 2016 at 05:18:22PM +0100, Mark Brown wrote: > On Tue, Jun 28, 2016 at 06:23:38PM +0300, Mika Westerberg wrote: > > On Tue, Jun 28, 2016 at 04:07:52PM +0100, Mark Brown wrote: > > > > If that's the case then it's definitely broken - we either always warn > > > or never warn, neither is good. > > > For DT we warn if the device is used directly (by the node name) as > > previously. For ACPI we never warn because it is not possible to use the > > node name directly. We always require use of a proper compatible string. > > > I don't see how that is broken. > > So you're saying that it's not possible for ACPI to use anything > *except* an explicitly listed compatible string to bind? What we want > to avoid is any ACPI tables that explicitly list spidev since that's a > total abstraction failure. That's exactly what I'm saying. We never match using the node name (which is always 32-bit "string" like "SPI0"). For PRP0001 match to succeeed you need to have "compatible" property. The ASL code looks like: Device (SPID) { Name (_HID, "PRP0001") Name (_CRS, ResourceTempate () { SpiSerialBus (1, PolarityLow, FourWireMode, 0x08, ControllerInitiated, 0x007A1200, ClockPolarityLow, ClockPhaseSecond, "\\_SB.SPI1", 0x00, ResourceConsumer) }) Name (_DSD, Package () { ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"), Package () { Package () {"compatible", "rohm,dh2228fv"}, } }) } Without the "compatible" property you get a warning from ACPI core: \_SB.SPI1.SPID requires 'compatible' property and it will not match anything. -- To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jun 29, 2016 at 12:13:04PM +0300, Mika Westerberg wrote: > On Tue, Jun 28, 2016 at 05:18:22PM +0100, Mark Brown wrote: > > So you're saying that it's not possible for ACPI to use anything > > *except* an explicitly listed compatible string to bind? What we want > > to avoid is any ACPI tables that explicitly list spidev since that's a > > total abstraction failure. > That's exactly what I'm saying. We never match using the node name > (which is always 32-bit "string" like "SPI0"). For PRP0001 match to > succeeed you need to have "compatible" property. > Without the "compatible" property you get a warning from ACPI core: > \_SB.SPI1.SPID requires 'compatible' property > and it will not match anything. But can that compatible string be spidev and be matched via the paths DT uses while still ensuring that enough of the OF matching code is enabled to cause the check to warn? That's the problem. Life would be a lot eaiser if Intel would use DT where it's appropriate, or if the OF in ACPI stuff were more transparent and needed fewer special cases.
On Wed, Jun 29, 2016 at 05:21:27PM +0100, Mark Brown wrote: > On Wed, Jun 29, 2016 at 12:13:04PM +0300, Mika Westerberg wrote: > > On Tue, Jun 28, 2016 at 05:18:22PM +0100, Mark Brown wrote: > > > > So you're saying that it's not possible for ACPI to use anything > > > *except* an explicitly listed compatible string to bind? What we want > > > to avoid is any ACPI tables that explicitly list spidev since that's a > > > total abstraction failure. > > > That's exactly what I'm saying. We never match using the node name > > (which is always 32-bit "string" like "SPI0"). For PRP0001 match to > > succeeed you need to have "compatible" property. > > > Without the "compatible" property you get a warning from ACPI core: > > > \_SB.SPI1.SPID requires 'compatible' property > > > and it will not match anything. > > But can that compatible string be spidev and be matched via the paths DT > uses while still ensuring that enough of the OF matching code is enabled > to cause the check to warn? That's the problem. No it can't. The match is done in ACPI code not in DT code. See drivers/acpi/bus.c::acpi_of_match_device(). > Life would be a lot eaiser if Intel would use DT where it's appropriate, > or if the OF in ACPI stuff were more transparent and needed fewer > special cases. I don't get this. We added PRP0001 and properties because then we can reuse DT bindings in ACPI and thus there is no need to reinvent all these things for ACPI. This is not a special case at all. I'm merely enabling the same thing to work in ACPI systems (sans the node name match). -- To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jun 29, 2016 at 07:34:20PM +0300, Mika Westerberg wrote: > On Wed, Jun 29, 2016 at 05:21:27PM +0100, Mark Brown wrote: > > But can that compatible string be spidev and be matched via the paths DT > > uses while still ensuring that enough of the OF matching code is enabled > > to cause the check to warn? That's the problem. > No it can't. The match is done in ACPI code not in DT code. See > drivers/acpi/bus.c::acpi_of_match_device(). And we're *sure* that's going to be maintained? People do use the fallback matching that DT does, I don't trust the ACPI maintainers not to do the same thing. > > Life would be a lot eaiser if Intel would use DT where it's appropriate, > > or if the OF in ACPI stuff were more transparent and needed fewer > > special cases. > I don't get this. We added PRP0001 and properties because then we can > reuse DT bindings in ACPI and thus there is no need to reinvent all > these things for ACPI. Right, but rather than just define a translation from ACPI to DT and use the DT code paths in their entirety for the relevant nodes what's happening is that there's a shim layer between ACPI and DT which sort of emulates bits of the DT interfaces but not quite and can lead to surprises. > This is not a special case at all. I'm merely enabling the same thing to > work in ACPI systems (sans the node name match). The force DT into ACPI layer that exists in the ACPI code is the special case here.
On Wed, Jun 29, 2016 at 07:31:01PM +0100, Mark Brown wrote: > On Wed, Jun 29, 2016 at 07:34:20PM +0300, Mika Westerberg wrote: > > On Wed, Jun 29, 2016 at 05:21:27PM +0100, Mark Brown wrote: > > > > But can that compatible string be spidev and be matched via the paths DT > > > uses while still ensuring that enough of the OF matching code is enabled > > > to cause the check to warn? That's the problem. > > > No it can't. The match is done in ACPI code not in DT code. See > > drivers/acpi/bus.c::acpi_of_match_device(). > > And we're *sure* that's going to be maintained? People do use the > fallback matching that DT does, I don't trust the ACPI maintainers not > to do the same thing. That fallback matching does not even work in ACPI like I told you already. We have no plans to do anything like that either. That's the reason why we complain if there is PRP0001 without compatible string. > > > Life would be a lot eaiser if Intel would use DT where it's appropriate, > > > or if the OF in ACPI stuff were more transparent and needed fewer > > > special cases. > > > I don't get this. We added PRP0001 and properties because then we can > > reuse DT bindings in ACPI and thus there is no need to reinvent all > > these things for ACPI. > > Right, but rather than just define a translation from ACPI to DT and use > the DT code paths in their entirety for the relevant nodes what's > happening is that there's a shim layer between ACPI and DT which sort of > emulates bits of the DT interfaces but not quite and can lead to > surprises. Maybe but currently it works fine. And if problems are found they will be fixed as usual. -- To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jun 29, 2016 at 09:51:55PM +0300, Mika Westerberg wrote: > On Wed, Jun 29, 2016 at 07:31:01PM +0100, Mark Brown wrote: > > And we're *sure* that's going to be maintained? People do use the > > fallback matching that DT does, I don't trust the ACPI maintainers not > > to do the same thing. > That fallback matching does not even work in ACPI like I told you > already. We have no plans to do anything like that either. That's the > reason why we complain if there is PRP0001 without compatible string. No, you're completely missing the point here. The problem is someone using something like linux,spidev as a compatible string and going into fallback matching on SPI bus IDs rather than compatible strings. I just don't have confidence that someone isn't going to try to add that fallback path given that it's used for DT. > > Right, but rather than just define a translation from ACPI to DT and use > > the DT code paths in their entirety for the relevant nodes what's > > happening is that there's a shim layer between ACPI and DT which sort of > > emulates bits of the DT interfaces but not quite and can lead to > > surprises. > Maybe but currently it works fine. And if problems are found they will > be fixed as usual. I'm worried that the reason it's working fine may be that there is very little usage, and that there may be more gotchas like we're already seeing lurking about.
On Wed, Jun 29, 2016 at 08:20:59PM +0100, Mark Brown wrote: > On Wed, Jun 29, 2016 at 09:51:55PM +0300, Mika Westerberg wrote: > > On Wed, Jun 29, 2016 at 07:31:01PM +0100, Mark Brown wrote: > > > > And we're *sure* that's going to be maintained? People do use the > > > fallback matching that DT does, I don't trust the ACPI maintainers not > > > to do the same thing. > > > That fallback matching does not even work in ACPI like I told you > > already. We have no plans to do anything like that either. That's the > > reason why we complain if there is PRP0001 without compatible string. > > No, you're completely missing the point here. The problem is someone > using something like linux,spidev as a compatible string and going into > fallback matching on SPI bus IDs rather than compatible strings. I just > don't have confidence that someone isn't going to try to add that > fallback path given that it's used for DT. If someone is using "linux,spidev" as compatible string it will not match anything and we are not going to add any kind of fallback to the ACPI core for that. Of course we cannot predict what happens in the future but is it really preventing merging of this patch? What I'm simply trying to achieve is to use the existing two DT compatible strings from ACPI. I've tested that it works fine and provided you all the evidence that it does not break the check for misusing DT. > > > Right, but rather than just define a translation from ACPI to DT and use > > > the DT code paths in their entirety for the relevant nodes what's > > > happening is that there's a shim layer between ACPI and DT which sort of > > > emulates bits of the DT interfaces but not quite and can lead to > > > surprises. > > > Maybe but currently it works fine. And if problems are found they will > > be fixed as usual. > > I'm worried that the reason it's working fine may be that there is very > little usage, and that there may be more gotchas like we're already > seeing lurking about. With more usage I'm sure we can improve things. After all the end goal is to let single driver to work on both DT and ACPI systems with minimal changes. If you think the current implementation is flawed, nothing prevents you from proposing better alternative :-) -- To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Jun 30, 2016 at 03:49:31PM +0300, Mika Westerberg wrote: > If someone is using "linux,spidev" as compatible string it will not > match anything and we are not going to add any kind of fallback to the > ACPI core for that. > Of course we cannot predict what happens in the future but is it really > preventing merging of this patch? Yes. People are *way* too happy to abuse spidev in board descriptions, especially people working in the areas these boards seem to target. There seems to be little engagement with the abstractions involved or interest in keeping things working well long term in more general applications, it's all about quick hacks now. Given the push for compatibility with DT and reuse of the existing software stack I just don't have any confidence that the fallback path isn't going to be implemented, and relying on anyone noticing that these warnings are broken when it does get added (or that spidev is affected at all) just doesn't seem wise to me. > > I'm worried that the reason it's working fine may be that there is very > > little usage, and that there may be more gotchas like we're already > > seeing lurking about. > With more usage I'm sure we can improve things. After all the end goal > is to let single driver to work on both DT and ACPI systems with minimal > changes. If you think the current implementation is flawed, nothing > prevents you from proposing better alternative :-) I've been suggesting that you should just use DT!
On Thu, Jun 30, 2016 at 03:49:31PM +0300, Mika Westerberg wrote: > On Wed, Jun 29, 2016 at 08:20:59PM +0100, Mark Brown wrote: > > On Wed, Jun 29, 2016 at 09:51:55PM +0300, Mika Westerberg wrote: > > > On Wed, Jun 29, 2016 at 07:31:01PM +0100, Mark Brown wrote: > > > > > > And we're *sure* that's going to be maintained? People do use the > > > > fallback matching that DT does, I don't trust the ACPI maintainers not > > > > to do the same thing. > > > > > That fallback matching does not even work in ACPI like I told you > > > already. We have no plans to do anything like that either. That's the > > > reason why we complain if there is PRP0001 without compatible string. > > > > No, you're completely missing the point here. The problem is someone > > using something like linux,spidev as a compatible string and going into > > fallback matching on SPI bus IDs rather than compatible strings. I just > > don't have confidence that someone isn't going to try to add that > > fallback path given that it's used for DT. > > If someone is using "linux,spidev" as compatible string it will not > match anything and we are not going to add any kind of fallback to the > ACPI core for that. > > Of course we cannot predict what happens in the future but is it really > preventing merging of this patch? > > What I'm simply trying to achieve is to use the existing two DT > compatible strings from ACPI. I've tested that it works fine and > provided you all the evidence that it does not break the check for > misusing DT. Actually there seems to be alternative to this. Windows seems to have a similar spidev raw interface in their MITT test suite here: https://msdn.microsoft.com/fi-fi/windows/hardware/drivers/spb/spi-tests-in-mitt It exposes three ACPI SPI devices with ACPI IDs of SPT0001, SPT0002 and SPT0003. I'm thinking that instead of using the existing DT compatible strings we could use these ACPI IDs in the driver. -- To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Jul 01, 2016 at 01:19:12PM +0300, Mika Westerberg wrote: > Windows seems to have a similar spidev raw interface in their MITT test > suite here: > https://msdn.microsoft.com/fi-fi/windows/hardware/drivers/spb/spi-tests-in-mitt > It exposes three ACPI SPI devices with ACPI IDs of SPT0001, SPT0002 and > SPT0003. I'm thinking that instead of using the existing DT compatible > strings we could use these ACPI IDs in the driver. Ugh :( But yes, if there's existing ACPI IDs for this functionality clearly we should support those. Probably with an equivalent warning about how they're only for non-production systems.
On Fri, Jul 01, 2016 at 04:50:35PM +0200, Mark Brown wrote: > On Fri, Jul 01, 2016 at 01:19:12PM +0300, Mika Westerberg wrote: > > > Windows seems to have a similar spidev raw interface in their MITT test > > suite here: > > > https://msdn.microsoft.com/fi-fi/windows/hardware/drivers/spb/spi-tests-in-mitt > > > It exposes three ACPI SPI devices with ACPI IDs of SPT0001, SPT0002 and > > SPT0003. I'm thinking that instead of using the existing DT compatible > > strings we could use these ACPI IDs in the driver. > > Ugh :( But yes, if there's existing ACPI IDs for this functionality > clearly we should support those. Probably with an equivalent warning > about how they're only for non-production systems. OK, good :) Let me explain some background why I'm doing this. Maybe it brings better alternatives. There are these boards for Makers and IoT stuff which basically have pin header where you can connect different low speed peripherals, like sensors and so on. The main point is that you don't always know beforehand what devices will be connected to the board. Now, it seems that IoT/Maker folks solved this in userspace so that they are not using existing drivers provided by Linux kernel but instead they are using raw access to buses like I2C and SPI, and provide their own "drivers" for those peripherals. For I2C it is easy because we have i2c-dev and it does not require any kind of firmware support. For SPI we need to be more careful because of bus signals like chip selects. Instead of writing out-of-tree board files to provide proper configuration for SPI (spidev) we can at least try to take advantage of the boot firmware, like ACPI which already has a way to describe devices connected to SPI bus. Now, since spidev is just Linux software abstraction for raw access to the SPI bus I don't think it is good idea to allocate special ACPI ID just for that - it does not describe hardware. So instead I'm thinking we could re-use those Windows ACPI IDs in the driver. With this we can either stick these devices with the boot firmware shipping with boards or alternatively provide overlays which can be loaded to the existing firmware as needed. Users of these boards can then take mainline Linux and use whatever existing IoT userspace components. I'm going to prepare a new patch adding these ACPI IDs to the spidev early next week. -- To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sat, Jul 02, 2016 at 12:01:14PM +0300, Mika Westerberg wrote: > There are these boards for Makers and IoT stuff which basically have pin > header where you can connect different low speed peripherals, like > sensors and so on. The main point is that you don't always know > beforehand what devices will be connected to the board. Now, it seems Sure, that's where most of this crap comes from. > that IoT/Maker folks solved this in userspace so that they are not using > existing drivers provided by Linux kernel but instead they are using raw > access to buses like I2C and SPI, and provide their own "drivers" for > those peripherals. ...which is of course a terrible solution which results in a huge amount of wheel reinvention and effort duplication. The best practice here is to use overlays, there's people doing active work on standardizing connectors for DT at the minute which will help a lot there but engagement from the maker community in general seems depressingly poor even though some of this stuff ends up getting productized. Unfortunately you're going to have to redo all the overlay work for ACPI to use it on x86. What would be really nice would be if the schematic capture tools were able to output DT overlays automatically, that'd make it really simple to design an add on board/circuit and get the software support up with no bother. > Now, since spidev is just Linux software abstraction for raw access to > the SPI bus I don't think it is good idea to allocate special ACPI ID > just for that - it does not describe hardware. So instead I'm thinking > we could re-use those Windows ACPI IDs in the driver. Yes, exactly - spidev should never appear directly in ACPI or DT since our ideas about the best way to control the hardware may change. > With this we can either stick these devices with the boot firmware > shipping with boards or alternatively provide overlays which can be > loaded to the existing firmware as needed. Users of these boards can > then take mainline Linux and use whatever existing IoT userspace > components. It really needs to be overlays, not everyone using these boards is doing the roll your own driver stuff - they also get used by people developing more substantial add on modules for example, or people prototyping more real systems who want normal software support. > I'm going to prepare a new patch adding these ACPI IDs to the spidev > early next week. Please also ensure that we complain loudly when we use them like we do for spidev. We shouldn't be encouraging people to develop software like this.
diff --git a/drivers/spi/spidev.c b/drivers/spi/spidev.c index e3c19f30f591..86a4c28b9e58 100644 --- a/drivers/spi/spidev.c +++ b/drivers/spi/spidev.c @@ -691,14 +691,12 @@ static const struct file_operations spidev_fops = { static struct class *spidev_class; -#ifdef CONFIG_OF static const struct of_device_id spidev_dt_ids[] = { { .compatible = "rohm,dh2228fv" }, { .compatible = "lineartechnology,ltc2488" }, {}, }; MODULE_DEVICE_TABLE(of, spidev_dt_ids); -#endif /*-------------------------------------------------------------------------*/ @@ -788,7 +786,7 @@ static int spidev_remove(struct spi_device *spi) static struct spi_driver spidev_spi_driver = { .driver = { .name = "spidev", - .of_match_table = of_match_ptr(spidev_dt_ids), + .of_match_table = spidev_dt_ids, }, .probe = spidev_probe, .remove = spidev_remove,
Allow the same devices to be used in ACPI based systems if they provide proper DT compatible string. Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com> --- drivers/spi/spidev.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)