diff mbox series

[v3,05/11] dt-bindings: i3c: Document core bindings

Message ID 20180323110020.19080-6-boris.brezillon@bootlin.com
State Not Applicable
Headers show
Series Add the I3C subsystem | expand

Commit Message

Boris Brezillon March 23, 2018, 11 a.m. UTC
From: Boris Brezillon <boris.brezillon@free-electrons.com>

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 v3:
- Rename {i2c,i3c}-scl-frequency DT prop into {i2c,i3c}-scl-hz
- Rework the way we expose the provisional ID and LVR information
- Rename dynamic-address into assigned-address
- Enforce the I3C master node name

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 | 140 ++++++++++++++++++++++++++
 1 file changed, 140 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/i3c/i3c.txt

Comments

Peter Rosin March 23, 2018, 12:47 p.m. UTC | #1
On 2018-03-23 12:00, Boris Brezillon wrote:
> From: Boris Brezillon <boris.brezillon@free-electrons.com>
> 
> 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 v3:
> - Rename {i2c,i3c}-scl-frequency DT prop into {i2c,i3c}-scl-hz
> - Rework the way we expose the provisional ID and LVR information
> - Rename dynamic-address into assigned-address
> - Enforce the I3C master node name
> 
> 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 | 140 ++++++++++++++++++++++++++
>  1 file changed, 140 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..ed858228d26b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/i3c/i3c.txt
> @@ -0,0 +1,140 @@
> +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 <3>. Read more about addresses below.
> +- #size-cells     - should be <0>.
> +- compatible      - name of the I3C master controller driving the I3C bus
> +
> +For other required properties e.g. to describe register sets,
> +clocks, etc. check the binding documentation of the specific driver.
> +The node describing an I3C bus should be named i3c-master.
> +
> +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-hz: frequency of the SCL signal used for I3C transfers.
> +	      When undefined the core sets it to 12.5MHz.
> +
> +- i2c-scl-hz: frequency of the SCL signal used for I2C transfers.
> +	      When undefined, the core looks at LVR (Legacy Virtual Register)
> +	      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. All
> +properties described in Documentation/devicetree/bindings/i2c/i2c.txt are
> +valid here, but several new properties have been added.
> +
> +New constraint on existing properties:
> +--------------------------------------
> +- reg: contains 3 cells
> +  + first cell : still encoding the I2C address
> +
> +  + second cell: should have bit 31 set to 1 signify that this is an I2C
> +		 device. Bits 0 to 7 encode the I3C LVR (Legacy Virtual
> +		 Register):
> +
> +	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 (Fast Mode) or FM+ mode
> +	* 0: FM+ mode
> +	* 1: FM mode
> +
> +	bit[3:0]: device type
> +	* 0-15: reserved
> +
> +  + third cell: should be 0
> +
> +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).
> +
> +The I3C device should be names <device-type>@<static-address>,<i3c-pid>,
> +where device-type is describing the type of device connected on the bus
> +(gpio-controller, sensor, ...).
> +
> +Required properties
> +-------------------
> +- reg: contains 3 cells
> +  + first cell : encodes the I2C address. Should be 0 if the device does not
> +		 have one (0 is not a valid I3C address).
> +
> +  + second and third cells: should encode the ProvisionalID. The second cell
> +			    contains the manufacturer ID left-shifted by 1.
> +			    The third cell contains ORing of the part ID
> +			    left-shifted by 16, the instance ID left-shifted
> +			    by 12 and the extra information. This encoding is
> +			    following the PID definition provided by the I3C
> +			    specification.
> +
> +Optional properties
> +-------------------
> +- assigned-address: dynamic address to be assigned to this device. This
> +		    property is only valid if the I3C device has a static
> +		    address (first cell of the reg property != 0).
> +
> +
> +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>;

Another s/frequency/hz/ instance, similar to those reported by Thomas.

Cheers,
Peter

