diff mbox series

[v8,4/5] dt-bindings: input: pwm-beeper: add volume

Message ID 20230126091825.220646-5-manuel.traut@mt.com
State New
Headers
Series input: pwm-beeper: add feature to set volume level |

Commit Message

Manuel Traut Jan. 26, 2023, 9:18 a.m. UTC 
  Adds an array of supported volume levels and a default volume level.

Signed-off-by: Manuel Traut <manuel.traut@mt.com>
---
 .../devicetree/bindings/input/pwm-beeper.yaml | 20 +++++++++++++++++++
 1 file changed, 20 insertions(+)

Comments

Krzysztof Kozlowski Jan. 26, 2023, 12:36 p.m. UTC | #1 
On 26/01/2023 10:18, Manuel Traut wrote:
> Adds an array of supported volume levels and a default volume level.
> 
> Signed-off-by: Manuel Traut <manuel.traut@mt.com>

This is the second patch. Bindings must be introduced before you start
using them.

> ---
>  .../devicetree/bindings/input/pwm-beeper.yaml | 20 +++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/input/pwm-beeper.yaml b/Documentation/devicetree/bindings/input/pwm-beeper.yaml
> index 351df83d5cbe..f1f9283ca855 100644
> --- a/Documentation/devicetree/bindings/input/pwm-beeper.yaml
> +++ b/Documentation/devicetree/bindings/input/pwm-beeper.yaml
> @@ -26,6 +26,24 @@ properties:
>    beeper-hz:
>      description: bell frequency in Hz
>  
> +  volume-levels:

use -bp suffix:
https://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/property-units.yaml#L44

which will mean the unit is 1/100 of %, not 1/10. Then you can also drop
the $ref.


> +    description: >
> +      Array of PWM duty cycle values that correspond to
> +      linear volume levels. These need to be in the range of
> +      0 to 500, while 0 means 0% duty cycle (mute) and 500
> +      means 50% duty cycle (max volume).
> +      Please note that the actual volume of most beepers is
> +      highly non-linear, which means that low volume levels
> +      are probably somewhere in the range of 1 to 30 (0.1-3%
> +      duty cycle).
> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> +
> +  default-volume-level:

I propose to use just the value, not the index, so the name should
finish with '-bp' and the $ref can be dropped.

> +    description: >
> +      The default volume level (index into the array defined
> +      by the "volume-levels" property).
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +
>  required:
>    - compatible
>    - pwms
> @@ -45,4 +63,6 @@ examples:
>        compatible = "pwm-beeper";
>        pwms = <&pwm0>;
>        amp-supply = <&beeper_amp>;
> +      volume-levels = <0 8 20 40 500>;
> +      default-volume-level = <4>;
>      };

Best regards,
Krzysztof
Manuel Traut Jan. 26, 2023, 3:11 p.m. UTC | #2 
> This is the second patch. Bindings must be introduced before you start
> using them.

OK, will be done in v9.

> > ---
> >  .../devicetree/bindings/input/pwm-beeper.yaml | 20 +++++++++++++++++++
> >  1 file changed, 20 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/input/pwm-beeper.yaml b/Documentation/devicetree/bindings/input/pwm-beeper.yaml
> > index 351df83d5cbe..f1f9283ca855 100644
> > --- a/Documentation/devicetree/bindings/input/pwm-beeper.yaml
> > +++ b/Documentation/devicetree/bindings/input/pwm-beeper.yaml
> > @@ -26,6 +26,24 @@ properties:
> >    beeper-hz:
> >      description: bell frequency in Hz
> >  
> > +  volume-levels:
> 
> use -bp suffix:
> https://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/property-units.yaml#L44
> 
> which will mean the unit is 1/100 of %, not 1/10. Then you can also drop
> the $ref.

OK, this is also fine for me. Chaned the description and implementation
for v9.
 
> > +    description: >
> > +      Array of PWM duty cycle values that correspond to
> > +      linear volume levels. These need to be in the range of
> > +      0 to 500, while 0 means 0% duty cycle (mute) and 500
> > +      means 50% duty cycle (max volume).
> > +      Please note that the actual volume of most beepers is
> > +      highly non-linear, which means that low volume levels
> > +      are probably somewhere in the range of 1 to 30 (0.1-3%
> > +      duty cycle).
> > +    $ref: /schemas/types.yaml#/definitions/uint32-array
> > +
> > +  default-volume-level:
> 
> I propose to use just the value, not the index, so the name should
> finish with '-bp' and the $ref can be dropped.

I am not so happy with this suggestion. What shall be displayed in
sysfs as volume if a user specifies a value that is not defined in
the array.. So I tend to keep this as is.

Thanks for your feedback
Manuel

