| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <c7fc31f1247332196516394a22f6feef9733a0b4.camel@pengutronix.de>
Date: Wed, 08 Oct 2025 10:34:13 +0200
From: Philipp Zabel <p.zabel@...gutronix.de>
To: Claudiu <claudiu.beznea@...on.dev>, vkoul@...nel.org, kishon@...nel.org,
robh@...nel.org, krzk+dt@...nel.org, conor+dt@...nel.org,
geert+renesas@...der.be, magnus.damm@...il.com,
yoshihiro.shimoda.uh@...esas.com, biju.das.jz@...renesas.com
Cc: linux-phy@...ts.infradead.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-renesas-soc@...r.kernel.org, Claudiu
Beznea <claudiu.beznea.uj@...renesas.com>, Wolfram Sang
<wsa+renesas@...g-engineering.com>
Subject: Re: [PATCH v7 4/7] reset: rzg2l-usbphy-ctrl: Add support for USB
PWRRDY
Hi Claudiu,
On Do, 2025-09-25 at 13:02 +0300, Claudiu wrote:
> From: Claudiu Beznea <claudiu.beznea.uj@...renesas.com>
>
> On the Renesas RZ/G3S SoC, the USB PHY block has an input signal called
> PWRRDY. This signal is managed by the system controller and must be
> de-asserted after powering on the area where USB PHY resides and asserted
> before powering it off.
>
> On power-on the USB PWRRDY signal need to be de-asserted before enabling
> clock and switching the module to normal state (through MSTOP support). The
> power-on configuration sequence
The wording makes me wonder, have you considered implementing this as a
power sequencing driver?
> must be:
>
> 1/ PWRRDY=0
> 2/ CLK_ON=1
> 3/ MSTOP=0
>
> On power-off the configuration sequence should be:
>
> 1/ MSTOP=1
> 2/ CLK_ON=0
> 3/ PWRRDY=1
>
> The CLK_ON and MSTOP functionalities are controlled by clock drivers.
>
> After long discussions with the internal HW team, it has been confirmed
> that the HW connection b/w USB PHY block, the USB channels, the system
> controller, clock, MSTOP, PWRRDY signal is as follows:
>
> ┌──────────────────────────────┐
> │ │◄── CPG_CLKON_USB.CLK0_ON
> │ USB CH0 │
> ┌──────────────────────────┐ │┌───────────────────────────┐ │◄── CPG_CLKON_USB.CLK2_ON
> │ ┌────────┐ ││host controller registers │ │
> │ │ │ ││function controller registers│
> │ │ PHY0 │◄──┤└───────────────────────────┘ │
> │ USB PHY │ │ └────────────▲─────────────────┘
> │ └────────┘ │
> │ │ CPG_BUS_PERI_COM_MSTOP.MSTOP{6, 5}_ON
> │┌──────────────┐ ┌────────┐
> ││USHPHY control│ │ │
> ││ registers │ │ PHY1 │ ┌──────────────────────────────┐
> │└──────────────┘ │ │◄──┤ USB CH1 │
> │ └────────┘ │┌───────────────────────────┐ │◄── CPG_CLKON_USB.CLK1_ON
> └─▲───────▲─────────▲──────┘ ││ host controller registers │ │
> │ │ │ │└───────────────────────────┘ │
> │ │ │ └────────────▲─────────────────┘
> │ │ │ │
> │ │ │ CPG_BUS_PERI_COM_MSTOP.MSTOP7_ON
> │PWRRDY │ │
> │ │ CPG_CLK_ON_USB.CLK3_ON
> │ │
> │ CPG_BUS_PERI_COM_MSTOP.MSTOP4_ON
> │
> ┌────┐
> │SYSC│
> └────┘
>
> where:
> - CPG_CLKON_USB.CLK.CLKX_ON is the register bit controlling the clock X
> of different USB blocks, X in {0, 1, 2, 3}
> - CPG_BUS_PERI_COM_MSTOP.MSTOPX_ON is the register bit controlling the
> MSTOP of different USB blocks, X in {4, 5, 6, 7}
> - USB PHY is the USB PHY block exposing 2 ports, port0 and port1, used
> by the USB CH0, USB CH1
> - SYSC is the system controller block controlling the PWRRDY signal
> - USB CHx are individual USB block with host and function capabilities
> (USB CH0 have both host and function capabilities, USB CH1 has only
> host capabilities)
>
> The USBPHY control registers are controlled though the
> reset-rzg2l-usbphy-ctrl driver. The USB PHY ports are controlled by
> phy_rcar_gen3_usb2 (drivers/phy/renesas/phy-rcar-gen3-usb2.c file). The
> USB PHY ports request resets from the reset-rzg2l-usbphy-ctrl driver.
>
> The connection b/w the system controller and the USB PHY CTRL driver is
> implemented through the renesas,sysc-pwrrdy device tree property
> proposed in this patch. This property specifies the register offset and the
> bitmask required to control the PWRRDY signal.
>
> Since the USB PHY CTRL driver needs to be probed before any other
> USB-specific driver on RZ/G3S, control of PWRRDY is passed exclusively
> to it. This guarantees the correct configuration sequence between clocks,
> MSTOP bits, and the PWRRDY bit. At the same time, changes are kept minimal
> by avoiding modifications to the USB PHY driver to also handle the PWRRDY
> itself.
>
> Tested-by: Wolfram Sang <wsa+renesas@...g-engineering.com>
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@...renesas.com>
> ---
>
> Changes in v7:
> - used proper regmap update value on rzg2l_usbphy_ctrl_set_pwrrdy()
>
> Changes in v6:
> - used syscon_regmap_lookup_by_phandle_args() to simplify the code
> - collected tags
>
> Changes in v5:
> - none
>
> Changes in v4:
> - updated patch description
> - updated rzg2l_usbphy_ctrl_pwrrdy_init() to map directly the
> "renesas,sysc-pwrrdy" as the SYSC signal abstraction was dropped
> in this version, along with rz_sysc_get_signal_map()
> - dropped priv member of rzg2l_usbphy_ctrl_pwrrdy_init() as it is
> not needed in this version
> - shift left !power_on with pwrrdy->mask as this is how the
> regmap_update_bits() needs the last member to be
> - selected MFD_SYSCON
>
> Changes in v3:
> - none; this patch is new
>
>
> drivers/reset/Kconfig | 1 +
> drivers/reset/reset-rzg2l-usbphy-ctrl.c | 62 +++++++++++++++++++++++++
> 2 files changed, 63 insertions(+)
>
> diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
> index 78b7078478d4..329730cbcfb9 100644
> --- a/drivers/reset/Kconfig
> +++ b/drivers/reset/Kconfig
> @@ -237,6 +237,7 @@ config RESET_RASPBERRYPI
> config RESET_RZG2L_USBPHY_CTRL
> tristate "Renesas RZ/G2L USBPHY control driver"
> depends on ARCH_RZG2L || COMPILE_TEST
> + select MFD_SYSCON
> help
> Support for USBPHY Control found on RZ/G2L family. It mainly
> controls reset and power down of the USB/PHY.
> diff --git a/drivers/reset/reset-rzg2l-usbphy-ctrl.c b/drivers/reset/reset-rzg2l-usbphy-ctrl.c
> index 8a7f167e405e..be315199e2b0 100644
> --- a/drivers/reset/reset-rzg2l-usbphy-ctrl.c
> +++ b/drivers/reset/reset-rzg2l-usbphy-ctrl.c
> @@ -13,6 +13,7 @@
> #include <linux/regmap.h>
> #include <linux/reset.h>
> #include <linux/reset-controller.h>
> +#include <linux/mfd/syscon.h>
>
> #define RESET 0x000
> #define VBENCTL 0x03c
> @@ -41,6 +42,18 @@ struct rzg2l_usbphy_ctrl_priv {
>
> #define rcdev_to_priv(x) container_of(x, struct rzg2l_usbphy_ctrl_priv, rcdev)
>
> +/**
> + * struct rzg2l_usbphy_ctrl_pwrrdy - SYSC PWRRDY signal descriptor
> + * @regmap: SYSC regmap
> + * @offset: offset into the SYSC address space for accessing PWRRDY
> + * @mask: mask into the register at offset for accessing PWRRDY
> + */
> +struct rzg2l_usbphy_ctrl_pwrrdy {
> + struct regmap *regmap;
> + u32 offset;
> + u32 mask;
> +};
> +
> static int rzg2l_usbphy_ctrl_assert(struct reset_controller_dev *rcdev,
> unsigned long id)
> {
> @@ -91,6 +104,8 @@ static int rzg2l_usbphy_ctrl_status(struct reset_controller_dev *rcdev,
> return !!(readl(priv->base + RESET) & port_mask);
> }
>
> +#define RZG2L_USBPHY_CTRL_PWRRDY 1
> +
> static const struct of_device_id rzg2l_usbphy_ctrl_match_table[] = {
> { .compatible = "renesas,rzg2l-usbphy-ctrl" },
> { /* Sentinel */ }
> @@ -110,6 +125,49 @@ static const struct regmap_config rzg2l_usb_regconf = {
> .max_register = 1,
> };
>
> +static void rzg2l_usbphy_ctrl_set_pwrrdy(struct rzg2l_usbphy_ctrl_pwrrdy *pwrrdy,
> + bool power_on)
> +{
> + u32 val = (!power_on << (ffs(pwrrdy->mask) - 1)) & pwrrdy->mask;
Why not just:
u32 val = power_on ? 0 : pwrrdy->mask;
You could simplify this further by using a regmap_field instead of
rzg2l_usbphy_ctrl_pwrrdy.
> +
> + regmap_update_bits(pwrrdy->regmap, pwrrdy->offset, pwrrdy->mask, val);
> +}
> +
> +static void rzg2l_usbphy_ctrl_pwrrdy_off(void *data)
> +{
> + rzg2l_usbphy_ctrl_set_pwrrdy(data, false);
> +}
> +
> +static int rzg2l_usbphy_ctrl_pwrrdy_init(struct device *dev)
> +{
> + struct rzg2l_usbphy_ctrl_pwrrdy *pwrrdy;
> + struct regmap *regmap;
> + const int *data;
> + u32 args[2];
> +
> + data = device_get_match_data(dev);
> + if (data != (int *)RZG2L_USBPHY_CTRL_PWRRDY)
Better not to compare pointers here:
if ((uintptr_t)data != RZG2L_USBPHY_CTRL_PWRRDY)
> + return 0;
> +
> + regmap = syscon_regmap_lookup_by_phandle_args(dev->of_node,
> + "renesas,sysc-pwrrdy",
> + ARRAY_SIZE(args), args);
> + if (IS_ERR(regmap))
> + return PTR_ERR(regmap);
> +
> + pwrrdy = devm_kzalloc(dev, sizeof(*pwrrdy), GFP_KERNEL);
> + if (!pwrrdy)
> + return -ENOMEM;
> +
> + pwrrdy->regmap = regmap;
> + pwrrdy->offset = args[0];
> + pwrrdy->mask = args[1];
> +
> + rzg2l_usbphy_ctrl_set_pwrrdy(pwrrdy, true);
> +
> + return devm_add_action_or_reset(dev, rzg2l_usbphy_ctrl_pwrrdy_off, pwrrdy);
> +}
So we deassert PWRRDY on probe and assert on remove. What about
suspend/resume ordering?
regards
Philipp
Powered by blists - more mailing lists