* [PATCH 0/3] phy: renesas: rcar-gen3-usb2: enable/disable independent irqs @ 2019-04-01 12:01 Yoshihiro Shimoda 2019-04-01 12:01 ` [PATCH 1/3] dt-bindings: phy: rcar-gen3-phy-usb2: Revise #phy-cells property Yoshihiro Shimoda ` (2 more replies) 0 siblings, 3 replies; 14+ messages in thread From: Yoshihiro Shimoda @ 2019-04-01 12:01 UTC (permalink / raw) To: kishon, robh+dt, mark.rutland Cc: linux-kernel, devicetree, linux-renesas-soc, Yoshihiro Shimoda Since the previous code enabled/disabled the irqs both OHCI and EHCI, it is possible to cause unexpected interruptions. To avoid this, this patch creates multiple phy instances from phandle and enables/disables independent irqs by the instances. Yoshihiro Shimoda (3): dt-bindings: phy: rcar-gen3-phy-usb2: Revise #phy-cells property phy: renesas: rcar-gen3-usb2: Use pdev's device pointer on dev_vdbg() phy: renesas: rcar-gen3-usb2: enable/disable independent irqs .../devicetree/bindings/phy/rcar-gen3-phy-usb2.txt | 7 +- drivers/phy/renesas/phy-rcar-gen3-usb2.c | 191 +++++++++++++++++---- 2 files changed, 168 insertions(+), 30 deletions(-) -- 2.7.4 ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/3] dt-bindings: phy: rcar-gen3-phy-usb2: Revise #phy-cells property 2019-04-01 12:01 [PATCH 0/3] phy: renesas: rcar-gen3-usb2: enable/disable independent irqs Yoshihiro Shimoda @ 2019-04-01 12:01 ` Yoshihiro Shimoda 2019-04-03 9:20 ` Simon Horman 2019-04-03 10:35 ` Fabrizio Castro 2019-04-01 12:01 ` [PATCH 2/3] phy: renesas: rcar-gen3-usb2: Use pdev's device pointer on dev_vdbg() Yoshihiro Shimoda 2019-04-01 12:01 ` [PATCH 3/3] phy: renesas: rcar-gen3-usb2: enable/disable independent irqs Yoshihiro Shimoda 2 siblings, 2 replies; 14+ messages in thread From: Yoshihiro Shimoda @ 2019-04-01 12:01 UTC (permalink / raw) To: kishon, robh+dt, mark.rutland Cc: linux-kernel, devicetree, linux-renesas-soc, Yoshihiro Shimoda To have the detailed property on each PHY specifier, this patch revices the #phy-cells property. Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> --- Documentation/devicetree/bindings/phy/rcar-gen3-phy-usb2.txt | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/phy/rcar-gen3-phy-usb2.txt b/Documentation/devicetree/bindings/phy/rcar-gen3-phy-usb2.txt index ad9c290..7f0d4bf 100644 --- a/Documentation/devicetree/bindings/phy/rcar-gen3-phy-usb2.txt +++ b/Documentation/devicetree/bindings/phy/rcar-gen3-phy-usb2.txt @@ -27,7 +27,12 @@ Required properties: - reg: offset and length of the partial USB 2.0 Host register block. - clocks: clock phandle and specifier pair(s). -- #phy-cells: see phy-bindings.txt in the same directory, must be <0>. +- #phy-cells: see phy-bindings.txt in the same directory, must be <1>. + +The phandle's argument in the PHY specifier is the INT_STATUS bit of controller: +- 1 = USBH_INTA (OHCI) +- 2 = USBH_INTB (EHCI) +- 3 = UCOM_INT (OTG and BC) Optional properties: To use a USB channel where USB 2.0 Host and HSUSB (USB 2.0 Peripheral) are -- 2.7.4 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] dt-bindings: phy: rcar-gen3-phy-usb2: Revise #phy-cells property 2019-04-01 12:01 ` [PATCH 1/3] dt-bindings: phy: rcar-gen3-phy-usb2: Revise #phy-cells property Yoshihiro Shimoda @ 2019-04-03 9:20 ` Simon Horman 2019-04-08 6:13 ` Yoshihiro Shimoda 2019-04-03 10:35 ` Fabrizio Castro 1 sibling, 1 reply; 14+ messages in thread From: Simon Horman @ 2019-04-03 9:20 UTC (permalink / raw) To: Yoshihiro Shimoda Cc: kishon, robh+dt, mark.rutland, linux-kernel, devicetree, linux-renesas-soc Hi Shimoda-san, On Mon, Apr 01, 2019 at 09:01:21PM +0900, Yoshihiro Shimoda wrote: > To have the detailed property on each PHY specifier, this patch revices > the #phy-cells property. > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > --- > Documentation/devicetree/bindings/phy/rcar-gen3-phy-usb2.txt | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/phy/rcar-gen3-phy-usb2.txt b/Documentation/devicetree/bindings/phy/rcar-gen3-phy-usb2.txt > index ad9c290..7f0d4bf 100644 > --- a/Documentation/devicetree/bindings/phy/rcar-gen3-phy-usb2.txt > +++ b/Documentation/devicetree/bindings/phy/rcar-gen3-phy-usb2.txt > @@ -27,7 +27,12 @@ Required properties: > > - reg: offset and length of the partial USB 2.0 Host register block. > - clocks: clock phandle and specifier pair(s). > -- #phy-cells: see phy-bindings.txt in the same directory, must be <0>. The driver currently supports <0> and with this changeset continues to do so. I also assume there are existing DTs that use <0>. Would it be better to describe this as deprecated? > +- #phy-cells: see phy-bindings.txt in the same directory, must be <1>. > + > +The phandle's argument in the PHY specifier is the INT_STATUS bit of controller: > +- 1 = USBH_INTA (OHCI) > +- 2 = USBH_INTB (EHCI) > +- 3 = UCOM_INT (OTG and BC) > > Optional properties: > To use a USB channel where USB 2.0 Host and HSUSB (USB 2.0 Peripheral) are > -- > 2.7.4 > ^ permalink raw reply [flat|nested] 14+ messages in thread
* RE: [PATCH 1/3] dt-bindings: phy: rcar-gen3-phy-usb2: Revise #phy-cells property 2019-04-03 9:20 ` Simon Horman @ 2019-04-08 6:13 ` Yoshihiro Shimoda 0 siblings, 0 replies; 14+ messages in thread From: Yoshihiro Shimoda @ 2019-04-08 6:13 UTC (permalink / raw) To: Simon Horman Cc: kishon, robh+dt, mark.rutland, linux-kernel, devicetree, linux-renesas-soc Hi Simon-san, Thank you for your review! > From: Simon Horman, Sent: Wednesday, April 3, 2019 6:21 PM > > Hi Shimoda-san, > > On Mon, Apr 01, 2019 at 09:01:21PM +0900, Yoshihiro Shimoda wrote: > > To have the detailed property on each PHY specifier, this patch revices > > the #phy-cells property. > > > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > > --- > > Documentation/devicetree/bindings/phy/rcar-gen3-phy-usb2.txt | 7 ++++++- > > 1 file changed, 6 insertions(+), 1 deletion(-) > > > > diff --git a/Documentation/devicetree/bindings/phy/rcar-gen3-phy-usb2.txt > b/Documentation/devicetree/bindings/phy/rcar-gen3-phy-usb2.txt > > index ad9c290..7f0d4bf 100644 > > --- a/Documentation/devicetree/bindings/phy/rcar-gen3-phy-usb2.txt > > +++ b/Documentation/devicetree/bindings/phy/rcar-gen3-phy-usb2.txt > > @@ -27,7 +27,12 @@ Required properties: > > > > - reg: offset and length of the partial USB 2.0 Host register block. > > - clocks: clock phandle and specifier pair(s). > > -- #phy-cells: see phy-bindings.txt in the same directory, must be <0>. > > The driver currently supports <0> and with this changeset continues to do > so. I also assume there are existing DTs that use <0>. Would it be > better to describe this as deprecated? I got it. I'll fix this in v2. Best regards, Yoshihiro Shimoda > > +- #phy-cells: see phy-bindings.txt in the same directory, must be <1>. > > + > > +The phandle's argument in the PHY specifier is the INT_STATUS bit of controller: > > +- 1 = USBH_INTA (OHCI) > > +- 2 = USBH_INTB (EHCI) > > +- 3 = UCOM_INT (OTG and BC) > > > > Optional properties: > > To use a USB channel where USB 2.0 Host and HSUSB (USB 2.0 Peripheral) are > > -- > > 2.7.4 > > ^ permalink raw reply [flat|nested] 14+ messages in thread
* RE: [PATCH 1/3] dt-bindings: phy: rcar-gen3-phy-usb2: Revise #phy-cells property 2019-04-01 12:01 ` [PATCH 1/3] dt-bindings: phy: rcar-gen3-phy-usb2: Revise #phy-cells property Yoshihiro Shimoda 2019-04-03 9:20 ` Simon Horman @ 2019-04-03 10:35 ` Fabrizio Castro 2019-04-08 6:22 ` Yoshihiro Shimoda 1 sibling, 1 reply; 14+ messages in thread From: Fabrizio Castro @ 2019-04-03 10:35 UTC (permalink / raw) To: Yoshihiro Shimoda, kishon, robh+dt, mark.rutland Cc: linux-kernel, devicetree, linux-renesas-soc, Yoshihiro Shimoda Hello Yoshihiro-san, Thank you for your patch! > From: linux-kernel-owner@vger.kernel.org <linux-kernel-owner@vger.kernel.org> On Behalf Of Yoshihiro Shimoda > Sent: 01 April 2019 13:01 > Subject: [PATCH 1/3] dt-bindings: phy: rcar-gen3-phy-usb2: Revise #phy-cells property > > To have the detailed property on each PHY specifier, this patch revices s/revices/revises/g > the #phy-cells property. > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > --- > Documentation/devicetree/bindings/phy/rcar-gen3-phy-usb2.txt | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/phy/rcar-gen3-phy-usb2.txt b/Documentation/devicetree/bindings/phy/rcar-gen3- > phy-usb2.txt > index ad9c290..7f0d4bf 100644 > --- a/Documentation/devicetree/bindings/phy/rcar-gen3-phy-usb2.txt > +++ b/Documentation/devicetree/bindings/phy/rcar-gen3-phy-usb2.txt > @@ -27,7 +27,12 @@ Required properties: > > - reg: offset and length of the partial USB 2.0 Host register block. > - clocks: clock phandle and specifier pair(s). > -- #phy-cells: see phy-bindings.txt in the same directory, must be <0>. > +- #phy-cells: see phy-bindings.txt in the same directory, must be <1>. > + > +The phandle's argument in the PHY specifier is the INT_STATUS bit of controller: From the driver we are also looking for value 0, as in rcar_gen3_get_dr_mode we have a for loop that starts from 0, and we call of_usb_get_dr_mode_by_phy with arg0 == 0, why are we leaving this out from this list? Thanks, Fab > +- 1 = USBH_INTA (OHCI) > +- 2 = USBH_INTB (EHCI) > +- 3 = UCOM_INT (OTG and BC) > > Optional properties: > To use a USB channel where USB 2.0 Host and HSUSB (USB 2.0 Peripheral) are > -- > 2.7.4 ^ permalink raw reply [flat|nested] 14+ messages in thread
* RE: [PATCH 1/3] dt-bindings: phy: rcar-gen3-phy-usb2: Revise #phy-cells property 2019-04-03 10:35 ` Fabrizio Castro @ 2019-04-08 6:22 ` Yoshihiro Shimoda 0 siblings, 0 replies; 14+ messages in thread From: Yoshihiro Shimoda @ 2019-04-08 6:22 UTC (permalink / raw) To: Fabrizio Castro, kishon, robh+dt, mark.rutland Cc: linux-kernel, devicetree, linux-renesas-soc Hi Fabrizio-san, Thank you for your review! > From: Fabrizio Castro, Sent: Wednesday, April 3, 2019 7:35 PM > > Hello Yoshihiro-san, > > Thank you for your patch! > > > From: linux-kernel-owner@vger.kernel.org <linux-kernel-owner@vger.kernel.org> On Behalf Of Yoshihiro Shimoda > > Sent: 01 April 2019 13:01 > > Subject: [PATCH 1/3] dt-bindings: phy: rcar-gen3-phy-usb2: Revise #phy-cells property > > > > To have the detailed property on each PHY specifier, this patch revices > > s/revices/revises/g Oops! I'll fix it in v2. > > the #phy-cells property. > > > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > > --- > > Documentation/devicetree/bindings/phy/rcar-gen3-phy-usb2.txt | 7 ++++++- > > 1 file changed, 6 insertions(+), 1 deletion(-) > > > > diff --git a/Documentation/devicetree/bindings/phy/rcar-gen3-phy-usb2.txt > b/Documentation/devicetree/bindings/phy/rcar-gen3- > > phy-usb2.txt > > index ad9c290..7f0d4bf 100644 > > --- a/Documentation/devicetree/bindings/phy/rcar-gen3-phy-usb2.txt > > +++ b/Documentation/devicetree/bindings/phy/rcar-gen3-phy-usb2.txt > > @@ -27,7 +27,12 @@ Required properties: > > > > - reg: offset and length of the partial USB 2.0 Host register block. > > - clocks: clock phandle and specifier pair(s). > > -- #phy-cells: see phy-bindings.txt in the same directory, must be <0>. > > +- #phy-cells: see phy-bindings.txt in the same directory, must be <1>. > > + > > +The phandle's argument in the PHY specifier is the INT_STATUS bit of controller: > > From the driver we are also looking for value 0, as in rcar_gen3_get_dr_mode we > have a for loop that starts from 0, and we call of_usb_get_dr_mode_by_phy with > arg0 == 0 This is backward compatibility for old DTS file (phy-cells = 0). > why are we leaving this out from this list? This is because I don't assume any device nodes doesn't have such a property (the bit 0 of INT_STATUS). Best regards, Yoshihiro Shimoda > Thanks, > Fab > > > +- 1 = USBH_INTA (OHCI) > > +- 2 = USBH_INTB (EHCI) > > +- 3 = UCOM_INT (OTG and BC) > > > > Optional properties: > > To use a USB channel where USB 2.0 Host and HSUSB (USB 2.0 Peripheral) are > > -- > > 2.7.4 ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 2/3] phy: renesas: rcar-gen3-usb2: Use pdev's device pointer on dev_vdbg() 2019-04-01 12:01 [PATCH 0/3] phy: renesas: rcar-gen3-usb2: enable/disable independent irqs Yoshihiro Shimoda 2019-04-01 12:01 ` [PATCH 1/3] dt-bindings: phy: rcar-gen3-phy-usb2: Revise #phy-cells property Yoshihiro Shimoda @ 2019-04-01 12:01 ` Yoshihiro Shimoda 2019-04-03 9:21 ` Simon Horman 2019-04-03 10:36 ` Fabrizio Castro 2019-04-01 12:01 ` [PATCH 3/3] phy: renesas: rcar-gen3-usb2: enable/disable independent irqs Yoshihiro Shimoda 2 siblings, 2 replies; 14+ messages in thread From: Yoshihiro Shimoda @ 2019-04-01 12:01 UTC (permalink / raw) To: kishon, robh+dt, mark.rutland Cc: linux-kernel, devicetree, linux-renesas-soc, Yoshihiro Shimoda To implement multiple phy instances in the future, this patch uses pdev's device pointer on dev_vdbg() instead of the phy's device pointer. Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> --- drivers/phy/renesas/phy-rcar-gen3-usb2.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/drivers/phy/renesas/phy-rcar-gen3-usb2.c b/drivers/phy/renesas/phy-rcar-gen3-usb2.c index 0a34782..4bdb2ed 100644 --- a/drivers/phy/renesas/phy-rcar-gen3-usb2.c +++ b/drivers/phy/renesas/phy-rcar-gen3-usb2.c @@ -80,6 +80,7 @@ struct rcar_gen3_chan { void __iomem *base; + struct device *dev; /* platform_device's device */ struct extcon_dev *extcon; struct phy *phy; struct regulator *vbus; @@ -120,7 +121,7 @@ static void rcar_gen3_set_host_mode(struct rcar_gen3_chan *ch, int host) void __iomem *usb2_base = ch->base; u32 val = readl(usb2_base + USB2_COMMCTRL); - dev_vdbg(&ch->phy->dev, "%s: %08x, %d\n", __func__, val, host); + dev_vdbg(ch->dev, "%s: %08x, %d\n", __func__, val, host); if (host) val &= ~USB2_COMMCTRL_OTG_PERI; else @@ -133,7 +134,7 @@ static void rcar_gen3_set_linectrl(struct rcar_gen3_chan *ch, int dp, int dm) void __iomem *usb2_base = ch->base; u32 val = readl(usb2_base + USB2_LINECTRL1); - dev_vdbg(&ch->phy->dev, "%s: %08x, %d, %d\n", __func__, val, dp, dm); + dev_vdbg(ch->dev, "%s: %08x, %d, %d\n", __func__, val, dp, dm); val &= ~(USB2_LINECTRL1_DP_RPD | USB2_LINECTRL1_DM_RPD); if (dp) val |= USB2_LINECTRL1_DP_RPD; @@ -147,7 +148,7 @@ static void rcar_gen3_enable_vbus_ctrl(struct rcar_gen3_chan *ch, int vbus) void __iomem *usb2_base = ch->base; u32 val = readl(usb2_base + USB2_ADPCTRL); - dev_vdbg(&ch->phy->dev, "%s: %08x, %d\n", __func__, val, vbus); + dev_vdbg(ch->dev, "%s: %08x, %d\n", __func__, val, vbus); if (vbus) val |= USB2_ADPCTRL_DRVVBUS; else @@ -401,7 +402,7 @@ static irqreturn_t rcar_gen3_phy_usb2_irq(int irq, void *_ch) irqreturn_t ret = IRQ_NONE; if (status & USB2_OBINT_BITS) { - dev_vdbg(&ch->phy->dev, "%s: %08x\n", __func__, status); + dev_vdbg(ch->dev, "%s: %08x\n", __func__, status); writel(USB2_OBINT_BITS, usb2_base + USB2_OBINTSTA); rcar_gen3_device_recognition(ch); ret = IRQ_HANDLED; @@ -499,6 +500,7 @@ static int rcar_gen3_phy_usb2_probe(struct platform_device *pdev) platform_set_drvdata(pdev, channel); phy_set_drvdata(channel->phy, channel); + channel->dev = dev; provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate); if (IS_ERR(provider)) { -- 2.7.4 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 2/3] phy: renesas: rcar-gen3-usb2: Use pdev's device pointer on dev_vdbg() 2019-04-01 12:01 ` [PATCH 2/3] phy: renesas: rcar-gen3-usb2: Use pdev's device pointer on dev_vdbg() Yoshihiro Shimoda @ 2019-04-03 9:21 ` Simon Horman 2019-04-03 10:36 ` Fabrizio Castro 1 sibling, 0 replies; 14+ messages in thread From: Simon Horman @ 2019-04-03 9:21 UTC (permalink / raw) To: Yoshihiro Shimoda Cc: kishon, robh+dt, mark.rutland, linux-kernel, devicetree, linux-renesas-soc On Mon, Apr 01, 2019 at 09:01:22PM +0900, Yoshihiro Shimoda wrote: > To implement multiple phy instances in the future, this patch uses > pdev's device pointer on dev_vdbg() instead of the phy's device > pointer. > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> LGTM Reviewed-by: Simon Horman <horms+renesas@verge.net.au> ^ permalink raw reply [flat|nested] 14+ messages in thread
* RE: [PATCH 2/3] phy: renesas: rcar-gen3-usb2: Use pdev's device pointer on dev_vdbg() 2019-04-01 12:01 ` [PATCH 2/3] phy: renesas: rcar-gen3-usb2: Use pdev's device pointer on dev_vdbg() Yoshihiro Shimoda 2019-04-03 9:21 ` Simon Horman @ 2019-04-03 10:36 ` Fabrizio Castro 1 sibling, 0 replies; 14+ messages in thread From: Fabrizio Castro @ 2019-04-03 10:36 UTC (permalink / raw) To: Yoshihiro Shimoda, kishon, robh+dt, mark.rutland Cc: linux-kernel, devicetree, linux-renesas-soc, Yoshihiro Shimoda Hello Yoshihiro-san, Thank you for your patch! > From: linux-kernel-owner@vger.kernel.org <linux-kernel-owner@vger.kernel.org> On Behalf Of Yoshihiro Shimoda > Sent: 01 April 2019 13:01 > Subject: [PATCH 2/3] phy: renesas: rcar-gen3-usb2: Use pdev's device pointer on dev_vdbg() > > To implement multiple phy instances in the future, this patch uses > pdev's device pointer on dev_vdbg() instead of the phy's device > pointer. > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> Reviewed-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com> > --- > drivers/phy/renesas/phy-rcar-gen3-usb2.c | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/drivers/phy/renesas/phy-rcar-gen3-usb2.c b/drivers/phy/renesas/phy-rcar-gen3-usb2.c > index 0a34782..4bdb2ed 100644 > --- a/drivers/phy/renesas/phy-rcar-gen3-usb2.c > +++ b/drivers/phy/renesas/phy-rcar-gen3-usb2.c > @@ -80,6 +80,7 @@ > > struct rcar_gen3_chan { > void __iomem *base; > + struct device *dev; /* platform_device's device */ > struct extcon_dev *extcon; > struct phy *phy; > struct regulator *vbus; > @@ -120,7 +121,7 @@ static void rcar_gen3_set_host_mode(struct rcar_gen3_chan *ch, int host) > void __iomem *usb2_base = ch->base; > u32 val = readl(usb2_base + USB2_COMMCTRL); > > - dev_vdbg(&ch->phy->dev, "%s: %08x, %d\n", __func__, val, host); > + dev_vdbg(ch->dev, "%s: %08x, %d\n", __func__, val, host); > if (host) > val &= ~USB2_COMMCTRL_OTG_PERI; > else > @@ -133,7 +134,7 @@ static void rcar_gen3_set_linectrl(struct rcar_gen3_chan *ch, int dp, int dm) > void __iomem *usb2_base = ch->base; > u32 val = readl(usb2_base + USB2_LINECTRL1); > > - dev_vdbg(&ch->phy->dev, "%s: %08x, %d, %d\n", __func__, val, dp, dm); > + dev_vdbg(ch->dev, "%s: %08x, %d, %d\n", __func__, val, dp, dm); > val &= ~(USB2_LINECTRL1_DP_RPD | USB2_LINECTRL1_DM_RPD); > if (dp) > val |= USB2_LINECTRL1_DP_RPD; > @@ -147,7 +148,7 @@ static void rcar_gen3_enable_vbus_ctrl(struct rcar_gen3_chan *ch, int vbus) > void __iomem *usb2_base = ch->base; > u32 val = readl(usb2_base + USB2_ADPCTRL); > > - dev_vdbg(&ch->phy->dev, "%s: %08x, %d\n", __func__, val, vbus); > + dev_vdbg(ch->dev, "%s: %08x, %d\n", __func__, val, vbus); > if (vbus) > val |= USB2_ADPCTRL_DRVVBUS; > else > @@ -401,7 +402,7 @@ static irqreturn_t rcar_gen3_phy_usb2_irq(int irq, void *_ch) > irqreturn_t ret = IRQ_NONE; > > if (status & USB2_OBINT_BITS) { > - dev_vdbg(&ch->phy->dev, "%s: %08x\n", __func__, status); > + dev_vdbg(ch->dev, "%s: %08x\n", __func__, status); > writel(USB2_OBINT_BITS, usb2_base + USB2_OBINTSTA); > rcar_gen3_device_recognition(ch); > ret = IRQ_HANDLED; > @@ -499,6 +500,7 @@ static int rcar_gen3_phy_usb2_probe(struct platform_device *pdev) > > platform_set_drvdata(pdev, channel); > phy_set_drvdata(channel->phy, channel); > + channel->dev = dev; > > provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate); > if (IS_ERR(provider)) { > -- > 2.7.4 ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 3/3] phy: renesas: rcar-gen3-usb2: enable/disable independent irqs 2019-04-01 12:01 [PATCH 0/3] phy: renesas: rcar-gen3-usb2: enable/disable independent irqs Yoshihiro Shimoda 2019-04-01 12:01 ` [PATCH 1/3] dt-bindings: phy: rcar-gen3-phy-usb2: Revise #phy-cells property Yoshihiro Shimoda 2019-04-01 12:01 ` [PATCH 2/3] phy: renesas: rcar-gen3-usb2: Use pdev's device pointer on dev_vdbg() Yoshihiro Shimoda @ 2019-04-01 12:01 ` Yoshihiro Shimoda 2019-04-03 9:19 ` Simon Horman 2019-04-03 10:49 ` Fabrizio Castro 2 siblings, 2 replies; 14+ messages in thread From: Yoshihiro Shimoda @ 2019-04-01 12:01 UTC (permalink / raw) To: kishon, robh+dt, mark.rutland Cc: linux-kernel, devicetree, linux-renesas-soc, Yoshihiro Shimoda Since the previous code enabled/disabled the irqs both OHCI and EHCI, it is possible to cause unexpected interruptions. To avoid this, this patch creates multiple phy instances from phandle and enables/disables independent irqs by the instances. Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> --- drivers/phy/renesas/phy-rcar-gen3-usb2.c | 181 ++++++++++++++++++++++++++----- 1 file changed, 156 insertions(+), 25 deletions(-) diff --git a/drivers/phy/renesas/phy-rcar-gen3-usb2.c b/drivers/phy/renesas/phy-rcar-gen3-usb2.c index 4bdb2ed..bbe0fe5 100644 --- a/drivers/phy/renesas/phy-rcar-gen3-usb2.c +++ b/drivers/phy/renesas/phy-rcar-gen3-usb2.c @@ -37,11 +37,8 @@ /* INT_ENABLE */ #define USB2_INT_ENABLE_UCOM_INTEN BIT(3) -#define USB2_INT_ENABLE_USBH_INTB_EN BIT(2) -#define USB2_INT_ENABLE_USBH_INTA_EN BIT(1) -#define USB2_INT_ENABLE_INIT (USB2_INT_ENABLE_UCOM_INTEN | \ - USB2_INT_ENABLE_USBH_INTB_EN | \ - USB2_INT_ENABLE_USBH_INTA_EN) +#define USB2_INT_ENABLE_USBH_INTB_EN BIT(2) /* For EHCI */ +#define USB2_INT_ENABLE_USBH_INTA_EN BIT(1) /* For OHCI */ /* USBCTR */ #define USB2_USBCTR_DIRPD BIT(2) @@ -78,11 +75,33 @@ #define USB2_ADPCTRL_IDPULLUP BIT(5) /* 1 = ID sampling is enabled */ #define USB2_ADPCTRL_DRVVBUS BIT(4) +#define NUM_OF_PHYS 4 +#define PHY_INDEX_BOTH_HC 0 +#define PHY_INDEX_OHCI 1 +#define PHY_INDEX_EHCI 2 +#define PHY_INDEX_HSUSB 3 + +static const u32 rcar_gen3_int_enable[NUM_OF_PHYS] = { + USB2_INT_ENABLE_USBH_INTB_EN | USB2_INT_ENABLE_USBH_INTA_EN, + USB2_INT_ENABLE_USBH_INTA_EN, + USB2_INT_ENABLE_USBH_INTB_EN, + 0 +}; + +struct rcar_gen3_phy { + struct phy *phy; + struct rcar_gen3_chan *ch; + u32 int_enable_bits; + bool initialized; + bool otg_initialized; + bool powered; +}; + struct rcar_gen3_chan { void __iomem *base; struct device *dev; /* platform_device's device */ struct extcon_dev *extcon; - struct phy *phy; + struct rcar_gen3_phy rphys[NUM_OF_PHYS]; struct regulator *vbus; struct work_struct work; enum usb_dr_mode dr_mode; @@ -250,6 +269,42 @@ static enum phy_mode rcar_gen3_get_phy_mode(struct rcar_gen3_chan *ch) return PHY_MODE_USB_DEVICE; } +static bool rcar_gen3_is_any_rphy_initialized(struct rcar_gen3_chan *ch) +{ + int i; + + for (i = 0; i < NUM_OF_PHYS; i++) { + if (ch->rphys[i].initialized) + return true; + } + + return false; +} + +static bool rcar_gen3_needs_init_otg(struct rcar_gen3_chan *ch) +{ + int i; + + for (i = 0; i < NUM_OF_PHYS; i++) { + if (ch->rphys[i].otg_initialized) + return false; + } + + return true; +} + +static bool rcar_gen3_are_all_rphys_power_off(struct rcar_gen3_chan *ch) +{ + int i; + + for (i = 0; i < NUM_OF_PHYS; i++) { + if (ch->rphys[i].powered) + return false; + } + + return true; +} + static ssize_t role_store(struct device *dev, struct device_attribute *attr, const char *buf, size_t count) { @@ -257,7 +312,7 @@ static ssize_t role_store(struct device *dev, struct device_attribute *attr, bool is_b_device; enum phy_mode cur_mode, new_mode; - if (!ch->is_otg_channel || !ch->phy->init_count) + if (!ch->is_otg_channel || !rcar_gen3_is_any_rphy_initialized(ch)) return -EIO; if (!strncmp(buf, "host", strlen("host"))) @@ -295,7 +350,7 @@ static ssize_t role_show(struct device *dev, struct device_attribute *attr, { struct rcar_gen3_chan *ch = dev_get_drvdata(dev); - if (!ch->is_otg_channel || !ch->phy->init_count) + if (!ch->is_otg_channel || !rcar_gen3_is_any_rphy_initialized(ch)) return -EIO; return sprintf(buf, "%s\n", rcar_gen3_is_host(ch) ? "host" : @@ -329,37 +384,62 @@ static void rcar_gen3_init_otg(struct rcar_gen3_chan *ch) static int rcar_gen3_phy_usb2_init(struct phy *p) { - struct rcar_gen3_chan *channel = phy_get_drvdata(p); + struct rcar_gen3_phy *rphy = phy_get_drvdata(p); + struct rcar_gen3_chan *channel = rphy->ch; void __iomem *usb2_base = channel->base; + u32 val; /* Initialize USB2 part */ - writel(USB2_INT_ENABLE_INIT, usb2_base + USB2_INT_ENABLE); + val = readl(usb2_base + USB2_INT_ENABLE); + val |= USB2_INT_ENABLE_UCOM_INTEN | rphy->int_enable_bits; + writel(val, usb2_base + USB2_INT_ENABLE); writel(USB2_SPD_RSM_TIMSET_INIT, usb2_base + USB2_SPD_RSM_TIMSET); writel(USB2_OC_TIMSET_INIT, usb2_base + USB2_OC_TIMSET); /* Initialize otg part */ - if (channel->is_otg_channel) - rcar_gen3_init_otg(channel); + if (channel->is_otg_channel) { + if (rcar_gen3_needs_init_otg(channel)) + rcar_gen3_init_otg(channel); + rphy->otg_initialized = true; + } + + rphy->initialized = true; return 0; } static int rcar_gen3_phy_usb2_exit(struct phy *p) { - struct rcar_gen3_chan *channel = phy_get_drvdata(p); + struct rcar_gen3_phy *rphy = phy_get_drvdata(p); + struct rcar_gen3_chan *channel = rphy->ch; + void __iomem *usb2_base = channel->base; + u32 val; - writel(0, channel->base + USB2_INT_ENABLE); + rphy->initialized = false; + + if (channel->is_otg_channel) + rphy->otg_initialized = false; + + val = readl(usb2_base + USB2_INT_ENABLE); + val &= ~rphy->int_enable_bits; + if (!rcar_gen3_is_any_rphy_initialized(channel)) + val &= ~USB2_INT_ENABLE_UCOM_INTEN; + writel(val, usb2_base + USB2_INT_ENABLE); return 0; } static int rcar_gen3_phy_usb2_power_on(struct phy *p) { - struct rcar_gen3_chan *channel = phy_get_drvdata(p); + struct rcar_gen3_phy *rphy = phy_get_drvdata(p); + struct rcar_gen3_chan *channel = rphy->ch; void __iomem *usb2_base = channel->base; u32 val; int ret; + if (!rcar_gen3_are_all_rphys_power_off(channel)) + return 0; + if (channel->vbus) { ret = regulator_enable(channel->vbus); if (ret) @@ -372,14 +452,22 @@ static int rcar_gen3_phy_usb2_power_on(struct phy *p) val &= ~USB2_USBCTR_PLL_RST; writel(val, usb2_base + USB2_USBCTR); + rphy->powered = true; + return 0; } static int rcar_gen3_phy_usb2_power_off(struct phy *p) { - struct rcar_gen3_chan *channel = phy_get_drvdata(p); + struct rcar_gen3_phy *rphy = phy_get_drvdata(p); + struct rcar_gen3_chan *channel = rphy->ch; int ret = 0; + rphy->powered = false; + + if (!rcar_gen3_are_all_rphys_power_off(channel)) + return 0; + if (channel->vbus) ret = regulator_disable(channel->vbus); @@ -426,13 +514,51 @@ static const unsigned int rcar_gen3_phy_cable[] = { EXTCON_NONE, }; +static struct phy *rcar_gen3_phy_usb2_xlate(struct device *dev, + struct of_phandle_args *args) +{ + struct rcar_gen3_chan *ch = dev_get_drvdata(dev); + + if (args->args_count == 0) /* For old version dts */ + return ch->rphys[PHY_INDEX_BOTH_HC].phy; + else if (args->args_count > 1) /* Prevent invalid args count */ + return ERR_PTR(-ENODEV); + + if (args->args[0] >= NUM_OF_PHYS) + return ERR_PTR(-ENODEV); + + return ch->rphys[args->args[0]].phy; +} + +static enum usb_dr_mode rcar_gen3_get_dr_mode(struct device_node *np) +{ + enum usb_dr_mode candidate = USB_DR_MODE_UNKNOWN, tmp; + int i; + + /* + * If one of device nodes has other dr_mode except UNKNOWN, + * this function returns UNKNOWN. + */ + for (i = 0; i < NUM_OF_PHYS; i++) { + tmp = of_usb_get_dr_mode_by_phy(np, i); + if (tmp != USB_DR_MODE_UNKNOWN) { + if (candidate == USB_DR_MODE_UNKNOWN) + candidate = tmp; + else if (candidate != tmp) + return USB_DR_MODE_UNKNOWN; + } + } + + return candidate; +} + static int rcar_gen3_phy_usb2_probe(struct platform_device *pdev) { struct device *dev = &pdev->dev; struct rcar_gen3_chan *channel; struct phy_provider *provider; struct resource *res; - int irq, ret = 0; + int irq, ret = 0, i; if (!dev->of_node) { dev_err(dev, "This driver needs device tree\n"); @@ -458,7 +584,7 @@ static int rcar_gen3_phy_usb2_probe(struct platform_device *pdev) dev_err(dev, "No irq handler (%d)\n", irq); } - channel->dr_mode = of_usb_get_dr_mode_by_phy(dev->of_node, 0); + channel->dr_mode = rcar_gen3_get_dr_mode(dev->of_node); if (channel->dr_mode != USB_DR_MODE_UNKNOWN) { int ret; @@ -482,11 +608,17 @@ static int rcar_gen3_phy_usb2_probe(struct platform_device *pdev) * And then, phy-core will manage runtime pm for this device. */ pm_runtime_enable(dev); - channel->phy = devm_phy_create(dev, NULL, &rcar_gen3_phy_usb2_ops); - if (IS_ERR(channel->phy)) { - dev_err(dev, "Failed to create USB2 PHY\n"); - ret = PTR_ERR(channel->phy); - goto error; + for (i = 0; i < NUM_OF_PHYS; i++) { + channel->rphys[i].phy = devm_phy_create(dev, NULL, + &rcar_gen3_phy_usb2_ops); + if (IS_ERR(channel->rphys[i].phy)) { + dev_err(dev, "Failed to create USB2 PHY\n"); + ret = PTR_ERR(channel->rphys[i].phy); + goto error; + } + channel->rphys[i].ch = channel; + channel->rphys[i].int_enable_bits = rcar_gen3_int_enable[i]; + phy_set_drvdata(channel->rphys[i].phy, &channel->rphys[i]); } channel->vbus = devm_regulator_get_optional(dev, "vbus"); @@ -499,10 +631,9 @@ static int rcar_gen3_phy_usb2_probe(struct platform_device *pdev) } platform_set_drvdata(pdev, channel); - phy_set_drvdata(channel->phy, channel); channel->dev = dev; - provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate); + provider = devm_of_phy_provider_register(dev, rcar_gen3_phy_usb2_xlate); if (IS_ERR(provider)) { dev_err(dev, "Failed to register PHY provider\n"); ret = PTR_ERR(provider); -- 2.7.4 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] phy: renesas: rcar-gen3-usb2: enable/disable independent irqs 2019-04-01 12:01 ` [PATCH 3/3] phy: renesas: rcar-gen3-usb2: enable/disable independent irqs Yoshihiro Shimoda @ 2019-04-03 9:19 ` Simon Horman 2019-04-09 9:47 ` Yoshihiro Shimoda 2019-04-03 10:49 ` Fabrizio Castro 1 sibling, 1 reply; 14+ messages in thread From: Simon Horman @ 2019-04-03 9:19 UTC (permalink / raw) To: Yoshihiro Shimoda Cc: kishon, robh+dt, mark.rutland, linux-kernel, devicetree, linux-renesas-soc Hi Shimoda-san, On Mon, Apr 01, 2019 at 09:01:23PM +0900, Yoshihiro Shimoda wrote: > Since the previous code enabled/disabled the irqs both OHCI and EHCI, > it is possible to cause unexpected interruptions. To avoid this, > this patch creates multiple phy instances from phandle and > enables/disables independent irqs by the instances. > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> I noted a few nits below but overall this looks good to me. Reviewed-by: Simon Horman <horms+renesas@verge.net.au> > --- > drivers/phy/renesas/phy-rcar-gen3-usb2.c | 181 ++++++++++++++++++++++++++----- > 1 file changed, 156 insertions(+), 25 deletions(-) > > diff --git a/drivers/phy/renesas/phy-rcar-gen3-usb2.c b/drivers/phy/renesas/phy-rcar-gen3-usb2.c > index 4bdb2ed..bbe0fe5 100644 > --- a/drivers/phy/renesas/phy-rcar-gen3-usb2.c > +++ b/drivers/phy/renesas/phy-rcar-gen3-usb2.c > @@ -37,11 +37,8 @@ > > /* INT_ENABLE */ > #define USB2_INT_ENABLE_UCOM_INTEN BIT(3) > -#define USB2_INT_ENABLE_USBH_INTB_EN BIT(2) > -#define USB2_INT_ENABLE_USBH_INTA_EN BIT(1) > -#define USB2_INT_ENABLE_INIT (USB2_INT_ENABLE_UCOM_INTEN | \ > - USB2_INT_ENABLE_USBH_INTB_EN | \ > - USB2_INT_ENABLE_USBH_INTA_EN) > +#define USB2_INT_ENABLE_USBH_INTB_EN BIT(2) /* For EHCI */ > +#define USB2_INT_ENABLE_USBH_INTA_EN BIT(1) /* For OHCI */ > > /* USBCTR */ > #define USB2_USBCTR_DIRPD BIT(2) > @@ -78,11 +75,33 @@ > #define USB2_ADPCTRL_IDPULLUP BIT(5) /* 1 = ID sampling is enabled */ > #define USB2_ADPCTRL_DRVVBUS BIT(4) > > +#define NUM_OF_PHYS 4 > +#define PHY_INDEX_BOTH_HC 0 > +#define PHY_INDEX_OHCI 1 > +#define PHY_INDEX_EHCI 2 > +#define PHY_INDEX_HSUSB 3 nit: I think the above #defines would be better expressed as an enum. ... > +static struct phy *rcar_gen3_phy_usb2_xlate(struct device *dev, > + struct of_phandle_args *args) > +{ > + struct rcar_gen3_chan *ch = dev_get_drvdata(dev); > + > + if (args->args_count == 0) /* For old version dts */ > + return ch->rphys[PHY_INDEX_BOTH_HC].phy; > + else if (args->args_count > 1) /* Prevent invalid args count */ > + return ERR_PTR(-ENODEV); > + > + if (args->args[0] >= NUM_OF_PHYS) > + return ERR_PTR(-ENODEV); > + > + return ch->rphys[args->args[0]].phy; > +} > + > +static enum usb_dr_mode rcar_gen3_get_dr_mode(struct device_node *np) > +{ > + enum usb_dr_mode candidate = USB_DR_MODE_UNKNOWN, tmp; nit: I think that there could be a better name for tmp, f.e. mode nit: The scope of tmp could be limited to inside the for loop. > + int i; > + > + /* > + * If one of device nodes has other dr_mode except UNKNOWN, > + * this function returns UNKNOWN. > + */ > + for (i = 0; i < NUM_OF_PHYS; i++) { > + tmp = of_usb_get_dr_mode_by_phy(np, i); > + if (tmp != USB_DR_MODE_UNKNOWN) { > + if (candidate == USB_DR_MODE_UNKNOWN) > + candidate = tmp; > + else if (candidate != tmp) > + return USB_DR_MODE_UNKNOWN; > + } > + } > + > + return candidate; > +} ... ^ permalink raw reply [flat|nested] 14+ messages in thread
* RE: [PATCH 3/3] phy: renesas: rcar-gen3-usb2: enable/disable independent irqs 2019-04-03 9:19 ` Simon Horman @ 2019-04-09 9:47 ` Yoshihiro Shimoda 0 siblings, 0 replies; 14+ messages in thread From: Yoshihiro Shimoda @ 2019-04-09 9:47 UTC (permalink / raw) To: Simon Horman Cc: kishon, robh+dt, mark.rutland, linux-kernel, devicetree, linux-renesas-soc Hi Simon-san, > From: Simon Horman, Sent: Wednesday, April 3, 2019 6:20 PM > > Hi Shimoda-san, > > On Mon, Apr 01, 2019 at 09:01:23PM +0900, Yoshihiro Shimoda wrote: > > Since the previous code enabled/disabled the irqs both OHCI and EHCI, > > it is possible to cause unexpected interruptions. To avoid this, > > this patch creates multiple phy instances from phandle and > > enables/disables independent irqs by the instances. > > > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > > I noted a few nits below but overall this looks good to me. > > Reviewed-by: Simon Horman <horms+renesas@verge.net.au> Thank you for your review! <snip> > > +#define NUM_OF_PHYS 4 > > +#define PHY_INDEX_BOTH_HC 0 > > +#define PHY_INDEX_OHCI 1 > > +#define PHY_INDEX_EHCI 2 > > +#define PHY_INDEX_HSUSB 3 > > nit: I think the above #defines would be better expressed as an enum. I'll modify it. <snip> > > +static enum usb_dr_mode rcar_gen3_get_dr_mode(struct device_node *np) > > +{ > > + enum usb_dr_mode candidate = USB_DR_MODE_UNKNOWN, tmp; > > nit: I think that there could be a better name for tmp, f.e. mode > nit: The scope of tmp could be limited to inside the for loop. I'll modify it. Best regards, Yoshihiro Shimoda ^ permalink raw reply [flat|nested] 14+ messages in thread
* RE: [PATCH 3/3] phy: renesas: rcar-gen3-usb2: enable/disable independent irqs 2019-04-01 12:01 ` [PATCH 3/3] phy: renesas: rcar-gen3-usb2: enable/disable independent irqs Yoshihiro Shimoda 2019-04-03 9:19 ` Simon Horman @ 2019-04-03 10:49 ` Fabrizio Castro 2019-04-09 9:49 ` Yoshihiro Shimoda 1 sibling, 1 reply; 14+ messages in thread From: Fabrizio Castro @ 2019-04-03 10:49 UTC (permalink / raw) To: Yoshihiro Shimoda, kishon, robh+dt, mark.rutland Cc: linux-kernel, devicetree, linux-renesas-soc, Yoshihiro Shimoda Hello Yoshihiro-san, Thank you for your partch! > From: devicetree-owner@vger.kernel.org <devicetree-owner@vger.kernel.org> On Behalf Of Yoshihiro Shimoda > Sent: 01 April 2019 13:01 > Subject: [PATCH 3/3] phy: renesas: rcar-gen3-usb2: enable/disable independent irqs > > Since the previous code enabled/disabled the irqs both OHCI and EHCI, > it is possible to cause unexpected interruptions. To avoid this, > this patch creates multiple phy instances from phandle and > enables/disables independent irqs by the instances. > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> Reviewed-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com> Tested-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com> > --- > drivers/phy/renesas/phy-rcar-gen3-usb2.c | 181 ++++++++++++++++++++++++++----- > 1 file changed, 156 insertions(+), 25 deletions(-) > > diff --git a/drivers/phy/renesas/phy-rcar-gen3-usb2.c b/drivers/phy/renesas/phy-rcar-gen3-usb2.c > index 4bdb2ed..bbe0fe5 100644 > --- a/drivers/phy/renesas/phy-rcar-gen3-usb2.c > +++ b/drivers/phy/renesas/phy-rcar-gen3-usb2.c > @@ -37,11 +37,8 @@ > > /* INT_ENABLE */ > #define USB2_INT_ENABLE_UCOM_INTEN BIT(3) > -#define USB2_INT_ENABLE_USBH_INTB_EN BIT(2) > -#define USB2_INT_ENABLE_USBH_INTA_EN BIT(1) > -#define USB2_INT_ENABLE_INIT (USB2_INT_ENABLE_UCOM_INTEN | \ > - USB2_INT_ENABLE_USBH_INTB_EN | \ > - USB2_INT_ENABLE_USBH_INTA_EN) > +#define USB2_INT_ENABLE_USBH_INTB_EN BIT(2) /* For EHCI */ > +#define USB2_INT_ENABLE_USBH_INTA_EN BIT(1) /* For OHCI */ > > /* USBCTR */ > #define USB2_USBCTR_DIRPD BIT(2) > @@ -78,11 +75,33 @@ > #define USB2_ADPCTRL_IDPULLUP BIT(5) /* 1 = ID sampling is enabled */ > #define USB2_ADPCTRL_DRVVBUS BIT(4) > > +#define NUM_OF_PHYS 4 > +#define PHY_INDEX_BOTH_HC 0 > +#define PHY_INDEX_OHCI 1 > +#define PHY_INDEX_EHCI 2 > +#define PHY_INDEX_HSUSB 3 > + > +static const u32 rcar_gen3_int_enable[NUM_OF_PHYS] = { > + USB2_INT_ENABLE_USBH_INTB_EN | USB2_INT_ENABLE_USBH_INTA_EN, > + USB2_INT_ENABLE_USBH_INTA_EN, > + USB2_INT_ENABLE_USBH_INTB_EN, > + 0 > +}; > + > +struct rcar_gen3_phy { > + struct phy *phy; > + struct rcar_gen3_chan *ch; > + u32 int_enable_bits; > + bool initialized; > + bool otg_initialized; > + bool powered; > +}; > + > struct rcar_gen3_chan { > void __iomem *base; > struct device *dev; /* platform_device's device */ > struct extcon_dev *extcon; > - struct phy *phy; > + struct rcar_gen3_phy rphys[NUM_OF_PHYS]; > struct regulator *vbus; > struct work_struct work; > enum usb_dr_mode dr_mode; > @@ -250,6 +269,42 @@ static enum phy_mode rcar_gen3_get_phy_mode(struct rcar_gen3_chan *ch) > return PHY_MODE_USB_DEVICE; > } > > +static bool rcar_gen3_is_any_rphy_initialized(struct rcar_gen3_chan *ch) > +{ > + int i; > + > + for (i = 0; i < NUM_OF_PHYS; i++) { > + if (ch->rphys[i].initialized) > + return true; > + } > + > + return false; > +} > + > +static bool rcar_gen3_needs_init_otg(struct rcar_gen3_chan *ch) > +{ > + int i; > + > + for (i = 0; i < NUM_OF_PHYS; i++) { > + if (ch->rphys[i].otg_initialized) > + return false; > + } > + > + return true; > +} > + > +static bool rcar_gen3_are_all_rphys_power_off(struct rcar_gen3_chan *ch) > +{ > + int i; > + > + for (i = 0; i < NUM_OF_PHYS; i++) { > + if (ch->rphys[i].powered) > + return false; > + } > + > + return true; > +} > + > static ssize_t role_store(struct device *dev, struct device_attribute *attr, > const char *buf, size_t count) > { > @@ -257,7 +312,7 @@ static ssize_t role_store(struct device *dev, struct device_attribute *attr, > bool is_b_device; > enum phy_mode cur_mode, new_mode; > > - if (!ch->is_otg_channel || !ch->phy->init_count) > + if (!ch->is_otg_channel || !rcar_gen3_is_any_rphy_initialized(ch)) > return -EIO; > > if (!strncmp(buf, "host", strlen("host"))) > @@ -295,7 +350,7 @@ static ssize_t role_show(struct device *dev, struct device_attribute *attr, > { > struct rcar_gen3_chan *ch = dev_get_drvdata(dev); > > - if (!ch->is_otg_channel || !ch->phy->init_count) > + if (!ch->is_otg_channel || !rcar_gen3_is_any_rphy_initialized(ch)) > return -EIO; > > return sprintf(buf, "%s\n", rcar_gen3_is_host(ch) ? "host" : > @@ -329,37 +384,62 @@ static void rcar_gen3_init_otg(struct rcar_gen3_chan *ch) > > static int rcar_gen3_phy_usb2_init(struct phy *p) > { > - struct rcar_gen3_chan *channel = phy_get_drvdata(p); > + struct rcar_gen3_phy *rphy = phy_get_drvdata(p); > + struct rcar_gen3_chan *channel = rphy->ch; > void __iomem *usb2_base = channel->base; > + u32 val; > > /* Initialize USB2 part */ > - writel(USB2_INT_ENABLE_INIT, usb2_base + USB2_INT_ENABLE); > + val = readl(usb2_base + USB2_INT_ENABLE); > + val |= USB2_INT_ENABLE_UCOM_INTEN | rphy->int_enable_bits; > + writel(val, usb2_base + USB2_INT_ENABLE); > writel(USB2_SPD_RSM_TIMSET_INIT, usb2_base + USB2_SPD_RSM_TIMSET); > writel(USB2_OC_TIMSET_INIT, usb2_base + USB2_OC_TIMSET); > > /* Initialize otg part */ > - if (channel->is_otg_channel) > - rcar_gen3_init_otg(channel); > + if (channel->is_otg_channel) { > + if (rcar_gen3_needs_init_otg(channel)) > + rcar_gen3_init_otg(channel); > + rphy->otg_initialized = true; > + } > + > + rphy->initialized = true; > > return 0; > } > > static int rcar_gen3_phy_usb2_exit(struct phy *p) > { > - struct rcar_gen3_chan *channel = phy_get_drvdata(p); > + struct rcar_gen3_phy *rphy = phy_get_drvdata(p); > + struct rcar_gen3_chan *channel = rphy->ch; > + void __iomem *usb2_base = channel->base; > + u32 val; > > - writel(0, channel->base + USB2_INT_ENABLE); > + rphy->initialized = false; > + > + if (channel->is_otg_channel) > + rphy->otg_initialized = false; > + > + val = readl(usb2_base + USB2_INT_ENABLE); > + val &= ~rphy->int_enable_bits; > + if (!rcar_gen3_is_any_rphy_initialized(channel)) > + val &= ~USB2_INT_ENABLE_UCOM_INTEN; > + writel(val, usb2_base + USB2_INT_ENABLE); > > return 0; > } > > static int rcar_gen3_phy_usb2_power_on(struct phy *p) > { > - struct rcar_gen3_chan *channel = phy_get_drvdata(p); > + struct rcar_gen3_phy *rphy = phy_get_drvdata(p); > + struct rcar_gen3_chan *channel = rphy->ch; > void __iomem *usb2_base = channel->base; > u32 val; > int ret; > > + if (!rcar_gen3_are_all_rphys_power_off(channel)) > + return 0; > + > if (channel->vbus) { > ret = regulator_enable(channel->vbus); > if (ret) > @@ -372,14 +452,22 @@ static int rcar_gen3_phy_usb2_power_on(struct phy *p) > val &= ~USB2_USBCTR_PLL_RST; > writel(val, usb2_base + USB2_USBCTR); > > + rphy->powered = true; > + > return 0; > } > > static int rcar_gen3_phy_usb2_power_off(struct phy *p) > { > - struct rcar_gen3_chan *channel = phy_get_drvdata(p); > + struct rcar_gen3_phy *rphy = phy_get_drvdata(p); > + struct rcar_gen3_chan *channel = rphy->ch; > int ret = 0; > > + rphy->powered = false; > + > + if (!rcar_gen3_are_all_rphys_power_off(channel)) > + return 0; > + > if (channel->vbus) > ret = regulator_disable(channel->vbus); > > @@ -426,13 +514,51 @@ static const unsigned int rcar_gen3_phy_cable[] = { > EXTCON_NONE, > }; > > +static struct phy *rcar_gen3_phy_usb2_xlate(struct device *dev, > + struct of_phandle_args *args) > +{ > + struct rcar_gen3_chan *ch = dev_get_drvdata(dev); > + > + if (args->args_count == 0) /* For old version dts */ > + return ch->rphys[PHY_INDEX_BOTH_HC].phy; > + else if (args->args_count > 1) /* Prevent invalid args count */ > + return ERR_PTR(-ENODEV); > + > + if (args->args[0] >= NUM_OF_PHYS) > + return ERR_PTR(-ENODEV); > + > + return ch->rphys[args->args[0]].phy; > +} > + > +static enum usb_dr_mode rcar_gen3_get_dr_mode(struct device_node *np) > +{ > + enum usb_dr_mode candidate = USB_DR_MODE_UNKNOWN, tmp; > + int i; > + > + /* > + * If one of device nodes has other dr_mode except UNKNOWN, > + * this function returns UNKNOWN. > + */ > + for (i = 0; i < NUM_OF_PHYS; i++) { > + tmp = of_usb_get_dr_mode_by_phy(np, i); We are calling of_usb_get_dr_mode_by_phy with i == 0, but we don't document it in the dt-bindings document? > + if (tmp != USB_DR_MODE_UNKNOWN) { > + if (candidate == USB_DR_MODE_UNKNOWN) > + candidate = tmp; > + else if (candidate != tmp) > + return USB_DR_MODE_UNKNOWN; > + } > + } > + > + return candidate; > +} > + > static int rcar_gen3_phy_usb2_probe(struct platform_device *pdev) > { > struct device *dev = &pdev->dev; > struct rcar_gen3_chan *channel; > struct phy_provider *provider; > struct resource *res; > - int irq, ret = 0; > + int irq, ret = 0, i; > > if (!dev->of_node) { > dev_err(dev, "This driver needs device tree\n"); > @@ -458,7 +584,7 @@ static int rcar_gen3_phy_usb2_probe(struct platform_device *pdev) > dev_err(dev, "No irq handler (%d)\n", irq); > } > > - channel->dr_mode = of_usb_get_dr_mode_by_phy(dev->of_node, 0); > + channel->dr_mode = rcar_gen3_get_dr_mode(dev->of_node); > if (channel->dr_mode != USB_DR_MODE_UNKNOWN) { > int ret; > > @@ -482,11 +608,17 @@ static int rcar_gen3_phy_usb2_probe(struct platform_device *pdev) > * And then, phy-core will manage runtime pm for this device. > */ > pm_runtime_enable(dev); > - channel->phy = devm_phy_create(dev, NULL, &rcar_gen3_phy_usb2_ops); > - if (IS_ERR(channel->phy)) { > - dev_err(dev, "Failed to create USB2 PHY\n"); > - ret = PTR_ERR(channel->phy); > - goto error; > + for (i = 0; i < NUM_OF_PHYS; i++) { > + channel->rphys[i].phy = devm_phy_create(dev, NULL, > + &rcar_gen3_phy_usb2_ops); > + if (IS_ERR(channel->rphys[i].phy)) { > + dev_err(dev, "Failed to create USB2 PHY\n"); > + ret = PTR_ERR(channel->rphys[i].phy); > + goto error; > + } > + channel->rphys[i].ch = channel; > + channel->rphys[i].int_enable_bits = rcar_gen3_int_enable[i]; > + phy_set_drvdata(channel->rphys[i].phy, &channel->rphys[i]); > } > > channel->vbus = devm_regulator_get_optional(dev, "vbus"); > @@ -499,10 +631,9 @@ static int rcar_gen3_phy_usb2_probe(struct platform_device *pdev) > } > > platform_set_drvdata(pdev, channel); > - phy_set_drvdata(channel->phy, channel); > channel->dev = dev; > > - provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate); > + provider = devm_of_phy_provider_register(dev, rcar_gen3_phy_usb2_xlate); > if (IS_ERR(provider)) { > dev_err(dev, "Failed to register PHY provider\n"); > ret = PTR_ERR(provider); > -- > 2.7.4 ^ permalink raw reply [flat|nested] 14+ messages in thread
* RE: [PATCH 3/3] phy: renesas: rcar-gen3-usb2: enable/disable independent irqs 2019-04-03 10:49 ` Fabrizio Castro @ 2019-04-09 9:49 ` Yoshihiro Shimoda 0 siblings, 0 replies; 14+ messages in thread From: Yoshihiro Shimoda @ 2019-04-09 9:49 UTC (permalink / raw) To: Fabrizio Castro Cc: linux-kernel, devicetree, linux-renesas-soc, kishon, robh+dt, mark.rutland Hello Fabrizio-san, > From: Fabrizio Castro, Sent: Wednesday, April 3, 2019 7:49 PM > > Hello Yoshihiro-san, > > Thank you for your partch! > > > From: devicetree-owner@vger.kernel.org <devicetree-owner@vger.kernel.org> On Behalf Of Yoshihiro Shimoda > > Sent: 01 April 2019 13:01 > > Subject: [PATCH 3/3] phy: renesas: rcar-gen3-usb2: enable/disable independent irqs > > > > Since the previous code enabled/disabled the irqs both OHCI and EHCI, > > it is possible to cause unexpected interruptions. To avoid this, > > this patch creates multiple phy instances from phandle and > > enables/disables independent irqs by the instances. > > > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > > Reviewed-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com> > Tested-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com> Thank you for your review and test! <snip> > > +static enum usb_dr_mode rcar_gen3_get_dr_mode(struct device_node *np) > > +{ > > + enum usb_dr_mode candidate = USB_DR_MODE_UNKNOWN, tmp; > > + int i; > > + > > + /* > > + * If one of device nodes has other dr_mode except UNKNOWN, > > + * this function returns UNKNOWN. > > + */ > > + for (i = 0; i < NUM_OF_PHYS; i++) { > > + tmp = of_usb_get_dr_mode_by_phy(np, i); > > We are calling of_usb_get_dr_mode_by_phy with i == 0, but we don't document it > in the dt-bindings document? This (i == 0) is for backward compatibility. So, I'll add such a comment on the driver. Best regards, Yoshihiro Shimoda ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2019-04-09 9:49 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-04-01 12:01 [PATCH 0/3] phy: renesas: rcar-gen3-usb2: enable/disable independent irqs Yoshihiro Shimoda 2019-04-01 12:01 ` [PATCH 1/3] dt-bindings: phy: rcar-gen3-phy-usb2: Revise #phy-cells property Yoshihiro Shimoda 2019-04-03 9:20 ` Simon Horman 2019-04-08 6:13 ` Yoshihiro Shimoda 2019-04-03 10:35 ` Fabrizio Castro 2019-04-08 6:22 ` Yoshihiro Shimoda 2019-04-01 12:01 ` [PATCH 2/3] phy: renesas: rcar-gen3-usb2: Use pdev's device pointer on dev_vdbg() Yoshihiro Shimoda 2019-04-03 9:21 ` Simon Horman 2019-04-03 10:36 ` Fabrizio Castro 2019-04-01 12:01 ` [PATCH 3/3] phy: renesas: rcar-gen3-usb2: enable/disable independent irqs Yoshihiro Shimoda 2019-04-03 9:19 ` Simon Horman 2019-04-09 9:47 ` Yoshihiro Shimoda 2019-04-03 10:49 ` Fabrizio Castro 2019-04-09 9:49 ` Yoshihiro Shimoda
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).