On Fri, May 19, 2017 at 2:13 AM, Stefan Wahren <stefan.wahren@xxxxxxxx> wrote: > Hi Rob, > > Am 12.05.2017 um 08:43 schrieb Jakub Kicinski: >> On Fri, 12 May 2017 06:15:52 +0000, Michael Heimpold wrote: >>> Hi, >>> >>> Zitat von Jakub Kicinski <kubakici@xxxxx>: >>> >>>> On Thu, 11 May 2017 21:12:22 +0200, Michael Heimpold wrote: >>>>> Am Mittwoch, 10. Mai 2017, 10:53:26 CEST schrieb Stefan Wahren: >>>>>> This merges the serdev binding for the QCA7000 UART driver (Ethernet over >>>>>> UART) into the existing document. >>>>>> >>>>>> Signed-off-by: Stefan Wahren <stefan.wahren@xxxxxxxx> >>>>>> --- >>>>>> .../devicetree/bindings/net/qca-qca7000.txt | 32 >>>>>> ++++++++++++++++++++++ 1 file changed, 32 insertions(+) >>>>>> >>>>>> diff --git a/Documentation/devicetree/bindings/net/qca-qca7000.txt >>>>>> b/Documentation/devicetree/bindings/net/qca-qca7000.txt index >>>>>> a37f656..08364c3 100644 >>>>>> --- a/Documentation/devicetree/bindings/net/qca-qca7000.txt >>>>>> +++ b/Documentation/devicetree/bindings/net/qca-qca7000.txt >>>>>> @@ -54,3 +54,35 @@ ssp2: spi@80014000 { >>>>>> local-mac-address = [ A0 B0 C0 D0 E0 F0 ]; >>>>>> }; >>>>>> }; >>>>>> + >>>>>> +(b) Ethernet over UART >>>>>> + >>>>>> +In order to use the QCA7000 as UART slave it must be defined as >>>>> a child of >>>>>> a +UART master in the device tree. It is possible to preconfigure the UART >>>>>> +settings of the QCA7000 firmware, but it's not possible to change them >>>>>> during +runtime. >>>>>> + >>>>>> +Required properties: >>>>>> +- compatible : Should be "qca,qca7000-uart" >>>>> I already discussed this with Stefan off-list a little bit, but I would like >>>>> to bring this to a broader audience: I'm not sure whether the compatible >>>>> should contain the "-uart" suffix, because the hardware chip is the >>>>> very same >>>>> QCA7000 chip which can also be used with SPI protocol. >>>>> The only difference is the loaded firmware within the chip which can either >>>>> speak SPI or UART protocol (but not both at the same time - due to shared >>>>> pins). So the hardware design decides which interface type is used. >>>>> >>>>> At the moment, this patch series adds a dedicated driver for the UART >>>>> protocol, in parallel to the existing SPI driver. So a different compatible >>>>> string is needed here to match against the new driver. >>>>> >>>>> An alternative approach would be to re-use the existing compatible string >>>>> "qca,qca7000" for both, the SPI and UART protocol, because a "smarter" >>>>> (combined) driver would detect which protocol to use. For example the driver >>>>> could check for spi-cpha and/or spi-cpol which are required for SPI >>>>> protocol: >>>>> if these exists the driver could assume that SPI must be used, if both are >>>>> missing then UART protocol should be used. >>>>> (This way it would not be necessary to check whether the node is a child of >>>>> a SPI or UART master node - but maybe this is even easier - I don't know) >>>>> >>>>> Or in shorter words: my concern is that while "qca7000-uart" describes the >>>>> hardware, it's too closely coupled to the driver implementation. Having >>>>> some feedback of the experts would be nice :-) >>>> I'm no expert, but devices which can do both I2C and SPI are quite >>>> common, and they usually have the same compatible string for both >>>> buses. >>> do you have an example driver at hand? I only found GPIO mcp23s08 driver, >>> which can handle both I2C and SPI chips, but there are different compatible >>> strings used to distinguish several chip models. >> I think drivers/tty/serial/sc16is7xx.c has the same strings, and some >> Kconfig magic to work when either bus is enabled in .config. >> >> Quick grep shows there are couple more potential ones to look at: >> >> $ find . -name Kconfig | xargs grep -n 'SPI_MASTER.* I2C' >> ./drivers/tty/serial/Kconfig:1208: depends on (SPI_MASTER && !I2C) || I2C >> ./drivers/mfd/Kconfig:327: depends on (SPI_MASTER || I2C) >> ./drivers/iio/dac/Kconfig:10: depends on (SPI_MASTER && I2C!=m) || I2C >> ./drivers/iio/dac/Kconfig:34: depends on (SPI_MASTER && I2C!=m) || I2C >> ./drivers/iio/dac/Kconfig:57: depends on (SPI_MASTER && I2C!=m) || I2C >> ./drivers/gpio/Kconfig:1231: depends on (SPI_MASTER && !I2C) || I2C >> $ find . -name Kconfig | xargs grep -n 'I2C.*||.*SPI_MASTER' >> ./drivers/mfd/Kconfig:1094: depends on (I2C=y || SPI_MASTER=y) >> ./drivers/iio/gyro/Kconfig:55: depends on (I2C || SPI_MASTER) >> ./drivers/iio/gyro/Kconfig:107: depends on (I2C || SPI_MASTER) && SYSFS >> ./drivers/iio/accel/Kconfig:153: depends on (I2C || SPI_MASTER) && SYSFS >> ./drivers/iio/pressure/Kconfig:20: depends on (I2C || SPI_MASTER) >> ./drivers/iio/pressure/Kconfig:161: depends on (I2C || SPI_MASTER) && SYSFS >> ./drivers/iio/magnetometer/Kconfig:118: depends on (I2C || SPI_MASTER) && SYSFS >> >> drivers/mfd/mc13xxx-*.c seems to have the same strings. The iio/dac drivers >> don't support DT but do share names. The MCP GPIO chip you mention indeed has >> different product names based on the bus it's made for (0 vs s in the middle >> of the name), so I gather less relevant case? drivers/iio/pressure/bmp280-*.c >> has the same strings, if I'm looking correctly... I didn't look at the others. > > are you okay with the suggestion to use the compatible "qca,qca7000" for > both drivers? I believe I said I was fine either way. > Should we mark "qca,qca7000-spi" as deprecated? Yes. Rob -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html