Re: [PATCH 5/5] ASoC: cs34l45: Hibernation support

Re: [PATCH 5/5] ASoC: cs34l45: Hibernation support

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 239 bytes --]

On Thu, Mar 02, 2023 at 11:11:54AM -0600, Vlad Karpovich wrote:
> From: "Vlad.Karpovich" <vkarpovi@opensource.cirrus.com>
> 
> Adds support for the amplifier hibernation controlled by
> DSP firmware.

What is amplifier hibernation?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 5/5] ASoC: cs34l45: Hibernation support

[Vlad Karpovich]

The CS35L45 features a low-power Hibernation State. In this state, all 
register contents are lost, but the contents of
RAM are retained. In the Hibernation State, only always-on digital 
functions to support wake-up are enabled.
Entry to this state is achieved via the register interface (either by an 
external driver using the control port, or the
programmable DSP). Exit from this state is triggered by activity on 
device GPIO pins, intended SPI transaction, or I2C
transaction with intended slave address

On 3/2/23 11:19, Mark Brown wrote:
> On Thu, Mar 02, 2023 at 11:11:54AM -0600, Vlad Karpovich wrote:
>> From: "Vlad.Karpovich" <vkarpovi@opensource.cirrus.com>
>>
>> Adds support for the amplifier hibernation controlled by
>> DSP firmware.
> What is amplifier hibernation?

Re: [PATCH 5/5] ASoC: cs34l45: Hibernation support

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1203 bytes --]

On Thu, Mar 02, 2023 at 11:59:05AM -0600, Vlad Karpovich wrote:
> The CS35L45 features a low-power Hibernation State. In this state, all
> register contents are lost, but the contents of
> RAM are retained. In the Hibernation State, only always-on digital functions
> to support wake-up are enabled.
> Entry to this state is achieved via the register interface (either by an
> external driver using the control port, or the
> programmable DSP). Exit from this state is triggered by activity on device
> GPIO pins, intended SPI transaction, or I2C
> transaction with intended slave address

OK, so it's essentially just a faster mechanism for bringing the device
out of runtime suspend?  I would suggest doing something in the code to
clarify that this is not the same thing as system level hibernation,
having references to hibernate in the driver is likely to lead to
confusion down the line.  I'd also include a bit more description in the
commit message too.

Please don't top post, reply in line with needed context.  This allows
readers to readily follow the flow of conversation and understand what
you are talking about and also helps ensure that everything in the
discussion is being addressed.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 5/5] ASoC: cs34l45: Hibernation support

[Vlad Karpovich]

On 3/2/23 12:08, Mark Brown wrote:
> On Thu, Mar 02, 2023 at 11:59:05AM -0600, Vlad Karpovich wrote:
>> The CS35L45 features a low-power Hibernation State. In this state, all
>> register contents are lost, but the contents of
>> RAM are retained. In the Hibernation State, only always-on digital functions
>> to support wake-up are enabled.
>> Entry to this state is achieved via the register interface (either by an
>> external driver using the control port, or the
>> programmable DSP). Exit from this state is triggered by activity on device
>> GPIO pins, intended SPI transaction, or I2C
>> transaction with intended slave address
> OK, so it's essentially just a faster mechanism for bringing the device
> out of runtime suspend?

I don't think it is a faster way since it requires interaction with DSP 
and restoring all wiped registers.

But it saves a some power comparing a low power state in the current driver

>   I would suggest doing something in the code to
> clarify that this is not the same thing as system level hibernation,
> having references to hibernate in the driver is likely to lead to
> confusion down the line.

The feature is named hibernation in the data sheet.  Renaming it in the 
driver will add confusing for the driver user.

>   I'd also include a bit more description in the
> commit message too.
I will do in next version.
> Please don't top post, reply in line with needed context.  This allows
> readers to readily follow the flow of conversation and understand what
> you are talking about and also helps ensure that everything in the
> discussion is being addressed.
Thanks. I will do.