mbox series

[v1,0/2] Add PDM controller for StarFive JH8100 SoC

Message ID 20240307033708.139535-1-xingyu.wu@starfivetech.com
Headers show
Series Add PDM controller for StarFive JH8100 SoC | expand

Message

Xingyu Wu March 7, 2024, 3:37 a.m. UTC
The Pulse Density Modulation (PDM) controller is a digital PDM
microphone interface controller and decoder that supports both
mono/stereo PDM format, and a Inter-IC Sound (I2S) transmitter
that outputs standard stereo audio data to another device. 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.
    
On the JH8100 SoC, PDM and I2S are fixedly connected as follow:
    PDM module 0 --> I2S channel 0
    PDM module 1 --> I2S channel 1

The first patch adds documentation to describe PDM controller
bindings. And the second patch adds PDM driver for the StarFive
JH8100 SoC. The DTS patch of PDM controller will be submitted
after the patchs of JH8100 DTS are accepted.

Xingyu Wu (2):
  dt-bindings: ASoC: Add PDM controller for the StarFive JH8100 SoC
  ASoC: starfive: Add PDM controller support

 .../bindings/sound/starfive,jh8100-pdm.yaml   |  84 ++++
 MAINTAINERS                                   |   7 +
 sound/soc/starfive/Kconfig                    |   7 +
 sound/soc/starfive/Makefile                   |   1 +
 sound/soc/starfive/jh8100_pdm.c               | 395 ++++++++++++++++++
 5 files changed, 494 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/sound/starfive,jh8100-pdm.yaml
 create mode 100644 sound/soc/starfive/jh8100_pdm.c

Comments

Xingyu Wu March 8, 2024, 7:49 a.m. UTC | #1
> 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
Xingyu Wu March 8, 2024, 9:19 a.m. UTC | #2
> 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
Krzysztof Kozlowski March 8, 2024, 10:05 a.m. UTC | #3
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