From: Rob Herring <robh@kernel.org> To: Felipe Balbi <felipe.balbi@linux.intel.com> Cc: Chen Yu <chenyu56@huawei.com>, Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>, Linux USB List <linux-usb@vger.kernel.org>, devicetree@vger.kernel.org, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, Wangbinghui <wangbinghui@hisilicon.com>, Suzhuangluan <suzhuangluan@hisilicon.com>, kongfei@hisilicon.com, Greg Kroah-Hartman <gregkh@linuxfoundation.org>, Mark Rutland <mark.rutland@arm.com>, John Stultz <john.stultz@linaro.org> Subject: Re: [PATCH v1 01/12] dt-bindings: usb: add support for dwc3 controller on HiSilicon SoCs Date: Thu, 20 Dec 2018 07:48:43 -0600 [thread overview] Message-ID: <CAL_JsqKooF=R93enNTvP9MLaJgoygoOEYyh4OU6-RP6E3Fv0vA@mail.gmail.com> (raw) In-Reply-To: <87y38ku24b.fsf@linux.intel.com> On Thu, Dec 20, 2018 at 12:46 AM Felipe Balbi <felipe.balbi@linux.intel.com> wrote: > > > Hi, > > Rob Herring <robh@kernel.org> writes: > >> >>>> +Example: > >> >>>> + usb3: hisi_dwc3 { > >> >>>> + compatible = "hisilicon,hi3660-dwc3"; > >> >>>> + #address-cells = <2>; > >> >>>> + #size-cells = <2>; > >> >>>> + ranges; > >> >>>> + > >> >>>> + clocks = <&crg_ctrl HI3660_CLK_ABB_USB>, > >> >>>> + <&crg_ctrl HI3660_ACLK_GATE_USB3OTG>; > >> >>>> + clock-names = "clk_usb3phy_ref", "aclk_usb3otg"; > >> >>>> + assigned-clocks = <&crg_ctrl HI3660_ACLK_GATE_USB3OTG>; > >> >>>> + assigned-clock-rates = <229000000>; > >> >>>> + resets = <&crg_rst 0x90 8>, > >> >>>> + <&crg_rst 0x90 7>, > >> >>>> + <&crg_rst 0x90 6>, > >> >>>> + <&crg_rst 0x90 5>; > >> >>>> + > >> >>>> + dwc3: dwc3@ff100000 { > > > > Please combine these into a single node. Unless you have a wrapper with > > registers, you don't need these 2 nodes. Clocks and reset can go in the > > dwc3 node. > > > >> >>> > >> >>> According to the DT spec, the node names should be generic, not chip specific, i.e. usb@ff100000 in this case. > >> >>> > >> >> > >> >> Do you mean it should be usb@ff100000: dwc3@ff100000 ? > >> > > >> > dwc3: usb@ff100000 > >> > > >> > "dwc3:" is a label, not name. > >> > >> I use the node name "dwc3@ff100000" according to Documentation/devicetree/bindings/usb/dwc3.txt > >> and documentations of vendor drivers, i.e. qcom,dwc3.txt, rockchip,dwc3.txt. > >> > >> In these documentations, the dwc3 sub-node name uses "dwc3@xxxxxxxx". > >> > >> I think it is better to be same as the other vendor's dwc3 drivers. > > > > It's not. The other bindings are wrong. Follow the DT Spec. > > what's wrong about them? They clearly describe the HW: > > 1) a company-specific glue/adaptation/integration IP > 2) a generic licensed IP inside it That is *every* licensed IP block and DWC3 is the oddball where we did this 2 node thing. It is not a pattern we should continue. If there's registers in the wrapper, then yes, having 2 nodes makes sense. But just additional clocks or resets, no. I would guess these extra clocks and resets are inter-connect related and are needed as an artifact of not describing and managing inter-connects. I can just as easily argue it doesn't describe the hardware. I'm pretty sure the DWC3 has clocks and resets yet there are none in the DWC3 node. How can it operate with no clocks? > dwc3.ko is compatible with Synopsys' documentation and there's only one > incarnation of dwc3. Everything that can be detected in runtime, we do > so. Everything that can't, we use quirk flags. Keep in mind dwc3.ko is > also used as is by non-DT systems where we can't simply change a > compatible flag. Linux driver architecture doesn't dictate bindings. Rob
WARNING: multiple messages have this Message-ID (diff)
From: Rob Herring <robh@kernel.org> To: Felipe Balbi <felipe.balbi@linux.intel.com> Cc: Chen Yu <chenyu56@huawei.com>, Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>, Linux USB List <linux-usb@vger.kernel.org>, devicetree@vger.kernel.org, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, Wangbinghui <wangbinghui@hisilicon.com>, Suzhuangluan <suzhuangluan@hisilicon.com>, kongfei@hisilicon.com, Greg Kroah-Hartman <gregkh@linuxfoundation.org>, Mark Rutland <mark.rutland@arm.com>, John Stultz <john.stultz@linaro.org> Subject: [v1,01/12] dt-bindings: usb: add support for dwc3 controller on HiSilicon SoCs Date: Thu, 20 Dec 2018 07:48:43 -0600 [thread overview] Message-ID: <CAL_JsqKooF=R93enNTvP9MLaJgoygoOEYyh4OU6-RP6E3Fv0vA@mail.gmail.com> (raw) On Thu, Dec 20, 2018 at 12:46 AM Felipe Balbi <felipe.balbi@linux.intel.com> wrote: > > > Hi, > > Rob Herring <robh@kernel.org> writes: > >> >>>> +Example: > >> >>>> + usb3: hisi_dwc3 { > >> >>>> + compatible = "hisilicon,hi3660-dwc3"; > >> >>>> + #address-cells = <2>; > >> >>>> + #size-cells = <2>; > >> >>>> + ranges; > >> >>>> + > >> >>>> + clocks = <&crg_ctrl HI3660_CLK_ABB_USB>, > >> >>>> + <&crg_ctrl HI3660_ACLK_GATE_USB3OTG>; > >> >>>> + clock-names = "clk_usb3phy_ref", "aclk_usb3otg"; > >> >>>> + assigned-clocks = <&crg_ctrl HI3660_ACLK_GATE_USB3OTG>; > >> >>>> + assigned-clock-rates = <229000000>; > >> >>>> + resets = <&crg_rst 0x90 8>, > >> >>>> + <&crg_rst 0x90 7>, > >> >>>> + <&crg_rst 0x90 6>, > >> >>>> + <&crg_rst 0x90 5>; > >> >>>> + > >> >>>> + dwc3: dwc3@ff100000 { > > > > Please combine these into a single node. Unless you have a wrapper with > > registers, you don't need these 2 nodes. Clocks and reset can go in the > > dwc3 node. > > > >> >>> > >> >>> According to the DT spec, the node names should be generic, not chip specific, i.e. usb@ff100000 in this case. > >> >>> > >> >> > >> >> Do you mean it should be usb@ff100000: dwc3@ff100000 ? > >> > > >> > dwc3: usb@ff100000 > >> > > >> > "dwc3:" is a label, not name. > >> > >> I use the node name "dwc3@ff100000" according to Documentation/devicetree/bindings/usb/dwc3.txt > >> and documentations of vendor drivers, i.e. qcom,dwc3.txt, rockchip,dwc3.txt. > >> > >> In these documentations, the dwc3 sub-node name uses "dwc3@xxxxxxxx". > >> > >> I think it is better to be same as the other vendor's dwc3 drivers. > > > > It's not. The other bindings are wrong. Follow the DT Spec. > > what's wrong about them? They clearly describe the HW: > > 1) a company-specific glue/adaptation/integration IP > 2) a generic licensed IP inside it That is *every* licensed IP block and DWC3 is the oddball where we did this 2 node thing. It is not a pattern we should continue. If there's registers in the wrapper, then yes, having 2 nodes makes sense. But just additional clocks or resets, no. I would guess these extra clocks and resets are inter-connect related and are needed as an artifact of not describing and managing inter-connects. I can just as easily argue it doesn't describe the hardware. I'm pretty sure the DWC3 has clocks and resets yet there are none in the DWC3 node. How can it operate with no clocks? > dwc3.ko is compatible with Synopsys' documentation and there's only one > incarnation of dwc3. Everything that can be detected in runtime, we do > so. Everything that can't, we use quirk flags. Keep in mind dwc3.ko is > also used as is by non-DT systems where we can't simply change a > compatible flag. Linux driver architecture doesn't dictate bindings. Rob
next prev parent reply other threads:[~2018-12-20 13:49 UTC|newest] Thread overview: 130+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-12-03 3:45 [PATCH v1 00/12] Add support for usb on Hikey960 Yu Chen 2018-12-03 3:45 ` Yu Chen 2018-12-03 3:45 ` [PATCH v1 01/12] dt-bindings: usb: add support for dwc3 controller on HiSilicon SoCs Yu Chen 2018-12-03 3:45 ` [v1,01/12] " Yu Chen 2018-12-03 3:45 ` [PATCH v1 01/12] " Yu Chen 2018-12-03 8:35 ` Sergei Shtylyov 2018-12-03 8:35 ` [v1,01/12] " Sergei Shtylyov 2018-12-03 8:51 ` [PATCH v1 01/12] " Chen Yu 2018-12-03 8:51 ` [v1,01/12] " Yu Chen 2018-12-03 8:51 ` [PATCH v1 01/12] " Chen Yu 2018-12-03 8:59 ` Sergei Shtylyov 2018-12-03 8:59 ` [v1,01/12] " Sergei Shtylyov 2018-12-03 9:28 ` [PATCH v1 01/12] " Chen Yu 2018-12-03 9:28 ` [v1,01/12] " Yu Chen 2018-12-03 9:28 ` [PATCH v1 01/12] " Chen Yu 2018-12-19 14:09 ` Rob Herring 2018-12-19 14:09 ` [v1,01/12] " Rob Herring 2018-12-20 1:33 ` [PATCH v1 01/12] " Chen Yu 2018-12-20 1:33 ` [v1,01/12] " Yu Chen 2018-12-20 1:33 ` [PATCH v1 01/12] " Chen Yu 2018-12-20 6:46 ` Felipe Balbi 2018-12-20 6:46 ` [v1,01/12] " Felipe Balbi 2018-12-20 13:48 ` Rob Herring [this message] 2018-12-20 13:48 ` Rob Herring 2018-12-03 3:45 ` [PATCH v1 02/12] dt-bindings: phy: Add support for HiSilicon's hi3660 USB PHY Yu Chen 2018-12-03 3:45 ` [v1,02/12] " Yu Chen 2018-12-03 3:45 ` [PATCH v1 02/12] " Yu Chen 2018-12-19 14:14 ` Rob Herring 2018-12-19 14:14 ` [v1,02/12] " Rob Herring 2018-12-20 1:52 ` [PATCH v1 02/12] " Chen Yu 2018-12-20 1:52 ` [v1,02/12] " Yu Chen 2018-12-20 1:52 ` [PATCH v1 02/12] " Chen Yu 2018-12-03 3:45 ` [PATCH v1 03/12] dt-bindings: misc: Add bindings for HiSilicon usb hub and data role switch functionality on HiKey960 Yu Chen 2018-12-03 3:45 ` [v1,03/12] " Yu Chen 2018-12-03 3:45 ` [PATCH v1 03/12] " Yu Chen 2018-12-19 14:21 ` Rob Herring 2018-12-19 14:21 ` [v1,03/12] " Rob Herring 2018-12-20 2:02 ` [PATCH v1 03/12] " Chen Yu 2018-12-20 2:02 ` [v1,03/12] " Yu Chen 2018-12-20 2:02 ` [PATCH v1 03/12] " Chen Yu 2018-12-03 3:45 ` [PATCH v1 04/12] usb: dwc3: dwc3-hisi: Add code for dwc3 of Hisilicon Soc Platform Yu Chen 2018-12-03 3:45 ` [v1,04/12] " Yu Chen 2018-12-03 3:45 ` [PATCH v1 04/12] " Yu Chen 2018-12-03 8:12 ` Andy Shevchenko 2018-12-03 8:12 ` [v1,04/12] " Andy Shevchenko 2018-12-03 8:30 ` [PATCH v1 04/12] " Chen Yu 2018-12-03 8:30 ` [v1,04/12] " Yu Chen 2018-12-03 8:30 ` [PATCH v1 04/12] " Chen Yu 2018-12-03 3:45 ` [PATCH v1 05/12] usb: dwc3: Add two quirks for Hisilicon Kirin " Yu Chen 2018-12-03 3:45 ` [v1,05/12] " Yu Chen 2018-12-03 3:45 ` [PATCH v1 05/12] " Yu Chen 2018-12-03 8:02 ` Andy Shevchenko 2018-12-03 8:02 ` [v1,05/12] " Andy Shevchenko 2018-12-03 8:23 ` [PATCH v1 05/12] " Chen Yu 2018-12-03 8:23 ` [v1,05/12] " Yu Chen 2018-12-03 8:23 ` [PATCH v1 05/12] " Chen Yu 2018-12-03 3:45 ` [PATCH v1 06/12] phy: Add usb phy support for hi3660 Soc of Hisilicon Yu Chen 2018-12-03 3:45 ` [v1,06/12] " Yu Chen 2018-12-03 3:45 ` [PATCH v1 06/12] " Yu Chen 2018-12-03 8:14 ` Andy Shevchenko 2018-12-03 8:14 ` [v1,06/12] " Andy Shevchenko 2018-12-03 3:45 ` [PATCH v1 07/12] usb: roles: Find the usb role switch by also matching against the device node Yu Chen 2018-12-03 3:45 ` [v1,07/12] " Yu Chen 2018-12-03 3:45 ` [PATCH v1 07/12] " Yu Chen 2018-12-03 8:17 ` Andy Shevchenko 2018-12-03 8:17 ` [v1,07/12] " Andy Shevchenko 2018-12-03 12:25 ` [PATCH v1 07/12] " Heikki Krogerus 2018-12-03 12:25 ` [v1,07/12] " Heikki Krogerus 2019-01-21 8:36 ` [PATCH v1 07/12] " Chen Yu 2019-01-21 8:36 ` [v1,07/12] " Yu Chen 2019-01-21 8:36 ` [PATCH v1 07/12] " Chen Yu 2019-01-21 13:51 ` Heikki Krogerus 2019-01-21 13:51 ` [v1,07/12] " Heikki Krogerus 2019-01-25 12:35 ` [PATCH v1 07/12] " Heikki Krogerus 2019-01-25 12:35 ` [v1,07/12] " Heikki Krogerus 2018-12-03 3:45 ` [PATCH v1 08/12] usb: roles: Add usb role switch notifier Yu Chen 2018-12-03 3:45 ` [v1,08/12] " Yu Chen 2018-12-03 3:45 ` [PATCH v1 08/12] " Yu Chen 2018-12-03 12:38 ` Heikki Krogerus 2018-12-03 12:38 ` [v1,08/12] " Heikki Krogerus 2018-12-04 1:07 ` [PATCH v1 08/12] " Chen Yu 2018-12-04 1:07 ` [v1,08/12] " Yu Chen 2018-12-04 1:07 ` [PATCH v1 08/12] " Chen Yu 2018-12-03 3:45 ` [PATCH v1 09/12] usb: dwc3: Registering a role switch in the DRD code Yu Chen 2018-12-03 3:45 ` [v1,09/12] " Yu Chen 2018-12-03 3:45 ` [PATCH v1 09/12] " Yu Chen 2018-12-03 9:15 ` Andy Shevchenko 2018-12-03 9:15 ` [v1,09/12] " Andy Shevchenko 2018-12-03 9:30 ` [PATCH v1 09/12] " Chen Yu 2018-12-03 9:30 ` [v1,09/12] " Yu Chen 2018-12-03 9:30 ` [PATCH v1 09/12] " Chen Yu 2018-12-03 12:43 ` Heikki Krogerus 2018-12-03 12:43 ` [v1,09/12] " Heikki Krogerus 2018-12-04 10:54 ` [PATCH v1 09/12] " Heikki Krogerus 2018-12-04 10:54 ` [v1,09/12] " Heikki Krogerus 2018-12-05 0:56 ` [PATCH v1 09/12] " Chen Yu 2018-12-05 0:56 ` [v1,09/12] " Yu Chen 2018-12-05 0:56 ` [PATCH v1 09/12] " Chen Yu 2018-12-03 3:45 ` [PATCH v1 10/12] hikey960: Support usb functionality of Hikey960 Yu Chen 2018-12-03 3:45 ` [v1,10/12] " Yu Chen 2018-12-03 3:45 ` [PATCH v1 10/12] " Yu Chen 2018-12-03 9:23 ` Andy Shevchenko 2018-12-03 9:23 ` [v1,10/12] " Andy Shevchenko 2018-12-04 1:40 ` [PATCH v1 10/12] " Chen Yu 2018-12-04 1:40 ` [v1,10/12] " Yu Chen 2018-12-04 1:40 ` [PATCH v1 10/12] " Chen Yu 2018-12-04 17:47 ` Andy Shevchenko 2018-12-04 17:47 ` [v1,10/12] " Andy Shevchenko 2018-12-05 1:57 ` [PATCH v1 10/12] " Chen Yu 2018-12-05 1:57 ` [v1,10/12] " Yu Chen 2018-12-05 1:57 ` [PATCH v1 10/12] " Chen Yu 2018-12-05 8:44 ` Andy Shevchenko 2018-12-05 8:44 ` [v1,10/12] " Andy Shevchenko 2018-12-03 3:45 ` [PATCH v1 11/12] usb: gadget: Add configfs attribuite for controling match_existing_only Yu Chen 2018-12-03 3:45 ` [v1,11/12] " Yu Chen 2018-12-03 3:45 ` [PATCH v1 11/12] " Yu Chen 2018-12-03 9:25 ` Andy Shevchenko 2018-12-03 9:25 ` [v1,11/12] " Andy Shevchenko 2018-12-04 1:22 ` [PATCH v1 11/12] " Chen Yu 2018-12-04 1:22 ` [v1,11/12] " Yu Chen 2018-12-04 1:22 ` [PATCH v1 11/12] " Chen Yu 2018-12-03 9:53 ` Krzysztof Opasiak 2018-12-03 9:53 ` [v1,11/12] " Krzysztof Opasiak 2018-12-03 3:45 ` [PATCH v1 12/12] dts: hi3660: Add support for usb on Hikey960 Yu Chen 2018-12-03 3:45 ` Yu Chen 2018-12-03 3:45 ` [v1,12/12] " Yu Chen 2018-12-03 3:45 ` [PATCH v1 12/12] " Yu Chen 2018-12-04 12:45 ` Heikki Krogerus 2018-12-04 12:45 ` Heikki Krogerus 2018-12-04 12:45 ` [v1,12/12] " Heikki Krogerus
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='CAL_JsqKooF=R93enNTvP9MLaJgoygoOEYyh4OU6-RP6E3Fv0vA@mail.gmail.com' \ --to=robh@kernel.org \ --cc=chenyu56@huawei.com \ --cc=devicetree@vger.kernel.org \ --cc=felipe.balbi@linux.intel.com \ --cc=gregkh@linuxfoundation.org \ --cc=john.stultz@linaro.org \ --cc=kongfei@hisilicon.com \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-usb@vger.kernel.org \ --cc=mark.rutland@arm.com \ --cc=sergei.shtylyov@cogentembedded.com \ --cc=suzhuangluan@hisilicon.com \ --cc=wangbinghui@hisilicon.com \ /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.