lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Fri, 26 Jan 2024 09:27:47 +0100
From: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To: Shenghao Ding <shenghao-ding@...com>, broonie@...nel.org,
 conor+dt@...nel.org
Cc: robh+dt@...nel.org, andriy.shevchenko@...ux.intel.com, kevin-lu@...com,
 baojun.xu@...com, devicetree@...r.kernel.org, v-po@...com,
 lgirdwood@...il.com, perex@...ex.cz, pierre-louis.bossart@...ux.intel.com,
 13916275206@....com, mohit.chawla@...com, linux-sound@...r.kernel.org,
 linux-kernel@...r.kernel.org, liam.r.girdwood@...el.com, soyer@....hu,
 jkhuang3@...com, tiwai@...e.de, pdjuandi@...com, j-mcpherson@...com,
 navada@...com
Subject: Re: [PATCH v2 4/4] ASoc: dt-bindings: PCM6240: Add initial DT binding

On 26/01/2024 04:58, Shenghao Ding wrote:
> PCM6240 driver implements a flexible and configurable setting for register
> and filter coefficients, to one, two or even multiple PCM6240 Family Audio
> chips.
> 

Subject: you just ignored my comment...

Please use scripts/get_maintainers.pl to get a list of necessary people
and lists to CC (and consider --no-git-fallback argument). It might
happen, that command when run on an older kernel, gives you outdated
entries. Therefore please be sure you base your patches on recent Linux
kernel.

Tools like b4 or scripts_getmaintainer.pl provide you proper list of
people, so fix your workflow. Tools might also fail if you work on some
ancient tree (don't, use mainline), work on fork of kernel (don't, use
mainline) or you ignore some maintainers (really don't). Just use b4 and
all the problems go away.

..

> +      ti,pcmd3140: Four-channel PDM-input to TDM or I�S output converter.

This does not look like proper encoding.

> +
> +      ti,pcmd3180: Eight-channel pulse-density-modulation input to TDM or
> +      I�S output converter.
> +
> +      ti,taa5212: Low-power high-performance stereo audio ADC with 118-dB
> +      dynamic range.
> +
> +      ti,tad5212: Low-power stereo audio DAC with 120-dB dynamic range.
> +    oneOf:
> +      - items:
> +          - enum:
> +              - ti,adc3120
> +              - ti,adc5120
> +              - ti,pcm3120
> +              - ti,pcm5120
> +              - ti,pcm6120
> +          - const: ti,adc6120
> +      - items:
> +          - enum:
> +              - ti,pcm6260
> +              - ti,pcm6140
> +              - ti,pcm3140
> +              - ti,pcm5140
> +          - const: ti,pcm6240
> +      - items:
> +          - const: ti,dix4192
> +          - const: ti,pcm6240
> +          - const: ti,adc6120

It does not make sense. You said above adc6120 is not compatible with
pcm6240.

> +      - items:
> +          - const: ti,pcm1690
> +          - const: ti,pcm9211
> +          - const: ti,pcmd512x
> +      - items:
> +          - enum:
> +              - ti,pcmd3180
> +          - const: ti,pcmd3140
> +      - items:
> +          - enum:
> +              - taa5412
> +          - const: ti,taa5212
> +      - items:
> +          - enum:
> +              - tad5412
> +          - const: ti,tad5212
> +      - items:

No need for items.

> +          - enum:
> +              - ti,pcm6240
> +              - ti,pcmd3140
> +              - ti,taa5212
> +              - ti,tad5212
> +              - ti,pcmd3180

Where is adc6120 and others?

Missing blank line.

> +  reg:
> +    description:
> +      I2C address, in multiple pcmdevices case, all the i2c address
> +      aggregate as one Audio Device to support multiple audio slots.
> +    minItems: 1
> +    maxItems: 4
> +
> +  reset-gpios:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +    description:
> +      Invalid only for ti,pcm1690 because of no INT pin.
> +
> +  '#sound-dai-cells':
> +    const: 0
> +
> +required:
> +  - compatible
> +  - reg
> +
> +allOf:
> +  - $ref: dai-common.yaml#
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - ti,pcm1690
> +    then:
> +      properties:
> +        reg:
> +          items:
> +            minimum: 0x4c
> +            maximum: 0x4f

Nothing improved.

> +        interrupts: false
> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - ti,adc3120
> +              - ti,adc5120
> +              - ti,adc6120
> +              - ti,pcm3120
> +              - ti,pcm5120
> +              - ti,pcm6120
> +              - ti,pcmd3140
> +    then:
> +      properties:
> +        reg:
> +          maxItems: 1
> +          items:
> +            maximum: 0x4e
> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - ti,dix4192
> +    then:
> +      properties:
> +        reg:
> +          items:
> +            minimum: 0x70
> +            maximum: 0x73

Drop entire if

> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - ti,pcm6240
> +              - ti,pcm6260
> +    then:
> +      properties:
> +        reg:
> +          minItems: 1
> +          maxItems: 4
> +          items:
> +            minimum: 0x48
> +            maximum: 0x4b

Drop entire if

> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - ti,pcm9211
> +    then:
> +      properties:
> +        reg:
> +          items:
> +            minimum: 0x40
> +            maximum: 0x43

Drop entire if


> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - ti,taa5212
> +              - ti,taa5412
> +              - ti,tad5212
> +              - ti,tad5412
> +    then:
> +      properties:
> +        reg:
> +          items:
> +            minimum: 0x50
> +            maximum: 0x53

Drop entire if


> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +   #include <dt-bindings/gpio/gpio.h>
> +   i2c {
> +     /* example for two devices with interrupt support */
> +     #address-cells = <1>;
> +     #size-cells = <0>;
> +     pcm6240: audio-codec@48 {
> +       compatible = "ti,pcm6240";
> +       reg = <0x48>, /* primary-device */
> +            <0x4b>; /* secondary-device */

Looks misaligned.



Best regards,
Krzysztof


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