From mboxrd@z Thu Jan 1 00:00:00 1970 From: Boris Brezillon Subject: Re: [PATCH v3 05/11] dt-bindings: i3c: Document core bindings Date: Mon, 26 Mar 2018 13:19:46 +0200 Message-ID: <20180326131946.5513c6e2@bbrezillon> References: <20180323110020.19080-1-boris.brezillon@bootlin.com> <20180323110020.19080-6-boris.brezillon@bootlin.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: Sender: linux-kernel-owner@vger.kernel.org To: Geert Uytterhoeven Cc: Wolfram Sang , Linux I2C , 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 , Cyprian Wronka , Suresh Punnoose , Rafal Ciepiela , Thomas Petazzoni , Nishanth Menon , Rob Herring , Pawel Moll , Mark Rutland , Ian List-Id: linux-gpio@vger.kernel.org Hi Geert, On Mon, 26 Mar 2018 12:22:24 +0200 Geert Uytterhoeven wrote: > Hi Boris, > > On Fri, Mar 23, 2018 at 12:00 PM, Boris Brezillon > wrote: > > From: Boris Brezillon > > > > 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 > > Thanks for your patch! > > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/i3c/i3c.txt > > @@ -0,0 +1,140 @@ > > > +I3C devices > > +=========== > > + > > +All I3C devices are supposed to support DAA (Dynamic Address Assignment), and > > +are thus discoverable. So, by default, I3C devices do not have to be described > > +in the device tree. > > But if they're described, they should have a compatible value, no? What's the point of having a compatible here? I mean, I3C devices are already attached to the relevant drivers using the Provisional ID, why would we need a compatible if we don't parse it? > > > +This being said, one might want to attach extra resources to these devices, > > +and those resources may have to be described in the device tree, which in turn > > +means we have to describe I3C devices. > > + > > +Another use case for describing an I3C device in the device tree is when this > > +I3C device has a static address and we want to assign it a specific dynamic > > +address before the DAA takes place (so that other devices on the bus can't > > +take this dynamic address). > > + > > +The I3C device should be names @,, > > named Oops. Will fix the typo. > > So the i3c-pid in the unit address is represented as a 64-bit number, not as two > comma-separated 32-bit numbers? Right. I've changed my mind so many times that I ended up mixing the 2 representations I have considered. Here are the choices we have: 1/ expose the raw ProvisionalID which is a 48-bit integer formed by the concatenation of the vendor ID, part ID and instance ID: ProvisionalID = VendorID << 33 | PartID << 16 | InstanceID << 12 | ExtraInfo The I3C dev node name should in this case be something like @, 2/ split the fields we are really interested in in different cells: 2nd cell: vendorID 3rd cell: partID 4th cell: instanceID In this case, the node name should be @,,, Note that we can still change for the second representation if Rob is okay. > > > +Example: > > + > > + i3c-master@d040000 { > > + compatible = "cdns,i3c-master"; > > + clocks = <&coreclock>, <&i3csysclock>; > > + clock-names = "pclk", "sysclk"; > > + interrupts = <3 0>; > > + reg = <0x0d040000 0x1000>; > > + #address-cells = <3>; > > + #size-cells = <0>; > > + > > + status = "okay"; > > + i2c-scl-frequency = <100000>; > > + > > + /* I2C device. */ > > + nunchuk: nunchuk@52 { > > @52,8000001000000000? Well, I2C devs is a special case, and I'm not sure we want to add the extra LVR information + the IS_I2C_DEV bit in the node name. > > > + compatible = "nintendo,nunchuk"; > > + reg = <0x52 0x80000010 0x0>; One more detail: people are unlikely to define reg using raw values: I provide macros to do that in patch 6. > > + }; > > + > > + /* I3C device with a static address. */ > > + thermal_sensor: sensor@68,39200144004 { > > No compatible value? No, as said above, it's not needed. > > > + reg = <0x68 0x392 0x144004>; > > + assigned-address = <0xa>; > > + }; > > + > > + /* > > + * I3C device without a static address but requiring resources > > + * described in the DT. > > + */ > > + sensor@0,39200154004 { > > No compatible value? Ditto. Thanks for your review. Boris -- Boris Brezillon, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering https://bootlin.com