Received-SPF: pass (google.com: domain of linux-kernel+bounces-22654-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Date: Wed, 10 Jan 2024 11:23:29 -0800
From: Bjorn Andersson <quic_bjorande@quicinc.com>
To: Krishna Kurapati PSSNV <quic_kriskura@quicinc.com>
CC: Wesley Cheng <quic_wcheng@quicinc.com>, Johan Hovold <johan@kernel.org>,
        Thinh Nguyen <Thinh.Nguyen@synopsys.com>,
        <linux-arm-msm@vger.kernel.org>, <linux-usb@vger.kernel.org>,
        Conor Dooley <conor+dt@kernel.org>, Felipe Balbi
	<balbi@kernel.org>,
        Philipp Zabel <p.zabel@pengutronix.de>,
        "Greg
 Kroah-Hartman" <gregkh@linuxfoundation.org>,
        Konrad Dybcio
	<konrad.dybcio@linaro.org>,
        Krzysztof Kozlowski
	<krzysztof.kozlowski+dt@linaro.org>,
        Rob Herring <robh+dt@kernel.org>,
        "Bjorn
 Andersson" <andersson@kernel.org>,
        <devicetree@vger.kernel.org>, <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 11/12] usb: dwc3: qcom: Flatten the Qualcomm dwc3 binding
 and implementation
Message-ID: <20240110192329.GN1315173@hu-bjorande-lv.qualcomm.com>
References: <20231016-dwc3-refactor-v1-0-ab4a84165470@quicinc.com>
 <20231016-dwc3-refactor-v1-11-ab4a84165470@quicinc.com>
 <1c706807-a216-47cd-a1f6-0546bec27242@quicinc.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <1c706807-a216-47cd-a1f6-0546bec27242@quicinc.com>

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,