| Message ID | 20171214151610.19153-6-boris.brezillon@free-electrons.com |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | Add the I3C subsystem | expand |
Hi Boris, On Thu, Dec 14, 2017 at 4:16 PM, Boris Brezillon <boris.brezillon@free-electrons.com> 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 <boris.brezillon@free-electrons.com> Thanks for your patch! > --- /dev/null > +++ b/Documentation/devicetree/bindings/i3c/i3c.txt > @@ -0,0 +1,128 @@ > +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. sets > + > +- i2c-scl-frequency: frequency (in Hz) of the SCL signal used for I2C > + transfers. When undefined, the core looks at LVR values LVR (Legacy I2C Virtual Register) > + 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: This colon looks a bit funny here, as below is a sentence, not a list. > + > +All properties described in Documentation/devicetree/bindings/i2c/i2c.txt are > +valid here. Perhaps rewrite as: Each I2C device connected to the bus should be described in a subnode with properties. All properties described in Documentation/devicetree/bindings/i2c/i2c.txt are valid here, but several new properties have been added. > + > +New required properties: > +------------------------ > +- i3c-lvr: 32 bits integer property (only the lowest 8 bits are meaningful) > + describing device capabilities as described in the I3C > + specification. > + > + bit[31:8]: unused > + bit[7:5]: I2C device index. Possible values > + * 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 As this is the only reference to "FM", perhaps clarify the acronym, like you do for DAA below. > + > + bit[3:0]: device type > + * 0-15: reserved > + > +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. > +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). > + > +Required properties > +------------------- > +- i3c-pid: PID (Provisional ID). 64-bit property which is used to match a > + device discovered during DAA with its device tree definition. The > + PID is supposed to be unique on a given bus, which guarantees a 1:1 > + match. This property becomes optional if a reg property is defined, > + meaning that the device has a static address. > + > +Optional properties > +------------------- > +- reg: static address. Only valid is the device has a static address. if
On 2017-12-14 16:16, Boris Brezillon wrote: > +Optional properties > +------------------- > +- reg: static address. Only valid is the device has a static address. > +- i3c-dynamic-address: dynamic address to be assigned to this device. This > + property depends on the reg property. > + > +Example: > + > + i3c-master@0d040000 { i3c-master@d040000 (same in patch 7/7) Cheers, Peter -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Dec 14, 2017 at 04:16:08PM +0100, 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 <boris.brezillon@free-electrons.com> > --- > Changes in v2: > - Define how to describe I3C devices in the DT and when it should be > used. Note that the parsing of I3C devices is not yet implemented in > the framework. Will be added when someone really needs it. > --- > Documentation/devicetree/bindings/i3c/i3c.txt | 128 ++++++++++++++++++++++++++ > 1 file changed, 128 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..79a214dee025 > --- /dev/null > +++ b/Documentation/devicetree/bindings/i3c/i3c.txt > @@ -0,0 +1,128 @@ > +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. generic names isn't anything recommended. One problem we have with i2c buses is we can't identify them in the DT other than with a list of all controller's compatible strings. We can fix this by defining the controller node name. So please define the node name to be "i3c-controller". That's more inline with other node names than i3c-master that you used below. > + > +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. Add '-hz' suffix. You could drop 'frequency' as that would be implied by Hz. > + > +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) > + describing device capabilities as described in the I3C > + specification. > + > + bit[31:8]: unused > + bit[7:5]: I2C device index. Possible values > + * 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 > + > +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. > +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). > + > +Required properties > +------------------- > +- i3c-pid: PID (Provisional ID). 64-bit property which is used to match a > + device discovered during DAA with its device tree definition. The > + PID is supposed to be unique on a given bus, which guarantees a 1:1 > + match. This property becomes optional if a reg property is defined, > + meaning that the device has a static address. What determines this number? > + > +Optional properties > +------------------- > +- reg: static address. Only valid is the device has a static address. > +- i3c-dynamic-address: dynamic address to be assigned to this device. This > + property depends on the reg property. Perhaps "assigned-address" property would be appropriate. I'm not all that familiar with it though. > + > +Example: > + > + i3c-master@0d040000 { Drop leading 0. > + 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>; > + > + /* I2C device. */ > + nunchuk: nunchuk@52 { > + compatible = "nintendo,nunchuk"; > + reg = <0x52>; > + i3c-lvr = <0x10>; > + }; > + > + /* I3C device with a static address. */ > + thermal_sensor: sensor@68 { > + reg = <0x68>; > + i3c-dynamic-address = <0xa>; > + }; > + > + /* > + * I3C device without a static address but requiring resources > + * described in the DT. > + */ > + sensor2 { It's not great that we can't follow using generic node names. Maybe the PID can be used as the address? In USB for example, we use hub ports for DT addresses rather than USB addresses since those are dynamic. > + i3c-pid = /bits/ 64 <0x39200144004>; > + clocks = <&clock_provider 0>; > + }; > + }; > + > -- > 2.11.0 > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sat, 16 Dec 2017 11:20:40 -0600 Rob Herring <robh@kernel.org> wrote: > On Thu, Dec 14, 2017 at 04:16:08PM +0100, 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 <boris.brezillon@free-electrons.com> > > --- > > Changes in v2: > > - Define how to describe I3C devices in the DT and when it should be > > used. Note that the parsing of I3C devices is not yet implemented in > > the framework. Will be added when someone really needs it. > > --- > > Documentation/devicetree/bindings/i3c/i3c.txt | 128 ++++++++++++++++++++++++++ > > 1 file changed, 128 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..79a214dee025 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/i3c/i3c.txt > > @@ -0,0 +1,128 @@ > > +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. > > generic names isn't anything recommended. Hm, I agree. Don't even know what I wanted to say (it's probably been copied from somewhere else). > > One problem we have with i2c buses is we can't identify them in the DT > other than with a list of all controller's compatible strings. We can > fix this by defining the controller node name. I'm fine with that. > So please define the node > name to be "i3c-controller". That's more inline with other node names > than i3c-master that you used below. Hm, not sure i3c-controller is appropriate though, because you can have slave controllers. Maybe i3c-host, but I'd prefer to keep the term master since it's employed everywhere in the spec. I can also be i3c-master-controller if you prefer. > > > + > > +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. > > Add '-hz' suffix. You could drop 'frequency' as that would be implied by > Hz. Okay. > > > + > > +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) > > + describing device capabilities as described in the I3C > > + specification. > > + > > + bit[31:8]: unused > > + bit[7:5]: I2C device index. Possible values > > + * 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 > > + > > +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. > > +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). > > + > > +Required properties > > +------------------- > > +- i3c-pid: PID (Provisional ID). 64-bit property which is used to match a > > + device discovered during DAA with its device tree definition. The > > + PID is supposed to be unique on a given bus, which guarantees a 1:1 > > + match. This property becomes optional if a reg property is defined, > > + meaning that the device has a static address. > > What determines this number? Part of it is fixed (manufacturer and part id) and the last few bits represent the device instance on the bus (so you can have several identical devices on the same bus). The manufacturer and part ids should be statically assigned during production, instance id is usually configurable through extra pins that you drive high or low at reset time. > > > + > > +Optional properties > > +------------------- > > +- reg: static address. Only valid is the device has a static address. > > +- i3c-dynamic-address: dynamic address to be assigned to this device. This > > + property depends on the reg property. > > Perhaps "assigned-address" property would be appropriate. I'm not all > that familiar with it though. Again, the spec use the term "dynamic address" everywhere, and I'd like to stay as close as possible to the spec. > > > + > > +Example: > > + > > + i3c-master@0d040000 { > > Drop leading 0. Yep, already mentioned by Peter, I'll fix 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>; > > + > > + /* I2C device. */ > > + nunchuk: nunchuk@52 { > > + compatible = "nintendo,nunchuk"; > > + reg = <0x52>; > > + i3c-lvr = <0x10>; > > + }; > > + > > + /* I3C device with a static address. */ > > + thermal_sensor: sensor@68 { > > + reg = <0x68>; > > + i3c-dynamic-address = <0xa>; > > + }; > > + > > + /* > > + * I3C device without a static address but requiring resources > > + * described in the DT. > > + */ > > + sensor2 { > > It's not great that we can't follow using generic node names. Maybe the > PID can be used as the address? In USB for example, we use hub ports for > DT addresses rather than USB addresses since those are dynamic. Hm, the problem is, we may have 2 numbering schemes here: one where reg is used (reg representing the I2C/static address), and another one where the PID is used. If you're okay with mixing those 2 schemes, then I'm fine with that too. > > > + i3c-pid = /bits/ 64 <0x39200144004>; > > + clocks = <&clock_provider 0>; > > + }; > > + }; > > + > > -- > > 2.11.0 > > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sat, Dec 16, 2017 at 07:35:37PM +0100, Boris Brezillon wrote: > On Sat, 16 Dec 2017 11:20:40 -0600 > Rob Herring <robh@kernel.org> wrote: > > > On Thu, Dec 14, 2017 at 04:16:08PM +0100, 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. [...] > > So please define the node > > name to be "i3c-controller". That's more inline with other node names > > than i3c-master that you used below. > > Hm, not sure i3c-controller is appropriate though, because you can have > slave controllers. Maybe i3c-host, but I'd prefer to keep the term > master since it's employed everywhere in the spec. I can also be > i3c-master-controller if you prefer. Okay, i3c-master is fine. Just make it explicit. > > > +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. > > > +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). > > > + > > > +Required properties > > > +------------------- > > > +- i3c-pid: PID (Provisional ID). 64-bit property which is used to match a > > > + device discovered during DAA with its device tree definition. The > > > + PID is supposed to be unique on a given bus, which guarantees a 1:1 > > > + match. This property becomes optional if a reg property is defined, > > > + meaning that the device has a static address. > > > > What determines this number? > > Part of it is fixed (manufacturer and part id) and the last few bits > represent the device instance on the bus (so you can have several > identical devices on the same bus). The manufacturer and part ids > should be statically assigned during production, instance id is usually > configurable through extra pins that you drive high or low at reset > time. Sounds like an I2C address at least for the pin strapping part... > > > + > > > +Optional properties > > > +------------------- > > > +- reg: static address. Only valid is the device has a static address. > > > +- i3c-dynamic-address: dynamic address to be assigned to this device. This > > > + property depends on the reg property. > > > > Perhaps "assigned-address" property would be appropriate. I'm not all > > that familiar with it though. > > Again, the spec use the term "dynamic address" everywhere, and I'd like > to stay as close as possible to the spec. I looked at assigned-addresses a bit more and that won't really fit because it should be the same format as reg. So I think reg should always be the PID as that is fixed and always present. Then the DAA address is separate and can be the i3c-dynamic-address property. However, there's still part I don't understand... > > > + /* I3C device with a static address. */ > > > + thermal_sensor: sensor@68 { > > > + reg = <0x68>; > > > + i3c-dynamic-address = <0xa>; I'm confused as to how/why you have both reg and dynamic address? > > > + }; > > > + > > > + /* > > > + * I3C device without a static address but requiring resources > > > + * described in the DT. > > > + */ > > > + sensor2 { > > > > It's not great that we can't follow using generic node names. Maybe the > > PID can be used as the address? In USB for example, we use hub ports for > > DT addresses rather than USB addresses since those are dynamic. > > Hm, the problem is, we may have 2 numbering schemes here: one where reg > is used (reg representing the I2C/static address), and another one > where the PID is used. > If you're okay with mixing those 2 schemes, then I'm fine with that too. Mixing I2C devices and I3C devices, yes. But you need to mix in a single device? IOW, do I3C devices also have an I2C address? Rob -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 20 Dec 2017 12:06:45 -0600 Rob Herring <robh@kernel.org> wrote: > On Sat, Dec 16, 2017 at 07:35:37PM +0100, Boris Brezillon wrote: > > On Sat, 16 Dec 2017 11:20:40 -0600 > > Rob Herring <robh@kernel.org> wrote: > > > > > On Thu, Dec 14, 2017 at 04:16:08PM +0100, 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. > > [...] > > > > So please define the node > > > name to be "i3c-controller". That's more inline with other node names > > > than i3c-master that you used below. > > > > Hm, not sure i3c-controller is appropriate though, because you can have > > slave controllers. Maybe i3c-host, but I'd prefer to keep the term > > master since it's employed everywhere in the spec. I can also be > > i3c-master-controller if you prefer. > > Okay, i3c-master is fine. Just make it explicit. Okay. > > > > > +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. > > > > +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). > > > > + > > > > +Required properties > > > > +------------------- > > > > +- i3c-pid: PID (Provisional ID). 64-bit property which is used to match a > > > > + device discovered during DAA with its device tree definition. The > > > > + PID is supposed to be unique on a given bus, which guarantees a 1:1 > > > > + match. This property becomes optional if a reg property is defined, > > > > + meaning that the device has a static address. > > > > > > What determines this number? > > > > Part of it is fixed (manufacturer and part id) and the last few bits > > represent the device instance on the bus (so you can have several > > identical devices on the same bus). The manufacturer and part ids > > should be statically assigned during production, instance id is usually > > configurable through extra pins that you drive high or low at reset > > time. > > Sounds like an I2C address at least for the pin strapping part... The address space of this instance-id is smaller (4bits) than the I2C one (7bits), and more importantly, the instance-id is not required to be unique, it's the aggregation of the vendor-id, part-id and instance-id that has to be unique. So, if you were thinking about using this id to uniquely identify the device on the bus it's not a good idea. > > > > > + > > > > +Optional properties > > > > +------------------- > > > > +- reg: static address. Only valid is the device has a static address. > > > > +- i3c-dynamic-address: dynamic address to be assigned to this device. This > > > > + property depends on the reg property. > > > > > > Perhaps "assigned-address" property would be appropriate. I'm not all > > > that familiar with it though. > > > > Again, the spec use the term "dynamic address" everywhere, and I'd like > > to stay as close as possible to the spec. > > I looked at assigned-addresses a bit more and that won't really fit > because it should be the same format as reg. So I think reg should > always be the PID as that is fixed and always present. Then the DAA > address is separate and can be the i3c-dynamic-address property. > > However, there's still part I don't understand... > > > > > + /* I3C device with a static address. */ > > > > + thermal_sensor: sensor@68 { > > > > + reg = <0x68>; > > > > + i3c-dynamic-address = <0xa>; > > I'm confused as to how/why you have both reg and dynamic address? Some I3C devices have an I2C address (also called static or legacy address in a few places). The static/I2C/legacy address is used until the I3C device is assigned a dynamic address by the master. The whole point of specifying both an I2C address (through the reg property) and a dynamic address (through the i3c-dynamic-address) is to tell the controller that a specific dynamic address should be assigned to this device using the SETSADA (Set Dynamic Address from Static Address) command before a DAA (Dynamic Address Assignment) procedure is started. This way, the device will not participate to the DAA (because it already has a valid DA) and the dynamic address can't be assigned to a different device (which is one of the problem with the automatic DAA procedure). > > > > > + }; > > > > + > > > > + /* > > > > + * I3C device without a static address but requiring resources > > > > + * described in the DT. > > > > + */ > > > > + sensor2 { > > > > > > It's not great that we can't follow using generic node names. Maybe the > > > PID can be used as the address? In USB for example, we use hub ports for > > > DT addresses rather than USB addresses since those are dynamic. > > > > Hm, the problem is, we may have 2 numbering schemes here: one where reg > > is used (reg representing the I2C/static address), and another one > > where the PID is used. > > If you're okay with mixing those 2 schemes, then I'm fine with that too. > > Mixing I2C devices and I3C devices, yes. But you need to mix in a single > device? IOW, do I3C devices also have an I2C address? Yes, some of them have both. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Dec 21, 2017 at 4:41 AM, Boris Brezillon <boris.brezillon@free-electrons.com> wrote: > On Wed, 20 Dec 2017 12:06:45 -0600 > Rob Herring <robh@kernel.org> wrote: > >> On Sat, Dec 16, 2017 at 07:35:37PM +0100, Boris Brezillon wrote: >> > On Sat, 16 Dec 2017 11:20:40 -0600 >> > Rob Herring <robh@kernel.org> wrote: >> > >> > > On Thu, Dec 14, 2017 at 04:16:08PM +0100, 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. >> >> [...] >> >> > > So please define the node >> > > name to be "i3c-controller". That's more inline with other node names >> > > than i3c-master that you used below. >> > >> > Hm, not sure i3c-controller is appropriate though, because you can have >> > slave controllers. Maybe i3c-host, but I'd prefer to keep the term >> > master since it's employed everywhere in the spec. I can also be >> > i3c-master-controller if you prefer. >> >> Okay, i3c-master is fine. Just make it explicit. > > Okay. > >> >> > > > +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. >> > > > +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). >> > > > + >> > > > +Required properties >> > > > +------------------- >> > > > +- i3c-pid: PID (Provisional ID). 64-bit property which is used to match a >> > > > + device discovered during DAA with its device tree definition. The >> > > > + PID is supposed to be unique on a given bus, which guarantees a 1:1 >> > > > + match. This property becomes optional if a reg property is defined, >> > > > + meaning that the device has a static address. >> > > >> > > What determines this number? >> > >> > Part of it is fixed (manufacturer and part id) and the last few bits >> > represent the device instance on the bus (so you can have several >> > identical devices on the same bus). The manufacturer and part ids >> > should be statically assigned during production, instance id is usually >> > configurable through extra pins that you drive high or low at reset >> > time. >> >> Sounds like an I2C address at least for the pin strapping part... > > The address space of this instance-id is smaller (4bits) than the I2C > one (7bits), and more importantly, the instance-id is not required to > be unique, it's the aggregation of the vendor-id, part-id and > instance-id that has to be unique. So, if you were thinking about using > this id to uniquely identify the device on the bus it's not a good idea. No, no. I was just commenting how I2C devices typically do the same pin strapping to make addresses unique. >> > > > +Optional properties >> > > > +------------------- >> > > > +- reg: static address. Only valid is the device has a static address. >> > > > +- i3c-dynamic-address: dynamic address to be assigned to this device. This >> > > > + property depends on the reg property. >> > > >> > > Perhaps "assigned-address" property would be appropriate. I'm not all >> > > that familiar with it though. >> > >> > Again, the spec use the term "dynamic address" everywhere, and I'd like >> > to stay as close as possible to the spec. >> >> I looked at assigned-addresses a bit more and that won't really fit >> because it should be the same format as reg. So I think reg should >> always be the PID as that is fixed and always present. Then the DAA >> address is separate and can be the i3c-dynamic-address property. >> >> However, there's still part I don't understand... >> >> > > > + /* I3C device with a static address. */ >> > > > + thermal_sensor: sensor@68 { >> > > > + reg = <0x68>; >> > > > + i3c-dynamic-address = <0xa>; >> >> I'm confused as to how/why you have both reg and dynamic address? > > Some I3C devices have an I2C address (also called static or legacy > address in a few places). The static/I2C/legacy address is used until > the I3C device is assigned a dynamic address by the master. The whole > point of specifying both an I2C address (through the reg property) and > a dynamic address (through the i3c-dynamic-address) is to tell the > controller that a specific dynamic address should be assigned to this > device using the SETSADA (Set Dynamic Address from Static Address) > command before a DAA (Dynamic Address Assignment) procedure is started. > This way, the device will not participate to the DAA (because it > already has a valid DA) and the dynamic address can't be assigned to > a different device (which is one of the problem with the automatic DAA > procedure). Okay, think I got it now. I think we should extend "reg" to have either I2C address, I3C PID, or both (in a defined order). I'm assuming you can always distinguish a static I2C address and an I3C PID just by upper bits all being 0s for I2C addresses. Maybe both is not needed? This means we'd have to allow 64-bit I2C addresses (#address-cells=2), but that should be easily fixed if that causes problems in the kernel. So i3c-pid would go away and i3c-dynamic-address stays. Rob -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Rob, On Tue, 26 Dec 2017 12:29:34 -0600 Rob Herring <robh@kernel.org> wrote: > >> > > > +Optional properties > >> > > > +------------------- > >> > > > +- reg: static address. Only valid is the device has a static address. > >> > > > +- i3c-dynamic-address: dynamic address to be assigned to this device. This > >> > > > + property depends on the reg property. > >> > > > >> > > Perhaps "assigned-address" property would be appropriate. I'm not all > >> > > that familiar with it though. > >> > > >> > Again, the spec use the term "dynamic address" everywhere, and I'd like > >> > to stay as close as possible to the spec. > >> > >> I looked at assigned-addresses a bit more and that won't really fit > >> because it should be the same format as reg. So I think reg should > >> always be the PID as that is fixed and always present. Then the DAA > >> address is separate and can be the i3c-dynamic-address property. > >> > >> However, there's still part I don't understand... > >> > >> > > > + /* I3C device with a static address. */ > >> > > > + thermal_sensor: sensor@68 { > >> > > > + reg = <0x68>; > >> > > > + i3c-dynamic-address = <0xa>; > >> > >> I'm confused as to how/why you have both reg and dynamic address? > > > > Some I3C devices have an I2C address (also called static or legacy > > address in a few places). The static/I2C/legacy address is used until > > the I3C device is assigned a dynamic address by the master. The whole > > point of specifying both an I2C address (through the reg property) and > > a dynamic address (through the i3c-dynamic-address) is to tell the > > controller that a specific dynamic address should be assigned to this > > device using the SETSADA (Set Dynamic Address from Static Address) > > command before a DAA (Dynamic Address Assignment) procedure is started. > > This way, the device will not participate to the DAA (because it > > already has a valid DA) and the dynamic address can't be assigned to > > a different device (which is one of the problem with the automatic DAA > > procedure). > > Okay, think I got it now. > > I think we should extend "reg" to have either I2C address, I3C PID, or > both (in a defined order). I'm assuming you can always distinguish a > static I2C address and an I3C PID just by upper bits all being 0s for > I2C addresses. Maybe both is not needed? This means we'd have to allow > 64-bit I2C addresses (#address-cells=2), but that should be easily > fixed if that causes problems in the kernel. > > So i3c-pid would go away and i3c-dynamic-address stays. Hm, actually I'm not sure this is a good idea. Sounds like we're abusing the purpose of reg here. For busses, reg is supposed to encode the id of the device on the bus that is used to communicate with this device (CS line for SPI, I2C address for I2C devs, ...). With I3C, the PID is just a way to uniquely identify a device, but is not used during communications (we either use the static/I2C address or the dynamic address assigned by the master). If your concern is just about I3C dev naming convention, maybe we could have something like: i3c-master@xxxx { ... i2cdev@xx { reg = <xx>; i3c-lvr = <yy>; ... }; ... i3cdev-<i3c-pid>[@zz] { i3c-pid = <pppppp>; /* * reg only defined if the device has a static * address. */ [reg = <zz>;] /* * i3c-dynamic-address only defined if a * specific dynamic address is requested. */ [i3c-dynamic-address = <dd>;] }; }; With this approach we have a way to quickly identify i3c devices by their pid when looking at their names (with the -<i3c-pid> suffix), and we keep reg for static/i2c addresses only. Regards, Boris -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Rob, On Sun, 7 Jan 2018 15:14:25 +0100 Boris Brezillon <boris.brezillon@free-electrons.com> wrote: > Hi Rob, > > On Tue, 26 Dec 2017 12:29:34 -0600 > Rob Herring <robh@kernel.org> wrote: > > > >> > > > +Optional properties > > >> > > > +------------------- > > >> > > > +- reg: static address. Only valid is the device has a static address. > > >> > > > +- i3c-dynamic-address: dynamic address to be assigned to this device. This > > >> > > > + property depends on the reg property. > > >> > > > > >> > > Perhaps "assigned-address" property would be appropriate. I'm not all > > >> > > that familiar with it though. > > >> > > > >> > Again, the spec use the term "dynamic address" everywhere, and I'd like > > >> > to stay as close as possible to the spec. > > >> > > >> I looked at assigned-addresses a bit more and that won't really fit > > >> because it should be the same format as reg. So I think reg should > > >> always be the PID as that is fixed and always present. Then the DAA > > >> address is separate and can be the i3c-dynamic-address property. > > >> > > >> However, there's still part I don't understand... > > >> > > >> > > > + /* I3C device with a static address. */ > > >> > > > + thermal_sensor: sensor@68 { > > >> > > > + reg = <0x68>; > > >> > > > + i3c-dynamic-address = <0xa>; > > >> > > >> I'm confused as to how/why you have both reg and dynamic address? > > > > > > Some I3C devices have an I2C address (also called static or legacy > > > address in a few places). The static/I2C/legacy address is used until > > > the I3C device is assigned a dynamic address by the master. The whole > > > point of specifying both an I2C address (through the reg property) and > > > a dynamic address (through the i3c-dynamic-address) is to tell the > > > controller that a specific dynamic address should be assigned to this > > > device using the SETSADA (Set Dynamic Address from Static Address) > > > command before a DAA (Dynamic Address Assignment) procedure is started. > > > This way, the device will not participate to the DAA (because it > > > already has a valid DA) and the dynamic address can't be assigned to > > > a different device (which is one of the problem with the automatic DAA > > > procedure). > > > > Okay, think I got it now. > > > > I think we should extend "reg" to have either I2C address, I3C PID, or > > both (in a defined order). I'm assuming you can always distinguish a > > static I2C address and an I3C PID just by upper bits all being 0s for > > I2C addresses. Maybe both is not needed? This means we'd have to allow > > 64-bit I2C addresses (#address-cells=2), but that should be easily > > fixed if that causes problems in the kernel. > > > > So i3c-pid would go away and i3c-dynamic-address stays. > > Hm, actually I'm not sure this is a good idea. Sounds like we're > abusing the purpose of reg here. For busses, reg is supposed to encode > the id of the device on the bus that is used to communicate with this > device (CS line for SPI, I2C address for I2C devs, ...). With I3C, the > PID is just a way to uniquely identify a device, but is not used during > communications (we either use the static/I2C address or the dynamic > address assigned by the master). > > If your concern is just about I3C dev naming convention, maybe we > could have something like: > > i3c-master@xxxx { > ... > i2cdev@xx { > reg = <xx>; > i3c-lvr = <yy>; > ... > }; > ... > > i3cdev-<i3c-pid>[@zz] { > i3c-pid = <pppppp>; > /* > * reg only defined if the device has a static > * address. > */ > [reg = <zz>;] > /* > * i3c-dynamic-address only defined if a > * specific dynamic address is requested. > */ > [i3c-dynamic-address = <dd>;] > }; > }; > > With this approach we have a way to quickly identify i3c devices by > their pid when looking at their names (with the -<i3c-pid> suffix), and > we keep reg for static/i2c addresses only. Did you have time to read this email (AKA ping)? Regards, Boris -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/Documentation/devicetree/bindings/i3c/i3c.txt b/Documentation/devicetree/bindings/i3c/i3c.txt new file mode 100644 index 000000000000..79a214dee025 --- /dev/null +++ b/Documentation/devicetree/bindings/i3c/i3c.txt @@ -0,0 +1,128 @@ +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) + describing device capabilities as described in the I3C + specification. + + bit[31:8]: unused + bit[7:5]: I2C device index. Possible values + * 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 + +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. +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). + +Required properties +------------------- +- i3c-pid: PID (Provisional ID). 64-bit property which is used to match a + device discovered during DAA with its device tree definition. The + PID is supposed to be unique on a given bus, which guarantees a 1:1 + match. This property becomes optional if a reg property is defined, + meaning that the device has a static address. + +Optional properties +------------------- +- reg: static address. Only valid is the device has a static address. +- i3c-dynamic-address: dynamic address to be assigned to this device. This + property depends on the reg property. + +Example: + + i3c-master@0d040000 { + 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>; + + /* I2C device. */ + nunchuk: nunchuk@52 { + compatible = "nintendo,nunchuk"; + reg = <0x52>; + i3c-lvr = <0x10>; + }; + + /* I3C device with a static address. */ + thermal_sensor: sensor@68 { + reg = <0x68>; + i3c-dynamic-address = <0xa>; + }; + + /* + * I3C device without a static address but requiring resources + * described in the DT. + */ + sensor2 { + i3c-pid = /bits/ 64 <0x39200144004>; + clocks = <&clock_provider 0>; + }; + }; +
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 <boris.brezillon@free-electrons.com> --- Changes in v2: - Define how to describe I3C devices in the DT and when it should be used. Note that the parsing of I3C devices is not yet implemented in the framework. Will be added when someone really needs it. --- Documentation/devicetree/bindings/i3c/i3c.txt | 128 ++++++++++++++++++++++++++ 1 file changed, 128 insertions(+) create mode 100644 Documentation/devicetree/bindings/i3c/i3c.txt