Re: [Patch v6] driver/clk/clk-si5338: Add common clock framework driver for si5338

[Michael Turquette <mturquette@baylibre.com>]

Hello York Sun,

Quoting York Sun (2015-10-07 10:59:10)
> +static const struct clk_ops si5338_xtal_ops =3D {
> +       .prepare =3D si5338_xtal_prepare,

There are many instances of .prepare where there is no matching
.unprepare. Why is that? And what are these .prepare callbacks doing?
Are they changing rates or just enabling the clock signals?

> +static const struct clk_ops si5338_clkout_ops =3D {
> +       .prepare =3D si5338_clkout_prepare,
> +       .unprepare =3D si5338_clkout_unprepare,
> +       .enable =3D si5338_clkout_enable,
> +       .disable =3D si5338_clkout_disable,

This is an i2c device, so I'm confused how an .enable or .disable
callback is appropriate.

> +       /*
> +        * To form clock names, concatentate name prefix with each name.
> +        * The result string is up to MAX_NAME_LENGTH including terminati=
on.
> +        */
> +
> +       /* Register xtal input clock */
> +       if (!IS_ERR_OR_NULL(drvdata->pxtal)) {
> +               strlcpy(register_name, drvdata->name_prefix, MAX_NAME_PRE=
FIX);
> +               strncat(register_name, si5338_input_names[4], STRNCAT_LEN=
GTH);
> +               drvdata->pxtal_name =3D __clk_get_name(drvdata->pxtal);

I don't really understand what's going on here. Why is this necessary?
Are you not able to get this info from DT?

Regards,
Mike