2023-02-07 10:41:15

by Lucas Tanure

[permalink] [raw]
Subject: [PATCH 2/2] Documentation: cs35l41: Shared boost properties

Describe the properties used for shared boost
configuration.

Signed-off-by: Lucas Tanure <[email protected]>
---
.../devicetree/bindings/sound/cirrus,cs35l41.yaml | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/sound/cirrus,cs35l41.yaml b/Documentation/devicetree/bindings/sound/cirrus,cs35l41.yaml
index 18fb471aa891..6f5f01bec6f1 100644
--- a/Documentation/devicetree/bindings/sound/cirrus,cs35l41.yaml
+++ b/Documentation/devicetree/bindings/sound/cirrus,cs35l41.yaml
@@ -85,11 +85,20 @@ properties:
boost-cap-microfarad.
External Boost must have GPIO1 as GPIO output. GPIO1 will be set high to
enable boost voltage.
+ Shared boost allows two amplifiers to share a single boost circuit by
+ communicating on the MDSYNC bus. The passive amplifier does not control
+ the boost and receives data from the active amplifier. GPIO1 should be
+ configured for Sync when shared boost is used. Shared boost is not
+ compatible with External boost. Active amplifier requires
+ boost-peak-milliamp, boost-ind-nanohenry and boost-cap-microfarad.
0 = Internal Boost
1 = External Boost
+ 2 = Reserved
+ 3 = Shared Boost Active
+ 4 = Shared Boost Passive
$ref: /schemas/types.yaml#/definitions/uint32
minimum: 0
- maximum: 1
+ maximum: 4

cirrus,gpio1-polarity-invert:
description:
--
2.39.1



2023-02-07 10:43:03

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 2/2] Documentation: cs35l41: Shared boost properties

On 07/02/2023 11:40, Lucas Tanure wrote:
> Describe the properties used for shared boost
> configuration.

Use subject prefixes matching the subsystem (which you can get for
example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
your patch is touching).

>
> Signed-off-by: Lucas Tanure <[email protected]>
> ---
> .../devicetree/bindings/sound/cirrus,cs35l41.yaml | 11 ++++++++++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/sound/cirrus,cs35l41.yaml b/Documentation/devicetree/bindings/sound/cirrus,cs35l41.yaml
> index 18fb471aa891..6f5f01bec6f1 100644
> --- a/Documentation/devicetree/bindings/sound/cirrus,cs35l41.yaml
> +++ b/Documentation/devicetree/bindings/sound/cirrus,cs35l41.yaml
> @@ -85,11 +85,20 @@ properties:
> boost-cap-microfarad.
> External Boost must have GPIO1 as GPIO output. GPIO1 will be set high to
> enable boost voltage.
> + Shared boost allows two amplifiers to share a single boost circuit by
> + communicating on the MDSYNC bus. The passive amplifier does not control
> + the boost and receives data from the active amplifier. GPIO1 should be
> + configured for Sync when shared boost is used. Shared boost is not
> + compatible with External boost. Active amplifier requires
> + boost-peak-milliamp, boost-ind-nanohenry and boost-cap-microfarad.
> 0 = Internal Boost
> 1 = External Boost
> + 2 = Reserved

How binding can be reserved? For what and why? Drop. 2 is shared active,
3 is shared passive.

Best regards,
Krzysztof


2023-02-07 15:46:15

by Lucas Tanure

[permalink] [raw]
Subject: Re: [PATCH 2/2] Documentation: cs35l41: Shared boost properties

