linux-kernel - Re: [PATCH v5 15/17] dt-bindings: qca7000: append UART interface to binding

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1702237.66ccflAQAJ@kerker>
Date:   Thu, 11 May 2017 21:12:22 +0200
From:   Michael Heimpold <mhei@...mpold.de>
To:     Rob Herring <robh@...nel.org>
Cc:     Stefan Wahren <stefan.wahren@...e.com>,
        "David S. Miller" <davem@...emloft.net>,
        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,

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 :-)

Thanks,
Michael

> +
> +Optional properties:
> +- local-mac-address : see ./ethernet.txt
> +- current-speed     : current baud rate of QCA7000 which defaults to 115200
> +		      if absent, see also ../serial/slave-device.txt
> +
> +UART Example:
> +
> +/* Freescale i.MX28 UART */
> +auart0: serial@...6a000 {
> +	compatible = "fsl,imx28-auart", "fsl,imx23-auart";
> +	reg = <0x8006a000 0x2000>;
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&auart0_2pins_a>;
> +	status = "okay";
> +
> +	qca7000: ethernet {
> +		compatible = "qca,qca7000-uart";
> +		local-mac-address = [ A0 B0 C0 D0 E0 F0 ];
> +		current-speed = <38400>;
> +	};
> +};