Re: [PATCH v3 3/9] clk: at91: add audio pll clock drivers

[Stephen Boyd]

On 07/24, Quentin Schulz wrote:
> Hi Stephen,
> 
> On 22/07/2017 00:20, Stephen Boyd wrote:
> > On 07/13, Quentin Schulz wrote:
> >> diff --git a/drivers/clk/at91/clk-audio-pll-pad.c 
> >> b/drivers/clk/at91/clk-audio-pll-pad.c
> >> new file mode 100644
> >> index ..10dd6d625696
> >> --- /dev/null
> >> +++ b/drivers/clk/at91/clk-audio-pll-pad.c
> >> +  struct regmap *regmap;
> >> +  const char *parent_name;
> >> +  const char *name = np->name;
> >> +  int ret;
> >> +
> >> +  parent_name = of_clk_get_parent_name(np, 0);
> >> +
> >> +  of_property_read_string(np, "clock-output-names", );
> >> +
> >> +  regmap = syscon_node_to_regmap(of_get_parent(np));
> >> +  if (IS_ERR(regmap))
> >> +  return;
> >> +
> >> +  apad_ck = kzalloc(sizeof(*apad_ck), GFP_KERNEL);
> >> +  if (!apad_ck)
> >> +  return;
> >> +
> >> +  init.name = name;
> >> +  init.ops = _pll_pad_ops;
> >> +  init.parent_names = (parent_name ? _name : NULL);
> > 
> > Use of_clk_parent_fill()?
> > 
> 
> [Deleting `parent_name = of_clk_get_parent_name(np, 0);`]
> [Deleting `init.parent_names = (parent_name ? _name : NULL);`]
> 
> + const char *parent_names[1];
> [...]
> + of_clk_parent_fill(np, parent_names, 1);
> + init.parent_names = parent_names;
> 
> Is it what you're asking?
> 

Yes.

> >> +  init.num_parents = 1;
> >> +  init.flags = CLK_SET_RATE_GATE | CLK_SET_PARENT_GATE |
> >> +  CLK_SET_RATE_PARENT;
> >> +
> >> +  apad_ck->hw.init = 
> >> +  apad_ck->regmap = regmap;
> >> +
> >> +  ret = clk_hw_register(NULL, _ck->hw);
> >> +  if (ret)
> >> +  kfree(apad_ck);
> >> +  else
> >> +  of_clk_add_hw_provider(np, of_clk_hw_simple_get, _ck->hw);
> > 
> > Maybe we should make this register sequence a helper function.
> > Seems common.
> > 
> 
> I can put such an helper in an header if this is what you meant.

No big deal either way.

