[PATCH v2 10/15] phy: renesas: rcar-gen3-usb2: Add support for PWRRDY

[Claudiu Beznea]

Hi, Geert,

On 28.11.2024 17:07, Geert Uytterhoeven wrote:
> Hi Claudiu,
> 
> On Tue, Nov 26, 2024 at 10:21 AM Claudiu <claudiu.beznea at tuxon.dev> wrote:
>> From: Claudiu Beznea <claudiu.beznea.uj at bp.renesas.com>
>>
>> On the Renesas RZ/G3S SoC, the USB PHY 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.
>>
>> The connection b/w the system controller and the USB PHY is implemented
>> through the renesas,sysc-signal device tree property. This property
>> specifies the register offset and the bitmask required to control the
>> signal. The system controller exports the syscon regmap, and the read/write
>> access to the memory area of the PWRRDY signal is reference-counted, as the
>> same system controller signal is connected to both RZ/G3S USB PHYs.
>>
>> Add support for the PWRRDY signal control.
>>
>> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj at bp.renesas.com>
> 
> Thanks for your patch!
> 
>> --- a/drivers/phy/renesas/phy-rcar-gen3-usb2.c
>> +++ b/drivers/phy/renesas/phy-rcar-gen3-usb2.c
>> @@ -707,6 +718,55 @@ static int rcar_gen3_phy_usb2_init_bus(struct rcar_gen3_chan *channel)
>>         return ret;
>>  }
>>
>> +static void rcar_gen3_phy_usb2_set_pwrrdy(struct rcar_gen3_chan *channel, bool power_on)
>> +{
>> +       struct rcar_gen3_pwrrdy *pwrrdy = channel->pwrrdy;
>> +
>> +       /* N/A on this platform. */
>> +       if (!pwrrdy)
>> +               return;
> 
> This cannot happen?

You're right, currently it can't happen.

It might be useful for the suspend to RAM support (that will be posted
after initial support will be integrated) to have this function called
unconditionally on suspend/resume APIs.

I can drop it if it's preferred.

Thank you for  your review,
Claudiu

> 
>> +
>> +       regmap_update_bits(pwrrdy->regmap, pwrrdy->offset, pwrrdy->mask, !power_on);
>> +}
>> +
>> +static void rcar_gen3_phy_usb2_pwrrdy_off(void *data)
>> +{
>> +       rcar_gen3_phy_usb2_set_pwrrdy(data, false);
>> +}
>> +
>> +static int rcar_gen3_phy_usb2_init_pwrrdy(struct rcar_gen3_chan *channel)
>> +{
>> +       struct device *dev = channel->dev;
>> +       struct rcar_gen3_pwrrdy *pwrrdy;
>> +       struct of_phandle_args args;
>> +       int ret;
>> +
>> +       pwrrdy = devm_kzalloc(dev, sizeof(*pwrrdy), GFP_KERNEL);
>> +       if (!pwrrdy)
>> +               return -ENOMEM;
>> +
>> +       ret = of_parse_phandle_with_args(dev->of_node, "renesas,sysc-signal",
>> +                                        "#renesas,sysc-signal-cells", 0, &args);
>> +       if (ret)
>> +               return ret;
>> +
>> +       pwrrdy->regmap = syscon_node_to_regmap(args.np);
>> +       pwrrdy->offset = args.args[0];
>> +       pwrrdy->mask = args.args[1];
>> +
>> +       of_node_put(args.np);
>> +
>> +       if (IS_ERR(pwrrdy->regmap))
>> +               return PTR_ERR(pwrrdy->regmap);
>> +
>> +       channel->pwrrdy = pwrrdy;
>> +
>> +       /* Power it ON. */
>> +       rcar_gen3_phy_usb2_set_pwrrdy(channel, true);
>> +
>> +       return devm_add_action_or_reset(dev, rcar_gen3_phy_usb2_pwrrdy_off, channel);
>> +}
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
>