Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752286AbdHJIuG convert rfc822-to-8bit (ORCPT ); Thu, 10 Aug 2017 04:50:06 -0400 Received: from mail.free-electrons.com ([62.4.15.54]:46213 "EHLO mail.free-electrons.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751485AbdHJIuC (ORCPT ); Thu, 10 Aug 2017 04:50:02 -0400 Date: Thu, 10 Aug 2017 10:49:58 +0200 From: Boris Brezillon To: Rob Herring Cc: Wolfram Sang , linux-i2c@vger.kernel.org, Jonathan Corbet , linux-doc@vger.kernel.org, Greg Kroah-Hartman , Arnd Bergmann , Przemyslaw Sroka , Arkadiusz Golec , Alan Douglas , Bartosz Folta , Damian Kos , Alicja Jurasik-Urbaniak , Jan Kotas , Cyprian Wronka , Alexandre Belloni , Thomas Petazzoni , Nishanth Menon , Pawel Moll , Mark Rutland , Ian Campbell , Kumar Gala , devicetree@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [RFC 3/5] dt-bindings: i3c: Document core bindings Message-ID: <20170810104958.7e34332f@bbrezillon> In-Reply-To: <20170809234302.ffcr3gwpiy4roj7l@rob-hp-laptop> References: <1501518290-5723-1-git-send-email-boris.brezillon@free-electrons.com> <1501518290-5723-4-git-send-email-boris.brezillon@free-electrons.com> <20170809234302.ffcr3gwpiy4roj7l@rob-hp-laptop> X-Mailer: Claws Mail 3.14.1 (GTK+ 2.24.31; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8BIT Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 6018 Lines: 178 Hi Rob, Le Wed, 9 Aug 2017 18:43:02 -0500, Rob Herring a écrit : > On Mon, Jul 31, 2017 at 06:24:48PM +0200, Boris Brezillon wrote: > > A new I3C subsystem has been added and a generic description has been > > created to represent the I3C bus and the devices connected on it. > > > > Document this generic representation. > > > > Signed-off-by: Boris Brezillon > > --- > > Documentation/devicetree/bindings/i3c/i3c.txt | 90 +++++++++++++++++++++++++++ > > 1 file changed, 90 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/i3c/i3c.txt > > > > diff --git a/Documentation/devicetree/bindings/i3c/i3c.txt b/Documentation/devicetree/bindings/i3c/i3c.txt > > new file mode 100644 > > index 000000000000..49261dec7b01 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/i3c/i3c.txt > > @@ -0,0 +1,90 @@ > > +Generic device tree bindings for I3C busses > > +=========================================== > > + > > +This document describes generic bindings that should be used to describe I3C > > +busses in a device tree. > > + > > +Required properties > > +------------------- > > + > > +- #address-cells - should be <1>. Read more about addresses below. > > +- #size-cells - should be <0>. > > +- compatible - name of I3C bus controller following generic names > > + recommended practice. > > + > > +For other required properties e.g. to describe register sets, > > +clocks, etc. check the binding documentation of the specific driver. > > + > > +Optional properties > > +------------------- > > + > > +These properties may not be supported by all I3C master drivers. Each I3C > > +master bindings should specify which of them are supported. > > + > > +- i3c-scl-frequency: frequency (in Hz) of the SCL signal used for I3C > > + transfers. When undefined the core set it to 12.5MHz. > > + > > +- i2c-scl-frequency: frequency (in Hz) of the SCL signal used for I2C > > + transfers. When undefined, the core looks at LVR values > > + of I2C devices described in the device tree to determine > > + the maximum I2C frequency. > > + > > +I2C devices > > +=========== > > + > > +Each I2C device connected to the bus should be described in a subnode with > > +the following properties: > > + > > +All properties described in Documentation/devicetree/bindings/i2c/i2c.txt are > > +valid here. > > + > > +New required properties: > > +------------------------ > > +- i3c-lvr: 32 bits integer property (only the lowest 8 bits are meaningful) > > What does lvr mean? Legacy Virtual Register. It's the name used in the I3C specification and a short description is given in the I3C doc (patch 2 of this RFC): " Backward compatibility with I2C devices ======================================= The I3C protocol has been designed to be backward compatible with I2C devices. This backward compatibility allows one to connect a mix of I2C and I3C device on the same bus, though, in order to be really efficient, I2C devices should be equipped with 50 ns spike filters. I2C devices can't be discovered like I3C ones and have to be statically declared. In order to let the master know what these devices are capable of (both in terms of bus related limitations and functionalities), the software has to provide some information, which is done through the LVR (Legacy I2C Virtual Register). " > > > + describing device capabilities as described in the I3C > > + specification. > > + > > + bit[31:8]: unused > > + bit[7:5]: I2C device index. Possible values > > index? Yes, I also find this name inappropriate, but that's directly extracted from the specification. > Seems more like flags These are not described as independent flags in the spec. It's an enum with only 3 values on 7 currently defined. I guess MIPI reserves other values for future use (v2 of the spec?). > > > + * 0: I2C device has a 50 ns spike filter > > + * 1: I2C device does not have a 50 ns spike filter but supports high > > + frequency on SCL > > + * 2: I2C device does not have a 50 ns spike filter and is not > > + tolerant to high frequencies > > + * 3-7: reserved > > + > > + bit[4]: tell whether the device operates in FM or FM+ mode > > + * 0: FM+ mode > > + * 1: FM mode > > + > > + bit[3:0]: device type > > + * 0-15: reserved > > That's useful... Unfortunately, v1 of the spec does not define any of these device types. Probably something that will be extended in future versions. > > > + > > +I3C devices > > +=========== > > + > > +I3C are not described in the device tree yet. We could decide to represent them > > +at some point to assign a specific dynamic address to a device or to force an > > +I3C device to act as an I2C device if it has a static address. > > I think we need to define this sooner rather than later if there's not a > standard connector. That's the only thing that would enforce any sort of > standard. Of course, that didn't help with SDIO. I'm perfectly fine with that. I'll try to come up with a proposal in v2 of this patchset. > > > + > > +Example: > > + > > + i3c-master@0d040000 { > > The node name should go into the DT spec. I tend to think "i3c" would be > sufficient and aligned with i2c. Well, I3C slave IPs are around the corner (actually I used one to test this framework), and I thought clarifying things from the beginning would be beneficial. But if you think i3c implicitly means i3c-master, I can change that. > > > + compatible = "cdns,i3c-master"; > > + clocks = <&coreclock>, <&i3csysclock>; > > + clock-names = "pclk", "sysclk"; > > + interrupts = <3 0>; > > + reg = <0x0d040000 0x1000>; > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + status = "okay"; > > + i2c-scl-frequency = <100000>; > > + > > + nunchuk: nunchuk@52 { > > + compatible = "nintendo,nunchuk"; > > + reg = <0x52>; > > + i3c-lvr = <0x10>; > > + }; > > + }; > > + > > -- > > 2.7.4 > >