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

[Stephen Boyd]

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.