> +
> +		/* I2C device. */
> +		nunchuk: nunchuk@52 {
> +			compatible = "nintendo,nunchuk";
> +			reg = <0x52 0x80000010 0x0>;
> +		};
> +
> +		/* I3C device with a static address. */
> +		thermal_sensor: sensor@68,39200144004 {
> +			reg = <0x68 0x392 0x144004>;
> +			assigned-address = <0xa>;
> +		};
> +
> +		/*
> +		 * I3C device without a static address but requiring resources
> +		 * described in the DT.
> +		 */
> +		sensor@0,39200154004 {
> +			reg = <0x0 0x392 0x154004>;
> +			clocks = <&clock_provider 0>;
> +		};
> +	};
> +
>
Boris Brezillon March 23, 2018, 1:58 p.m. UTC | #2
On Fri, 23 Mar 2018 13:47:49 +0100
Peter Rosin <peda@axentia.se> wrote:

> > +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>;  
> 
> Another s/frequency/hz/ instance, similar to those reported by Thomas.

Will fix it in v4.

Thanks,

Boris
Geert Uytterhoeven March 26, 2018, 10:22 a.m. UTC | #3
Hi Boris,

On Fri, Mar 23, 2018 at 12:00 PM, Boris Brezillon
<boris.brezillon@bootlin.com> wrote:
> From: Boris Brezillon <boris.brezillon@free-electrons.com>
>
> 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,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?

> +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 <device-type>@<static-address>,<i3c-pid>,

named

So the i3c-pid in the unit address is represented as a 64-bit number, not as two
comma-separated 32-bit numbers?

> +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?

> +                       compatible = "nintendo,nunchuk";
> +                       reg = <0x52 0x80000010 0x0>;
> +               };
> +
> +               /* I3C device with a static address. */
> +               thermal_sensor: sensor@68,39200144004 {

No compatible value?

> +                       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?

> +                       reg = <0x0 0x392 0x154004>;
> +                       clocks = <&clock_provider 0>;
> +               };
> +       };

Gr{oetje,eeting}s,

                        Geert
Boris Brezillon March 26, 2018, 11:19 a.m. UTC | #4
Hi Geert,

On Mon, 26 Mar 2018 12:22:24 +0200
Geert Uytterhoeven <geert@linux-m68k.org> wrote:

> Hi Boris,
> 
> On Fri, Mar 23, 2018 at 12:00 PM, Boris Brezillon
> <boris.brezillon@bootlin.com> wrote:
> > From: Boris Brezillon <boris.brezillon@free-electrons.com>
> >
> > 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,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 <device-type>@<static-address>,<i3c-pid>,  
> 
> 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
   <device-type>@<static-address>,<i3c-pid>

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
   <device-type>@<static-address>,<i3c-vendor>,<i3c-partid>,<i3c-instanceid>

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
Rob Herring March 26, 2018, 10:24 p.m. UTC | #5
On Fri, Mar 23, 2018 at 12:00:14PM +0100, Boris Brezillon wrote:
> From: Boris Brezillon <boris.brezillon@free-electrons.com>
> 
> 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.

Mostly looks fine, a couple of clarifications below.

> 
> Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> ---
> Changes in v3:
> - Rename {i2c,i3c}-scl-frequency DT prop into {i2c,i3c}-scl-hz
> - Rework the way we expose the provisional ID and LVR information
> - Rename dynamic-address into assigned-address
> - Enforce the I3C master node name
> 
> 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 | 140 ++++++++++++++++++++++++++
>  1 file changed, 140 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..ed858228d26b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/i3c/i3c.txt
> @@ -0,0 +1,140 @@
> +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 <3>. Read more about addresses below.
> +- #size-cells     - should be <0>.
> +- compatible      - name of the I3C master controller driving the I3C bus
> +
> +For other required properties e.g. to describe register sets,
> +clocks, etc. check the binding documentation of the specific driver.
> +The node describing an I3C bus should be named i3c-master.
> +
> +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-hz: frequency of the SCL signal used for I3C transfers.
> +	      When undefined the core sets it to 12.5MHz.
> +
> +- i2c-scl-hz: frequency of the SCL signal used for I2C transfers.
> +	      When undefined, the core looks at LVR (Legacy Virtual Register)
> +	      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. All
> +properties described in Documentation/devicetree/bindings/i2c/i2c.txt are
> +valid here, but several new properties have been added.
> +
> +New constraint on existing properties:
> +--------------------------------------
> +- reg: contains 3 cells
> +  + first cell : still encoding the I2C address
> +
> +  + second cell: should have bit 31 set to 1 signify that this is an I2C
> +		 device. Bits 0 to 7 encode the I3C LVR (Legacy Virtual
> +		 Register):
> +
> +	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 (Fast Mode) or FM+ mode
> +	* 0: FM+ mode
> +	* 1: FM mode
> +
> +	bit[3:0]: device type
> +	* 0-15: reserved
> +
> +  + third cell: should be 0
> +
> +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

