| Message ID | 20240307033708.139535-2-xingyu.wu@starfivetech.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Add PDM controller for StarFive JH8100 SoC | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/patch-applied | success | |
| robh/dtbs-check | warning | build log |
| robh/dt-meta-schema | success |
On 07/03/2024 04:37, Xingyu Wu wrote: > Add bindings for the PDM controller for the StarFive JH8100 SoC. Please use subject prefixes matching the subsystem. You can get them for example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory your patch is touching. > > Signed-off-by: Xingyu Wu <xingyu.wu@starfivetech.com> > --- > .../bindings/sound/starfive,jh8100-pdm.yaml | 84 +++++++++++++++++++ > 1 file changed, 84 insertions(+) > create mode 100644 Documentation/devicetree/bindings/sound/starfive,jh8100-pdm.yaml > > diff --git a/Documentation/devicetree/bindings/sound/starfive,jh8100-pdm.yaml b/Documentation/devicetree/bindings/sound/starfive,jh8100-pdm.yaml > new file mode 100644 > index 000000000000..a91b47d39ad3 > --- /dev/null > +++ b/Documentation/devicetree/bindings/sound/starfive,jh8100-pdm.yaml > @@ -0,0 +1,84 @@ > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/sound/starfive,jh8100-pdm.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: StarFive JH8100 PDM controller > + > +description: | > + The Pulse Density Modulation (PDM) controller is a digital PDM out > + microphone interface controller and decoder that supports both > + mono/stereo PDM format, and an Inter-IC Sound (I2S) transmitter that > + outputs standard stereo audio data to another device. The I2S transmitter > + can be configured to operate either a master or a slave (default mode). > + The PDM controller includes two PDM modules, each PDM module can drive > + one bitstream sampling clock and two bitstream coming data with sampling > + clock rising and falling edge. > + > +maintainers: > + - Xingyu Wu <xingyu.wu@starfivetech.com> > + - Walker Chen <walker.chen@starfivetech.com> > + > +properties: > + compatible: > + const: starfive,jh8100-pdm > + > + reg: > + maxItems: 1 > + > + clocks: > + items: > + - description: DMIC output clock > + - description: Main ICG clock > + > + clock-names: > + items: > + - const: dmic > + - const: icg > + > + resets: > + maxItems: 1 > + > + starfive,syscon: > + $ref: /schemas/types.yaml#/definitions/phandle-array > + items: > + - items: > + - description: phandle to System Register Controller sys_syscon_ne node. > + - description: PDM source enabled control offset of SYS_SYSCON_NE register. > + - description: PDM source enabled control mask > + description: > + The phandle to System Register Controller syscon node and the PDM source > + from I2S enabled control offset and mask of SYS_SYSCON_NE register. > + > + starfive,pdm-modulex: > + $ref: /schemas/types.yaml#/definitions/uint32 > + enum: [0, 1] > + description: > + The module x will be using in PDM controller. Default use module 0. This is an index of the block instance? If so, then it's not allowed. Otherwise I don't understand the description. Anyway, don't repeat constraints in free form text. default: 0, if this is going to stay. > + > + "#sound-dai-cells": > + const: 0 > + > +required: > + - compatible > + - reg > + - clocks > + - clock-names > + - resets > + - starfive,syscon > + > +unevaluatedProperties: false This is wrong without $ref, which points you to missing $ref to dai-common. > + > +examples: > + - | > + pdm@12250000 { > + compatible = "starfive,jh8100-pdm"; > + reg = <0x12250000 0x1000>; > + clocks = <&syscrg_ne 142>, > + <&syscrg_ne 171>; > + clock-names = "dmic", "icg"; > + resets = <&syscrg_ne 44>; > + starfive,syscon = <&sys_syscon_ne 0xC 0xFF>; Lowercase hex only. > + #sound-dai-cells = <0>; > + }; Best regards, Krzysztof
> On 07/03/2024 04:37, Xingyu Wu wrote: > > Add bindings for the PDM controller for the StarFive JH8100 SoC. > > Please use subject prefixes matching the subsystem. You can get them for > example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory your > patch is touching. > Okay, will fix. > > > > Signed-off-by: Xingyu Wu <xingyu.wu@starfivetech.com> > > --- > > .../bindings/sound/starfive,jh8100-pdm.yaml | 84 +++++++++++++++++++ > > 1 file changed, 84 insertions(+) > > create mode 100644 > > Documentation/devicetree/bindings/sound/starfive,jh8100-pdm.yaml > > > > diff --git > > a/Documentation/devicetree/bindings/sound/starfive,jh8100-pdm.yaml > > b/Documentation/devicetree/bindings/sound/starfive,jh8100-pdm.yaml > > new file mode 100644 > > index 000000000000..a91b47d39ad3 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/sound/starfive,jh8100-pdm.yaml > > @@ -0,0 +1,84 @@ > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) %YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/sound/starfive,jh8100-pdm.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: StarFive JH8100 PDM controller > > + > > +description: | > > + The Pulse Density Modulation (PDM) controller is a digital PDM out > > + microphone interface controller and decoder that supports both > > + mono/stereo PDM format, and an Inter-IC Sound (I2S) transmitter > > +that > > + outputs standard stereo audio data to another device. The I2S > > +transmitter > > + can be configured to operate either a master or a slave (default mode). > > + The PDM controller includes two PDM modules, each PDM module can > > +drive > > + one bitstream sampling clock and two bitstream coming data with > > +sampling > > + clock rising and falling edge. > > + > > +maintainers: > > + - Xingyu Wu <xingyu.wu@starfivetech.com> > > + - Walker Chen <walker.chen@starfivetech.com> > > + > > +properties: > > + compatible: > > + const: starfive,jh8100-pdm > > + > > + reg: > > + maxItems: 1 > > + > > + clocks: > > + items: > > + - description: DMIC output clock > > + - description: Main ICG clock > > + > > + clock-names: > > + items: > > + - const: dmic > > + - const: icg > > + > > + resets: > > + maxItems: 1 > > + > > + starfive,syscon: > > + $ref: /schemas/types.yaml#/definitions/phandle-array > > + items: > > + - items: > > + - description: phandle to System Register Controller > sys_syscon_ne node. > > + - description: PDM source enabled control offset of > SYS_SYSCON_NE register. > > + - description: PDM source enabled control mask > > + description: > > + The phandle to System Register Controller syscon node and the PDM > source > > + from I2S enabled control offset and mask of SYS_SYSCON_NE register. > > + > > + starfive,pdm-modulex: > > + $ref: /schemas/types.yaml#/definitions/uint32 > > + enum: [0, 1] > > + description: > > + The module x will be using in PDM controller. Default use module 0. > > This is an index of the block instance? If so, then it's not allowed. > Otherwise I don't understand the description. > No, this is just one instance. The PDM have two internal and independent modules or called channels. They can be configured and used separately, and the user can choose which channel to use. > Anyway, don't repeat constraints in free form text. default: 0, if this is going to > stay. > Oh, I just want to describe that if no this property, the driver default use module 0. I will make this clear in next version. > > + > > + "#sound-dai-cells": > > + const: 0 > > + > > +required: > > + - compatible > > + - reg > > + - clocks > > + - clock-names > > + - resets > > + - starfive,syscon > > + > > +unevaluatedProperties: false > > This is wrong without $ref, which points you to missing $ref to dai-common. Will add it. Thanks. > > > + > > +examples: > > + - | > > + pdm@12250000 { > > + compatible = "starfive,jh8100-pdm"; > > + reg = <0x12250000 0x1000>; > > + clocks = <&syscrg_ne 142>, > > + <&syscrg_ne 171>; > > + clock-names = "dmic", "icg"; > > + resets = <&syscrg_ne 44>; > > + starfive,syscon = <&sys_syscon_ne 0xC 0xFF>; > > Lowercase hex only. Will fix. > > > + #sound-dai-cells = <0>; > > + }; > > Best regards, > Krzysztof Thanks, Xingyu Wu
On 08/03/2024 08:49, Xingyu Wu wrote: >>> + >>> + starfive,pdm-modulex: >>> + $ref: /schemas/types.yaml#/definitions/uint32 >>> + enum: [0, 1] >>> + description: >>> + The module x will be using in PDM controller. Default use module 0. >> >> This is an index of the block instance? If so, then it's not allowed. >> Otherwise I don't understand the description. >> > > No, this is just one instance. The PDM have two internal and independent modules or called channels. > They can be configured and used separately, and the user can choose which channel to use. > Do the modulex differ? Why different boards would choose one over another? Best regards, Krzysztof
> On 08/03/2024 08:49, Xingyu Wu wrote: > >>> + > >>> + starfive,pdm-modulex: > >>> + $ref: /schemas/types.yaml#/definitions/uint32 > >>> + enum: [0, 1] > >>> + description: > >>> + The module x will be using in PDM controller. Default use module 0. > >> > >> This is an index of the block instance? If so, then it's not allowed. > >> Otherwise I don't understand the description. > >> > > > > No, this is just one instance. The PDM have two internal and independent > modules or called channels. > > They can be configured and used separately, and the user can choose which > channel to use. > > > > Do the modulex differ? Why different boards would choose one over another? > They are same. The choice between them is base on the match with I2S. The DMA data channel of hardware between them is fixed linked: PDM module 0 --> I2S channel 0, PDM module 1 --> I2S channel 1 I2S uses higher-number channels first for capture (like channel 1), so PDM should skips module 0 and uses module 1. Oh, I just thought of a way to fix them that change the priority of I2S channel to use lower-number channels first and PDM need not skip module0. Best regards, Xingyu Wu
On 08/03/2024 10:19, Xingyu Wu wrote: >> On 08/03/2024 08:49, Xingyu Wu wrote: >>>>> + >>>>> + starfive,pdm-modulex: >>>>> + $ref: /schemas/types.yaml#/definitions/uint32 >>>>> + enum: [0, 1] >>>>> + description: >>>>> + The module x will be using in PDM controller. Default use module 0. >>>> >>>> This is an index of the block instance? If so, then it's not allowed. >>>> Otherwise I don't understand the description. >>>> >>> >>> No, this is just one instance. The PDM have two internal and independent >> modules or called channels. >>> They can be configured and used separately, and the user can choose which >> channel to use. >>> >> >> Do the modulex differ? Why different boards would choose one over another? >> > > They are same. The choice between them is base on the match with I2S. > The DMA data channel of hardware between them is fixed linked: > PDM module 0 --> I2S channel 0, > PDM module 1 --> I2S channel 1 > I2S uses higher-number channels first for capture (like channel 1), so PDM should skips module 0 and uses module 1. > Oh, I just thought of a way to fix them that change the priority of I2S channel to use lower-number channels first and PDM need not skip module0. > Hm, then maybe this should be somehow linked with choice of I2C channel? Do you have anywhere a link to complete DTS with sound card? Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/sound/starfive,jh8100-pdm.yaml b/Documentation/devicetree/bindings/sound/starfive,jh8100-pdm.yaml new file mode 100644 index 000000000000..a91b47d39ad3 --- /dev/null +++ b/Documentation/devicetree/bindings/sound/starfive,jh8100-pdm.yaml @@ -0,0 +1,84 @@ +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/sound/starfive,jh8100-pdm.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: StarFive JH8100 PDM controller + +description: | + The Pulse Density Modulation (PDM) controller is a digital PDM out + microphone interface controller and decoder that supports both + mono/stereo PDM format, and an Inter-IC Sound (I2S) transmitter that + outputs standard stereo audio data to another device. The I2S transmitter + can be configured to operate either a master or a slave (default mode). + The PDM controller includes two PDM modules, each PDM module can drive + one bitstream sampling clock and two bitstream coming data with sampling + clock rising and falling edge. + +maintainers: + - Xingyu Wu <xingyu.wu@starfivetech.com> + - Walker Chen <walker.chen@starfivetech.com> + +properties: + compatible: + const: starfive,jh8100-pdm + + reg: + maxItems: 1 + + clocks: + items: + - description: DMIC output clock + - description: Main ICG clock + + clock-names: + items: + - const: dmic + - const: icg + + resets: + maxItems: 1 + + starfive,syscon: + $ref: /schemas/types.yaml#/definitions/phandle-array + items: + - items: + - description: phandle to System Register Controller sys_syscon_ne node. + - description: PDM source enabled control offset of SYS_SYSCON_NE register. + - description: PDM source enabled control mask + description: + The phandle to System Register Controller syscon node and the PDM source + from I2S enabled control offset and mask of SYS_SYSCON_NE register. + + starfive,pdm-modulex: + $ref: /schemas/types.yaml#/definitions/uint32 + enum: [0, 1] + description: + The module x will be using in PDM controller. Default use module 0. + + "#sound-dai-cells": + const: 0 + +required: + - compatible + - reg + - clocks + - clock-names + - resets + - starfive,syscon + +unevaluatedProperties: false + +examples: + - | + pdm@12250000 { + compatible = "starfive,jh8100-pdm"; + reg = <0x12250000 0x1000>; + clocks = <&syscrg_ne 142>, + <&syscrg_ne 171>; + clock-names = "dmic", "icg"; + resets = <&syscrg_ne 44>; + starfive,syscon = <&sys_syscon_ne 0xC 0xFF>; + #sound-dai-cells = <0>; + };
Add bindings for the PDM controller for the StarFive JH8100 SoC. Signed-off-by: Xingyu Wu <xingyu.wu@starfivetech.com> --- .../bindings/sound/starfive,jh8100-pdm.yaml | 84 +++++++++++++++++++ 1 file changed, 84 insertions(+) create mode 100644 Documentation/devicetree/bindings/sound/starfive,jh8100-pdm.yaml