Re: [PATCH 00/12] usb: dwc3: qcom: Flatten dwc3 structure

From: Bjorn Andersson <quic_bjorande@quicinc.com>
To: Johan Hovold <johan@kernel.org>
Cc: Bjorn Andersson <andersson@kernel.org>,
	Konrad Dybcio <konrad.dybcio@linaro.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Rob Herring <robh+dt@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Conor Dooley <conor+dt@kernel.org>,
	Wesley Cheng <quic_wcheng@quicinc.com>,
	Thinh Nguyen <Thinh.Nguyen@synopsys.com>,
	Felipe Balbi <balbi@kernel.org>,
	Philipp Zabel <p.zabel@pengutronix.de>,
	<linux-arm-msm@vger.kernel.org>, <linux-usb@vger.kernel.org>,
	<devicetree@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
	Krishna Kurapati PSSNV <quic_kriskura@quicinc.com>
Subject: Re: [PATCH 00/12] usb: dwc3: qcom: Flatten dwc3 structure
Date: Mon, 8 Jan 2024 08:46:17 -0800	[thread overview]
Message-ID: <20240108164617.GJ1315173@hu-bjorande-lv.qualcomm.com> (raw)
In-Reply-To: <ZV3OgorG4G4mwvv1@hovoldconsulting.com>

On Wed, Nov 22, 2023 at 10:48:50AM +0100, Johan Hovold wrote:
> On Mon, Oct 16, 2023 at 08:11:08PM -0700, Bjorn Andersson wrote:
> > The USB IP-block found in most Qualcomm platforms is modelled in the
> > Linux kernel as 3 different independent device drivers, but as shown by
> > the already existing layering violations in the Qualcomm glue driver
> > they can not be operated independently.
> > 
> > With the current implementation, the glue driver registers the core and
> > has no way to know when this is done. As a result, e.g. the suspend
> > callbacks needs to guard against NULL pointer dereferences when trying
> > to peek into the struct dwc3 found in the drvdata of the child.
> > 
> > Missing from the upstream Qualcomm USB support is handling of role
> > switching, in which the glue needs to be notified upon DRD mode changes.
> > Several attempts has been made through the years to register callbacks
> > etc, but they always fall short when it comes to handling of the core's
> > probe deferral on resources etc.
> 
> Nice to see this finally being worked on. It's not clear why mode-change
> notifications would be a problem though, as if you get such a
> notification, you know that core has been probed.
>  

The problem here is that the usb_role_switch is implemented in the core,
but the glue needs to act upon the notification as well - and there's
currently no way to have the core notify the glue about such changes.

You can see this "solved" in the case of extcon, where both the Qualcomm
glue and core listens to and act upon the extcon updates. This isn't
pretty, but with the of-graph based role-switching description (and good
judgement) it's not possible to replicate this on modern platforms.

Which means that this leaves a TODO to investigate if we can drop the
extcon support from dwc3-qcom.c

Regards,
Bjorn

> > Furhtermore, the DeviceTree binding is a direct representation of the
> > Linux driver model, and doesn't necessarily describe "the USB IP-block".
> 
> True.
> 
> Johan