Hi Alexey,
On Wed, Apr 5, 2017 at 1:46 PM, Alexey Firago <alexey_firago@xxxxxxxxxx> wrote:
> Introduce vc5_chip_info structure to describe features of a particular
> VC5 chip (id, number of FODs, number of outputs, flags).
> For now flags are only used to indicate if chip has internal XTAL.
> vc5_chip_info is set on probe from the matched of_device_id->data.
>
> Also add defines to specify maximum number of FODs and clock outputs
> supported by the driver.
>
> With these changes it should be easier to extend driver to support
> more VC5 models.
>
> Signed-off-by: Alexey Firago <alexey_firago@xxxxxxxxxx>
Thanks for your patch!
Reviewed-by: Geert Uytterhoeven <geert+renesas@xxxxxxxxx>
> --- a/drivers/clk/clk-versaclock5.c
> +++ b/drivers/clk/clk-versaclock5.c
> @@ -113,12 +113,30 @@
> #define VC5_MUX_IN_XIN BIT(0)
> #define VC5_MUX_IN_CLKIN BIT(1)
>
> +/* Maximum number of clk_out supported by this driver */
> +#define VC5_MAX_CLK_OUT_NUM 3
> +
> +/* Maximum number of FODs supported by this driver */
> +#define VC5_MAX_FOD_NUM 2
> +
> +/* flags to describe chip features */
> +/* chip has built-in oscilator */
> +#define VC5_HAS_INTERNAL_XTAL BIT(0)
VC5_HAS_INTERNAL_OSC?
> +
> /* Supported IDT VC5 models. */
> enum vc5_model {
> IDT_VC5_5P49V5923,
> IDT_VC5_5P49V5933,
> };
>
> +/* Structure to describe features of a particular VC5 model */
> +struct vc5_chip_info {
> + const enum vc5_model model;
> + const int clk_fod_cnt;
> + const int clk_out_cnt;
const unsigned int (both)
> + u32 flags;
> +};
> +
> struct vc5_driver_data;
>
> struct vc5_hw_data {
> @@ -132,15 +150,15 @@ struct vc5_hw_data {
> struct vc5_driver_data {
> struct i2c_client *client;
> struct regmap *regmap;
> - enum vc5_model model;
> + struct vc5_chip_info *chip_info;
const struct vc5_chip_info *chip_info;
> @@ -591,7 +609,7 @@ static int vc5_probe(struct i2c_client *client,
> struct vc5_driver_data *vc5;
> struct clk_init_data init;
> const char *parent_names[2];
> - unsigned int n, idx;
> + unsigned int n, idx = 0;
> int ret;
>
> vc5 = devm_kzalloc(&client->dev, sizeof(*vc5), GFP_KERNEL);
> @@ -600,7 +618,12 @@ static int vc5_probe(struct i2c_client *client,
>
> i2c_set_clientdata(client, vc5);
> vc5->client = client;
> - vc5->model = (enum vc5_model)of_id->data;
> +
> + vc5->chip_info = (struct vc5_chip_info *)of_id->data;
I think the cast is no longer needed when chip_info becomes const.
BTW, of_id is not really needed if you write it like:
vc5->chip_info = of_device_get_match_data(&client->dev);
> + if (!vc5->chip_info) {
This can't really happen, can it?
> + dev_err(&client->dev, "No device match found\n");
> + return -ENODEV;
> + }
>
> vc5->pin_xin = devm_clk_get(&client->dev, "xin");
> if (PTR_ERR(vc5->pin_xin) == -EPROBE_DEFER)
> @@ -622,8 +645,8 @@ static int vc5_probe(struct i2c_client *client,
> if (!IS_ERR(vc5->pin_xin)) {
> vc5->clk_mux_ins |= VC5_MUX_IN_XIN;
> parent_names[init.num_parents++] = __clk_get_name(vc5->pin_xin);
> - } else if (vc5->model == IDT_VC5_5P49V5933) {
> - /* IDT VC5 5P49V5933 has built-in oscilator. */
> + } else if (vc5->chip_info->flags & VC5_HAS_INTERNAL_XTAL) {
> + /* chip has built-in oscilator. */
The comment is no longer needed when the bit is named VC5_HAS_INTERNAL_OSC :-)
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