On 07-02-2023 10:42, Krzysztof Kozlowski wrote:
> On 07/02/2023 11:40, Lucas Tanure wrote:
>> Describe the properties used for shared boost
>> configuration.
>
> Use subject prefixes matching the subsystem (which you can get for
> example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
> your patch is touching).
ack
>
>>
>> Signed-off-by: Lucas Tanure <[email protected]>
>> ---
>> .../devicetree/bindings/sound/cirrus,cs35l41.yaml | 11 ++++++++++-
>> 1 file changed, 10 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/sound/cirrus,cs35l41.yaml b/Documentation/devicetree/bindings/sound/cirrus,cs35l41.yaml
>> index 18fb471aa891..6f5f01bec6f1 100644
>> --- a/Documentation/devicetree/bindings/sound/cirrus,cs35l41.yaml
>> +++ b/Documentation/devicetree/bindings/sound/cirrus,cs35l41.yaml
>> @@ -85,11 +85,20 @@ properties:
>> boost-cap-microfarad.
>> External Boost must have GPIO1 as GPIO output. GPIO1 will be set high to
>> enable boost voltage.
>> + Shared boost allows two amplifiers to share a single boost circuit by
>> + communicating on the MDSYNC bus. The passive amplifier does not control
>> + the boost and receives data from the active amplifier. GPIO1 should be
>> + configured for Sync when shared boost is used. Shared boost is not
>> + compatible with External boost. Active amplifier requires
>> + boost-peak-milliamp, boost-ind-nanohenry and boost-cap-microfarad.
>> 0 = Internal Boost
>> 1 = External Boost
>> + 2 = Reserved
>
> How binding can be reserved? For what and why? Drop. 2 is shared active,
> 3 is shared passive.
2 Is shared boost without VSPK switch, a mode not supported for new
system designs. But there is laptops using it, so we need to keep
supporting in the driver.

>
> Best regards,
> Krzysztof
>


2023-02-07 16:14:07

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 2/2] Documentation: cs35l41: Shared boost properties

On 07/02/2023 16:46, Lucas Tanure wrote:
>>> + Shared boost allows two amplifiers to share a single boost circuit by
>>> + communicating on the MDSYNC bus. The passive amplifier does not control
>>> + the boost and receives data from the active amplifier. GPIO1 should be
>>> + configured for Sync when shared boost is used. Shared boost is not
>>> + compatible with External boost. Active amplifier requires
>>> + boost-peak-milliamp, boost-ind-nanohenry and boost-cap-microfarad.
>>> 0 = Internal Boost
>>> 1 = External Boost
>>> + 2 = Reserved
>>
>> How binding can be reserved? For what and why? Drop. 2 is shared active,
>> 3 is shared passive.
> 2 Is shared boost without VSPK switch, a mode not supported for new
> system designs. But there is laptops using it, so we need to keep
> supporting in the driver.

That's not the answer. 2 is nothing here, so it cannot be reserved.
Aren't you mixing now some register value with bindings?

Best regards,
Krzysztof


2023-02-07 16:34:49

by Lucas Tanure

[permalink] [raw]
Subject: Re: [PATCH 2/2] Documentation: cs35l41: Shared boost properties

On 07-02-2023 16:13, Krzysztof Kozlowski wrote:
> On 07/02/2023 16:46, Lucas Tanure wrote:
>>>> + Shared boost allows two amplifiers to share a single boost circuit by
>>>> + communicating on the MDSYNC bus. The passive amplifier does not control
>>>> + the boost and receives data from the active amplifier. GPIO1 should be
>>>> + configured for Sync when shared boost is used. Shared boost is not
>>>> + compatible with External boost. Active amplifier requires
>>>> + boost-peak-milliamp, boost-ind-nanohenry and boost-cap-microfarad.
>>>> 0 = Internal Boost
>>>> 1 = External Boost
>>>> + 2 = Reserved
>>>
>>> How binding can be reserved? For what and why? Drop. 2 is shared active,
>>> 3 is shared passive.
>> 2 Is shared boost without VSPK switch, a mode not supported for new
>> system designs. But there is laptops using it, so we need to keep
>> supporting in the driver.
>
> That's not the answer. 2 is nothing here, so it cannot be reserved.
> Aren't you mixing now some register value with bindings?
>
> Best regards,
> Krzysztof
>
>
I have added a new patch with propper documentation.
And I would like to use 3 and 4 for shared boost as
CS35L41_EXT_BOOST_NO_VSPK_SWITCH already exist as 2 and is used in the
current driver.
The laptop that uses CS35L41_EXT_BOOST_NO_VSPK_SWITCH doesn't have the
property "cirrus,boost-type", but to make everything consistent I would
prefer to use 3 and 4 for the new boost types.
Is that ok with you?

