From: Laurent Pinchart <laurent.pinchart@ideasonboard.com> To: Vladimir Zapolskiy <vz@mleia.com> Cc: Lee Jones <lee.jones@linaro.org>, Linus Walleij <linus.walleij@linaro.org>, Rob Herring <robh+dt@kernel.org>, Marek Vasut <marek.vasut@gmail.com>, Wolfram Sang <wsa@the-dreams.de>, devicetree@vger.kernel.org, linux-gpio@vger.kernel.org, linux-media@vger.kernel.org, linux-kernel@vger.kernel.org, Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com> Subject: Re: [PATCH 2/7] dt-bindings: mfd: ds90ux9xx: add description of TI DS90Ux9xx I2C bridge Date: Fri, 12 Oct 2018 14:54:16 +0300 [thread overview] Message-ID: <1728543.TWj8z2etku@avalon> (raw) In-Reply-To: <20181008211205.2900-3-vz@mleia.com> Hi Vladimir, Thank you for the patch. On Tuesday, 9 October 2018 00:12:00 EEST Vladimir Zapolskiy wrote: > From: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com> > > TI DS90Ux9xx de-/serializers are capable to route I2C messages to > I2C slave devices connected to a remote de-/serializer in a pair, > the change adds description of device tree bindings of the subcontroller > to configure and enable this functionality. > > Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com> > --- > .../bindings/mfd/ti,ds90ux9xx-i2c-bridge.txt | 61 +++++++++++++++++++ > 1 file changed, 61 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/mfd/ti,ds90ux9xx-i2c-bridge.txt > > diff --git > a/Documentation/devicetree/bindings/mfd/ti,ds90ux9xx-i2c-bridge.txt > b/Documentation/devicetree/bindings/mfd/ti,ds90ux9xx-i2c-bridge.txt new > file mode 100644 > index 000000000000..4169e382073a > --- /dev/null > +++ b/Documentation/devicetree/bindings/mfd/ti,ds90ux9xx-i2c-bridge.txt > @@ -0,0 +1,61 @@ > +TI DS90Ux9xx de-/serializer I2C bridge subcontroller > + > +Required properties: > +- compatible: Must contain a generic "ti,ds90ux9xx-i2c-bridge" value and > + may contain one more specific value from the list: > + "ti,ds90ux925-i2c-bridge", > + "ti,ds90ux926-i2c-bridge", > + "ti,ds90ux927-i2c-bridge", > + "ti,ds90ux928-i2c-bridge", > + "ti,ds90ux940-i2c-bridge". I still don't think the I2C bridge should be modeled with a separate compatible string, as explained in my comments to the cover letter and patch 1/7. > +Required properties of a de-/serializer device connected to a local I2C > bus: > +- ti,i2c-bridges: List of phandles to remote de-/serializer devices with > + two arguments: id of a local de-/serializer FPD link and an assigned > + I2C address of a remote de-/serializer to be accessed on a local > + I2C bus. Please don't. Just make the deserializer a child of the serializer. This is how DT works, devices are organized in a tree based on the main control bus. When the deserializer is controlled from the serializer through the FPD link, it should be a child of the serializer. > +Optional properties of a de-/serializer device connected to a local I2C > bus: > +- ti,i2c-bridge-maps: List of 3-cell values: > + - the first argument is id of a local de-/serializer FPD link, > + - the second argument is an I2C address of a device connected to > + a remote de-/serializer IC, > + - the third argument is an I2C address of the remote I2C device > + for access on a local I2C bus. > +- ti,i2c-bridge-auto-ack: Enables AUTO ACK mode. >From my experience with GMSL this isn't something you can just enable or disable unconditionally, it needs to be handled at runtime, and doesn't belong to DT anyway. > +- ti,i2c-bridge-pass-all: Enables PASS ALL mode, remote I2C slave devices > + are accessible on a local (host) I2C bus without I2C address > + remappings. This property also doesn't seem like a hardware description. Please try to focus on what DT describes, not what the effect of the properties are. Bindings need to describe the model of the device, and that description then leads to effects. It shouldn't be the other way around. > +Remote de-/serializer device may contain a list of device nodes, each > +one represents an I2C device connected to that remote de-/serializer IC. > + > +Example (remote device is a deserializer with Atmel MXT touchscreen): > + > +serializer: serializer@c { > + compatible = "ti,ds90ub927q", "ti,ds90ux9xx"; > + reg = <0xc>; > + > + i2c-bridge { > + compatible = "ti,ds90ux927-i2c-bridge", > + "ti,ds90ux9xx-i2c-bridge"; > + ti,i2c-bridges = <&deserializer 0 0x3b>; > + ti,i2c-bridge-maps = <0 0x4b 0x64>; > + }; > +}; > + > +deserializer: deserializer { > + compatible = "ti,ds90ub928q", "ti,ds90ux9xx"; > + > + i2c-bridge { > + compatible = "ti,ds90ux928-i2c-bridge", > + "ti,ds90ux9xx-i2c-bridge"; > + #address-cells = <1>; > + #size-cells = <0>; > + > + touchscreen@4b { > + compatible = "atmel,maxtouch"; > + reg = <0x4b>; > + }; > + }; > +}; -- Regards, Laurent Pinchart
WARNING: multiple messages have this Message-ID (diff)
From: Laurent Pinchart <laurent.pinchart@ideasonboard.com> To: Vladimir Zapolskiy <vz@mleia.com> Cc: Lee Jones <lee.jones@linaro.org>, Linus Walleij <linus.walleij@linaro.org>, Rob Herring <robh+dt@kernel.org>, Marek Vasut <marek.vasut@gmail.com>, Wolfram Sang <wsa@the-dreams.de>, devicetree@vger.kernel.org, linux-gpio@vger.kernel.org, linux-media@vger.kernel.org, linux-kernel@vger.kernel.org, Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com> Subject: Re: [PATCH 2/7] dt-bindings: mfd: ds90ux9xx: add description of TI DS90Ux9xx I2C bridge Date: Fri, 12 Oct 2018 14:54:16 +0300 [thread overview] Message-ID: <1728543.TWj8z2etku@avalon> (raw) In-Reply-To: <20181008211205.2900-3-vz@mleia.com> Hi Vladimir, Thank you for the patch. On Tuesday, 9 October 2018 00:12:00 EEST Vladimir Zapolskiy wrote: > From: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com> > > TI DS90Ux9xx de-/serializers are capable to route I2C messages to > I2C slave devices connected to a remote de-/serializer in a pair, > the change adds description of device tree bindings of the subcontroller > to configure and enable this functionality. > > Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com> > --- > .../bindings/mfd/ti,ds90ux9xx-i2c-bridge.txt | 61 +++++++++++++++++++ > 1 file changed, 61 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/mfd/ti,ds90ux9xx-i2c-bridge.txt > > diff --git > a/Documentation/devicetree/bindings/mfd/ti,ds90ux9xx-i2c-bridge.txt > b/Documentation/devicetree/bindings/mfd/ti,ds90ux9xx-i2c-bridge.txt new > file mode 100644 > index 000000000000..4169e382073a > --- /dev/null > +++ b/Documentation/devicetree/bindings/mfd/ti,ds90ux9xx-i2c-bridge.txt > @@ -0,0 +1,61 @@ > +TI DS90Ux9xx de-/serializer I2C bridge subcontroller > + > +Required properties: > +- compatible: Must contain a generic "ti,ds90ux9xx-i2c-bridge" value and > + may contain one more specific value from the list: > + "ti,ds90ux925-i2c-bridge", > + "ti,ds90ux926-i2c-bridge", > + "ti,ds90ux927-i2c-bridge", > + "ti,ds90ux928-i2c-bridge", > + "ti,ds90ux940-i2c-bridge". I still don't think the I2C bridge should be modeled with a separate compatible string, as explained in my comments to the cover letter and patch 1/7. > +Required properties of a de-/serializer device connected to a local I2C > bus: > +- ti,i2c-bridges: List of phandles to remote de-/serializer devices with > + two arguments: id of a local de-/serializer FPD link and an assigned > + I2C address of a remote de-/serializer to be accessed on a local > + I2C bus. Please don't. Just make the deserializer a child of the serializer. This is how DT works, devices are organized in a tree based on the main control bus. When the deserializer is controlled from the serializer through the FPD link, it should be a child of the serializer. > +Optional properties of a de-/serializer device connected to a local I2C > bus: > +- ti,i2c-bridge-maps: List of 3-cell values: > + - the first argument is id of a local de-/serializer FPD link, > + - the second argument is an I2C address of a device connected to > + a remote de-/serializer IC, > + - the third argument is an I2C address of the remote I2C device > + for access on a local I2C bus. > +- ti,i2c-bridge-auto-ack: Enables AUTO ACK mode. From my experience with GMSL this isn't something you can just enable or disable unconditionally, it needs to be handled at runtime, and doesn't belong to DT anyway. > +- ti,i2c-bridge-pass-all: Enables PASS ALL mode, remote I2C slave devices > + are accessible on a local (host) I2C bus without I2C address > + remappings. This property also doesn't seem like a hardware description. Please try to focus on what DT describes, not what the effect of the properties are. Bindings need to describe the model of the device, and that description then leads to effects. It shouldn't be the other way around. > +Remote de-/serializer device may contain a list of device nodes, each > +one represents an I2C device connected to that remote de-/serializer IC. > + > +Example (remote device is a deserializer with Atmel MXT touchscreen): > + > +serializer: serializer@c { > + compatible = "ti,ds90ub927q", "ti,ds90ux9xx"; > + reg = <0xc>; > + > + i2c-bridge { > + compatible = "ti,ds90ux927-i2c-bridge", > + "ti,ds90ux9xx-i2c-bridge"; > + ti,i2c-bridges = <&deserializer 0 0x3b>; > + ti,i2c-bridge-maps = <0 0x4b 0x64>; > + }; > +}; > + > +deserializer: deserializer { > + compatible = "ti,ds90ub928q", "ti,ds90ux9xx"; > + > + i2c-bridge { > + compatible = "ti,ds90ux928-i2c-bridge", > + "ti,ds90ux9xx-i2c-bridge"; > + #address-cells = <1>; > + #size-cells = <0>; > + > + touchscreen@4b { > + compatible = "atmel,maxtouch"; > + reg = <0x4b>; > + }; > + }; > +}; -- Regards, Laurent Pinchart
next prev parent reply other threads:[~2018-10-12 11:54 UTC|newest] Thread overview: 77+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-10-08 21:11 [PATCH 0/7] mfd/pinctrl: add initial support of TI DS90Ux9xx ICs Vladimir Zapolskiy 2018-10-08 21:11 ` [PATCH 1/7] dt-bindings: mfd: ds90ux9xx: add description " Vladimir Zapolskiy 2018-10-09 0:13 ` Marek Vasut 2018-10-09 11:11 ` Vladimir Zapolskiy 2018-10-09 20:55 ` Vladimir Zapolskiy 2018-10-09 21:03 ` Marek Vasut 2018-10-10 8:41 ` Linus Walleij 2018-10-10 8:41 ` Linus Walleij 2018-10-10 8:41 ` Linus Walleij 2018-10-12 11:44 ` Laurent Pinchart 2018-10-13 14:28 ` Vladimir Zapolskiy 2018-10-13 14:28 ` Vladimir Zapolskiy 2018-10-16 12:30 ` Laurent Pinchart 2018-10-30 16:43 ` Luca Ceresoli 2018-10-30 23:40 ` Vladimir Zapolskiy 2018-10-08 21:12 ` [PATCH 2/7] dt-bindings: mfd: ds90ux9xx: add description of TI DS90Ux9xx I2C bridge Vladimir Zapolskiy 2018-10-12 11:54 ` Laurent Pinchart [this message] 2018-10-12 11:54 ` Laurent Pinchart 2018-10-30 16:43 ` Luca Ceresoli 2018-10-31 20:12 ` Vladimir Zapolskiy 2018-11-03 21:00 ` Luca Ceresoli 2018-11-03 22:07 ` Vladimir Zapolskiy 2018-10-08 21:12 ` [PATCH 3/7] dt-bindings: pinctrl: ds90ux9xx: add description of TI DS90Ux9xx pinmux Vladimir Zapolskiy 2018-10-10 8:45 ` Linus Walleij 2018-10-10 8:45 ` Linus Walleij 2018-10-17 15:02 ` Rob Herring 2018-10-17 15:02 ` Rob Herring 2018-10-12 12:01 ` Laurent Pinchart 2018-10-13 13:47 ` Vladimir Zapolskiy 2018-10-13 13:47 ` Vladimir Zapolskiy 2018-10-16 12:48 ` Laurent Pinchart 2018-10-30 16:44 ` Luca Ceresoli 2018-10-31 20:31 ` Vladimir Zapolskiy 2018-10-08 21:12 ` [PATCH 4/7] mfd: ds90ux9xx: add TI DS90Ux9xx de-/serializer MFD driver Vladimir Zapolskiy 2018-10-09 4:08 ` kbuild test robot 2018-10-09 11:14 ` Vladimir Zapolskiy 2018-10-12 6:03 ` Lee Jones 2018-10-12 7:41 ` Vladimir Zapolskiy 2018-10-12 7:41 ` Vladimir Zapolskiy 2018-10-12 8:39 ` Lee Jones 2018-10-12 9:20 ` Kieran Bingham 2018-10-12 10:58 ` Vladimir Zapolskiy 2018-10-12 10:58 ` Vladimir Zapolskiy 2018-10-12 11:34 ` Lee Jones 2018-10-12 14:13 ` Vladimir Zapolskiy 2018-10-12 14:25 ` Lee Jones 2018-10-12 11:47 ` Kieran Bingham 2018-10-12 13:01 ` Laurent Pinchart 2018-10-12 13:59 ` Vladimir Zapolskiy 2018-10-16 13:08 ` Laurent Pinchart 2018-10-23 8:16 ` Vladimir Zapolskiy 2018-10-30 16:44 ` Luca Ceresoli 2018-10-13 15:10 ` Vladimir Zapolskiy 2018-10-13 15:10 ` Vladimir Zapolskiy 2018-10-16 13:12 ` Laurent Pinchart 2018-10-16 18:32 ` Vladimir Zapolskiy 2018-10-13 12:33 ` Vladimir Zapolskiy 2018-10-13 12:33 ` Vladimir Zapolskiy 2018-10-12 11:24 ` Vladimir Zapolskiy 2018-10-12 11:24 ` Vladimir Zapolskiy 2018-10-12 11:43 ` Lee Jones 2018-10-12 14:23 ` Vladimir Zapolskiy 2018-10-12 13:07 ` Laurent Pinchart 2018-10-08 21:12 ` [PATCH 5/7] mfd: ds90ux9xx: add I2C bridge/alias and link connection driver Vladimir Zapolskiy 2018-10-12 6:04 ` Lee Jones 2018-10-12 7:32 ` Vladimir Zapolskiy 2018-10-12 7:32 ` Vladimir Zapolskiy 2018-10-12 9:03 ` Lee Jones 2018-10-12 13:12 ` Laurent Pinchart 2018-10-12 13:12 ` Laurent Pinchart 2018-10-12 14:36 ` Vladimir Zapolskiy 2018-10-16 14:06 ` Laurent Pinchart 2018-10-08 21:12 ` [PATCH 6/7] pinctrl: ds90ux9xx: add TI DS90Ux9xx pinmux and GPIO controller driver Vladimir Zapolskiy 2018-10-10 9:04 ` Linus Walleij 2018-10-10 9:04 ` Linus Walleij 2018-10-08 21:12 ` [PATCH 7/7] MAINTAINERS: add entry for TI DS90Ux9xx FPD-Link III drivers Vladimir Zapolskiy 2018-10-12 11:34 ` [PATCH 0/7] mfd/pinctrl: add initial support of TI DS90Ux9xx ICs Laurent Pinchart
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=1728543.TWj8z2etku@avalon \ --to=laurent.pinchart@ideasonboard.com \ --cc=devicetree@vger.kernel.org \ --cc=lee.jones@linaro.org \ --cc=linus.walleij@linaro.org \ --cc=linux-gpio@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-media@vger.kernel.org \ --cc=marek.vasut@gmail.com \ --cc=robh+dt@kernel.org \ --cc=vladimir_zapolskiy@mentor.com \ --cc=vz@mleia.com \ --cc=wsa@the-dreams.de \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.