> [...]
> >> +static long clk_audio_pll_round_rate(struct clk_hw *hw, unsigned long 
> >> rate,
> >> +   unsigned long *parent_rate)
> >> +{
> >> +  long best_rate = -EINVAL;
> >> +  unsigned long fracr, nd;
> >> +  int ret;
> >> +
> >> +  pr_debug("A PLL: %s, rate = %lu (parent_rate = %lu)\n", __func__, rate,
> >> +   *parent_rate);
> >> +
> >> +  if (rate < AUDIO_PLL_FOUT_MIN)
> >> +  rate = AUDIO_PLL_FOUT_MIN;
> >> +  else if (rate > AUDIO_PLL_FOUT_MAX)
> >> +  rate = AUDIO_PLL_FOUT_MAX;
> > 
> > Use clamp. Also, did you want to use determine_rate callback and
> > clamp the requested rate range?
> > 
> 
> Didn't know this one, thanks!
> 
> I want determine_rate to return a valid rate for the pll so I clamp the
> requested rate range in this one. In set_rate, I just tell the user that
> any requested rate outside of the valid range is invalid. Does that
> answer your question?

I meant to use the determine rate op here instead of round_rate.
That way, the min/max ranges can be updated here and the core
should figure out that something went out of range. Of course,
the rounded rate needs to be clamped still, but the ranges could
be expressed back as well.

> 
> [...]
> >> +static void __init of_sama5d2_clk_audio_pll_setup(struct device_node *np)
> >> +{
> >> +  struct clk_audio_frac *fck;
> >> +  struct clk_init_data init;
> >> +  struct regmap *regmap;
> >> +  const char *parent_name;
> >> +  const char *name = np->name;
> >> +  int ret;
> >> +
> >> +  parent_name = of_clk_get_parent_name(np, 0);
> >> +
> >> +  of_property_read_string(np, "clock-output-names", );
> > 
> > Any way to not rely on clock-output-names?
> > 
> 
> I guess we could use the name of the DT node (as it's done in the
> variable initialization block above) and not override it by
> clock-output-names?

If that works, sure.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v3 3/9] clk: at91: add audio pll clock drivers

[Stephen Boyd]

On 07/24, Quentin Schulz wrote:
> Hi Stephen,
> 
> On 22/07/2017 00:20, Stephen Boyd wrote:
> > On 07/13, Quentin Schulz wrote:
> >> diff --git a/drivers/clk/at91/clk-audio-pll-pad.c 
> >> b/drivers/clk/at91/clk-audio-pll-pad.c
> >> new file mode 100644
> >> index ..10dd6d625696
> >> --- /dev/null
> >> +++ b/drivers/clk/at91/clk-audio-pll-pad.c
> >> +  struct regmap *regmap;
> >> +  const char *parent_name;
> >> +  const char *name = np->name;
> >> +  int ret;
> >> +
> >> +  parent_name = of_clk_get_parent_name(np, 0);
> >> +
> >> +  of_property_read_string(np, "clock-output-names", );
> >> +
> >> +  regmap = syscon_node_to_regmap(of_get_parent(np));
> >> +  if (IS_ERR(regmap))
> >> +  return;
> >> +
> >> +  apad_ck = kzalloc(sizeof(*apad_ck), GFP_KERNEL);
> >> +  if (!apad_ck)
> >> +  return;
> >> +
> >> +  init.name = name;
> >> +  init.ops = _pll_pad_ops;
> >> +  init.parent_names = (parent_name ? _name : NULL);
> > 
> > Use of_clk_parent_fill()?
> > 
> 
> [Deleting `parent_name = of_clk_get_parent_name(np, 0);`]
> [Deleting `init.parent_names = (parent_name ? _name : NULL);`]
> 
> + const char *parent_names[1];
> [...]
> + of_clk_parent_fill(np, parent_names, 1);
> + init.parent_names = parent_names;
> 
> Is it what you're asking?
> 

Yes.

> >> +  init.num_parents = 1;
> >> +  init.flags = CLK_SET_RATE_GATE | CLK_SET_PARENT_GATE |
> >> +  CLK_SET_RATE_PARENT;
> >> +
> >> +  apad_ck->hw.init = 
> >> +  apad_ck->regmap = regmap;
> >> +
> >> +  ret = clk_hw_register(NULL, _ck->hw);
> >> +  if (ret)
> >> +  kfree(apad_ck);
> >> +  else
> >> +  of_clk_add_hw_provider(np, of_clk_hw_simple_get, _ck->hw);
> > 
> > Maybe we should make this register sequence a helper function.
> > Seems common.
> > 
> 
> I can put such an helper in an header if this is what you meant.

No big deal either way.

> [...]
> >> +static long clk_audio_pll_round_rate(struct clk_hw *hw, unsigned long 
> >> rate,
> >> +   unsigned long *parent_rate)
> >> +{
> >> +  long best_rate = -EINVAL;
> >> +  unsigned long fracr, nd;
> >> +  int ret;
> >> +
> >> +  pr_debug("A PLL: %s, rate = %lu (parent_rate = %lu)\n", __func__, rate,
> >> +   *parent_rate);
> >> +
> >> +  if (rate < AUDIO_PLL_FOUT_MIN)
> >> +  rate = AUDIO_PLL_FOUT_MIN;
> >> +  else if (rate > AUDIO_PLL_FOUT_MAX)
> >> +  rate = AUDIO_PLL_FOUT_MAX;
> > 
> > Use clamp. Also, did you want to use determine_rate callback and
> > clamp the requested rate range?
> > 
> 
> Didn't know this one, thanks!
> 
> I want determine_rate to return a valid rate for the pll so I clamp the
> requested rate range in this one. In set_rate, I just tell the user that
> any requested rate outside of the valid range is invalid. Does that
> answer your question?

I meant to use the determine rate op here instead of round_rate.
That way, the min/max ranges can be updated here and the core
should figure out that something went out of range. Of course,
the rounded rate needs to be clamped still, but the ranges could
be expressed back as well.

> 
> [...]
> >> +static void __init of_sama5d2_clk_audio_pll_setup(struct device_node *np)
> >> +{
> >> +  struct clk_audio_frac *fck;
> >> +  struct clk_init_data init;
> >> +  struct regmap *regmap;
> >> +  const char *parent_name;
> >> +  const char *name = np->name;
> >> +  int ret;
> >> +
> >> +  parent_name = of_clk_get_parent_name(np, 0);
> >> +
> >> +  of_property_read_string(np, "clock-output-names", );
> > 
> > Any way to not rely on clock-output-names?
> > 
> 
> I guess we could use the name of the DT node (as it's done in the
> variable initialization block above) and not override it by
> clock-output-names?

If that works, sure.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v3 3/9] clk: at91: add audio pll clock drivers

[Stephen Boyd]

On 07/25, Quentin Schulz wrote:
> Hi Stephen,
> 
> I forgot to answer one of your questions:
> 
> On 22/07/2017 00:20, Stephen Boyd wrote:
> > On 07/13, Quentin Schulz wrote:
> >> diff --git a/drivers/clk/at91/clk-audio-pll-pad.c 
> >> b/drivers/clk/at91/clk-audio-pll-pad.c
> >> new file mode 100644
> >> index ..10dd6d625696
> >> --- /dev/null
> >> +++ b/drivers/clk/at91/clk-audio-pll-pad.c
> [...]
> >> +}
> >> +
> >> +CLK_OF_DECLARE(of_sama5d2_clk_audio_pll_pad_setup,
> >> + "atmel,sama5d2-clk-audio-pll-pad",
> >> + of_sama5d2_clk_audio_pll_pad_setup);
> > 
> > We can't have a device driver for this?
> > 
> 
> I tried to work on that but the device does not get probed because the
> parent node (pmc) does not register its children. We would need to
> modify the DT to add such a support and break DT compat.
> 
> As we are speaking about getting rid of clk subnodes (c.f. Rob and
> Alexandre answers on the first version of this patch series:
> https://patchwork.kernel.org/patch/9805991/) in favor of a clock
> controller node that would break the DT compat as well, IMHO it would be
> best to break the DT compat as few times as possible and thus, rework
> this driver once we switch to a clock controller node.
> 

Ok. That's fine. Is there work in progress to make a new binding
here?

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v3 3/9] clk: at91: add audio pll clock drivers

[Stephen Boyd]

On 07/25, Quentin Schulz wrote:
> Hi Stephen,
> 
> I forgot to answer one of your questions:
> 
> On 22/07/2017 00:20, Stephen Boyd wrote:
> > On 07/13, Quentin Schulz wrote:
> >> diff --git a/drivers/clk/at91/clk-audio-pll-pad.c 
> >> b/drivers/clk/at91/clk-audio-pll-pad.c
> >> new file mode 100644
> >> index ..10dd6d625696
> >> --- /dev/null
> >> +++ b/drivers/clk/at91/clk-audio-pll-pad.c
> [...]
> >> +}
> >> +
> >> +CLK_OF_DECLARE(of_sama5d2_clk_audio_pll_pad_setup,
> >> + "atmel,sama5d2-clk-audio-pll-pad",
> >> + of_sama5d2_clk_audio_pll_pad_setup);
> > 
> > We can't have a device driver for this?
> > 
> 
> I tried to work on that but the device does not get probed because the
> parent node (pmc) does not register its children. We would need to
> modify the DT to add such a support and break DT compat.
> 
> As we are speaking about getting rid of clk subnodes (c.f. Rob and
> Alexandre answers on the first version of this patch series:
> https://patchwork.kernel.org/patch/9805991/) in favor of a clock
> controller node that would break the DT compat as well, IMHO it would be
> best to break the DT compat as few times as possible and thus, rework
> this driver once we switch to a clock controller node.
> 

Ok. That's fine. Is there work in progress to make a new binding
here?

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v3 3/9] clk: at91: add audio pll clock drivers

[Quentin Schulz]

Hi Stephen,

I forgot to answer one of your questions:

On 22/07/2017 00:20, Stephen Boyd wrote:
> On 07/13, Quentin Schulz wrote:
>> diff --git a/drivers/clk/at91/clk-audio-pll-pad.c 
>> b/drivers/clk/at91/clk-audio-pll-pad.c
>> new file mode 100644
>> index ..10dd6d625696
>> --- /dev/null
>> +++ b/drivers/clk/at91/clk-audio-pll-pad.c
[...]
>> +}
>> +
>> +CLK_OF_DECLARE(of_sama5d2_clk_audio_pll_pad_setup,
>> +   "atmel,sama5d2-clk-audio-pll-pad",
>> +   of_sama5d2_clk_audio_pll_pad_setup);
> 
> We can't have a device driver for this?
> 

I tried to work on that but the device does not get probed because the
parent node (pmc) does not register its children. We would need to
modify the DT to add such a support and break DT compat.

As we are speaking about getting rid of clk subnodes (c.f. Rob and
Alexandre answers on the first version of this patch series:
https://patchwork.kernel.org/patch/9805991/) in favor of a clock
controller node that would break the DT compat as well, IMHO it would be
best to break the DT compat as few times as possible and thus, rework
this driver once we switch to a clock controller node.

Thanks,
Quentin
-- 
Quentin Schulz, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Re: [PATCH v3 3/9] clk: at91: add audio pll clock drivers

[Quentin Schulz]

Hi Stephen,

I forgot to answer one of your questions:

On 22/07/2017 00:20, Stephen Boyd wrote:
> On 07/13, Quentin Schulz wrote:
>> diff --git a/drivers/clk/at91/clk-audio-pll-pad.c 
>> b/drivers/clk/at91/clk-audio-pll-pad.c
>> new file mode 100644
>> index ..10dd6d625696
>> --- /dev/null
>> +++ b/drivers/clk/at91/clk-audio-pll-pad.c
[...]
>> +}
>> +
>> +CLK_OF_DECLARE(of_sama5d2_clk_audio_pll_pad_setup,
>> +   "atmel,sama5d2-clk-audio-pll-pad",
>> +   of_sama5d2_clk_audio_pll_pad_setup);
> 
> We can't have a device driver for this?
> 

I tried to work on that but the device does not get probed because the
parent node (pmc) does not register its children. We would need to
modify the DT to add such a support and break DT compat.

