[PATCH v2 3/3] clk: xilinx: vcu: Update vcu init/reset sequence

[Visavalia, Rohit]

Hi Geert,

Thanks for the review.

>-----Original Message-----
>From: Geert Uytterhoeven <geert at linux-m68k.org>
>Sent: Friday, January 3, 2025 1:01 AM
>To: Visavalia, Rohit <rohit.visavalia at amd.com>
>Cc: mturquette at baylibre.com; sboyd at kernel.org; Simek, Michal
><michal.simek at amd.com>; Sagar, Vishal <vishal.sagar at amd.com>;
>javier.carrasco.cruz at gmail.com; geert+renesas at glider.be; u.kleine-
>koenig at baylibre.com; linux-clk at vger.kernel.org; linux-arm-
>kernel at lists.infradead.org; linux-kernel at vger.kernel.org
>Subject: Re: [PATCH v2 3/3] clk: xilinx: vcu: Update vcu init/reset sequence
>
>Hi Rohit,
>
>On Thu, Jan 2, 2025 at 6:04 PM Rohit Visavalia <rohit.visavalia at amd.com> wrote:
>> Updated vcu init/reset sequence as per design changes.
>> If VCU reset GPIO is available then do assert and de-assert it before
>> enabling/disabling gasket isolation.
>> This GPIO is added because gasket isolation will be removed during
>> startup that requires access to SLCR register space. Post startup, the
>> ownership of the register interface lies with logiCORE IP.
>>
>> Signed-off-by: Rohit Visavalia <rohit.visavalia at amd.com>
>> ---
>> Changes in v2:
>>   - Changed patches sequence to have patches with "Fixes" as preceding in order
>>   - Used dev_err_probe()
>>   - Moved warning to dev_dbg() and updated print with more detail
>
>Thanks for the update!
>
>> --- a/drivers/clk/xilinx/xlnx_vcu.c
>> +++ b/drivers/clk/xilinx/xlnx_vcu.c
>> @@ -676,6 +679,24 @@ static int xvcu_probe(struct platform_device *pdev)
>>          * Bit 0 : Gasket isolation
>>          * Bit 1 : put VCU out of reset
>>          */
>> +       xvcu->reset_gpio = devm_gpiod_get_optional(&pdev->dev, "reset",
>> +                                                  GPIOD_OUT_LOW);
>
>This requires updating the DT bindings _first_.

Yes, shared patch for the same.
Link: https://lore.kernel.org/linux-clk/20250102163700.759712-1-rohit.visavalia@amd.com/

>
>> +       if (IS_ERR(xvcu->reset_gpio)) {
>> +               ret = PTR_ERR(xvcu->reset_gpio);
>> +               dev_err_probe(&pdev->dev, ret, "failed to get reset gpio for vcu.\n");
>> +               goto error_get_gpio;
>> +       }
>> +
>> +       if (xvcu->reset_gpio) {
>> +               gpiod_set_value(xvcu->reset_gpio, 0);
>> +               /* min 2 clock cycle of vcu pll_ref, slowest freq is 33.33KHz */
>> +               usleep_range(60, 120);
>> +               gpiod_set_value(xvcu->reset_gpio, 1);
>> +               usleep_range(60, 120);
>> +       } else {
>> +               dev_dbg(&pdev->dev, "No reset gpio info found in dts for VCU. This may
>result in incorrect functionality if VCU isolation is removed after initialization in
>designs where the VCU reset is driven by gpio.\n");
>> +       }
>> +
>>         regmap_write(xvcu->logicore_reg_ba, VCU_GASKET_INIT,
>> VCU_GASKET_VALUE);
>>
>>         ret = xvcu_register_clock_provider(xvcu);
>
>Gr{oetje,eeting}s,
>
>                        Geert
>
>--
>Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org
>
>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

Thanks
Rohit