2015-04-03 19:33:19

by Scott Branden

[permalink] [raw]
Subject: Re: [PATCH 0/2] Cygnus Audio Driver

Hi Mark,


On 15-03-30 11:43 PM, Mark Brown wrote:
> On Mon, Mar 30, 2015 at 08:16:22PM -0700, Scott Branden wrote:
>
>> The audio PLL is embedded in the audio block and only used
>> by the audio block. The audio PLL registers are also in the middle of
>> the audio register map.
>
> When you say it's only used by the audio block do you mean to say that
> the audio block exposes no clock signals other than the bit and frame
> clocks?
>
The audio block exposes the MCLK in addition to the bit and frame clock.

Regards,
Scott


2015-04-06 09:58:34

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH 0/2] Cygnus Audio Driver

On Fri, Apr 03, 2015 at 12:33:12PM -0700, Scott Branden wrote:
> On 15-03-30 11:43 PM, Mark Brown wrote:
> >On Mon, Mar 30, 2015 at 08:16:22PM -0700, Scott Branden wrote:

> >>The audio PLL is embedded in the audio block and only used
> >>by the audio block. The audio PLL registers are also in the middle of
> >>the audio register map.

> >When you say it's only used by the audio block do you mean to say that
> >the audio block exposes no clock signals other than the bit and frame
> >clocks?

> The audio block exposes the MCLK in addition to the bit and frame clock.

OK, then it's going to need to be a clock provider at some point - the
clock will be going into external devices which are going to need to be
able to interact with the clock (for example, to get the rate).


Attachments:
(No filename) (780.00 B)
signature.asc (473.00 B)
Digital signature
Download all attachments

2015-04-08 02:28:23

by Lori Hikichi

[permalink] [raw]
Subject: Re: [PATCH 0/2] Cygnus Audio Driver



On 15-04-06 02:58 AM, Mark Brown wrote:
> On Fri, Apr 03, 2015 at 12:33:12PM -0700, Scott Branden wrote:
>> On 15-03-30 11:43 PM, Mark Brown wrote:
>>> On Mon, Mar 30, 2015 at 08:16:22PM -0700, Scott Branden wrote:
>
>>>> The audio PLL is embedded in the audio block and only used
>>>> by the audio block. The audio PLL registers are also in the middle of
>>>> the audio register map.
>
>>> When you say it's only used by the audio block do you mean to say that
>>> the audio block exposes no clock signals other than the bit and frame
>>> clocks?
>
>> The audio block exposes the MCLK in addition to the bit and frame clock.
>
> OK, then it's going to need to be a clock provider at some point - the
> clock will be going into external devices which are going to need to be
> able to interact with the clock (for example, to get the rate).
>
Currently, the ASoC machine driver is responsible for requesting a
certain frequency of MCLK be generated from our driver and then also
sending the frequency information along to the external device (codec).
This is done via the snd_soc_dai_set_sysclk. That is the only clock
interaction we have needed for the core part of the driver. For
enhanced features, we also have the need to make minor adjustments
(tweaks) to the PLL. The tweaks are used to make the PLLs output
frequency match as closely as possible to a true reference frequency.
As such, we would like to provide the finest adjustment resolution as
possible. The clocking framework only seems to allow for a 1 Hz
adjustment. This limitation and the fact that no other device seems to
need to interact directly will the PLL are why we have not put it in the
clocking framework.

2015-04-08 18:54:49

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH 0/2] Cygnus Audio Driver

On Tue, Apr 07, 2015 at 07:28:40PM -0700, Lori Hikichi wrote:
> On 15-04-06 02:58 AM, Mark Brown wrote:

> >OK, then it's going to need to be a clock provider at some point - the
> >clock will be going into external devices which are going to need to be
> >able to interact with the clock (for example, to get the rate).

> Currently, the ASoC machine driver is responsible for requesting a certain
> frequency of MCLK be generated from our driver and then also sending the
> frequency information along to the external device (codec).
> This is done via the snd_soc_dai_set_sysclk. That is the only clock
> interaction we have needed for the core part of the driver. For enhanced

I have some passing familiarity with ASoC... if you look at newer
drivers, especially those for DT systems, you'll see that we're
transitioning CODEC drivers to use the clock API for their clocks since
this makes integrating with both generic ASoC things like simple card
and non-ASoC clocks.

> features, we also have the need to make minor adjustments (tweaks) to the
> PLL. The tweaks are used to make the PLLs output frequency match as closely
> as possible to a true reference frequency. As such, we would like to provide
> the finest adjustment resolution as possible. The clocking framework only
> seems to allow for a 1 Hz adjustment. This limitation and the fact that no
> other device seems to need to interact directly will the PLL are why we have
> not put it in the clocking framework.

That's going to be an issue no matter where you put the control - the
ASoC specific clocking APIs don't have any control here either. I don't
know if we want to add the functionality for doing very fine grained
adjustments into the clock API or not (the use cases seem limited though
I'm sure they exist), though I do think we should have that discussion
if only to confirm, but that's a separate thing to how we expose any
userspace control - the clock API is a kernel internal thing.


Attachments:
(No filename) (1.93 kB)
signature.asc (473.00 B)
Digital signature
Download all attachments

2015-04-11 03:06:31

by Lori Hikichi

[permalink] [raw]
Subject: Re: [PATCH 0/2] Cygnus Audio Driver



On 15-04-08 11:54 AM, Mark Brown wrote:
> On Tue, Apr 07, 2015 at 07:28:40PM -0700, Lori Hikichi wrote:
>> On 15-04-06 02:58 AM, Mark Brown wrote:
>
>>> OK, then it's going to need to be a clock provider at some point - the
>>> clock will be going into external devices which are going to need to be
>>> able to interact with the clock (for example, to get the rate).
>
>> Currently, the ASoC machine driver is responsible for requesting a certain
>> frequency of MCLK be generated from our driver and then also sending the
>> frequency information along to the external device (codec).
>> This is done via the snd_soc_dai_set_sysclk. That is the only clock
>> interaction we have needed for the core part of the driver. For enhanced
>
> I have some passing familiarity with ASoC... if you look at newer
> drivers, especially those for DT systems, you'll see that we're
> transitioning CODEC drivers to use the clock API for their clocks since
> this makes integrating with both generic ASoC things like simple card
> and non-ASoC clocks.
>
>> features, we also have the need to make minor adjustments (tweaks) to the
>> PLL. The tweaks are used to make the PLLs output frequency match as closely
>> as possible to a true reference frequency. As such, we would like to provide
>> the finest adjustment resolution as possible. The clocking framework only
>> seems to allow for a 1 Hz adjustment. This limitation and the fact that no
>> other device seems to need to interact directly will the PLL are why we have
>> not put it in the clocking framework.
>
> That's going to be an issue no matter where you put the control - the
> ASoC specific clocking APIs don't have any control here either. I don't
> know if we want to add the functionality for doing very fine grained
> adjustments into the clock API or not (the use cases seem limited though
> I'm sure they exist), though I do think we should have that discussion
> if only to confirm, but that's a separate thing to how we expose any
> userspace control - the clock API is a kernel internal thing.
>
Seems like there are some benefits to integrating with the clocking framework.
I will have to consider what kernel APIs we want to expose as well. I believe
we may have another kernel driver wanting to control this tweaking. Do you feel
it is ok to have to PLL code reside in this driver for now, and then we can patch
later after we get this clocking control sorted out.