As we are speaking about getting rid of clk subnodes (c.f. Rob and
Alexandre answers on the first version of this patch series:
https://patchwork.kernel.org/patch/9805991/) in favor of a clock
controller node that would break the DT compat as well, IMHO it would be
best to break the DT compat as few times as possible and thus, rework
this driver once we switch to a clock controller node.

Thanks,
Quentin
-- 
Quentin Schulz, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Re: [PATCH v3 3/9] clk: at91: add audio pll clock drivers

[Quentin Schulz]

Hi Stephen,

On 22/07/2017 00:20, Stephen Boyd wrote:
> On 07/13, Quentin Schulz wrote:
>> diff --git a/drivers/clk/at91/clk-audio-pll-pad.c 
>> b/drivers/clk/at91/clk-audio-pll-pad.c
>> new file mode 100644
>> index ..10dd6d625696
>> --- /dev/null
>> +++ b/drivers/clk/at91/clk-audio-pll-pad.c
>> @@ -0,0 +1,206 @@
>> +/*
>> + *  Copyright (C) 2016 Atmel Corporation,
>> + * Nicolas Ferre 
>> + *  Copyright (C) 2017 Free Electrons,
>> + * Quentin Schulz 
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>> + *
>> + */
>> +#include 
>> +#include 
>> +#include 
> 
> Used?
> 

Not really, I need slab.h for kzalloc tough which was included by clkdev.

[...]
>> +static int clk_audio_pll_pad_set_rate(struct clk_hw *hw, unsigned long rate,
>> +  unsigned long parent_rate)
>> +{
>> +struct clk_audio_pad *apad_ck = to_clk_audio_pad(hw);
>> +u8 tmp_div;
>> +
>> +pr_debug("A PLL/PAD: %s, rate = %lu (parent_rate = %lu)\n", __func__,
>> + rate, parent_rate);
>> +
>> +if (!rate)
>> +return -EINVAL;
> 
> This happens?
> 

I don't know, but it's better to do this quick check rather than being
exposed to a division by zero IMHO. Nothing in clk_ops states that the
rate given to set_rate is non-zero, so I made sure this can't happen.

>> +
>> +tmp_div = parent_rate / rate;
>> +if (tmp_div % 3 == 0) {
>> +apad_ck->qdaudio = tmp_div / 3;
>> +apad_ck->div = 3;
>> +} else {
>> +apad_ck->qdaudio = tmp_div / 2;
>> +apad_ck->div = 2;
>> +}[...]
>> +static void __init of_sama5d2_clk_audio_pll_pad_setup(struct device_node 
>> *np)
>> +{
>> +struct clk_audio_pad *apad_ck;
>> +struct clk_init_data init;
> 
> Best to initialize to { } just in case we add something later.
> 

ACK.

>> +struct regmap *regmap;
>> +const char *parent_name;
>> +const char *name = np->name;
>> +int ret;
>> +
>> +parent_name = of_clk_get_parent_name(np, 0);
>> +
>> +of_property_read_string(np, "clock-output-names", );
>> +
>> +regmap = syscon_node_to_regmap(of_get_parent(np));
>> +if (IS_ERR(regmap))
>> +return;
>> +
>> +apad_ck = kzalloc(sizeof(*apad_ck), GFP_KERNEL);
>> +if (!apad_ck)
>> +return;
>> +
>> +init.name = name;
>> +init.ops = _pll_pad_ops;
>> +init.parent_names = (parent_name ? _name : NULL);
> 
> Use of_clk_parent_fill()?
> 

[Deleting `parent_name = of_clk_get_parent_name(np, 0);`]
[Deleting `init.parent_names = (parent_name ? _name : NULL);`]

+ const char *parent_names[1];
[...]
+ of_clk_parent_fill(np, parent_names, 1);
+ init.parent_names = parent_names;

Is it what you're asking?

>> +init.num_parents = 1;
>> +init.flags = CLK_SET_RATE_GATE | CLK_SET_PARENT_GATE |
>> +CLK_SET_RATE_PARENT;
>> +
>> +apad_ck->hw.init = 
>> +apad_ck->regmap = regmap;
>> +
>> +ret = clk_hw_register(NULL, _ck->hw);
>> +if (ret)
>> +kfree(apad_ck);
>> +else
>> +of_clk_add_hw_provider(np, of_clk_hw_simple_get, _ck->hw);
> 
> Maybe we should make this register sequence a helper function.
> Seems common.
> 

I can put such an helper in an header if this is what you meant.

>> +}
>> +
>> +CLK_OF_DECLARE(of_sama5d2_clk_audio_pll_pad_setup,
>> +   "atmel,sama5d2-clk-audio-pll-pad",
>> +   of_sama5d2_clk_audio_pll_pad_setup);
> 
> We can't have a device driver for this?
> 

Could you elaborate please?

>> diff --git a/drivers/clk/at91/clk-audio-pll-pmc.c 
>> b/drivers/clk/at91/clk-audio-pll-pmc.c
>> new file mode 100644
>> index ..7b0847ed7a4b
>> --- /dev/null
>> +++ b/drivers/clk/at91/clk-audio-pll-pmc.c
[...]
>> +static int clk_audio_pll_pmc_set_rate(struct clk_hw *hw, unsigned long rate,
>> +  unsigned long parent_rate)
>> +{
>> +struct clk_audio_pmc *apmc_ck = to_clk_audio_pmc(hw);
>> +
>> +if (!rate)
>> +return -EINVAL;
>> +
>> +pr_debug("A PLL/PMC: %s, rate = %lu (parent_rate = %lu)\n", __func__,
>> + rate, parent_rate);
>> +
>> +apmc_ck->qdpmc = parent_rate / rate - 1;
> 
> Hopefully rate isn't 1 or that goes undefined.
> 

Thanks to operator precedence, the only check to do is rate != 0 (done
few lines above). Division has precedence over substraction.

[...]
>> +static void __init of_sama5d2_clk_audio_pll_pmc_setup(struct device_node 
>> *np)
>> +{
>> +struct clk_audio_pmc *apmc_ck;
>> +struct clk_init_data init;
>> +struct regmap *regmap;
>> +const char *parent_name;
>> +const char *name = np->name;
>> +int ret;
>> +
>> +

Re: [PATCH v3 3/9] clk: at91: add audio pll clock drivers

[Quentin Schulz]

Hi Stephen,

On 22/07/2017 00:20, Stephen Boyd wrote:
> On 07/13, Quentin Schulz wrote:
>> diff --git a/drivers/clk/at91/clk-audio-pll-pad.c 
>> b/drivers/clk/at91/clk-audio-pll-pad.c
>> new file mode 100644
>> index ..10dd6d625696
>> --- /dev/null
>> +++ b/drivers/clk/at91/clk-audio-pll-pad.c
>> @@ -0,0 +1,206 @@
>> +/*
>> + *  Copyright (C) 2016 Atmel Corporation,
>> + * Nicolas Ferre 
>> + *  Copyright (C) 2017 Free Electrons,
>> + * Quentin Schulz 
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>> + *
>> + */
>> +#include 
>> +#include 
>> +#include 
> 
> Used?
> 

Not really, I need slab.h for kzalloc tough which was included by clkdev.

[...]
>> +static int clk_audio_pll_pad_set_rate(struct clk_hw *hw, unsigned long rate,
>> +  unsigned long parent_rate)
>> +{
>> +struct clk_audio_pad *apad_ck = to_clk_audio_pad(hw);
>> +u8 tmp_div;
>> +
>> +pr_debug("A PLL/PAD: %s, rate = %lu (parent_rate = %lu)\n", __func__,
>> + rate, parent_rate);
>> +
>> +if (!rate)
>> +return -EINVAL;
> 
> This happens?
> 

I don't know, but it's better to do this quick check rather than being
exposed to a division by zero IMHO. Nothing in clk_ops states that the
rate given to set_rate is non-zero, so I made sure this can't happen.

>> +
>> +tmp_div = parent_rate / rate;
>> +if (tmp_div % 3 == 0) {
>> +apad_ck->qdaudio = tmp_div / 3;
>> +apad_ck->div = 3;
>> +} else {
>> +apad_ck->qdaudio = tmp_div / 2;
>> +apad_ck->div = 2;
>> +}[...]
>> +static void __init of_sama5d2_clk_audio_pll_pad_setup(struct device_node 
>> *np)
>> +{
>> +struct clk_audio_pad *apad_ck;
>> +struct clk_init_data init;
> 
> Best to initialize to { } just in case we add something later.
> 

ACK.

>> +struct regmap *regmap;
>> +const char *parent_name;
>> +const char *name = np->name;
>> +int ret;
>> +
>> +parent_name = of_clk_get_parent_name(np, 0);
>> +
>> +of_property_read_string(np, "clock-output-names", );
>> +
>> +regmap = syscon_node_to_regmap(of_get_parent(np));
>> +if (IS_ERR(regmap))
>> +return;
>> +
>> +apad_ck = kzalloc(sizeof(*apad_ck), GFP_KERNEL);
>> +if (!apad_ck)
>> +return;
>> +
>> +init.name = name;
>> +init.ops = _pll_pad_ops;
>> +init.parent_names = (parent_name ? _name : NULL);
> 
> Use of_clk_parent_fill()?
> 

[Deleting `parent_name = of_clk_get_parent_name(np, 0);`]
[Deleting `init.parent_names = (parent_name ? _name : NULL);`]

+ const char *parent_names[1];
[...]
+ of_clk_parent_fill(np, parent_names, 1);
+ init.parent_names = parent_names;

Is it what you're asking?

>> +init.num_parents = 1;
>> +init.flags = CLK_SET_RATE_GATE | CLK_SET_PARENT_GATE |
>> +CLK_SET_RATE_PARENT;
>> +
>> +apad_ck->hw.init = 
>> +apad_ck->regmap = regmap;
>> +
>> +ret = clk_hw_register(NULL, _ck->hw);
>> +if (ret)
>> +kfree(apad_ck);
>> +else
>> +of_clk_add_hw_provider(np, of_clk_hw_simple_get, _ck->hw);
> 
> Maybe we should make this register sequence a helper function.
> Seems common.
> 

I can put such an helper in an header if this is what you meant.

>> +}
>> +
>> +CLK_OF_DECLARE(of_sama5d2_clk_audio_pll_pad_setup,
>> +   "atmel,sama5d2-clk-audio-pll-pad",
>> +   of_sama5d2_clk_audio_pll_pad_setup);
> 
> We can't have a device driver for this?
> 

Could you elaborate please?

>> diff --git a/drivers/clk/at91/clk-audio-pll-pmc.c 
>> b/drivers/clk/at91/clk-audio-pll-pmc.c
>> new file mode 100644
>> index ..7b0847ed7a4b
>> --- /dev/null
>> +++ b/drivers/clk/at91/clk-audio-pll-pmc.c
[...]
>> +static int clk_audio_pll_pmc_set_rate(struct clk_hw *hw, unsigned long rate,
>> +  unsigned long parent_rate)
>> +{
>> +struct clk_audio_pmc *apmc_ck = to_clk_audio_pmc(hw);
>> +
>> +if (!rate)
>> +return -EINVAL;
>> +
>> +pr_debug("A PLL/PMC: %s, rate = %lu (parent_rate = %lu)\n", __func__,
>> + rate, parent_rate);
>> +
>> +apmc_ck->qdpmc = parent_rate / rate - 1;
> 
> Hopefully rate isn't 1 or that goes undefined.
> 

Thanks to operator precedence, the only check to do is rate != 0 (done
few lines above). Division has precedence over substraction.

[...]
>> +static void __init of_sama5d2_clk_audio_pll_pmc_setup(struct device_node 
>> *np)
>> +{
>> +struct clk_audio_pmc *apmc_ck;
>> +struct clk_init_data init;
>> +struct regmap *regmap;
>> +const char *parent_name;
>> +const char *name = np->name;
>> +int ret;
>> +
>> +parent_name = of_clk_get_parent_name(np, 0);
>> +
>> +

Re: [PATCH v3 3/9] clk: at91: add audio pll clock drivers

[Stephen Boyd]

On 07/13, Quentin Schulz wrote:
> diff --git a/drivers/clk/at91/clk-audio-pll-pad.c 
> b/drivers/clk/at91/clk-audio-pll-pad.c
> new file mode 100644
> index ..10dd6d625696
> --- /dev/null
> +++ b/drivers/clk/at91/clk-audio-pll-pad.c
> @@ -0,0 +1,206 @@
> +/*
> + *  Copyright (C) 2016 Atmel Corporation,
> + * Nicolas Ferre 
> + *  Copyright (C) 2017 Free Electrons,
> + *  Quentin Schulz 
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + */
> +#include 
> +#include 
> +#include 

Used?

> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "pmc.h"
> +
> +/*
> + * DOC: PAD output for fractional PLL clock for audio
> + *
> + * Traits of this clock:
> + * enable - clk_enable writes divisors and enables PAD output
> + * rate - rate is adjustable.
> + *clk->rate = parent->rate / (qdaudio * div))
> + * parent - fixed parent.  No clk_set_parent support
> + */
> +
> +#define AUDIO_PLL_QDPAD(qd, div) ((AT91_PMC_AUDIO_PLL_QDPAD_EXTDIV(qd) & 
> \
> +   AT91_PMC_AUDIO_PLL_QDPAD_EXTDIV_MASK) 
> | \
> +  (AT91_PMC_AUDIO_PLL_QDPAD_DIV(div) & \
> +   AT91_PMC_AUDIO_PLL_QDPAD_DIV_MASK))
> +
> +struct clk_audio_pad {
> + struct clk_hw hw;
> + struct regmap *regmap;
> + u8 qdaudio;
> + u8 div;
> +};
> +
> +#define to_clk_audio_pad(hw) container_of(hw, struct clk_audio_pad, hw)
> +
> +static int clk_audio_pll_pad_enable(struct clk_hw *hw)
> +{
> + struct clk_audio_pad *apad_ck = to_clk_audio_pad(hw);
> +
> + regmap_update_bits(apad_ck->regmap, AT91_PMC_AUDIO_PLL1,
> +AT91_PMC_AUDIO_PLL_QDPAD_MASK,
> +AUDIO_PLL_QDPAD(apad_ck->qdaudio, apad_ck->div));
> + regmap_update_bits(apad_ck->regmap, AT91_PMC_AUDIO_PLL0,
> +AT91_PMC_AUDIO_PLL_PADEN, AT91_PMC_AUDIO_PLL_PADEN);
> +
> + return 0;
> +}
> +
> +static void clk_audio_pll_pad_disable(struct clk_hw *hw)
> +{
> + struct clk_audio_pad *apad_ck = to_clk_audio_pad(hw);
> +
> + regmap_update_bits(apad_ck->regmap, AT91_PMC_AUDIO_PLL0,
> +AT91_PMC_AUDIO_PLL_PADEN, 0);
> +}
> +
> +static unsigned long clk_audio_pll_pad_recalc_rate(struct clk_hw *hw,
> +unsigned long parent_rate)
> +{
> + struct clk_audio_pad *apad_ck = to_clk_audio_pad(hw);
> + unsigned long apad_rate = 0;
> +
> + if (apad_ck->qdaudio && apad_ck->div)
> + apad_rate = parent_rate / (apad_ck->qdaudio * apad_ck->div);
> +
> + pr_debug("A PLL/PAD: %s, apad_rate = %lu (div = %u, qdaudio = %u)\n",
> +  __func__, apad_rate, apad_ck->div, apad_ck->qdaudio);
> +
> + return apad_rate;
> +}
> +
> +static long clk_audio_pll_pad_round_rate(struct clk_hw *hw, unsigned long 
> rate,
> +  unsigned long *parent_rate)
> +{
> + struct clk_hw *pclk = clk_hw_get_parent(hw);
> + long best_rate = -EINVAL;
> + unsigned long best_parent_rate;
> + unsigned long tmp_qd;
> + u32 div;
> + long tmp_rate;
> + int tmp_diff;
> + int best_diff = -1;
> +
> + pr_debug("A PLL/PAD: %s, rate = %lu (parent_rate = %lu)\n", __func__,
> +  rate, *parent_rate);
> +
> + /*
> +  * Rate divisor is actually made of two different divisors, multiplied
> +  * between themselves before dividing the rate.
> +  * tmp_qd goes from 1 to 31 and div is either 2 or 3.
> +  * In order to avoid testing twice the rate divisor (e.g. divisor 12 can
> +  * be found with (tmp_qd, div) = (2, 6) or (3, 4)), we remove any loop
> +  * for a rate divisor when div is 2 and tmp_qd is a multiple of 3.
> +  * We cannot inverse it (condition div is 3 and tmp_qd is even) or we
> +  * would miss some rate divisor that aren't reachable with div being 2
> +  * (e.g. rate divisor 90 is made with div = 3 and tmp_qd = 30, thus
> +  * tmp_qd is even so we skip it because we think div 2 could make this
> +  * rate divisor which isn't possible since tmp_qd has to be <= 31).
> +  */
> + for (tmp_qd = 1; tmp_qd < AT91_PMC_AUDIO_PLL_QDPAD_EXTDIV_MAX; tmp_qd++)
> + for (div = 2; div <= 3; div++) {
> + if (div == 2 && tmp_qd % 3 == 0)
> + continue;
> +
> + best_parent_rate = clk_hw_round_rate(pclk,
> + rate * tmp_qd * div);
> + tmp_rate = best_parent_rate / (div * tmp_qd);
> + tmp_diff = abs(rate - tmp_rate);
> +
> +   

Re: [PATCH v3 3/9] clk: at91: add audio pll clock drivers

[Stephen Boyd]

On 07/13, Quentin Schulz wrote:
> diff --git a/drivers/clk/at91/clk-audio-pll-pad.c 
> b/drivers/clk/at91/clk-audio-pll-pad.c
> new file mode 100644
> index ..10dd6d625696
> --- /dev/null
> +++ b/drivers/clk/at91/clk-audio-pll-pad.c
> @@ -0,0 +1,206 @@
> +/*
> + *  Copyright (C) 2016 Atmel Corporation,
> + * Nicolas Ferre 
> + *  Copyright (C) 2017 Free Electrons,
> + *  Quentin Schulz 
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + */
> +#include 
> +#include 
> +#include 

Used?

> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "pmc.h"
> +
> +/*
> + * DOC: PAD output for fractional PLL clock for audio
> + *
> + * Traits of this clock:
> + * enable - clk_enable writes divisors and enables PAD output
> + * rate - rate is adjustable.
> + *clk->rate = parent->rate / (qdaudio * div))
> + * parent - fixed parent.  No clk_set_parent support
> + */
> +
> +#define AUDIO_PLL_QDPAD(qd, div) ((AT91_PMC_AUDIO_PLL_QDPAD_EXTDIV(qd) & 
> \
> +   AT91_PMC_AUDIO_PLL_QDPAD_EXTDIV_MASK) 
> | \
> +  (AT91_PMC_AUDIO_PLL_QDPAD_DIV(div) & \
> +   AT91_PMC_AUDIO_PLL_QDPAD_DIV_MASK))
> +
> +struct clk_audio_pad {
> + struct clk_hw hw;
> + struct regmap *regmap;
> + u8 qdaudio;
> + u8 div;
> +};
> +
> +#define to_clk_audio_pad(hw) container_of(hw, struct clk_audio_pad, hw)
> +
> +static int clk_audio_pll_pad_enable(struct clk_hw *hw)
> +{
> + struct clk_audio_pad *apad_ck = to_clk_audio_pad(hw);
> +
> + regmap_update_bits(apad_ck->regmap, AT91_PMC_AUDIO_PLL1,
> +AT91_PMC_AUDIO_PLL_QDPAD_MASK,
> +AUDIO_PLL_QDPAD(apad_ck->qdaudio, apad_ck->div));
> + regmap_update_bits(apad_ck->regmap, AT91_PMC_AUDIO_PLL0,
> +AT91_PMC_AUDIO_PLL_PADEN, AT91_PMC_AUDIO_PLL_PADEN);
> +
> + return 0;
> +}
> +
> +static void clk_audio_pll_pad_disable(struct clk_hw *hw)
> +{
> + struct clk_audio_pad *apad_ck = to_clk_audio_pad(hw);
> +
> + regmap_update_bits(apad_ck->regmap, AT91_PMC_AUDIO_PLL0,
> +AT91_PMC_AUDIO_PLL_PADEN, 0);
> +}
> +
> +static unsigned long clk_audio_pll_pad_recalc_rate(struct clk_hw *hw,
> +unsigned long parent_rate)
> +{
> + struct clk_audio_pad *apad_ck = to_clk_audio_pad(hw);
> + unsigned long apad_rate = 0;
> +
> + if (apad_ck->qdaudio && apad_ck->div)
> + apad_rate = parent_rate / (apad_ck->qdaudio * apad_ck->div);
> +
> + pr_debug("A PLL/PAD: %s, apad_rate = %lu (div = %u, qdaudio = %u)\n",
> +  __func__, apad_rate, apad_ck->div, apad_ck->qdaudio);
> +
> + return apad_rate;
> +}
> +
> +static long clk_audio_pll_pad_round_rate(struct clk_hw *hw, unsigned long 
> rate,
> +  unsigned long *parent_rate)
> +{
> + struct clk_hw *pclk = clk_hw_get_parent(hw);
> + long best_rate = -EINVAL;
> + unsigned long best_parent_rate;
> + unsigned long tmp_qd;
> + u32 div;
> + long tmp_rate;
> + int tmp_diff;
> + int best_diff = -1;
> +
> + pr_debug("A PLL/PAD: %s, rate = %lu (parent_rate = %lu)\n", __func__,
> +  rate, *parent_rate);
> +
> + /*
> +  * Rate divisor is actually made of two different divisors, multiplied
> +  * between themselves before dividing the rate.
> +  * tmp_qd goes from 1 to 31 and div is either 2 or 3.
> +  * In order to avoid testing twice the rate divisor (e.g. divisor 12 can
> +  * be found with (tmp_qd, div) = (2, 6) or (3, 4)), we remove any loop
> +  * for a rate divisor when div is 2 and tmp_qd is a multiple of 3.
> +  * We cannot inverse it (condition div is 3 and tmp_qd is even) or we
> +  * would miss some rate divisor that aren't reachable with div being 2
> +  * (e.g. rate divisor 90 is made with div = 3 and tmp_qd = 30, thus
> +  * tmp_qd is even so we skip it because we think div 2 could make this
> +  * rate divisor which isn't possible since tmp_qd has to be <= 31).
> +  */
> + for (tmp_qd = 1; tmp_qd < AT91_PMC_AUDIO_PLL_QDPAD_EXTDIV_MAX; tmp_qd++)
> + for (div = 2; div <= 3; div++) {
> + if (div == 2 && tmp_qd % 3 == 0)
> + continue;
> +
> + best_parent_rate = clk_hw_round_rate(pclk,
> + rate * tmp_qd * div);
> + tmp_rate = best_parent_rate / (div * tmp_qd);
> + tmp_diff = abs(rate - tmp_rate);
> +
> + if (best_diff < 0 || best_diff > 

Re: [PATCH v3 3/9] clk: at91: add audio pll clock drivers

[Boris Brezillon]

On Thu, 13 Jul 2017 09:49:21 +0200
Quentin Schulz  wrote:

> This new clock driver set allows to have a fractional divided clock that
> would generate a precise clock particularly suitable for audio
> applications.
> 
> The main audio pll clock has two children clocks: one that is connected
> to the PMC, the other that can directly drive a pad. As these two routes
> have different enable bits and different dividers and divider formulas,
> they are handled by two different drivers. Each of them could modify the
> rate of the main audio pll parent.
> 
> The main audio pll clock can output 620MHz to 700MHz.
> 
> Signed-off-by: Nicolas Ferre 
> Signed-off-by: Quentin Schulz 

Acked-by: Boris Brezillon 

> ---
> 
> v2:
>   - split DT binding in a different patch,
>   - removed unused AUDIO_PLL_*FOUT* defines from clk-audio-pll-pmc,
>   - split classD modifications in a different patch,
> 
>  arch/arm/mach-at91/Kconfig   |   4 +
>  drivers/clk/at91/Makefile|   2 +
>  drivers/clk/at91/clk-audio-pll-pad.c | 206 ++
>  drivers/clk/at91/clk-audio-pll-pmc.c | 174 +
>  drivers/clk/at91/clk-audio-pll.c | 239 
> +++
>  include/linux/clk/at91_pmc.h |  25 
>  6 files changed, 650 insertions(+)
>  create mode 100644 drivers/clk/at91/clk-audio-pll-pad.c
>  create mode 100644 drivers/clk/at91/clk-audio-pll-pmc.c
>  create mode 100644 drivers/clk/at91/clk-audio-pll.c
> 
> diff --git a/arch/arm/mach-at91/Kconfig b/arch/arm/mach-at91/Kconfig
> index d735e5fc4772..9ae14d59a9ce 100644
> --- a/arch/arm/mach-at91/Kconfig
> +++ b/arch/arm/mach-at91/Kconfig
> @@ -26,6 +26,7 @@ config SOC_SAMA5D2
>   select HAVE_AT91_USB_CLK
>   select HAVE_AT91_H32MX
>   select HAVE_AT91_GENERATED_CLK
> + select HAVE_AT91_AUDIO_PLL
>   select PINCTRL_AT91PIO4
>   help
> Select this if ou are using one of Atmel's SAMA5D2 family SoC.
> @@ -125,6 +126,9 @@ config HAVE_AT91_H32MX
>  config HAVE_AT91_GENERATED_CLK
>   bool
>  
> +config HAVE_AT91_AUDIO_PLL
> + bool
> +
>  config SOC_SAM_V4_V5
>   bool
>  
> diff --git a/drivers/clk/at91/Makefile b/drivers/clk/at91/Makefile
> index 13e67bd35cff..c9353d17763a 100644
> --- a/drivers/clk/at91/Makefile
> +++ b/drivers/clk/at91/Makefile
> @@ -6,6 +6,8 @@ obj-y += pmc.o sckc.o
>  obj-y += clk-slow.o clk-main.o clk-pll.o clk-plldiv.o clk-master.o
>  obj-y += clk-system.o clk-peripheral.o clk-programmable.o
>  
> +obj-$(CONFIG_HAVE_AT91_AUDIO_PLL)+= clk-audio-pll.o
> +obj-$(CONFIG_HAVE_AT91_AUDIO_PLL)+= clk-audio-pll-pmc.o 
> clk-audio-pll-pad.o
>  obj-$(CONFIG_HAVE_AT91_UTMI) += clk-utmi.o
>  obj-$(CONFIG_HAVE_AT91_USB_CLK)  += clk-usb.o
>  obj-$(CONFIG_HAVE_AT91_SMD)  += clk-smd.o
> diff --git a/drivers/clk/at91/clk-audio-pll-pad.c 
> b/drivers/clk/at91/clk-audio-pll-pad.c
> new file mode 100644
> index ..10dd6d625696
> --- /dev/null
> +++ b/drivers/clk/at91/clk-audio-pll-pad.c
> @@ -0,0 +1,206 @@
> +/*
> + *  Copyright (C) 2016 Atmel Corporation,
> + * Nicolas Ferre 
> + *  Copyright (C) 2017 Free Electrons,
> + *  Quentin Schulz 
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + */
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "pmc.h"
> +
> +/*
> + * DOC: PAD output for fractional PLL clock for audio
> + *
> + * Traits of this clock:
> + * enable - clk_enable writes divisors and enables PAD output
> + * rate - rate is adjustable.
> + *clk->rate = parent->rate / (qdaudio * div))
> + * parent - fixed parent.  No clk_set_parent support
> + */
> +
> +#define AUDIO_PLL_QDPAD(qd, div) ((AT91_PMC_AUDIO_PLL_QDPAD_EXTDIV(qd) & 
> \
> +   AT91_PMC_AUDIO_PLL_QDPAD_EXTDIV_MASK) 
> | \
> +  (AT91_PMC_AUDIO_PLL_QDPAD_DIV(div) & \
> +   AT91_PMC_AUDIO_PLL_QDPAD_DIV_MASK))
> +
> +struct clk_audio_pad {
> + struct clk_hw hw;
> + struct regmap *regmap;
> + u8 qdaudio;
> + u8 div;
> +};
> +
> +#define to_clk_audio_pad(hw) container_of(hw, struct clk_audio_pad, hw)
> +
> +static int clk_audio_pll_pad_enable(struct clk_hw *hw)
> +{
> + struct clk_audio_pad *apad_ck = to_clk_audio_pad(hw);
> +
> + regmap_update_bits(apad_ck->regmap, AT91_PMC_AUDIO_PLL1,
> +AT91_PMC_AUDIO_PLL_QDPAD_MASK,
> +AUDIO_PLL_QDPAD(apad_ck->qdaudio, 

Re: [PATCH v3 3/9] clk: at91: add audio pll clock drivers

[Boris Brezillon]

On Thu, 13 Jul 2017 09:49:21 +0200
Quentin Schulz  wrote:

> This new clock driver set allows to have a fractional divided clock that
> would generate a precise clock particularly suitable for audio
> applications.
> 
> The main audio pll clock has two children clocks: one that is connected
> to the PMC, the other that can directly drive a pad. As these two routes
> have different enable bits and different dividers and divider formulas,
> they are handled by two different drivers. Each of them could modify the
> rate of the main audio pll parent.
> 
> The main audio pll clock can output 620MHz to 700MHz.
> 
> Signed-off-by: Nicolas Ferre 
> Signed-off-by: Quentin Schulz 

Acked-by: Boris Brezillon 

> ---
> 
> v2:
>   - split DT binding in a different patch,
>   - removed unused AUDIO_PLL_*FOUT* defines from clk-audio-pll-pmc,
>   - split classD modifications in a different patch,
> 
>  arch/arm/mach-at91/Kconfig   |   4 +
>  drivers/clk/at91/Makefile|   2 +
>  drivers/clk/at91/clk-audio-pll-pad.c | 206 ++
>  drivers/clk/at91/clk-audio-pll-pmc.c | 174 +
>  drivers/clk/at91/clk-audio-pll.c | 239 
> +++
>  include/linux/clk/at91_pmc.h |  25 
>  6 files changed, 650 insertions(+)
>  create mode 100644 drivers/clk/at91/clk-audio-pll-pad.c
>  create mode 100644 drivers/clk/at91/clk-audio-pll-pmc.c
>  create mode 100644 drivers/clk/at91/clk-audio-pll.c
> 
> diff --git a/arch/arm/mach-at91/Kconfig b/arch/arm/mach-at91/Kconfig
> index d735e5fc4772..9ae14d59a9ce 100644
> --- a/arch/arm/mach-at91/Kconfig
> +++ b/arch/arm/mach-at91/Kconfig
> @@ -26,6 +26,7 @@ config SOC_SAMA5D2
>   select HAVE_AT91_USB_CLK
>   select HAVE_AT91_H32MX
>   select HAVE_AT91_GENERATED_CLK
> + select HAVE_AT91_AUDIO_PLL
>   select PINCTRL_AT91PIO4
>   help
> Select this if ou are using one of Atmel's SAMA5D2 family SoC.
> @@ -125,6 +126,9 @@ config HAVE_AT91_H32MX
>  config HAVE_AT91_GENERATED_CLK
>   bool
>  
> +config HAVE_AT91_AUDIO_PLL
> + bool
> +
>  config SOC_SAM_V4_V5
>   bool
>  
> diff --git a/drivers/clk/at91/Makefile b/drivers/clk/at91/Makefile
> index 13e67bd35cff..c9353d17763a 100644
> --- a/drivers/clk/at91/Makefile
> +++ b/drivers/clk/at91/Makefile
> @@ -6,6 +6,8 @@ obj-y += pmc.o sckc.o
>  obj-y += clk-slow.o clk-main.o clk-pll.o clk-plldiv.o clk-master.o
>  obj-y += clk-system.o clk-peripheral.o clk-programmable.o
>  
> +obj-$(CONFIG_HAVE_AT91_AUDIO_PLL)+= clk-audio-pll.o
> +obj-$(CONFIG_HAVE_AT91_AUDIO_PLL)+= clk-audio-pll-pmc.o 
> clk-audio-pll-pad.o
>  obj-$(CONFIG_HAVE_AT91_UTMI) += clk-utmi.o
>  obj-$(CONFIG_HAVE_AT91_USB_CLK)  += clk-usb.o
>  obj-$(CONFIG_HAVE_AT91_SMD)  += clk-smd.o
> diff --git a/drivers/clk/at91/clk-audio-pll-pad.c 
> b/drivers/clk/at91/clk-audio-pll-pad.c
> new file mode 100644
> index ..10dd6d625696
> --- /dev/null
> +++ b/drivers/clk/at91/clk-audio-pll-pad.c
> @@ -0,0 +1,206 @@
> +/*
> + *  Copyright (C) 2016 Atmel Corporation,
> + * Nicolas Ferre 
> + *  Copyright (C) 2017 Free Electrons,
> + *  Quentin Schulz 
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + */
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "pmc.h"
> +
> +/*
> + * DOC: PAD output for fractional PLL clock for audio
> + *
> + * Traits of this clock:
> + * enable - clk_enable writes divisors and enables PAD output
> + * rate - rate is adjustable.
> + *clk->rate = parent->rate / (qdaudio * div))
> + * parent - fixed parent.  No clk_set_parent support
> + */
> +
> +#define AUDIO_PLL_QDPAD(qd, div) ((AT91_PMC_AUDIO_PLL_QDPAD_EXTDIV(qd) & 
> \
> +   AT91_PMC_AUDIO_PLL_QDPAD_EXTDIV_MASK) 
> | \
> +  (AT91_PMC_AUDIO_PLL_QDPAD_DIV(div) & \
> +   AT91_PMC_AUDIO_PLL_QDPAD_DIV_MASK))
> +
> +struct clk_audio_pad {
> + struct clk_hw hw;
> + struct regmap *regmap;
> + u8 qdaudio;
> + u8 div;
> +};
> +
> +#define to_clk_audio_pad(hw) container_of(hw, struct clk_audio_pad, hw)
> +
> +static int clk_audio_pll_pad_enable(struct clk_hw *hw)
> +{
> + struct clk_audio_pad *apad_ck = to_clk_audio_pad(hw);
> +
> + regmap_update_bits(apad_ck->regmap, AT91_PMC_AUDIO_PLL1,
> +AT91_PMC_AUDIO_PLL_QDPAD_MASK,
> +AUDIO_PLL_QDPAD(apad_ck->qdaudio, apad_ck->div));
> + regmap_update_bits(apad_ck->regmap, AT91_PMC_AUDIO_PLL0,
> +AT91_PMC_AUDIO_PLL_PADEN, AT91_PMC_AUDIO_PLL_PADEN);
> +
> + return 0;
> +}
> +
>