static is I2C address and dynamic is an I3C address. That could be 
clearer throughout.

> +take this dynamic address).
> +
> +The I3C device should be names <device-type>@<static-address>,<i3c-pid>,

s/static-address/static-i2c-address/

> +where device-type is describing the type of device connected on the bus
> +(gpio-controller, sensor, ...).
> +
> +Required properties
> +-------------------
> +- reg: contains 3 cells
> +  + first cell : encodes the I2C address. Should be 0 if the device does not
> +		 have one (0 is not a valid I3C address).

Change here to "encodes the static I2C address". 

0 is not a valid I2C address?

> +
> +  + second and third cells: should encode the ProvisionalID. The second cell
> +			    contains the manufacturer ID left-shifted by 1.
> +			    The third cell contains ORing of the part ID
> +			    left-shifted by 16, the instance ID left-shifted
> +			    by 12 and the extra information. This encoding is
> +			    following the PID definition provided by the I3C
> +			    specification.
> +
> +Optional properties
> +-------------------
> +- assigned-address: dynamic address to be assigned to this device. This
> +		    property is only valid if the I3C device has a static
> +		    address (first cell of the reg property != 0).
> +
> +
> +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 {
> +			compatible = "nintendo,nunchuk";
> +			reg = <0x52 0x80000010 0x0>;
> +		};
> +
> +		/* I3C device with a static address. */
> +		thermal_sensor: sensor@68,39200144004 {
> +			reg = <0x68 0x392 0x144004>;
> +			assigned-address = <0xa>;
> +		};
> +
> +		/*
> +		 * I3C device without a static address but requiring resources
> +		 * described in the DT.
> +		 */
> +		sensor@0,39200154004 {
> +			reg = <0x0 0x392 0x154004>;
> +			clocks = <&clock_provider 0>;
> +		};
> +	};
> +
> -- 
> 2.14.1
>
Boris Brezillon March 28, 2018, 8:19 a.m. UTC | #6
Hi Rob,

On Mon, 26 Mar 2018 17:24:58 -0500
Rob Herring <robh@kernel.org> wrote:

> > +
> > +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  
> 
> static is I2C address and dynamic is an I3C address. That could be 
> clearer throughout.

I'll clarify that.

> 
> > +take this dynamic address).
> > +
> > +The I3C device should be names <device-type>@<static-address>,<i3c-pid>,  
> 
> s/static-address/static-i2c-address/

Okay.

> 
> > +where device-type is describing the type of device connected on the bus
> > +(gpio-controller, sensor, ...).
> > +
> > +Required properties
> > +-------------------
> > +- reg: contains 3 cells
> > +  + first cell : encodes the I2C address. Should be 0 if the device does not
> > +		 have one (0 is not a valid I3C address).  
> 
> Change here to "encodes the static I2C address". 
> 
> 0 is not a valid I2C address?

According to [1] it is reserved, and it's reserved in the I3C spec
anyway (see "Table 9 I3C Slave Address Restrictions" in the I3C spec).

> 
> > +
> > +  + second and third cells: should encode the ProvisionalID. The second cell
> > +			    contains the manufacturer ID left-shifted by 1.
> > +			    The third cell contains ORing of the part ID
> > +			    left-shifted by 16, the instance ID left-shifted
> > +			    by 12 and the extra information. This encoding is
> > +			    following the PID definition provided by the I3C
> > +			    specification.

One extra question for you: should I refer to the I3C_DEV(),
I3C_DEV_WITH_STATIC_ADDR() and I2C_DEV() macros in the bindings doc?
And if I do, should I use them my example?

Thanks,

Boris

