From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751692AbdESHO3 (ORCPT ); Fri, 19 May 2017 03:14:29 -0400 Received: from mout.kundenserver.de ([217.72.192.73]:49588 "EHLO mout.kundenserver.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750793AbdESHOX (ORCPT ); Fri, 19 May 2017 03:14:23 -0400 Subject: Re: [PATCH v5 15/17] dt-bindings: qca7000: append UART interface to binding To: Rob Herring References: <1494406408-31760-1-git-send-email-stefan.wahren@i2se.com> <1494406408-31760-16-git-send-email-stefan.wahren@i2se.com> <1702237.66ccflAQAJ@kerker> <20170511194541.4e58cb7d@cakuba.netronome.com> <20170512061552.Horde.ggjCMHnIsr8LeADwTQwhnx7@www.mhei.heimpold.org> <20170511234342.575d9226@cakuba.netronome.com> Cc: Jakub Kicinski , Michael Heimpold , Mark Rutland , Greg Kroah-Hartman , Jiri Slaby , linux-serial@vger.kernel.org, linux-kernel@vger.kernel.org, netdev@vger.kernel.org, devicetree@vger.kernel.org From: Stefan Wahren Message-ID: <53509108-fac2-8e8d-9279-ed54e8c1af3b@i2se.com> Date: Fri, 19 May 2017 09:13:53 +0200 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: <20170511234342.575d9226@cakuba.netronome.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-Provags-ID: V03:K0:g/LUenJwiYkWAnE9Yv7yNw1OmFp/LCm9akbZ6up1d5tZIO+1jn6 EgKr9Rh1wKUcyzDbLxiQ+zBhye1myVt8Tgcl6b6sHZav9s4yFdGCZ67UJsOojW3cKGgSXDZ I5EofG/OAwASwtMOzYDbsUfOWsCQJjftsC9lcrAvreBcTYoJVxGN4bdX4dtdnV02qTZz6o/ WqpMaWkQ0v+S2s6ppe23w== X-UI-Out-Filterresults: notjunk:1;V01:K0:B2PNcw3SZsE=:7IPVNm+1IOReixcjqj2WPe QiP5xwn/CxZ7k4/CMfN29Rku2KE36UVTOIN4PKARen7mN8Wjc1+RAJIZ4Ed4mdTzZlmWRytoy OkiwCuh8HPcYOybXuLWJgK9+VWCuhLiMWsZFNQ3vmGPOZg/yncXMPBXyIHXvhMxC0OgRkRWV8 IpQtyDMsacxWW3VVbbcsFmBJJ4GEY+ckw1Tf3YpZ5WpSSALAucKfx5D0inwHr3y5wzXOLYENk cwV/eya/0OxPRVAJwDsXI72kFFexmpVuI6ov51pdJzdnKY2XDAIek+hUAAE7Gq8N0+RpaoEYm XTCgnsvyAotcF8B/ZwsMBUVJSa2wSdhE1F+pF3kqRRBiiYpEpSYVmPrGHYEoQ4mFrbUpBzZkg i4XKCvJc5SEJbIr15UFzxNYX/45xZsNkxdi8tM740VMi1dkWpecPZ0wqs/k2YcOKApE9cSke2 HVNepyvlPwhQBCOuAZniBd3kUvrrGq6jnBRnvHmqhM+4X7ZKh+6/DNU0qek62uh5AeUt3e9Hk Xh18gndxPWWzj7VxMvcpvMztd8ij7nTtriw5Har26FczRAfjcrHICNy16Bz7QDvlqTdHjp/84 bpLLvNGk6gtg+m9bZsBOe3mhnxaKHm8cETbVSouwZ7qDh4r6pRdQNhij1ZJvz61jdgMr6zrkK NJP1ZWdhbOIeLWVspkCo6CogsexZ9C+v2+88JRTPBqOPW+DI9GcGoa4XvnRHRpy7z1GnQN0j/ qejP1JzyAR+3NzeU Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 : >> >>> 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 >>>>> --- >>>>> .../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? Should we mark "qca,qca7000-spi" as deprecated? Regards Stefan