linux-kernel - Re: [PATCH 6/6] clk: clocking-wizard: move dynamic reconfig setup behind flag

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: <0161bb1640489c7a677ac26967b65ee1.sboyd@kernel.org>
Date: Thu, 05 Sep 2024 12:06:05 -0700
From: Stephen Boyd <sboyd@...nel.org>
To: Conor Dooley <conor+dt@...nel.org>, Harry Austen <hpausten@...tonmail.com>, Krzysztof Kozlowski <krzk+dt@...nel.org>, Michael Turquette <mturquette@...libre.com>, Michal Simek <michal.simek@....com>, Rob Herring <robh@...nel.org>
Cc: Shubhrajyoti Datta <shubhrajyoti.datta@....com>, linux-clk@...r.kernel.org, devicetree@...r.kernel.org, linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org, Harry Austen <hpausten@...tonmail.com>
Subject: Re: [PATCH 6/6] clk: clocking-wizard: move dynamic reconfig setup behind flag

Quoting Harry Austen (2024-08-31 04:13:26)
> diff --git a/drivers/clk/xilinx/clk-xlnx-clock-wizard.c b/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
> index 1a65a7d153c35..967eacc28050d 100644
> --- a/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
> +++ b/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
> @@ -1146,20 +1146,6 @@ static int clk_wzrd_probe(struct platform_device *pdev)
>         if (IS_ERR(clk_wzrd->base))
>                 return PTR_ERR(clk_wzrd->base);
>  
> -       ret = of_property_read_u32(np, "xlnx,speed-grade", &clk_wzrd->speed_grade);
> -       if (!ret) {
> -               if (clk_wzrd->speed_grade < 1 || clk_wzrd->speed_grade > 3) {
> -                       dev_warn(&pdev->dev, "invalid speed grade '%d'\n",
> -                                clk_wzrd->speed_grade);
> -                       clk_wzrd->speed_grade = 0;
> -               }
> -       }
> -
> -       clk_wzrd->clk_in1 = devm_clk_get(&pdev->dev, "clk_in1");
> -       if (IS_ERR(clk_wzrd->clk_in1))
> -               return dev_err_probe(&pdev->dev, PTR_ERR(clk_wzrd->clk_in1),
> -                                    "clk_in1 not found\n");
> -
>         clk_wzrd->axi_clk = devm_clk_get_enabled(&pdev->dev, "s_axi_aclk");
>         if (IS_ERR(clk_wzrd->axi_clk))
>                 return dev_err_probe(&pdev->dev, PTR_ERR(clk_wzrd->axi_clk),
> @@ -1170,31 +1156,48 @@ static int clk_wzrd_probe(struct platform_device *pdev)
>                 return -EINVAL;
>         }
>  
> -       ret = clk_wzrd_register_output_clocks(&pdev->dev, nr_outputs);
> -       if (ret)
> -               return ret;
> -
> -       clk_wzrd->clk_data.num = nr_outputs;
> -       ret = devm_of_clk_add_hw_provider(&pdev->dev, of_clk_hw_onecell_get, &clk_wzrd->clk_data);
> -       if (ret) {
> -               dev_err(&pdev->dev, "unable to register clock provider\n");
> -               return ret;
> -       }
> +       if (of_property_read_bool(np, "xlnx,dynamic-reconfig")) {

Is this going to break the existing DTBs? Before the property existed,
the driver seems to have always tried to read xlnx,speed-grade and then
setup a notifier, etc. Why would xlnx,speed-grade be set if
xlnx,dynamic-reconfig wasn't set?

The binding has implicitly had xlnx,dynamic-reconfig set before this
patch, and we should strive to maintain that. Perhaps the property
should be inverted, i.e. xlnx,static-config, and the absence of that
property can imply the reconfig property.