linux-kernel - Re: [PATCH 1/3] clk: xilinx: vcu: Update vcu init/reset sequence

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAMuHMdX57SvuVsRhisF1RDtP+TgQsa2i+hBg334yQUxTMsqDPw@mail.gmail.com>
Date: Tue, 31 Dec 2024 15:26:07 +0100
From: Geert Uytterhoeven <geert@...ux-m68k.org>
To: "Visavalia, Rohit" <rohit.visavalia@....com>
Cc: Stephen Boyd <sboyd@...nel.org>, "Simek, Michal" <michal.simek@....com>, 
	"mturquette@...libre.com" <mturquette@...libre.com>, "Sagar, Vishal" <vishal.sagar@....com>, 
	"javier.carrasco.cruz@...il.com" <javier.carrasco.cruz@...il.com>, 
	"geert+renesas@...der.be" <geert+renesas@...der.be>, 
	"u.kleine-koenig@...libre.com" <u.kleine-koenig@...libre.com>, 
	"linux-clk@...r.kernel.org" <linux-clk@...r.kernel.org>, 
	"linux-arm-kernel@...ts.infradead.org" <linux-arm-kernel@...ts.infradead.org>, 
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/3] clk: xilinx: vcu: Update vcu init/reset sequence

Hi Rohit,

On Tue, Dec 31, 2024 at 3:16 PM Visavalia, Rohit
<rohit.visavalia@....com> wrote:
> >Subject: Re: [PATCH 1/3] clk: xilinx: vcu: Update vcu init/reset sequence
> >Quoting Rohit Visavalia (2024-12-26 04:20:21)
> >> diff --git a/drivers/clk/xilinx/xlnx_vcu.c
> >> b/drivers/clk/xilinx/xlnx_vcu.c index 81501b48412e..f294a2398cb4
> >> 100644
> >> --- 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);
> >> +       if (IS_ERR(xvcu->reset_gpio)) {
> >> +               ret = PTR_ERR(xvcu->reset_gpio);
> >> +               dev_err(&pdev->dev, "failed to get reset gpio for
> >> + vcu.\n");
> >
> >Use dev_err_probe() and friends.
> I will take care in v2 patch series.
>
> >
> >> +               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_warn(&pdev->dev, "No reset gpio info from dts for
> >> + vcu. This may lead to incorrect functionality if VCU isolation is
> >> + removed post initialization.\n");
> >
> >Is it 'vcu' or 'VCU'? Pick one please. Also, this is going to be an unfixable warning
> >message if the reset isn't there. Why have this warning at all?
> I will use 'VCU' in next(v2) patch series.
> Added warning just to inform user that if design has the reset gpio and it is missing in dt node then it could lead to issue.

If it could lead to issues, shouldn't the reset GPIO be required instead of
optional?

Regardless, the reset GPIO should be documented in the DT bindings.
And perhaps marked required, so "make dtbs_check" will flag it when
it's missing?

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@...ux-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