Re: [PATCH 2/3] clk: vc5: Enable addition output configurations of the Versaclock

[Stephen Boyd]

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