Thanks
Lucas

2023-02-07 16:49:08

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 2/2] Documentation: cs35l41: Shared boost properties

On 07/02/2023 17:34, Lucas Tanure wrote:
> On 07-02-2023 16:13, Krzysztof Kozlowski wrote:
>> On 07/02/2023 16:46, Lucas Tanure wrote:
>>>>> + Shared boost allows two amplifiers to share a single boost circuit by
>>>>> + communicating on the MDSYNC bus. The passive amplifier does not control
>>>>> + the boost and receives data from the active amplifier. GPIO1 should be
>>>>> + configured for Sync when shared boost is used. Shared boost is not
>>>>> + compatible with External boost. Active amplifier requires
>>>>> + boost-peak-milliamp, boost-ind-nanohenry and boost-cap-microfarad.
>>>>> 0 = Internal Boost
>>>>> 1 = External Boost
>>>>> + 2 = Reserved
>>>>
>>>> How binding can be reserved? For what and why? Drop. 2 is shared active,
>>>> 3 is shared passive.
>>> 2 Is shared boost without VSPK switch, a mode not supported for new
>>> system designs. But there is laptops using it, so we need to keep
>>> supporting in the driver.
>>
>> That's not the answer. 2 is nothing here, so it cannot be reserved.
>> Aren't you mixing now some register value with bindings?
>>
>> Best regards,
>> Krzysztof
>>
>>
> I have added a new patch with propper documentation.
> And I would like to use 3 and 4 for shared boost as
> CS35L41_EXT_BOOST_NO_VSPK_SWITCH already exist as 2 and is used in the
> current driver.

I don't see CS35L41_EXT_BOOST_NO_VSPK_SWITCH in the bindings.

> The laptop that uses CS35L41_EXT_BOOST_NO_VSPK_SWITCH doesn't have the
> property "cirrus,boost-type", but to make everything consistent I would
> prefer to use 3 and 4 for the new boost types.
> Is that ok with you?

I don't see how it is related. The value does not exist, so whether
laptop has that property or not, is not really related, right?

Best regards,
Krzysztof


2023-02-07 17:03:39

by Lucas Tanure

[permalink] [raw]
Subject: Re: [PATCH 2/2] Documentation: cs35l41: Shared boost properties

On 2/7/23 4:48 PM, Krzysztof Kozlowski <[email protected]> wrote:
> On 07/02/2023 17:34, Lucas Tanure wrote:
> > On 07-02-2023 16:13, Krzysztof Kozlowski wrote:
> >> On 07/02/2023 16:46, Lucas Tanure wrote:
> >>>>> + Shared boost allows two amplifiers to share a single boost circuit by
> >>>>> + communicating on the MDSYNC bus. The passive amplifier does not control
> >>>>> + the boost and receives data from the active amplifier. GPIO1 should be
> >>>>> + configured for Sync when shared boost is used. Shared boost is not
> >>>>> + compatible with External boost. Active amplifier requires
> >>>>> + boost-peak-milliamp, boost-ind-nanohenry and boost-cap-microfarad.
> >>>>> 0 = Internal Boost
> >>>>> 1 = External Boost
> >>>>> + 2 = Reserved
> >>>>
> >>>> How binding can be reserved? For what and why? Drop. 2 is shared active,
> >>>> 3 is shared passive.
> >>> 2 Is shared boost without VSPK switch, a mode not supported for new
> >>> system designs. But there is laptops using it, so we need to keep
> >>> supporting in the driver.
> >>
> >> That's not the answer. 2 is nothing here, so it cannot be reserved.
> >> Aren't you mixing now some register value with bindings?
> >>
> >> Best regards,
> >> Krzysztof
> >>
> >>
> > I have added a new patch with propper documentation.
> > And I would like to use 3 and 4 for shared boost as
> > CS35L41_EXT_BOOST_NO_VSPK_SWITCH already exist as 2 and is used in the
> > current driver.
>
> I don't see CS35L41_EXT_BOOST_NO_VSPK_SWITCH in the bindings.
>
> > The laptop that uses CS35L41_EXT_BOOST_NO_VSPK_SWITCH doesn't have the
> > property "cirrus,boost-type", but to make everything consistent I would
> > prefer to use 3 and 4 for the new boost types.
> > Is that ok with you?
>
> I don't see how it is related. The value does not exist, so whether
> laptop has that property or not, is not really related, right?
>
> Best regards,
> Krzysztof
>
>
The value does exist in the code, but no device should have that in ACPI/DTB, so yes the value doesn't exist for ACPI/DTB purposes.
I can change CS35L41_EXT_BOOST_NO_VSPK_SWITCH to another value, like 99, and use 2 and 3 for shared boost.
I will re-submit that with v3.
Is that ok with you?