> > +    description: >
> > +      The default volume level (index into the array defined
> > +      by the "volume-levels" property).
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +
> >  required:
> >    - compatible
> >    - pwms
> > @@ -45,4 +63,6 @@ examples:
> >        compatible = "pwm-beeper";
> >        pwms = <&pwm0>;
> >        amp-supply = <&beeper_amp>;
> > +      volume-levels = <0 8 20 40 500>;
> > +      default-volume-level = <4>;
> >      };
> 
> Best regards,
> Krzysztof
>
Krzysztof Kozlowski Jan. 26, 2023, 3:20 p.m. UTC | #3 
On 26/01/2023 16:11, Manuel Traut wrote:
>> This is the second patch. Bindings must be introduced before you start
>> using them.
> 
> OK, will be done in v9.
> 
>>> ---
>>>  .../devicetree/bindings/input/pwm-beeper.yaml | 20 +++++++++++++++++++
>>>  1 file changed, 20 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/input/pwm-beeper.yaml b/Documentation/devicetree/bindings/input/pwm-beeper.yaml
>>> index 351df83d5cbe..f1f9283ca855 100644
>>> --- a/Documentation/devicetree/bindings/input/pwm-beeper.yaml
>>> +++ b/Documentation/devicetree/bindings/input/pwm-beeper.yaml
>>> @@ -26,6 +26,24 @@ properties:
>>>    beeper-hz:
>>>      description: bell frequency in Hz
>>>  
>>> +  volume-levels:
>>
>> use -bp suffix:
>> https://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/property-units.yaml#L44
>>
>> which will mean the unit is 1/100 of %, not 1/10. Then you can also drop
>> the $ref.
> 
> OK, this is also fine for me. Chaned the description and implementation
> for v9.
>  
>>> +    description: >
>>> +      Array of PWM duty cycle values that correspond to
>>> +      linear volume levels. These need to be in the range of
>>> +      0 to 500, while 0 means 0% duty cycle (mute) and 500
>>> +      means 50% duty cycle (max volume).
>>> +      Please note that the actual volume of most beepers is
>>> +      highly non-linear, which means that low volume levels
>>> +      are probably somewhere in the range of 1 to 30 (0.1-3%
>>> +      duty cycle).
>>> +    $ref: /schemas/types.yaml#/definitions/uint32-array
>>> +
>>> +  default-volume-level:
>>
>> I propose to use just the value, not the index, so the name should
>> finish with '-bp' and the $ref can be dropped.
> 
> I am not so happy with this suggestion. What shall be displayed in
> sysfs as volume if a user specifies a value that is not defined in
> the array.. So I tend to keep this as is.

sysfs is not related to bindings. To rephrase your question: what shall
be shown in anywhere if the index is not correct? The same problem, the
same solution.

Best regards,
Krzysztof
Manuel Traut Jan. 26, 2023, 4:08 p.m. UTC | #4 
> >>> +  default-volume-level:
> >>
> >> I propose to use just the value, not the index, so the name should
> >> finish with '-bp' and the $ref can be dropped.
> > 
> > I am not so happy with this suggestion. What shall be displayed in
> > sysfs as volume if a user specifies a value that is not defined in
> > the array.. So I tend to keep this as is.
> 
> sysfs is not related to bindings. To rephrase your question: what shall
> be shown in anywhere if the index is not correct? The same problem, the
> same solution.

got it, thanks for the hint. I will implement it in v9.

Regards
Manuel
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/input/pwm-beeper.yaml b/Documentation/devicetree/bindings/input/pwm-beeper.yaml
index 351df83d5cbe..f1f9283ca855 100644
--- a/Documentation/devicetree/bindings/input/pwm-beeper.yaml
+++ b/Documentation/devicetree/bindings/input/pwm-beeper.yaml
@@ -26,6 +26,24 @@  properties:
   beeper-hz:
     description: bell frequency in Hz
 
+  volume-levels:
+    description: >
+      Array of PWM duty cycle values that correspond to
+      linear volume levels. These need to be in the range of
+      0 to 500, while 0 means 0% duty cycle (mute) and 500
+      means 50% duty cycle (max volume).
+      Please note that the actual volume of most beepers is
+      highly non-linear, which means that low volume levels
+      are probably somewhere in the range of 1 to 30 (0.1-3%
+      duty cycle).
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+
+  default-volume-level:
+    description: >
+      The default volume level (index into the array defined
+      by the "volume-levels" property).
+    $ref: /schemas/types.yaml#/definitions/uint32
+
 required:
   - compatible
   - pwms
@@ -45,4 +63,6 @@  examples:
       compatible = "pwm-beeper";
       pwms = <&pwm0>;
       amp-supply = <&beeper_amp>;
+      volume-levels = <0 8 20 40 500>;
+      default-volume-level = <4>;
     };