Hi Claudiu,
On Thu, Aug 22, 2024 at 5:28 PM Claudiu <claudiu.beznea@xxxxxxxxx> wrote:
>
> 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.
>
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@xxxxxxxxxxxxxx>
Thanks for your patch, which is now commit 4eae16375357a2a7 ("phy:
renesas: rcar-gen3-usb2: Add support to initialize the bus") in
v6.12-rc1.
> --- a/drivers/phy/renesas/phy-rcar-gen3-usb2.c
> +++ b/drivers/phy/renesas/phy-rcar-gen3-usb2.c
> @@ -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;
Unrelated change?
> +
> + 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");
The reset is not asserted in the error path, only in .remove().
Oh, Christophe already sent a fix for that...
"[PATCH v3] phy: renesas: rcar-gen3-usb2: Fix an error handling path
in rcar_gen3_phy_usb2_probe()"
https://lore.kernel.org/all/290b25827e3f0742808940719455ff0c5cb9d01d.1726329925.git.christophe.jaillet@xxxxxxxxxx
> @@ -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);
> };
The rest LGTM.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
