Quoting Adam Ford (2020-04-14 12:36:15)
> @@ -865,6 +904,77 @@ static int vc5_probe(struct i2c_client *client,
> init.name);
> goto err_clk;
> }
> +
> + /* Fetch Clock Output configuration from DT (if specified) */
> + child_name = kasprintf(GFP_KERNEL, "OUT%d", n);
> + np_output = of_get_child_by_name(client->dev.of_node, child_name);
> + kfree(child_name);
> + if (!np_output)
> + continue;
> + if (!(ret || of_property_read_u32(np_output,
> + "idt,mode", &value))) {
> + vc5->clk_out[n].clk_output_cfg0_mask |= VC5_CLK_OUTPUT_CFG0_CFG_MASK;
> + switch (value) {
> + case VC5_CLK_OUTPUT_CFG0_CFG_LVPECL:
> + case VC5_CLK_OUTPUT_CFG0_CFG_CMOS:
> + case VC5_CLK_OUTPUT_CFG0_CFG_HCSL33:
> + case VC5_CLK_OUTPUT_CFG0_CFG_LVDS:
> + case VC5_CLK_OUTPUT_CFG0_CFG_CMOS2:
> + case VC5_CLK_OUTPUT_CFG0_CFG_CMOSD:
> + case VC5_CLK_OUTPUT_CFG0_CFG_HCSL25:
> + vc5->clk_out[n].clk_output_cfg0 |= value << VC5_CLK_OUTPUT_CFG0_CFG_SHIFT;
> + break;
> + default:
> + ret = -EINVAL;
> + break;
> + }
> + }
Can these three things be functions that are called and passed a
vc5->clk_out[n] pointer? Then the code would be something like
ret = prop1_parse_and_update(vc5->clk_out[n]);
if (ret)
goto err_clk;
ret = prop2_parse_and_update(...)
if (ret)
goto err_clk;
> + if (!(ret || of_property_read_u32(np_output,
> + "idt,voltage-microvolts", &value))) {
> + vc5->clk_out[n].clk_output_cfg0_mask |= VC5_CLK_OUTPUT_CFG0_PWR_MASK;
> diff --git a/include/dt-bindings/clk/versaclock.h b/include/dt-bindings/clk/versaclock.h
> new file mode 100644
> index 000000000000..30add3488713
> --- /dev/null
> +++ b/include/dt-bindings/clk/versaclock.h
> @@ -0,0 +1,13 @@
> +/* HEADER */
Any SPDX license for this in place of HEADER?
> +
> +/* This file defines field values used by the versaclock 6 family
> + * for defining output type
> + */
> +
> +#define VC5_LVPECL 0
> +#define VC5_CMOS 1
> +#define VC5_HCSL33 2
> +#define VC5_LVDS 3
> +#define VC5_CMOS2 4
> +#define VC5_CMOSD 5
> +#define VC5_HCSL25 6
