From: Conor Dooley <conor@kernel.org>
To: Xu Yang <xu.yang_2@nxp.com>
Cc: peter.chen@kernel.org, jun.li@nxp.com,
gregkh@linuxfoundation.org, linux-usb@vger.kernel.org,
linux-imx@nxp.com
Subject: Re: [PATCH v2 1/8] usb: chipidea: core: add controller resume support when controller is powered off
Date: Tue, 25 Oct 2022 14:47:22 +0100 [thread overview]
Message-ID: <Y1fo6v9Wm0HBdE67@spud> (raw)
In-Reply-To: <20221013151442.3262951-2-xu.yang_2@nxp.com>
On Thu, Oct 13, 2022 at 11:14:35PM +0800, Xu Yang wrote:
> For some SoCs, the controler's power will be off during the system
> suspend, and it needs some recovery operation to let the system back
> to workable. We add this support in this patch.
>
> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
>
> ---
> Changes since v1:
> - add static modifer for ci_handle_power_lost().
> ---
> drivers/usb/chipidea/core.c | 80 ++++++++++++++++++++++++++++---------
> drivers/usb/chipidea/otg.c | 2 +-
> drivers/usb/chipidea/otg.h | 1 +
> 3 files changed, 63 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
> index ae90fee75a32..80267b973c26 100644
> --- a/drivers/usb/chipidea/core.c
> +++ b/drivers/usb/chipidea/core.c
> @@ -637,6 +637,49 @@ static int ci_usb_role_switch_set(struct usb_role_switch *sw,
> return 0;
> }
>
> +static enum ci_role ci_get_role(struct ci_hdrc *ci)
> +{
> + enum ci_role role;
> +
> + if (ci->roles[CI_ROLE_HOST] && ci->roles[CI_ROLE_GADGET]) {
> + if (ci->is_otg) {
> + role = ci_otg_role(ci);
> + hw_write_otgsc(ci, OTGSC_IDIE, OTGSC_IDIE);
> + } else {
> + /*
> + * If the controller is not OTG capable, but support
> + * role switch, the defalt role is gadget, and the
> + * user can switch it through debugfs.
> + */
> + role = CI_ROLE_GADGET;
> + }
> + } else {
> + role = ci->roles[CI_ROLE_HOST] ? CI_ROLE_HOST
> + : CI_ROLE_GADGET;
> + }
> +
> + return role;
> +}
> +
> +static void ci_handle_power_lost(struct ci_hdrc *ci)
Hey,
This appears to have landed in -next and is breaking allmodconfig for
RISC-V:
../drivers/usb/chipidea/core.c:664:13: error: 'ci_handle_power_lost' defined but not used [-Werror=unused-function]
664 | static void ci_handle_power_lost(struct ci_hdrc *ci)
| ^~~~~~~~~~~~~~~~~~~~
CC [M] drivers/media/i2c/mt9t001.o
CC [M] drivers/net/ethernet/davicom/dm9051.o
CC [M] drivers/input/touchscreen/ti_am335x_tsc.o
CC [M] drivers/watchdog/wdt_pci.o
cc1: all warnings being treated as errors
make[5]: *** [../scripts/Makefile.build:250: drivers/usb/chipidea/core.o] Error 1
make[4]: *** [../scripts/Makefile.build:500: drivers/usb/chipidea] Error 2
make[3]: *** [../scripts/Makefile.build:500: drivers/usb] Error 2
make[3]: *** Waiting for unfinished jobs....
The only user seems to be wrapped in an #ifdef CONFIG_PM_SLEEP, and
while I haven't had the chance to investigate further yet that's
probably where I'd start looking.
Apologies if it's been reported, I had a quick look on lore but didn't
see anything.
Thanks,
Conor.
> +{
> + enum ci_role role;
> +
> + disable_irq_nosync(ci->irq);
> + if (!ci_otg_is_fsm_mode(ci)) {
> + role = ci_get_role(ci);
> +
> + if (ci->role != role) {
> + ci_handle_id_switch(ci);
> + } else if (role == CI_ROLE_GADGET) {
> + if (ci->is_otg && hw_read_otgsc(ci, OTGSC_BSV))
> + usb_gadget_vbus_connect(&ci->gadget);
> + }
> + }
> +
> + enable_irq(ci->irq);
> +}
> +
> static struct usb_role_switch_desc ci_role_switch = {
> .set = ci_usb_role_switch_set,
> .get = ci_usb_role_switch_get,
> @@ -1134,25 +1177,7 @@ static int ci_hdrc_probe(struct platform_device *pdev)
> }
> }
>
> - if (ci->roles[CI_ROLE_HOST] && ci->roles[CI_ROLE_GADGET]) {
> - if (ci->is_otg) {
> - ci->role = ci_otg_role(ci);
> - /* Enable ID change irq */
> - hw_write_otgsc(ci, OTGSC_IDIE, OTGSC_IDIE);
> - } else {
> - /*
> - * If the controller is not OTG capable, but support
> - * role switch, the defalt role is gadget, and the
> - * user can switch it through debugfs.
> - */
> - ci->role = CI_ROLE_GADGET;
> - }
> - } else {
> - ci->role = ci->roles[CI_ROLE_HOST]
> - ? CI_ROLE_HOST
> - : CI_ROLE_GADGET;
> - }
> -
> + ci->role = ci_get_role(ci);
> if (!ci_otg_is_fsm_mode(ci)) {
> /* only update vbus status for peripheral */
> if (ci->role == CI_ROLE_GADGET) {
> @@ -1374,8 +1399,16 @@ static int ci_suspend(struct device *dev)
> static int ci_resume(struct device *dev)
> {
> struct ci_hdrc *ci = dev_get_drvdata(dev);
> + bool power_lost;
> int ret;
>
> + /* Since ASYNCLISTADDR (host mode) and ENDPTLISTADDR (device
> + * mode) share the same register address. We can check if
> + * controller resume from power lost based on this address
> + * due to this register will be reset after power lost.
> + */
> + power_lost = !hw_read(ci, OP_ENDPTLISTADDR, ~0);
> +
> if (device_may_wakeup(dev))
> disable_irq_wake(ci->irq);
>
> @@ -1383,6 +1416,15 @@ static int ci_resume(struct device *dev)
> if (ret)
> return ret;
>
> + if (power_lost) {
> + /* shutdown and re-init for phy */
> + ci_usb_phy_exit(ci);
> + ci_usb_phy_init(ci);
> + }
> +
> + if (power_lost)
> + ci_handle_power_lost(ci);
> +
> if (ci->supports_runtime_pm) {
> pm_runtime_disable(dev);
> pm_runtime_set_active(dev);
> diff --git a/drivers/usb/chipidea/otg.c b/drivers/usb/chipidea/otg.c
> index 7b53274ef966..622c3b68aa1e 100644
> --- a/drivers/usb/chipidea/otg.c
> +++ b/drivers/usb/chipidea/otg.c
> @@ -165,7 +165,7 @@ static int hw_wait_vbus_lower_bsv(struct ci_hdrc *ci)
> return 0;
> }
>
> -static void ci_handle_id_switch(struct ci_hdrc *ci)
> +void ci_handle_id_switch(struct ci_hdrc *ci)
> {
> enum ci_role role = ci_otg_role(ci);
>
> diff --git a/drivers/usb/chipidea/otg.h b/drivers/usb/chipidea/otg.h
> index 5e7a6e571dd2..87629b81e03e 100644
> --- a/drivers/usb/chipidea/otg.h
> +++ b/drivers/usb/chipidea/otg.h
> @@ -14,6 +14,7 @@ int ci_hdrc_otg_init(struct ci_hdrc *ci);
> void ci_hdrc_otg_destroy(struct ci_hdrc *ci);
> enum ci_role ci_otg_role(struct ci_hdrc *ci);
> void ci_handle_vbus_change(struct ci_hdrc *ci);
> +void ci_handle_id_switch(struct ci_hdrc *ci);
> static inline void ci_otg_queue_work(struct ci_hdrc *ci)
> {
> disable_irq_nosync(ci->irq);
> --
> 2.34.1
>
>
next prev parent reply other threads:[~2022-10-25 13:47 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-10-13 15:14 [PATCH v2 0/8] add power lost support during system suspend/resume Xu Yang
2022-10-13 15:14 ` [PATCH v2 1/8] usb: chipidea: core: add controller resume support when controller is powered off Xu Yang
2022-10-17 0:56 ` Peter Chen
2022-10-25 13:47 ` Conor Dooley [this message]
2022-10-26 2:50 ` [EXT] " Xu Yang
2022-10-13 15:14 ` [PATCH v2 2/8] usb: chipidea: core: handle suspend/resume for each role Xu Yang
2022-10-17 0:57 ` Peter Chen
2022-10-13 15:14 ` [PATCH v2 3/8] usb: chipidea: host: add suspend/resume support for host controller Xu Yang
2022-10-17 1:06 ` Peter Chen
2022-10-13 15:14 ` [PATCH v2 4/8] usb: chipidea: udc: add suspend/resume support for device controller Xu Yang
2022-10-17 1:07 ` Peter Chen
2022-10-13 15:14 ` [PATCH v2 5/8] usb: chipidea: usbmisc: group usbmisc operations for PM Xu Yang
2022-10-13 15:14 ` [PATCH v2 6/8] usb: chipidea: usbmisc: add power lost check for imx6sx Xu Yang
2022-10-13 15:14 ` [PATCH v2 7/8] usb: chipidea: usbmisc: add power lost check for imx7d Xu Yang
2022-10-13 15:14 ` [PATCH v2 8/8] usb: chipidea: usbmisc: add power lost check for imx7ulp Xu Yang
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=Y1fo6v9Wm0HBdE67@spud \
--to=conor@kernel.org \
--cc=gregkh@linuxfoundation.org \
--cc=jun.li@nxp.com \
--cc=linux-imx@nxp.com \
--cc=linux-usb@vger.kernel.org \
--cc=peter.chen@kernel.org \
--cc=xu.yang_2@nxp.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).