RE: [PATCH 10/16] phy: renesas: rcar-gen3-usb2: Add support to initialize the bus

[Biju Das]

Hi Claudiu,

> -----Original Message-----
> From: claudiu beznea <claudiu.beznea@xxxxxxxxx>
> Sent: Friday, August 23, 2024 9:58 AM
-
> clk@xxxxxxxxxxxxxxx; linux-pm@xxxxxxxxxxxxxxx; Claudiu Beznea <claudiu.beznea.uj@xxxxxxxxxxxxxx>
> Subject: Re: [PATCH 10/16] phy: renesas: rcar-gen3-usb2: Add support to initialize the bus
> 
> Hi, Biju,
> 
> On 23.08.2024 10:35, Biju Das wrote:
> > Hi Claudiu,
> >
> >> -----Original Message-----
> >> From: Claudiu <claudiu.beznea@xxxxxxxxx>
> >> Sent: Thursday, August 22, 2024 4:28 PM
> >> Subject: [PATCH 10/16] phy: renesas: rcar-gen3-usb2: Add support to
> >> initialize the bus
> >>
> >> From: Claudiu Beznea <claudiu.beznea.uj@xxxxxxxxxxxxxx>
> >>
> >> The Renesas RZ/G3S need to initialize the USB BUS before transferring data due to hardware
> limitation.
> >> As the register that need to be touched for this is in the address
> >> space of the USB PHY, and the UBS PHY need to be initialized before
> >> any other USB drivers handling data transfer, add support to initialize the USB BUS.
> >>
> >> As the USB PHY is probed before any other USB drivers that enables
> >> clocks and de-assert the reset signals and the BUS initialization is
> >> done in the probe phase, we need to add code to de-assert reset signal and runtime resume the
> device (which enables its clocks) before accessing the registers.
> >>
> >> As the reset signals are not required by the USB PHY driver for the
> >> other USB PHY hardware variants, the reset signals and runtime PM was handled only in the function
> that initialize the USB BUS.
> >>
> >> The PHY initialization was done right after runtime PM enable to have
> >> all in place when the PHYs are registered.
> >
> > There is no user for this patch. The first user is RZ/G3S and you
> > should merge this patch with next one.
> 
> I think this is a matter of taste... This is how I usually format the patches (for scenarios like
> this) and got no request for squashing.

That is strange for trivial patches like this.

Cheers,
Biju



