| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Fri, 19 May 2017 09:13:53 +0200
From: Stefan Wahren <stefan.wahren@...e.com>
To: Rob Herring <robh@...nel.org>
Cc: Jakub Kicinski <kubakici@...pl>,
Michael Heimpold <mhei@...mpold.de>,
Mark Rutland <mark.rutland@....com>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Jiri Slaby <jslaby@...e.com>, linux-serial@...r.kernel.org,
linux-kernel@...r.kernel.org, netdev@...r.kernel.org,
devicetree@...r.kernel.org
Subject: Re: [PATCH v5 15/17] dt-bindings: qca7000: append UART interface to
binding
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@...pl>:
>>
>>> 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@...e.com>
>>>>> ---
>>>>> .../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@...14000 {
>>>>> 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?
Should we mark "qca,qca7000-spi" as deprecated?
Regards
Stefan
Powered by blists - more mailing lists