From: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
To: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>,
Kishon Vijay Abraham I <kishon@ti.com>
Cc: "linux-renesas-soc@vger.kernel.org"
<linux-renesas-soc@vger.kernel.org>,
Simon Horman <horms@verge.net.au>,
Chris Paterson <Chris.Paterson2@renesas.com>,
Biju Das <biju.das@bp.renesas.com>,
Geert Uytterhoeven <geert+renesas@glider.be>
Subject: RE: [PATCH 2/4] phy: renesas: rcar-gen3-usb2: No need to request IRQ for non-OTG
Date: Wed, 3 Apr 2019 10:24:17 +0000 [thread overview]
Message-ID: <TY1PR01MB17703687A7ECF803B338834EC0570@TY1PR01MB1770.jpnprd01.prod.outlook.com> (raw)
In-Reply-To: <TYAPR01MB4271A67EA5A43F11656DD34AD8560@TYAPR01MB4271.jpnprd01.prod.outlook.com>
Hello Yoshihiro-san, Kishon
> From: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> Sent: 02 April 2019 03:11
> Subject: RE: [PATCH 2/4] phy: renesas: rcar-gen3-usb2: No need to request IRQ for non-OTG
>
> Hi Kishon, Fabrizio,
>
> > From: Kishon Vijay Abraham I, Sent: Monday, April 1, 2019 9:26 PM
> >
> > On 01/03/19 4:35 PM, Fabrizio Castro wrote:
> > > As stated in the comment found on top of rcar_gen3_phy_usb2_work,
> > > we should not be registering the IRQ if !otg_channel || !uses_otg_pins.
> > > On top of that, we should also make sure that extcon has been
> > > allocated before requesting the irq as rcar_gen3_phy_usb2_work
> > > uses it, hence the patch.
> > >
> > > Fixes: 2b38543c8db1 ("phy: rcar-gen3-usb2: add extcon support")
> > > Fixes: 73801b90a38f ("phy: renesas: rcar-gen3-usb2: change a condition "dr_mode"")
> > > Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> >
> > merged, thanks.
>
> I'm very sorry, I didn't notice this patch...
> I'd like to fix this patch because this patch breaks for R-Car D3.
> In this case, should I submit an incremental patch?
Kishon, is it too late to drop the patch?
>
> > -Kishon
> > > ---
> > > drivers/phy/renesas/phy-rcar-gen3-usb2.c | 26 +++++++++++++-------------
> > > 1 file changed, 13 insertions(+), 13 deletions(-)
> > >
> > > diff --git a/drivers/phy/renesas/phy-rcar-gen3-usb2.c b/drivers/phy/renesas/phy-rcar-gen3-usb2.c
> > > index 0a34782..be1a392 100644
> > > --- a/drivers/phy/renesas/phy-rcar-gen3-usb2.c
> > > +++ b/drivers/phy/renesas/phy-rcar-gen3-usb2.c
> > > @@ -447,20 +447,8 @@ static int rcar_gen3_phy_usb2_probe(struct platform_device *pdev)
> > > if (IS_ERR(channel->base))
> > > return PTR_ERR(channel->base);
> > >
> > > - /* call request_irq for OTG */
> > > - irq = platform_get_irq(pdev, 0);
> > > - if (irq >= 0) {
> > > - INIT_WORK(&channel->work, rcar_gen3_phy_usb2_work);
> > > - irq = devm_request_irq(dev, irq, rcar_gen3_phy_usb2_irq,
> > > - IRQF_SHARED, dev_name(dev), channel);
> > > - if (irq < 0)
> > > - dev_err(dev, "No irq handler (%d)\n", irq);
> > > - }
> > > -
> > > channel->dr_mode = of_usb_get_dr_mode_by_phy(dev->of_node, 0);
> > > - if (channel->dr_mode != USB_DR_MODE_UNKNOWN) {
> > > - int ret;
> > > -
> > > + if (channel->dr_mode == USB_DR_MODE_OTG) {
>
> R-Car D3 (r8a77995-draak.dts) has dr_mode with "host". So, the board can use host mode
> as default and "role" mode to switch the role for peripheral later. However, after we
> applied this patch, the R-Car D3 USB2.0 cannot switch to the peripheral mode.
I understand, I am sorry for breaking it.
The probing function still needs some fixing, devm_request_irq uses rcar_gen3_phy_usb2_irq,
rcar_gen3_phy_usb2_irq calls rcar_gen3_device_recognition, rcar_gen3_device_recognition may
call either rcar_gen3_init_for_host or rcar_gen3_init_for_peri, both rcar_gen3_init_for_host and
rcar_gen3_init_for_peri schedule work for the work queue that calls into rcar_gen3_phy_usb2_work,
rcar_gen3_phy_usb2_work uses extcon, extcon gets initialized after devm_request_irq, which
means we should move devm_request_irq after extcon gets initialized,.
Also, we may register the irq, and never deal with extcon if dr_mode == USB_DR_MODE_UNKNOWN.
Would you like to come up with a patch to address those points?
Thanks,
Fab
>
> Best regards,
> Yoshihiro Shimoda
>
> > > channel->is_otg_channel = true;
> > > channel->uses_otg_pins = !of_property_read_bool(dev->of_node,
> > > "renesas,no-otg-pins");
> > > @@ -476,6 +464,18 @@ static int rcar_gen3_phy_usb2_probe(struct platform_device *pdev)
> > > }
> > > }
> > >
> > > + if (channel->is_otg_channel && channel->uses_otg_pins) {
> > > + /* call request_irq for OTG */
> > > + irq = platform_get_irq(pdev, 0);
> > > + if (irq >= 0) {
> > > + INIT_WORK(&channel->work, rcar_gen3_phy_usb2_work);
> > > + irq = devm_request_irq(dev, irq, rcar_gen3_phy_usb2_irq,
> > > + IRQF_SHARED, dev_name(dev), channel);
> > > + if (irq < 0)
> > > + dev_err(dev, "No irq handler (%d)\n", irq);
> > > + }
> > > + }
> > > +
> > > /*
> > > * devm_phy_create() will call pm_runtime_enable(&phy->dev);
> > > * And then, phy-core will manage runtime pm for this device.
> > >
next prev parent reply other threads:[~2019-04-03 10:24 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-03-01 11:05 [PATCH 0/4] Add USB-HOST support to cat874 Fabrizio Castro
2019-03-01 11:05 ` [PATCH 1/4] usb: common: Consider only available nodes for dr_mode Fabrizio Castro
2019-04-02 1:53 ` Yoshihiro Shimoda
2019-04-02 9:22 ` Fabrizio Castro
2019-04-02 11:09 ` Yoshihiro Shimoda
2019-04-02 11:47 ` Fabrizio Castro
2019-04-08 5:52 ` Yoshihiro Shimoda
2019-03-01 11:05 ` [PATCH 2/4] phy: renesas: rcar-gen3-usb2: No need to request IRQ for non-OTG Fabrizio Castro
2019-04-01 12:26 ` Kishon Vijay Abraham I
2019-04-02 2:11 ` Yoshihiro Shimoda
2019-04-03 10:24 ` Fabrizio Castro [this message]
2019-04-17 6:35 ` Kishon Vijay Abraham I
2019-03-01 11:05 ` [PATCH 3/4] arm64: dts: renesas: r8a774c0-cat874: Add USB-HOST support Fabrizio Castro
2019-04-03 10:29 ` Fabrizio Castro
2019-04-04 10:41 ` Simon Horman
2019-03-01 11:05 ` [PATCH 4/4] arm64: dts: renesas: r8a77995: draak: Remove hsusb node Fabrizio Castro
2019-04-03 10:26 ` Fabrizio Castro
2019-04-04 10:34 ` Simon Horman
2019-03-06 13:13 ` [PATCH 0/4] Add USB-HOST support to cat874 Simon Horman
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=TY1PR01MB17703687A7ECF803B338834EC0570@TY1PR01MB1770.jpnprd01.prod.outlook.com \
--to=fabrizio.castro@bp.renesas.com \
--cc=Chris.Paterson2@renesas.com \
--cc=biju.das@bp.renesas.com \
--cc=geert+renesas@glider.be \
--cc=horms@verge.net.au \
--cc=kishon@ti.com \
--cc=linux-renesas-soc@vger.kernel.org \
--cc=yoshihiro.shimoda.uh@renesas.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).