From: Felipe Balbi <balbi@kernel.org>
To: Jun Li <jun.li@nxp.com>,
"gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>
Cc: "shawnguo@kernel.org" <shawnguo@kernel.org>,
dl-linux-imx <linux-imx@nxp.com>,
"thunder.leizhen@huawei.com" <thunder.leizhen@huawei.com>,
"linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>
Subject: RE: [PATCH] usb: dwc3: imx8mp: detect dwc3 core node via compatible string
Date: Fri, 30 Apr 2021 13:00:10 +0300 [thread overview]
Message-ID: <87r1ishz2t.fsf@kernel.org> (raw)
In-Reply-To: <VI1PR04MB5935F1D26E1F281B7273993F895E9@VI1PR04MB5935.eurprd04.prod.outlook.com>
[-- Attachment #1: Type: text/plain, Size: 3114 bytes --]
Hi,
Jun Li <jun.li@nxp.com> writes:
>> -----Original Message-----
>> From: Felipe Balbi <balbi@kernel.org>
>> Sent: Friday, April 30, 2021 4:24 PM
>> To: Jun Li <jun.li@nxp.com>; gregkh@linuxfoundation.org
>> Cc: shawnguo@kernel.org; dl-linux-imx <linux-imx@nxp.com>;
>> thunder.leizhen@huawei.com; linux-usb@vger.kernel.org
>> Subject: Re: [PATCH] usb: dwc3: imx8mp: detect dwc3 core node via compatible
>> string
>>
>> Li Jun <jun.li@nxp.com> writes:
>>
>> > New schema of usb controller DT-node should be named with prefix
>> > "^usb(@.*)?", dt changed the node name, but missed counter part change
>> > in driver, fix it by switching to use compatible string as the dwc3
>> > core compatible string keeps "snps,dwc3" in all dt.
>> >
>> > Fixes: d1689cd3c0f4 ("arm64: dts: imx8mp: Use the correct name for
>> > child node "snps, dwc3"")
>> > Signed-off-by: Li Jun <jun.li@nxp.com>
>>
>>
>> Nice fix :-) It may break down if we have two dwc3 nodes as child of a single
>> parent, but I guess that's very unlikely anyway.
>>
>> Acked-by: Felipe Balbi <balbi@kernel.org>
>>
>> That being said, why do need to keep a pointer to the child? I had a quick
>> look at the driver and it doesn't seem like the pointer is necessary at all.
>
> I need keep the child pointer(dwc3 core platform device) to find the dwc3 core
> instance struct(struct dwc3), the wakeup setting need check the dwc3 core's
> current_dr_role and do runtime resume based on the child's dev.
Right, you're talking about the code below. Some suggestions inline:
> static void dwc3_imx8mp_wakeup_enable(struct dwc3_imx8mp *dwc3_imx)
> {
> struct dwc3 *dwc3 = platform_get_drvdata(dwc3_imx->dwc3);
> u32 val;
>
> if (!dwc3)
> return;
I don't think you need to care if dwc3 has probed or not here.
> val = readl(dwc3_imx->glue_base + USB_WAKEUP_CTRL);
>
> if ((dwc3->current_dr_role == DWC3_GCTL_PRTCAP_HOST) && dwc3->xhci)
> val |= USB_WAKEUP_EN | USB_WAKEUP_SS_CONN |
> USB_WAKEUP_U3_EN | USB_WAKEUP_DPDM_EN;
> else if (dwc3->current_dr_role == DWC3_GCTL_PRTCAP_DEVICE)
> val |= USB_WAKEUP_EN | USB_WAKEUP_VBUS_EN |
> USB_WAKEUP_VBUS_SRC_SESS_VAL;
for this, you could register a listener to the extcon notifier and
update these bits accordingly. With that, you would already *know* that
dwc3 is probed.
> static irqreturn_t dwc3_imx8mp_interrupt(int irq, void *_dwc3_imx)
> {
> struct dwc3_imx8mp *dwc3_imx = _dwc3_imx;
> struct dwc3 *dwc = platform_get_drvdata(dwc3_imx->dwc3);
>
> if (!dwc3_imx->pm_suspended)
> return IRQ_HANDLED;
>
> disable_irq_nosync(dwc3_imx->irq);
> dwc3_imx->wakeup_pending = true;
>
> if ((dwc->current_dr_role == DWC3_GCTL_PRTCAP_HOST) && dwc->xhci)
> pm_runtime_resume(&dwc->xhci->dev);
> else if (dwc->current_dr_role == DWC3_GCTL_PRTCAP_DEVICE)
> pm_runtime_get(dwc->dev);
>
> return IRQ_HANDLED;
> }
for this, maybe you need to teach dwc3 core about wakeup irqs
instead. Have a look dev_pm_set_dedicated_wake_irq().
That would clean things up, I think ;)
--
balbi
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 511 bytes --]
next prev parent reply other threads:[~2021-04-30 10:00 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-04-30 6:57 [PATCH] usb: dwc3: imx8mp: detect dwc3 core node via compatible string Li Jun
2021-04-30 8:24 ` Felipe Balbi
2021-04-30 9:38 ` Jun Li
2021-04-30 10:00 ` Felipe Balbi [this message]
2021-05-06 11:28 ` Jun Li
2021-05-06 14:31 ` Felipe Balbi
2021-05-07 7:31 ` Jun Li
2021-05-11 7:59 ` Heikki Krogerus
2021-05-11 9:23 ` Jun Li
2021-05-11 9:26 ` Felipe Balbi
2021-08-02 5:29 ` Jun Li
2021-08-02 7:49 ` Felipe Balbi
2021-08-02 10:43 ` Jun Li
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=87r1ishz2t.fsf@kernel.org \
--to=balbi@kernel.org \
--cc=gregkh@linuxfoundation.org \
--cc=jun.li@nxp.com \
--cc=linux-imx@nxp.com \
--cc=linux-usb@vger.kernel.org \
--cc=shawnguo@kernel.org \
--cc=thunder.leizhen@huawei.com \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).