Thanks
Lucas


2023-02-08 10:24:04

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 2/2] Documentation: cs35l41: Shared boost properties

On 07/02/2023 18:03, [email protected] wrote:
> On 2/7/23 4:48 PM, Krzysztof Kozlowski <[email protected]> wrote:
>> On 07/02/2023 17:34, Lucas Tanure wrote:
>>> On 07-02-2023 16:13, Krzysztof Kozlowski wrote:
>>>> On 07/02/2023 16:46, Lucas Tanure wrote:
>>>>>>> + Shared boost allows two amplifiers to share a single boost circuit by
>>>>>>> + communicating on the MDSYNC bus. The passive amplifier does not control
>>>>>>> + the boost and receives data from the active amplifier. GPIO1 should be
>>>>>>> + configured for Sync when shared boost is used. Shared boost is not
>>>>>>> + compatible with External boost. Active amplifier requires
>>>>>>> + boost-peak-milliamp, boost-ind-nanohenry and boost-cap-microfarad.
>>>>>>> 0 = Internal Boost
>>>>>>> 1 = External Boost
>>>>>>> + 2 = Reserved
>>>>>>
>>>>>> How binding can be reserved? For what and why? Drop. 2 is shared active,
>>>>>> 3 is shared passive.
>>>>> 2 Is shared boost without VSPK switch, a mode not supported for new
>>>>> system designs. But there is laptops using it, so we need to keep
>>>>> supporting in the driver.
>>>>
>>>> That's not the answer. 2 is nothing here, so it cannot be reserved.
>>>> Aren't you mixing now some register value with bindings?
>>>>
>>>> Best regards,
>>>> Krzysztof
>>>>
>>>>
>>> I have added a new patch with propper documentation.
>>> And I would like to use 3 and 4 for shared boost as
>>> CS35L41_EXT_BOOST_NO_VSPK_SWITCH already exist as 2 and is used in the
>>> current driver.
>>
>> I don't see CS35L41_EXT_BOOST_NO_VSPK_SWITCH in the bindings.
>>
>>> The laptop that uses CS35L41_EXT_BOOST_NO_VSPK_SWITCH doesn't have the
>>> property "cirrus,boost-type", but to make everything consistent I would
>>> prefer to use 3 and 4 for the new boost types.
>>> Is that ok with you?
>>
>> I don't see how it is related. The value does not exist, so whether
>> laptop has that property or not, is not really related, right?
>>
>> Best regards,
>> Krzysztof
>>
>>
> The value does exist in the code, but no device should have that in ACPI/DTB, so yes the value doesn't exist for ACPI/DTB purposes.
> I can change CS35L41_EXT_BOOST_NO_VSPK_SWITCH to another value, like 99, and use 2 and 3 for shared boost.
> I will re-submit that with v3.
> Is that ok with you?

I guess we still talk about different things. The code does not have a
binding for the boost, therefore it does not use boost binding. Whatever
it does with CS35L41_EXT_BOOST_NO_VSPK_SWITCH outside of DT, is not my
topic and I don't care.

That's why I asked folks to use strings for such enumerations, not
register values - to avoid any confusion between the code and bindings
(and also make it more readable for humans).

Best regards,
Krzysztof