Re: [PATCH 11/12] usb: dwc3: qcom: Flatten the Qualcomm dwc3 binding and implementation

[Bjorn Andersson]

On Wed, Jan 10, 2024 at 08:43:23AM +0530, Krishna Kurapati PSSNV wrote:
> 
> 
> On 10/17/2023 8:41 AM, Bjorn Andersson wrote:
> > The USB block found in most Qualcomm platforms is modelled as three
> > different independent device drivers, and represented in DeviceTree as
> > two layered nodes. But as shown by the already existing layering
> > violations in the Qualcomm glue driver they can not be operated
> > independently.
> > 
> > In the current model, the probing of the core is asynchronous, and in a
> > number of places there's risk that the driver dereferences NULL
> > pointers, as it peeks into the core's drvdata.
> > 
> > There is also no way, in the current design to make the core notify the
> > glue upon DRD mode changes. Among the past proposals have been attempts
> > to provide a callback registration API, but as there is no way to know
> > when the core is probed this doesn't work.
> > 
> > Based on the recent refactoring its now possible to instantiate the glue
> > and core from a single representation of the DWC3 IP-block. This will
> > also allow for the glue to pass a callback to be called for DRD mode
> > changes.
> > 
> > The only overlapping handling between the Qualcomm glue and the core is
> > the release of reset, which is left to the core to handle.
> > 
> 
> Hi Bjorn,
> 
>  I think the reset has to be handled by glue itself. I was testing this
> series and found one issue:
> 
>  During suspend, we suspend core first which will assert the reset and then
> suspend the glue which will disable the clocks. This path doesn't seem to
> have a problem somehow even in flattened implementation.
> 
>  During resume, we resume the glue first and then resume the core. During
> resume of glue, we enable the clocks and at this point, the reset is still
> kept asserted causing the clocks to never turn ON leading to a crash. This
> is the case in flattened implementation only as in normal case, the reset is
> handled by glue and we never meddle with reset other than the time of
> probing.
> 
> I tried to check if we explicitly de-assert the reset during start of resume
> sequence of glue (in addition to the de-assertion present in core) and
> things worked out fine. But if I try to balance the reset count and add an
> assert at end of suspend sequence of glue (in addition to the assertion
> present in core), then it crashes complaining a double assertion happened.
> So double de-asserting is not causing a problem but double asserting is
> causing an issue.
> 

You're right. I looked at it briefly but ended up moving the reset
handling in the wrong direction...

I expect that in any scenario where a glue driver is used the core can
not control the reset. So far we've dealt with this by just not telling
the core about the reset.

Thanks,
Bjorn

> Regards,
> Krishna,