On 06/09/2022 08:28, Claudiu Beznea - M18063 wrote: > On 30.08.2022 15:28, Conor Dooley wrote: >> Add a driver to support the PLLs in PolarFire SoC's Clock Conditioning >> Circuitry, an instance of which is located in each ordinal corner of >> the FPGA. Only get_rate() is supported as these clocks are intended to >> be statically configured by the FPGA design. Currently, the DLLs are >> not supported by this driver. For more information on the hardware, see >> "PolarFire SoC FPGA Clocking Resources" in the link below. >> >> Link: https://onlinedocs.microchip.com/pr/GUID-8F0CC4C0-0317-4262-89CA-CE7773ED1931-en-US-1/index.html >> Signed-off-by: Conor Dooley <conor.dooley@xxxxxxxxxxxxx> >> + >> +#define CLK_CCC_PLL(_id, _parents, _shift, _width, _flags, _offset) { \ >> + .id = _id, \ >> + .shift = _shift, \ >> + .width = _width, \ >> + .reg_offset = _offset, \ >> + .flags = _flags, \ >> + .parents = _parents,\ > > There is a bit of missalignment of \ in this macro. > Eww, there is... >> +static int mpfs_ccc_register_plls(struct device *dev, struct mpfs_ccc_pll_hw_clock *pll_hws, >> + unsigned int num_clks, struct mpfs_ccc_data *data) >> +{ >> + int ret; >> + >> + for (unsigned int i = 0; i < num_clks; i++) { >> + struct mpfs_ccc_pll_hw_clock *pll_hw = &pll_hws[i]; >> + char *name = devm_kzalloc(dev, 18, GFP_KERNEL); >> + >> + pll_hw->base = data->pll_base[i]; >> + snprintf(name, 18, "ccc%s_pll%u", strchrnul(dev->of_node->full_name, '@'), i); >> + pll_hw->name = (const char *)name; >> + pll_hw->hw.init = CLK_HW_INIT_PARENTS_DATA_FIXED_SIZE(pll_hw->name, >> + pll_hw->parents, >> + &mpfs_ccc_pll_ops, 0); >> + >> + ret = devm_clk_hw_register(dev, &pll_hw->hw); >> + if (ret) >> + return dev_err_probe(dev, ret, "failed to register ccc id: %d\n", >> + pll_hw->id); >> + >> + data->hw_data.hws[pll_hw->id] = &pll_hw->hw; >> + >> + ret = mpfs_ccc_register_outputs(dev, mpfs_ccc_pllout_clks[i], >> + MPFS_CCC_OUTPUTS_PER_PLL, data, pll_hw); >> + if (ret) >> + return ret; >> + } >> + >> + return 0; >> +} >> + >> +static int mpfs_ccc_probe(struct platform_device *pdev) >> +{ >> + struct mpfs_ccc_data *clk_data; >> + void __iomem *pll_base[ARRAY_SIZE(mpfs_ccc_pll_clks)]; >> + unsigned int num_clks; >> + int ret; >> + >> + num_clks = ARRAY_SIZE(mpfs_ccc_pll_clks) + ARRAY_SIZE(mpfs_ccc_pll0out_clks) >> + + ARRAY_SIZE(mpfs_ccc_pll1out_clks); > > Usually, as far as I can tell, there are no (or not too much) lines > starting with arithmetic signs, but these goes at the end of the previous > lines. This allows you to also align the ARRAY_SIZE() macros to each others. > > Other than that: > > Reviewed-by: Claudiu Beznea <claudiu.beznea@xxxxxxxxxxxxx> > I think I just started the line with the operator as I find it easier to read that way around, but not a big deal. Thanks, Conor.
