| Message ID | 20191028215919.83697-7-john.stultz@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Prereqs for HiKey960 USB support | expand |
Hi, John Stultz <john.stultz@linaro.org> writes: > The dwc3 core binding specifies one reset. > > However some variants of the hardware my not have more. ^^ may According to synopsys databook, there's a single *input* reset signal on this IP. What is this extra reset you have? Is this, perhaps, specific to your glue layer around the synopsys ip? Should, perhaps, your extra reset be managed by the glue layer?
On Tue, Oct 29, 2019 at 2:17 AM Felipe Balbi <balbi@kernel.org> wrote: > John Stultz <john.stultz@linaro.org> writes: > > The dwc3 core binding specifies one reset. > > > > However some variants of the hardware my not have more. > ^^ > may > > According to synopsys databook, there's a single *input* reset signal on > this IP. What is this extra reset you have? > > Is this, perhaps, specific to your glue layer around the synopsys ip? Likely (again, I unfortunately don't have a ton of detail on the hardware). > Should, perhaps, your extra reset be managed by the glue layer? So yes the dwc3-of-simple does much of this already (it handles multiple resets, and variable clocks), but unfortunately we seem to need new bindings for each device added? I think the suggestion from Rob was due to the sprawl of bindings for the glue code, and the extra complexity of the parent node. So I believe Rob just thought it made sense to collapse this down into the core? I'm not really passionate about either approach, and am happy to rework (as long as there is eventual progress :). Just let me know what you'd prefer. thanks -john
Hi, John Stultz <john.stultz@linaro.org> writes: > On Tue, Oct 29, 2019 at 2:17 AM Felipe Balbi <balbi@kernel.org> wrote: >> John Stultz <john.stultz@linaro.org> writes: >> > The dwc3 core binding specifies one reset. >> > >> > However some variants of the hardware my not have more. >> ^^ >> may >> >> According to synopsys databook, there's a single *input* reset signal on >> this IP. What is this extra reset you have? >> >> Is this, perhaps, specific to your glue layer around the synopsys ip? > > Likely (again, I unfortunately don't have a ton of detail on the hardware). > >> Should, perhaps, your extra reset be managed by the glue layer? > > So yes the dwc3-of-simple does much of this already (it handles > multiple resets, and variable clocks), but unfortunately we seem to > need new bindings for each device added? I think the suggestion from > Rob was due to the sprawl of bindings for the glue code, and the extra > complexity of the parent node. So I believe Rob just thought it made > sense to collapse this down into the core? > > I'm not really passionate about either approach, and am happy to > rework (as long as there is eventual progress :). > Just let me know what you'd prefer. Well, I was under the impression we were supposed to describe the HW. Synopsys IP has a single reset input :-p
On Wed, Oct 30, 2019 at 4:01 AM Felipe Balbi <balbi@kernel.org> wrote: > > > Hi, > > John Stultz <john.stultz@linaro.org> writes: > > > On Tue, Oct 29, 2019 at 2:17 AM Felipe Balbi <balbi@kernel.org> wrote: > >> John Stultz <john.stultz@linaro.org> writes: > >> > The dwc3 core binding specifies one reset. > >> > > >> > However some variants of the hardware my not have more. > >> ^^ > >> may > >> > >> According to synopsys databook, there's a single *input* reset signal on > >> this IP. What is this extra reset you have? > >> > >> Is this, perhaps, specific to your glue layer around the synopsys ip? > > > > Likely (again, I unfortunately don't have a ton of detail on the hardware). > > > >> Should, perhaps, your extra reset be managed by the glue layer? An extra clock or reset is a silly reason to have a whole other node and driver. If there's additional blocks and registers, then yes a glue node makes sense. > > So yes the dwc3-of-simple does much of this already (it handles > > multiple resets, and variable clocks), but unfortunately we seem to > > need new bindings for each device added? I think the suggestion from > > Rob was due to the sprawl of bindings for the glue code, and the extra > > complexity of the parent node. So I believe Rob just thought it made > > sense to collapse this down into the core? > > > > I'm not really passionate about either approach, and am happy to > > rework (as long as there is eventual progress :). > > Just let me know what you'd prefer. > > Well, I was under the impression we were supposed to describe the > HW. Synopsys IP has a single reset input :-p John is. His chip requires 2 resets to use the USB block and the compatible provides that distinction. Maybe HiSilicon has a newer or customized IP version that has 2 resets. The block could have external RAMs (because every process has its own) which may have their own reset. With NDA specifications and little knowledge of the full revision history, we can really never know. Also, omitting clocks and resets from the dwc3 node entirely is just as much not describing the h/w (only the glue needs clocks?). This block is the oddball. I think there's 1 or 2 other blocks where this glue node was done, but please stop. If we did this every time there's a variation in clocks or resets, we'd pretty much have glue nodes everywhere. Rob
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c index 4d4f1836b62c..ef52ffa5d6cb 100644 --- a/drivers/usb/dwc3/core.c +++ b/drivers/usb/dwc3/core.c @@ -1442,7 +1442,7 @@ static int dwc3_probe(struct platform_device *pdev) dwc3_get_properties(dwc); - dwc->reset = devm_reset_control_get_optional_shared(dev, NULL); + dwc->reset = devm_reset_control_array_get(dev, true, true); if (IS_ERR(dwc->reset)) return PTR_ERR(dwc->reset);
The dwc3 core binding specifies one reset. However some variants of the hardware my not have more. So this patch reworks the reading of the resets to fetch all the resets specified in the dts together. This patch was reccomended by Rob Herring <robh@kernel.org> as an alternative to creating multiple bindings for each variant of hardware when the only unique bits were clocks and resets. Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Cc: Rob Herring <robh+dt@kernel.org> Cc: Mark Rutland <mark.rutland@arm.com> CC: ShuFan Lee <shufan_lee@richtek.com> Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com> Cc: Suzuki K Poulose <suzuki.poulose@arm.com> Cc: Chunfeng Yun <chunfeng.yun@mediatek.com> Cc: Yu Chen <chenyu56@huawei.com> Cc: Felipe Balbi <balbi@kernel.org> Cc: Hans de Goede <hdegoede@redhat.com> Cc: Andy Shevchenko <andy.shevchenko@gmail.com> Cc: Jun Li <lijun.kernel@gmail.com> Cc: Valentin Schneider <valentin.schneider@arm.com> Cc: Jack Pham <jackp@codeaurora.org> Cc: linux-usb@vger.kernel.org Cc: devicetree@vger.kernel.org Suggested-by: Rob Herring <robh@kernel.org> Signed-off-by: John Stultz <john.stultz@linaro.org> --- v3: Rework dwc3 core rather then adding another dwc-of-simple binding. --- drivers/usb/dwc3/core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)