> > +
> > +Optional properties
> > +-------------------
> > +- assigned-address: dynamic address to be assigned to this device. This
> > +		    property is only valid if the I3C device has a static
> > +		    address (first cell of the reg property != 0).
> > +
> > +
> > +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 {
> > +			compatible = "nintendo,nunchuk";
> > +			reg = <0x52 0x80000010 0x0>;
> > +		};
> > +
> > +		/* I3C device with a static address. */
> > +		thermal_sensor: sensor@68,39200144004 {
> > +			reg = <0x68 0x392 0x144004>;
> > +			assigned-address = <0xa>;
> > +		};
> > +
> > +		/*
> > +		 * I3C device without a static address but requiring resources
> > +		 * described in the DT.
> > +		 */
> > +		sensor@0,39200154004 {
> > +			reg = <0x0 0x392 0x154004>;
> > +			clocks = <&clock_provider 0>;
> > +		};
> > +	};
> > +
> > -- 
> > 2.14.1
> >   

[1]http://www.i2c-bus.org/addressing
Rob Herring March 28, 2018, 4:42 p.m. UTC | #7
On Wed, Mar 28, 2018 at 3:19 AM, Boris Brezillon
<boris.brezillon@bootlin.com> wrote:
> Hi Rob,
>
> On Mon, 26 Mar 2018 17:24:58 -0500
> Rob Herring <robh@kernel.org> wrote:
>
>> > +
>> > +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
>>
>> static is I2C address and dynamic is an I3C address. That could be
>> clearer throughout.
>
> I'll clarify that.
>
>>
>> > +take this dynamic address).
>> > +
>> > +The I3C device should be names <device-type>@<static-address>,<i3c-pid>,
>>
>> s/static-address/static-i2c-address/
>
> Okay.
>
>>
>> > +where device-type is describing the type of device connected on the bus
>> > +(gpio-controller, sensor, ...).
>> > +
>> > +Required properties
>> > +-------------------
>> > +- reg: contains 3 cells
>> > +  + first cell : encodes the I2C address. Should be 0 if the device does not
>> > +            have one (0 is not a valid I3C address).
>>
>> Change here to "encodes the static I2C address".
>>
>> 0 is not a valid I2C address?
>
> According to [1] it is reserved, and it's reserved in the I3C spec
> anyway (see "Table 9 I3C Slave Address Restrictions" in the I3C spec).

Sorry, what I meant was s/I3C/I2C/. The first cell is I2C address and
0 is not valid.

>> > +
>> > +  + second and third cells: should encode the ProvisionalID. The second cell
>> > +                       contains the manufacturer ID left-shifted by 1.
>> > +                       The third cell contains ORing of the part ID
>> > +                       left-shifted by 16, the instance ID left-shifted
>> > +                       by 12 and the extra information. This encoding is
>> > +                       following the PID definition provided by the I3C
>> > +                       specification.
>
> One extra question for you: should I refer to the I3C_DEV(),
> I3C_DEV_WITH_STATIC_ADDR() and I2C_DEV() macros in the bindings doc?
> And if I do, should I use them my example?

Well, I don't want to see "device@I3C_DEV(...)" for unit-addresses.
You can use them for reg property, but it's somewhat pointless to use
it in one place and not the other.

Rob
Boris Brezillon March 28, 2018, 5:28 p.m. UTC | #8
On Wed, 28 Mar 2018 11:42:07 -0500
Rob Herring <robh@kernel.org> wrote:


> >>  
> >> > +where device-type is describing the type of device connected on the bus
> >> > +(gpio-controller, sensor, ...).
> >> > +
> >> > +Required properties
> >> > +-------------------
> >> > +- reg: contains 3 cells
> >> > +  + first cell : encodes the I2C address. Should be 0 if the device does not
> >> > +            have one (0 is not a valid I3C address).  
> >>
> >> Change here to "encodes the static I2C address".
> >>
> >> 0 is not a valid I2C address?  
> >
> > According to [1] it is reserved, and it's reserved in the I3C spec
> > anyway (see "Table 9 I3C Slave Address Restrictions" in the I3C spec).  
> 
> Sorry, what I meant was s/I3C/I2C/. The first cell is I2C address and
> 0 is not valid.

