Re: [PATCH] Fixed the schema binding according to test

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Thank you for your patch. There is something to discuss/improve.

On 02/02/2023 10:07, Kiseok Jo wrote:
> Modified according to the writing-schema.rst file and tested.

Use imperative, not past tense (Fixed->Fix, Modified->Modify).

> 
> Signed-off-by: Kiseok Jo <kiseok.jo@xxxxxxxxxxxxxx>

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). Therefore it should be:
"ASoC: dt-bindings: irondevice,sma1303: Rework binding and add missing
properties"



> ---
>  .../bindings/sound/irondevice,sma1303.yaml    | 46 +++++++++++++++++--
>  1 file changed, 43 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/sound/irondevice,sma1303.yaml b/Documentation/devicetree/bindings/sound/irondevice,sma1303.yaml
> index 162c52606635..35d9a046ef75 100644
> --- a/Documentation/devicetree/bindings/sound/irondevice,sma1303.yaml
> +++ b/Documentation/devicetree/bindings/sound/irondevice,sma1303.yaml
> @@ -10,22 +10,62 @@ maintainers:
>    - Kiseok Jo <kiseok.jo@xxxxxxxxxxxxxx>
>  
>  description:
> -  SMA1303 digital class-D audio amplifier with an integrated boost converter.
> +  SMA1303 digital class-D audio amplifier
> +  with an integrated boost converter.
>  
>  allOf:
> -  - $ref: name-prefix.yaml#
> +  - $ref: dai-common.yaml#
> +
> +properties:
> +  compatible:
> +    enum:
> +      - irondevice,sma1303
> +
> +  reg:
> +    maxItems: 1
> +
> +  '#sound-dai-cells':
> +    const: 1
> +
> +  i2c-retry:
> +    description: number of retries for I2C regmap.

Why do you need this? Why this fits the purpose of DT (or IOW why this
differs between boards)?

> +    maximum: 49
> +    default: 3
> +
> +  tdm-slot-rx:
> +    description: set the tdm rx start slot.

Aren't you now re-writing dai-tdm-slot-rx-mask property? Same for tx below.


> +    maximum: 7
> +    default: 0
> +
> +  tdm-slot-tx:
> +    description: set the tdm tx start slot.
> +    maximum: 7
> +    default: 0
> +
> +  sys-clk-id:
> +    description: select the using system clock.

What does it mean? Why do you need such property instead of clocks?

> +    default: 3
>  
>  required:
>    - compatible
>    - reg
> +  - '#sound-dai-cells'
>  

Best regards,
Krzysztof




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux