Quoting Maxime Ripard (2020-02-24 01:06:24)
> The firmware has an interface to discover the clocks it exposes.
>
> Let's use it to discover, register the clocks in the clocks framework and
> then expose them through the device tree for consumers to use them.
Everyone's doing it! :)
> diff --git a/drivers/clk/bcm/clk-raspberrypi.c b/drivers/clk/bcm/clk-raspberrypi.c
> index 3f21888a3e3e..bf6a1e2dc099 100644
> --- a/drivers/clk/bcm/clk-raspberrypi.c
> +++ b/drivers/clk/bcm/clk-raspberrypi.c
> @@ -285,6 +285,95 @@ static struct clk_hw *raspberrypi_register_pllb_arm(struct raspberrypi_clk *rpi)
> return &raspberrypi_clk_pllb_arm.hw;
> }
>
> +static long raspberrypi_fw_dumb_round_rate(struct clk_hw *hw,
> + unsigned long rate,
> + unsigned long *parent_rate)
> +{
> + return rate;
Can we get a comment here like "The firmware does the rounding but
doesn't tell us so we have to assume anything goes!"
> +}
> +
> +static const struct clk_ops raspberrypi_firmware_clk_ops = {
> + .is_prepared = raspberrypi_fw_is_prepared,
> + .recalc_rate = raspberrypi_fw_get_rate,
> + .round_rate = raspberrypi_fw_dumb_round_rate,
> + .set_rate = raspberrypi_fw_set_rate,
> +};
> +
> +static struct clk_hw *raspberrypi_clk_register(struct raspberrypi_clk *rpi,
> + unsigned int parent,
> + unsigned int id)
> +{
> + struct raspberrypi_clk_data *data;
> + struct clk_init_data init = {};
> + int ret;
> +
> + if (id == RPI_FIRMWARE_ARM_CLK_ID) {
> + struct clk_hw *hw;
> +
> + hw = raspberrypi_register_pllb(rpi);
> + if (IS_ERR(hw)) {
> + dev_err(rpi->dev, "Failed to initialize pllb, %ld\n",
> + PTR_ERR(hw));
> + return hw;
> + }
> +
> + hw = raspberrypi_register_pllb_arm(rpi);
> + if (IS_ERR(hw))
> + return hw;
> +
> + return hw;
Why the double return? Not just
return raspberrypi_register_pllb_arm(rpi);
> + }
> +
> + data = devm_kzalloc(rpi->dev, sizeof(data), GFP_KERNEL);
> + if (!data)
> + return ERR_PTR(-ENOMEM);
> + data->rpi = rpi;
> + data->id = id;
> +
> + init.name = devm_kasprintf(rpi->dev, GFP_KERNEL, "fw-clk-%u", id);
> + init.ops = &raspberrypi_firmware_clk_ops;
> + init.flags = CLK_GET_RATE_NOCACHE | CLK_IGNORE_UNUSED;
Why ignore unused? Doesn't seem to support enable/disable anyway so not
sure this flag adds any value.
> +
> + data->hw.init = &init;
> +
> + ret = devm_clk_hw_register(rpi->dev, &data->hw);
> + if (ret)
> + return ERR_PTR(ret);
> +
> + return &data->hw;
> +}
> +
> +static int raspberrypi_discover_clocks(struct raspberrypi_clk *rpi,
> + struct clk_hw_onecell_data *data)
> +{
> + struct rpi_firmware_get_clocks_response *clks;
> + size_t clks_size = NUM_FW_CLKS * sizeof(*clks);
> + int ret;
> +
> + clks = devm_kzalloc(rpi->dev, clks_size, GFP_KERNEL);
Use devm_kcalloc to avoid overflow and indicate it's an array.
> + if (!clks)
> + return -ENOMEM;
> +
> + ret = rpi_firmware_property(rpi->firmware, RPI_FIRMWARE_GET_CLOCKS,
> + clks, clks_size);
> + if (ret)
> + return ret;
> +
> + while (clks->id) {
> + struct clk_hw *hw;
> +
> + hw = raspberrypi_clk_register(rpi, clks->parent, clks->id);
> + if (IS_ERR(hw))
> + return PTR_ERR(hw);
> +
> + data->hws[clks->id] = hw;
> + data->num = clks->id + 1;
> + clks++;
> + }
> +
> + return 0;
> +}
> +
> static int raspberrypi_clk_probe(struct platform_device *pdev)
> {
> struct clk_hw_onecell_data *clk_data;
> diff --git a/include/soc/bcm2835/raspberrypi-firmware.h b/include/soc/bcm2835/raspberrypi-firmware.h
> index 7800e12ee042..e5b7a41bba6b 100644
> --- a/include/soc/bcm2835/raspberrypi-firmware.h
> +++ b/include/soc/bcm2835/raspberrypi-firmware.h
> @@ -135,6 +135,11 @@ enum rpi_firmware_property_tag {
> RPI_FIRMWARE_GET_DMA_CHANNELS = 0x00060001,
> };
>
> +struct rpi_firmware_get_clocks_response {
> + __le32 parent;
> + __le32 id;
Why double spaced or tabbed out?
> +};
> +
> #if IS_ENABLED(CONFIG_RASPBERRYPI_FIRMWARE)
> int rpi_firmware_property(struct rpi_firmware *fw,
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
