Re: [PATCH v5 1/2] dt-bindings: dma: Add bindings for intel LGM SOC

[Reddy, MallikarjunaX]

Hi Vinod,
Thanks for the review comments.

On 8/25/2020 7:21 PM, Vinod Koul wrote:
> On 18-08-20, 15:00, Reddy, MallikarjunaX wrote:
>
> > > > +
> > > > +            intel,chans:
> > > > +              $ref: /schemas/types.yaml#/definitions/uint32-array
> > > > +              description:
> > > > +                 The channels included on this port. Format is channel start
> > > > +                 number and how many channels on this port.
> > > Why does this need to be in DT? This all seems like it can be in the dma
> > > cells for each client.
> > (*ABC)
> > Yes. We need this.
> > for dma0(lgm-cdma) old SOC supports 16 channels and the new SOC supports 22
> > channels. and the logical channel mapping for the peripherals also differ
> > b/w old and new SOCs.
> >
> > Because of this hardware limitation we are trying to configure the total
> > channels and port-channel mapping dynamically from device tree.
> >
> > based on port name we are trying to configure the default values for
> > different peripherals(ports).
> > Example: burst length is not same for all ports, so using port name to do
> > default configurations.
> Sorry that does not make sense to me, why not specify the values to be
> used here instead of defining your own name scheme!
OK. Agreed. I will remove port name from DT and only use intel,chans
> Only older soc it should create 16 channels and new 22 (hint this is hw
> description so perfectly okay to specify in DT or in using driver_data
> and compatible for each version
>
> > > > +
> > > > +          required:
> > > > +            - reg
> > > > +            - intel,name
> > > > +            - intel,chans
> > > > +
> > > > +
> > > > + ldma-channels:
> > > > +    type: object
> > > > +    description:
> > > > +       This sub-node must contain a sub-node for each DMA channel.
> > > > +    properties:
> > > > +      '#address-cells':
> > > > +        const: 1
> > > > +      '#size-cells':
> > > > +        const: 0
> > > > +
> > > > +    patternProperties:
> > > > +      "^ldma-channels@[0-15]+$":
> > > > +          type: object
> > > > +
> > > > +          properties:
> > > > +            reg:
> > > > +              items:
> > > > +                - enum: [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15]
> > > > +              description:
> > > > +                 Which channel this node refers to.
> > > > +
> > > > +            intel,desc_num:
> > > > +              $ref: /schemas/types.yaml#/definitions/uint32
> > > > +              description:
> > > > +                 Per channel maximum descriptor number. The max value is 255.
> > > > +
> > > > +            intel,hdr-mode:
> > > > +              $ref: /schemas/types.yaml#/definitions/uint32-array
> > > > +              description:
> > > > +                 The first parameter is header mode size, the second
> > > > +                 parameter is checksum enable or disable. If enabled,
> > > > +                 header mode size is ignored. If disabled, header mode
> > > > +                 size must be provided.
> > > > +
> > > > +            intel,hw-desc:
> > > > +              $ref: /schemas/types.yaml#/definitions/uint32-array
> > > > +              description:
> > > > +                 Per channel dma hardware descriptor configuration.
> > > > +                 The first parameter is descriptor physical address and the
> > > > +                 second parameter hardware descriptor number.
> > > Again, this all seems like per client information for dma cells.
> >   Ok, if we move all these attributes to 'dmas' then 'dma-channels' child
> > node is not needed in dtsi.
> > #dma-cells number i am already using 7. If we move all these attributes to
> > 'dmas' then integer cells will increase.
> >
> > Is there any limitation in using a number of integer cells & as determined
> > by the #dma-cells property?
> No I dont think there is but it needs to make sense :-)
OK.
>