> >
> >>
> >> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@xxxxxxxxxxxxxx>
> >> ---
> >>  drivers/phy/renesas/phy-rcar-gen3-usb2.c | 50
> >> ++++++++++++++++++++++--
> >>  1 file changed, 47 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/phy/renesas/phy-rcar-gen3-usb2.c
> >> b/drivers/phy/renesas/phy-rcar-gen3-usb2.c
> >> index 7594f64eb737..cf4299cea579 100644
> >> --- a/drivers/phy/renesas/phy-rcar-gen3-usb2.c
> >> +++ b/drivers/phy/renesas/phy-rcar-gen3-usb2.c
> >> @@ -19,12 +19,14 @@
> >>  #include <linux/platform_device.h>
> >>  #include <linux/pm_runtime.h>
> >>  #include <linux/regulator/consumer.h>
> >> +#include <linux/reset.h>
> >>  #include <linux/string.h>
> >>  #include <linux/usb/of.h>
> >>  #include <linux/workqueue.h>
> >>
> >>  /******* USB2.0 Host registers (original offset is +0x200) *******/
> >>  #define USB2_INT_ENABLE		0x000
> >> +#define USB2_AHB_BUS_CTR	0x008
> >>  #define USB2_USBCTR		0x00c
> >>  #define USB2_SPD_RSM_TIMSET	0x10c
> >>  #define USB2_OC_TIMSET		0x110
> >> @@ -40,6 +42,10 @@
> >>  #define USB2_INT_ENABLE_USBH_INTB_EN	BIT(2)	/* For EHCI */
> >>  #define USB2_INT_ENABLE_USBH_INTA_EN	BIT(1)	/* For OHCI */
> >>
> >> +/* AHB_BUS_CTR */
> >> +#define USB2_AHB_BUS_CTR_MBL_MASK	GENMASK(1, 0)
> >> +#define USB2_AHB_BUS_CTR_MBL_INCR4	2
> >> +
> >>  /* USBCTR */
> >>  #define USB2_USBCTR_DIRPD	BIT(2)
> >>  #define USB2_USBCTR_PLL_RST	BIT(1)
> >> @@ -111,6 +117,7 @@ struct rcar_gen3_chan {
> >>  	struct extcon_dev *extcon;
> >>  	struct rcar_gen3_phy rphys[NUM_OF_PHYS];
> >>  	struct regulator *vbus;
> >> +	struct reset_control *rstc;
> >>  	struct work_struct work;
> >>  	struct mutex lock;	/* protects rphys[...].powered */
> >>  	enum usb_dr_mode dr_mode;
> >> @@ -125,6 +132,7 @@ struct rcar_gen3_chan {  struct rcar_gen3_phy_drv_data {
> >>  	const struct phy_ops *phy_usb2_ops;
> >>  	bool no_adp_ctrl;
> >> +	bool init_bus;
> >>  };
> >>
> >>  /*
> >> @@ -650,6 +658,35 @@ static enum usb_dr_mode rcar_gen3_get_dr_mode(struct device_node *np)
> >>  	return candidate;
> >>  }
> >>
> >> +static int rcar_gen3_phy_usb2_init_bus(struct rcar_gen3_chan
> >> +*channel) {
> >> +	struct device *dev = channel->dev;
> >> +	int ret;
> >> +	u32 val;
> >> +
> >> +	channel->rstc = devm_reset_control_array_get_shared(dev);
> >> +	if (IS_ERR(channel->rstc))
> >> +		return PTR_ERR(channel->rstc);
> >> +
> >> +	ret = pm_runtime_resume_and_get(dev);
> >> +	if (ret)
> >> +		return ret;
> >> +
> >> +	ret = reset_control_deassert(channel->rstc);
> >> +	if (ret)
> >> +		goto rpm_put;
> >> +
> >> +	val = readl(channel->base + USB2_AHB_BUS_CTR);
> >> +	val &= ~USB2_AHB_BUS_CTR_MBL_MASK;
> >> +	val |= USB2_AHB_BUS_CTR_MBL_INCR4;
> >> +	writel(val, channel->base + USB2_AHB_BUS_CTR);
> >> +
> >> +rpm_put:
> >> +	pm_runtime_put(dev);
> >> +
> >> +	return ret;
> >> +}
> >> +
> >>  static int rcar_gen3_phy_usb2_probe(struct platform_device *pdev)  {
> >>  	const struct rcar_gen3_phy_drv_data *phy_data; @@ -703,6 +740,15 @@
> >> static int rcar_gen3_phy_usb2_probe(struct platform_device *pdev)
> >>  		goto error;
> >>  	}
> >>
> >> +	platform_set_drvdata(pdev, channel);
> >> +	channel->dev = dev;
> >> +
> >> +	if (phy_data->init_bus) {
> >> +		ret = rcar_gen3_phy_usb2_init_bus(channel);
> >> +		if (ret)
> >> +			goto error;
> >> +	}
> >> +
> >>  	channel->soc_no_adp_ctrl = phy_data->no_adp_ctrl;
> >>  	if (phy_data->no_adp_ctrl)
> >>  		channel->obint_enable_bits = USB2_OBINT_IDCHG_EN; @@ -733,9 +779,6
> >> @@ static int rcar_gen3_phy_usb2_probe(struct platform_device *pdev)
> >>  		channel->vbus = NULL;
> >>  	}
> >>
> >> -	platform_set_drvdata(pdev, channel);
> >> -	channel->dev = dev;
> >> -
> >>  	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"); @@ -762,6
> >> +805,7 @@ static void rcar_gen3_phy_usb2_remove(struct platform_device *pdev)
> >>  	if (channel->is_otg_channel)
> >>  		device_remove_file(&pdev->dev, &dev_attr_role);
> >>
> >> +	reset_control_assert(channel->rstc);
> >>  	pm_runtime_disable(&pdev->dev);
> >>  };
> >>
> >> --
> >> 2.39.2
> >