Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   Fabrizio Castro <fabrizio.castro@bp.renesas.com>
To:     Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>,
        "kishon@ti.com" <kishon@ti.com>,
        "robh+dt@kernel.org" <robh+dt@kernel.org>,
        "mark.rutland@arm.com" <mark.rutland@arm.com>
CC:     "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
        "linux-renesas-soc@vger.kernel.org" 
        <linux-renesas-soc@vger.kernel.org>,
        Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Subject: RE: [PATCH 3/3] phy: renesas: rcar-gen3-usb2: enable/disable
 independent irqs
Thread-Topic: [PATCH 3/3] phy: renesas: rcar-gen3-usb2: enable/disable
 independent irqs
Thread-Index: AQHU6IM0XH8hd2Tue0yrb2jtPkZtk6YqQWUA
Date:   Wed, 3 Apr 2019 10:49:20 +0000
Message-ID: <TY1PR01MB17706362E3C2AB67C3ABD1E7C0570@TY1PR01MB1770.jpnprd01.prod.outlook.com>
References: <1554120083-29990-1-git-send-email-yoshihiro.shimoda.uh@renesas.com>
 <1554120083-29990-4-git-send-email-yoshihiro.shimoda.uh@renesas.com>
In-Reply-To: <1554120083-29990-4-git-send-email-yoshihiro.shimoda.uh@renesas.com>
Accept-Language: en-GB, en-US
Content-Language: en-US
received-spf: None (protection.outlook.com: bp.renesas.com does not designate
 permitted sender hosts)
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 6ab6e058-75cc-4c2d-cd03-08d6b822071f
X-MS-Exchange-CrossTenant-originalarrivaltime: 03 Apr 2019 10:49:20.7027
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 53d82571-da19-47e4-9cb4-625a166a4a2a
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-Transport-CrossTenantHeadersStamped: TY1PR01MB1849
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

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 indepen=
dent irqs
>=20
> 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.
>=20
> 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(-)
>=20
> diff --git a/drivers/phy/renesas/phy-rcar-gen3-usb2.c b/drivers/phy/renes=
as/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 @@
>=20
>  /* 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 */
>=20
>  /* USBCTR */
>  #define USB2_USBCTR_DIRPD	BIT(2)
> @@ -78,11 +75,33 @@
>  #define USB2_ADPCTRL_IDPULLUP		BIT(5)	/* 1 =3D ID sampling is enabled */
>  #define USB2_ADPCTRL_DRVVBUS		BIT(4)
>=20
> +#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] =3D {
> +	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 r=
car_gen3_chan *ch)
>  	return PHY_MODE_USB_DEVICE;
>  }
>=20
> +static bool rcar_gen3_is_any_rphy_initialized(struct rcar_gen3_chan *ch)
> +{
> +	int i;
> +
> +	for (i =3D 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 =3D 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 =3D 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 *a=
ttr,
>  			  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;
>=20
> -	if (!ch->is_otg_channel || !ch->phy->init_count)
> +	if (!ch->is_otg_channel || !rcar_gen3_is_any_rphy_initialized(ch))
>  		return -EIO;
>=20
>  	if (!strncmp(buf, "host", strlen("host")))
> @@ -295,7 +350,7 @@ static ssize_t role_show(struct device *dev, struct d=
evice_attribute *attr,
>  {
>  	struct rcar_gen3_chan *ch =3D dev_get_drvdata(dev);
>=20
> -	if (!ch->is_otg_channel || !ch->phy->init_count)
> +	if (!ch->is_otg_channel || !rcar_gen3_is_any_rphy_initialized(ch))
>  		return -EIO;
>=20
>  	return sprintf(buf, "%s\n", rcar_gen3_is_host(ch) ? "host" :
> @@ -329,37 +384,62 @@ static void rcar_gen3_init_otg(struct rcar_gen3_cha=
n *ch)
>=20
>  static int rcar_gen3_phy_usb2_init(struct phy *p)
>  {
> -	struct rcar_gen3_chan *channel =3D phy_get_drvdata(p);
> +	struct rcar_gen3_phy *rphy =3D phy_get_drvdata(p);
> +	struct rcar_gen3_chan *channel =3D rphy->ch;
>  	void __iomem *usb2_base =3D channel->base;
> +	u32 val;
>=20
>  	/* Initialize USB2 part */
> -	writel(USB2_INT_ENABLE_INIT, usb2_base + USB2_INT_ENABLE);
> +	val =3D readl(usb2_base + USB2_INT_ENABLE);
> +	val |=3D 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);
>=20
>  	/* 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 =3D true;
> +	}
> +
> +	rphy->initialized =3D true;
>=20
>  	return 0;
>  }
>=20
>  static int rcar_gen3_phy_usb2_exit(struct phy *p)
>  {
> -	struct rcar_gen3_chan *channel =3D phy_get_drvdata(p);
> +	struct rcar_gen3_phy *rphy =3D phy_get_drvdata(p);
> +	struct rcar_gen3_chan *channel =3D rphy->ch;
> +	void __iomem *usb2_base =3D channel->base;
> +	u32 val;
>=20
> -	writel(0, channel->base + USB2_INT_ENABLE);
> +	rphy->initialized =3D false;
> +
> +	if (channel->is_otg_channel)
> +		rphy->otg_initialized =3D false;
> +
> +	val =3D readl(usb2_base + USB2_INT_ENABLE);
> +	val &=3D ~rphy->int_enable_bits;
> +	if (!rcar_gen3_is_any_rphy_initialized(channel))
> +		val &=3D ~USB2_INT_ENABLE_UCOM_INTEN;
> +	writel(val, usb2_base + USB2_INT_ENABLE);
>=20
>  	return 0;
>  }
>=20
>  static int rcar_gen3_phy_usb2_power_on(struct phy *p)
>  {
> -	struct rcar_gen3_chan *channel =3D phy_get_drvdata(p);
> +	struct rcar_gen3_phy *rphy =3D phy_get_drvdata(p);
> +	struct rcar_gen3_chan *channel =3D rphy->ch;
>  	void __iomem *usb2_base =3D channel->base;
>  	u32 val;
>  	int ret;
>=20
> +	if (!rcar_gen3_are_all_rphys_power_off(channel))
> +		return 0;
> +
>  	if (channel->vbus) {
>  		ret =3D regulator_enable(channel->vbus);
>  		if (ret)
> @@ -372,14 +452,22 @@ static int rcar_gen3_phy_usb2_power_on(struct phy *=
p)
>  	val &=3D ~USB2_USBCTR_PLL_RST;
>  	writel(val, usb2_base + USB2_USBCTR);
>=20
> +	rphy->powered =3D true;
> +
>  	return 0;
>  }
>=20
>  static int rcar_gen3_phy_usb2_power_off(struct phy *p)
>  {
> -	struct rcar_gen3_chan *channel =3D phy_get_drvdata(p);
> +	struct rcar_gen3_phy *rphy =3D phy_get_drvdata(p);
> +	struct rcar_gen3_chan *channel =3D rphy->ch;
>  	int ret =3D 0;
>=20
> +	rphy->powered =3D false;
> +
> +	if (!rcar_gen3_are_all_rphys_power_off(channel))
> +		return 0;
> +
>  	if (channel->vbus)
>  		ret =3D regulator_disable(channel->vbus);
>=20
> @@ -426,13 +514,51 @@ static const unsigned int rcar_gen3_phy_cable[] =3D=
 {
>  	EXTCON_NONE,
>  };
>=20
> +static struct phy *rcar_gen3_phy_usb2_xlate(struct device *dev,
> +					    struct of_phandle_args *args)
> +{
> +	struct rcar_gen3_chan *ch =3D dev_get_drvdata(dev);
> +
> +	if (args->args_count =3D=3D 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] >=3D 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 =3D USB_DR_MODE_UNKNOWN, tmp;
> +	int i;
> +
> +	/*
> +	 * If one of device nodes has other dr_mode except UNKNOWN,
> +	 * this function returns UNKNOWN.
> +	 */
> +	for (i =3D 0; i < NUM_OF_PHYS; i++) {
> +		tmp =3D of_usb_get_dr_mode_by_phy(np, i);

We are calling of_usb_get_dr_mode_by_phy with i =3D=3D 0, but we don't docu=
ment it
in the dt-bindings document?

> +		if (tmp !=3D USB_DR_MODE_UNKNOWN) {
> +			if (candidate =3D=3D USB_DR_MODE_UNKNOWN)
> +				candidate =3D tmp;
> +			else if (candidate !=3D tmp)
> +				return USB_DR_MODE_UNKNOWN;
> +		}
> +	}
> +
> +	return candidate;
> +}
> +
>  static int rcar_gen3_phy_usb2_probe(struct platform_device *pdev)
>  {
>  	struct device *dev =3D &pdev->dev;
>  	struct rcar_gen3_chan *channel;
>  	struct phy_provider *provider;
>  	struct resource *res;
> -	int irq, ret =3D 0;
> +	int irq, ret =3D 0, i;
>=20
>  	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_d=
evice *pdev)
>  			dev_err(dev, "No irq handler (%d)\n", irq);
>  	}
>=20
> -	channel->dr_mode =3D of_usb_get_dr_mode_by_phy(dev->of_node, 0);
> +	channel->dr_mode =3D rcar_gen3_get_dr_mode(dev->of_node);
>  	if (channel->dr_mode !=3D USB_DR_MODE_UNKNOWN) {
>  		int ret;
>=20
> @@ -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 =3D 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 =3D PTR_ERR(channel->phy);
> -		goto error;
> +	for (i =3D 0; i < NUM_OF_PHYS; i++) {
> +		channel->rphys[i].phy =3D 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 =3D PTR_ERR(channel->rphys[i].phy);
> +			goto error;
> +		}
> +		channel->rphys[i].ch =3D channel;
> +		channel->rphys[i].int_enable_bits =3D rcar_gen3_int_enable[i];
> +		phy_set_drvdata(channel->rphys[i].phy, &channel->rphys[i]);
>  	}
>=20
>  	channel->vbus =3D devm_regulator_get_optional(dev, "vbus");
> @@ -499,10 +631,9 @@ static int rcar_gen3_phy_usb2_probe(struct platform_=
device *pdev)
>  	}
>=20
>  	platform_set_drvdata(pdev, channel);
> -	phy_set_drvdata(channel->phy, channel);
>  	channel->dev =3D dev;
>=20
> -	provider =3D devm_of_phy_provider_register(dev, of_phy_simple_xlate);
> +	provider =3D devm_of_phy_provider_register(dev, rcar_gen3_phy_usb2_xlat=
e);
>  	if (IS_ERR(provider)) {
>  		dev_err(dev, "Failed to register PHY provider\n");
>  		ret =3D PTR_ERR(provider);
> --
> 2.7.4