All of lore.kernel.org
 help / color / mirror / Atom feed
From: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
To: Krishna Kurapati <krishna.kurapati@oss.qualcomm.com>,
	Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Cc: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	"linux-arm-msm@vger.kernel.org" <linux-arm-msm@vger.kernel.org>,
	"linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v3] usb: dwc3: qcom: Modify interrupt handling for eUSB2 Phy targets
Date: Thu, 28 May 2026 00:46:53 +0000	[thread overview]
Message-ID: <aheMJFKI_kBTH3jH@vbox> (raw)
In-Reply-To: <ahd_VjrHp-u6thMT@vbox>

On Wed, May 27, 2026, Thinh Nguyen wrote:
> On Thu, May 21, 2026, Konrad Dybcio wrote:
> > On 5/13/26 10:48 PM, Dmitry Baryshkov wrote:
> > > On Mon, May 11, 2026 at 03:14:22PM +0530, Krishna Kurapati wrote:
> > >> eUSB2 targets handle wakeup interrupts differently depending on device
> > >> speed when operating in host mode.
> > >>
> > >> According to the eUSB2 specification, remote wakeup signaling in host
> > >> mode is detected via different data-line assertions based on the
> > >> connected device speed.
> > >>
> > >> When a low-speed device is connected, the host repeater drives eD+ to
> > >> logic '1' upon detecting a K-state on the USB lines during remote wakeup
> > >> (eUSB2 specification, Section 5.5.14).
> > >>
> > >> When a full-speed or high-speed device is connected, the host repeater
> > >> drives eD- to logic '1' upon detecting a K-state on the USB line during
> > >> remote wakeup (eUSB2 specification, Sections 5.5.15 and 5.5.18).
> > >>
> > >> Since the eUSB2 PHY's "DP" and "DM" interrupt lines monitor the eD+ and
> > >> eD- line states, configure the wakeup interrupts accordingly
> > >>
> > >> Signed-off-by: Krishna Kurapati <krishna.kurapati@oss.qualcomm.com>
> > >> ---
> > 
> > [...]
> > 
> > >> +	{
> > >> +		.compatible = "qcom,x1e80100-dwc3-mp",
> > >> +		.data = &dwc3_qcom_glymur_pdata,
> > > 
> > > This would result in the list of the platforms keeping on growing.
> > > Would you mind instead detecting eUSB2 by checking that HS PHY has its
> > > own phys property?
> > 
> > I think we've had a very similar issue on some other patch.. we could
> > use phy_mode, but it'd require first fixing this mess:
> > 
> > 28:     PHY_MODE_USB_HOST,
> > 29:     PHY_MODE_USB_HOST_LS,
> > 30:     PHY_MODE_USB_HOST_FS,
> > 31:     PHY_MODE_USB_HOST_HS,
> > 32:     PHY_MODE_USB_HOST_SS,
> > 33:     PHY_MODE_USB_DEVICE,
> > 34:     PHY_MODE_USB_DEVICE_LS,
> > 35:     PHY_MODE_USB_DEVICE_FS,
> > 36:     PHY_MODE_USB_DEVICE_HS,
> > 37:     PHY_MODE_USB_DEVICE_SS,
> > 38:     PHY_MODE_USB_OTG
> > 
> > into PHY_MODE_USB + submodes and phy_opts
> > 
> 
> IMHO, this seems to fit better in the DT binding, ie. a DT property
> "qcom,eusb2-phy" would be a cleaner solution than trying to match this
> with to compatible string.

Actually, it should be "qcom,has-eusb2-phy".

> 
> Also Krishna, your patch is corrupted. Please fix it.
> 

I notice all the qcom phy with eusb2 phy as a eusb2-phy substring. We
can (maybe) match the phy compatible substring:

/* not tested */
static bool dwc3_qcom_has_eusb2_phy(struct device *dev)
{
	struct device_node *hs_phy;
	int i;
	bool ret;

	if (of_property_match_string(dev->of_node, "phy-names", "usb2-phy"))
		return false;

	hs_phy = of_parse_phandle(dev->of_node, "phys", i);
	if (!hs_phy)
		return false;

	ret = of_device_is_compatible(hs_phy, "eusb2-phy");
	of_node_put(hs_phy);
	return ret;
}

But I don't like this approach either. Seems very fragile.

BR,
Thinh

  reply	other threads:[~2026-05-28  0:47 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-11  9:44 [PATCH v3] usb: dwc3: qcom: Modify interrupt handling for eUSB2 Phy targets Krishna Kurapati
2026-05-13 20:48 ` Dmitry Baryshkov
2026-05-21  9:48   ` Konrad Dybcio
2026-05-27 23:43     ` Thinh Nguyen
2026-05-28  0:46       ` Thinh Nguyen [this message]
2026-05-28  5:38         ` Dmitry Baryshkov
2026-05-29  0:34           ` Thinh Nguyen

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=aheMJFKI_kBTH3jH@vbox \
    --to=thinh.nguyen@synopsys.com \
    --cc=dmitry.baryshkov@oss.qualcomm.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=konrad.dybcio@oss.qualcomm.com \
    --cc=krishna.kurapati@oss.qualcomm.com \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    /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: link
Be 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.