From: Heiko Stuebner <heiko@sntech.de> To: Enric Balletbo i Serra <enric.balletbo@collabora.com> Cc: linux-kernel@vger.kernel.org, amstan@chromium.org, groeck@google.com, kieran.bingham@ideasonboard.com, kernel@collabora.com, bleung@chromium.org, devicetree@vger.kernel.org, Frank Wang <frank.wang@rock-chips.com>, linux-rockchip@lists.infradead.org, Rob Herring <robh+dt@kernel.org>, Kishon Vijay Abraham I <kishon@ti.com>, Mark Rutland <mark.rutland@arm.com>, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH 2/4] dt-bindings: phy-rockchip-inno-usb2: add documentation for extcon and utmi-avalid properties. Date: Wed, 15 Aug 2018 13:22:35 +0200 [thread overview] Message-ID: <4665926.GcjFtTh8TS@phil> (raw) In-Reply-To: <37512ec9-8d9a-6c0c-8808-6351b8501002@collabora.com> Hi Enric, Am Mittwoch, 15. August 2018, 13:08:00 CEST schrieb Enric Balletbo i Serra: > On 15/08/18 12:29, Heiko Stuebner wrote: > > Am Mittwoch, 15. August 2018, 11:59:32 CEST schrieb Enric Balletbo i Serra: > >> Commit 98898f3bc83c8 ("phy: rockchip-inno-usb2: support otg-port for > >> rk3399") introduces two new properties. The extcon property is used to > >> detect the cable-state, and the rockchip,utmi-avalid is used to indicate > >> which register should be used to detect the vbus state. > >> > >> Document these properties in the documentation binding. > >> > >> Fixes: 98898f3bc83c8 ("phy: rockchip-inno-usb2: support otg-port for rk3399") > >> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com> [...] > >> @@ -45,6 +46,8 @@ Required properties (port (child) node): > >> Optional properties: > >> - phy-supply : phandle to a regulator that provides power to VBUS. > >> See ./phy-bindings.txt for details. > >> + - rockchip,utmi-avalid : boolean, use the avalid register to get vbus status. > >> + Otherwise, use the bvalid register. > > > > Not having looked to deeply into the usb2 phy, this might raise questions > > on why this is a hardware-description? Is this needed when something is not > > connected on the board? > > I asked myself the same question and even I thought in just remove that code. > > After some investigation, though, I saw that the UTMI+ specification [1] has two > signals similar to ID signal (page 11), the AValid signal is used to indicate if > the session for an A-peripheral is valid and the BValid signal that is used to > indicate if the session for a B-peripheral is valid. I suppose that use of one > or the other matters in some cases, but AFAICT this is not used and I didn't see > any binding using it. > > Maybe someone else can give us more clues on the importance or not of this property? so I've looked in mainline, chromeos-4.4 and the Rockchip vendor-kernel and the only board using that property at all is the rk3399-evb-rev1 and -rev2 in the vendor kernel. The existence of a further -rev3 (which also looks way better cared for compared rev1+2) indicates that the older ones are probably some sort of preproduction models, where some wiring (on the soc or board) may have gone wrong. So while I would keep all the avalid settings in the driver, we could just drop reading that property quietly - as Rob wrote some days ago "it's only an incompatible change if someone notices" [0] and from the above it doesn't look like it ;-) . Heiko [0] https://www.spinics.net/lists/devicetree/msg243978.html
WARNING: multiple messages have this Message-ID (diff)
From: heiko@sntech.de (Heiko Stuebner) To: linux-arm-kernel@lists.infradead.org Subject: [PATCH 2/4] dt-bindings: phy-rockchip-inno-usb2: add documentation for extcon and utmi-avalid properties. Date: Wed, 15 Aug 2018 13:22:35 +0200 [thread overview] Message-ID: <4665926.GcjFtTh8TS@phil> (raw) In-Reply-To: <37512ec9-8d9a-6c0c-8808-6351b8501002@collabora.com> Hi Enric, Am Mittwoch, 15. August 2018, 13:08:00 CEST schrieb Enric Balletbo i Serra: > On 15/08/18 12:29, Heiko Stuebner wrote: > > Am Mittwoch, 15. August 2018, 11:59:32 CEST schrieb Enric Balletbo i Serra: > >> Commit 98898f3bc83c8 ("phy: rockchip-inno-usb2: support otg-port for > >> rk3399") introduces two new properties. The extcon property is used to > >> detect the cable-state, and the rockchip,utmi-avalid is used to indicate > >> which register should be used to detect the vbus state. > >> > >> Document these properties in the documentation binding. > >> > >> Fixes: 98898f3bc83c8 ("phy: rockchip-inno-usb2: support otg-port for rk3399") > >> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com> [...] > >> @@ -45,6 +46,8 @@ Required properties (port (child) node): > >> Optional properties: > >> - phy-supply : phandle to a regulator that provides power to VBUS. > >> See ./phy-bindings.txt for details. > >> + - rockchip,utmi-avalid : boolean, use the avalid register to get vbus status. > >> + Otherwise, use the bvalid register. > > > > Not having looked to deeply into the usb2 phy, this might raise questions > > on why this is a hardware-description? Is this needed when something is not > > connected on the board? > > I asked myself the same question and even I thought in just remove that code. > > After some investigation, though, I saw that the UTMI+ specification [1] has two > signals similar to ID signal (page 11), the AValid signal is used to indicate if > the session for an A-peripheral is valid and the BValid signal that is used to > indicate if the session for a B-peripheral is valid. I suppose that use of one > or the other matters in some cases, but AFAICT this is not used and I didn't see > any binding using it. > > Maybe someone else can give us more clues on the importance or not of this property? so I've looked in mainline, chromeos-4.4 and the Rockchip vendor-kernel and the only board using that property at all is the rk3399-evb-rev1 and -rev2 in the vendor kernel. The existence of a further -rev3 (which also looks way better cared for compared rev1+2) indicates that the older ones are probably some sort of preproduction models, where some wiring (on the soc or board) may have gone wrong. So while I would keep all the avalid settings in the driver, we could just drop reading that property quietly - as Rob wrote some days ago "it's only an incompatible change if someone notices" [0] and from the above it doesn't look like it ;-) . Heiko [0] https://www.spinics.net/lists/devicetree/msg243978.html
next prev parent reply other threads:[~2018-08-15 11:23 UTC|newest] Thread overview: 40+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-08-15 9:59 [PATCH 0/4] phy: rockchip-inno-usb2: document improvements and allow to force B-device valid session bit Enric Balletbo i Serra 2018-08-15 9:59 ` Enric Balletbo i Serra 2018-08-15 9:59 ` [PATCH 1/4] phy: rockchip-inno-usb2: fix misspelling and kernel-doc documentation Enric Balletbo i Serra 2018-08-15 9:59 ` Enric Balletbo i Serra 2018-08-15 10:31 ` Heiko Stuebner 2018-08-15 10:31 ` Heiko Stuebner 2019-01-08 17:35 ` Enric Balletbo Serra 2019-01-08 17:35 ` Enric Balletbo Serra 2019-01-08 17:38 ` Heiko Stübner 2019-01-08 17:38 ` Heiko Stübner 2018-08-15 9:59 ` [PATCH 2/4] dt-bindings: phy-rockchip-inno-usb2: add documentation for extcon and utmi-avalid properties Enric Balletbo i Serra 2018-08-15 9:59 ` Enric Balletbo i Serra 2018-08-15 10:29 ` Heiko Stuebner 2018-08-15 10:29 ` Heiko Stuebner 2018-08-15 11:08 ` Enric Balletbo i Serra 2018-08-15 11:08 ` Enric Balletbo i Serra 2018-08-15 11:22 ` Heiko Stuebner [this message] 2018-08-15 11:22 ` Heiko Stuebner 2018-08-15 22:21 ` Rob Herring 2018-08-15 22:21 ` Rob Herring 2018-08-16 8:38 ` Enric Balletbo i Serra 2018-08-16 8:38 ` Enric Balletbo i Serra 2018-08-15 9:59 ` [PATCH 3/4] phy: rockchip-inno-usb2: allow to force the B-Device Session Valid bit Enric Balletbo i Serra 2018-08-15 9:59 ` Enric Balletbo i Serra 2018-08-15 10:18 ` Heiko Stuebner 2018-08-15 10:18 ` Heiko Stuebner 2018-08-15 10:34 ` Enric Balletbo i Serra 2018-08-15 10:34 ` Enric Balletbo i Serra 2018-08-15 10:36 ` Heiko Stuebner 2018-08-15 10:36 ` Heiko Stuebner 2018-08-15 9:59 ` [PATCH 4/4] dt-bindings: phy-rockchip-inno-usb2: add new rockchip,force-bvalid property Enric Balletbo i Serra 2018-08-15 9:59 ` [PATCH 4/4] dt-bindings: phy-rockchip-inno-usb2: add new rockchip, force-bvalid property Enric Balletbo i Serra 2018-08-15 10:34 ` [PATCH 4/4] dt-bindings: phy-rockchip-inno-usb2: add new rockchip,force-bvalid property Heiko Stuebner 2018-08-15 10:34 ` [PATCH 4/4] dt-bindings: phy-rockchip-inno-usb2: add new rockchip, force-bvalid property Heiko Stuebner 2018-08-15 22:26 ` [PATCH 4/4] dt-bindings: phy-rockchip-inno-usb2: add new rockchip,force-bvalid property Rob Herring 2018-08-15 22:26 ` Rob Herring 2019-01-10 9:06 ` Enric Balletbo Serra 2019-01-10 9:06 ` Enric Balletbo Serra 2019-01-10 12:31 ` Heiko Stuebner 2019-01-10 12:31 ` [PATCH 4/4] dt-bindings: phy-rockchip-inno-usb2: add new rockchip, force-bvalid property Heiko Stuebner
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=4665926.GcjFtTh8TS@phil \ --to=heiko@sntech.de \ --cc=amstan@chromium.org \ --cc=bleung@chromium.org \ --cc=devicetree@vger.kernel.org \ --cc=enric.balletbo@collabora.com \ --cc=frank.wang@rock-chips.com \ --cc=groeck@google.com \ --cc=kernel@collabora.com \ --cc=kieran.bingham@ideasonboard.com \ --cc=kishon@ti.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-rockchip@lists.infradead.org \ --cc=mark.rutland@arm.com \ --cc=robh+dt@kernel.org \ /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.