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

From: Stephen Boyd
Date: Thu Sep 05 2024 - 15:06:18 EST


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.