Okay, got it now :-).
> 
> >> > +
> >> > +  + second and third cells: should encode the ProvisionalID. The second cell
> >> > +                       contains the manufacturer ID left-shifted by 1.
> >> > +                       The third cell contains ORing of the part ID
> >> > +                       left-shifted by 16, the instance ID left-shifted
> >> > +                       by 12 and the extra information. This encoding is
> >> > +                       following the PID definition provided by the I3C
> >> > +                       specification.  
> >
> > One extra question for you: should I refer to the I3C_DEV(),
> > I3C_DEV_WITH_STATIC_ADDR() and I2C_DEV() macros in the bindings doc?
> > And if I do, should I use them my example?  
> 
> Well, I don't want to see "device@I3C_DEV(...)" for unit-addresses.

That wouldn't work anyway.

> You can use them for reg property, but it's somewhat pointless to use
> it in one place and not the other.

Not sure I follow you. These macros have been added to ease definitions
of reg, but you'll still have to manually define the unit-address
manually. Are you saying I should not use them in dts files or just that
I should not mention it in the doc. If this is the former, then patch 6
should be dropped.
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/i3c/i3c.txt b/Documentation/devicetree/bindings/i3c/i3c.txt
new file mode 100644
index 000000000000..ed858228d26b
--- /dev/null
+++ b/Documentation/devicetree/bindings/i3c/i3c.txt
@@ -0,0 +1,140 @@ 
+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 <3>. Read more about addresses below.
+- #size-cells     - should be <0>.
+- compatible      - name of the I3C master controller driving the I3C bus
+
+For other required properties e.g. to describe register sets,
+clocks, etc. check the binding documentation of the specific driver.
+The node describing an I3C bus should be named i3c-master.
+
+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-hz: frequency of the SCL signal used for I3C transfers.
+	      When undefined the core sets it to 12.5MHz.
+
+- i2c-scl-hz: frequency of the SCL signal used for I2C transfers.
+	      When undefined, the core looks at LVR (Legacy Virtual Register)
+	      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. All
+properties described in Documentation/devicetree/bindings/i2c/i2c.txt are
+valid here, but several new properties have been added.
+
+New constraint on existing properties:
+--------------------------------------
+- reg: contains 3 cells
+  + first cell : still encoding the I2C address
+
+  + second cell: should have bit 31 set to 1 signify that this is an I2C
+		 device. Bits 0 to 7 encode the I3C LVR (Legacy Virtual
+		 Register):
+
+	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 (Fast Mode) or FM+ mode
+	* 0: FM+ mode
+	* 1: FM mode
+
+	bit[3:0]: device type
+	* 0-15: reserved
+
+  + third cell: should be 0
+
+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).
+
+The I3C device should be names <device-type>@<static-address>,<i3c-pid>,
+where device-type is describing the type of device connected on the bus
+(gpio-controller, sensor, ...).
+
+Required properties
+-------------------
+- reg: contains 3 cells
+  + first cell : encodes the I2C address. Should be 0 if the device does not
+		 have one (0 is not a valid I3C address).
+
+  + second and third cells: should encode the ProvisionalID. The second cell
+			    contains the manufacturer ID left-shifted by 1.
+			    The third cell contains ORing of the part ID
+			    left-shifted by 16, the instance ID left-shifted
+			    by 12 and the extra information. This encoding is
+			    following the PID definition provided by the I3C
+			    specification.
+
+Optional properties
+-------------------
+- assigned-address: dynamic address to be assigned to this device. This
+		    property is only valid if the I3C device has a static
+		    address (first cell of the reg property != 0).
+
+
+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 {
+			compatible = "nintendo,nunchuk";
+			reg = <0x52 0x80000010 0x0>;
+		};
+
+		/* I3C device with a static address. */
+		thermal_sensor: sensor@68,39200144004 {
+			reg = <0x68 0x392 0x144004>;
+			assigned-address = <0xa>;
+		};
+
+		/*
+		 * I3C device without a static address but requiring resources
+		 * described in the DT.
+		 */
+		sensor@0,39200154004 {
+			reg = <0x0 0x392 0x154004>;
+			clocks = <&clock_provider 0>;
+		};
+	};
+