RE: [PATCH net-next V3 1/3] dt-bindings: net: xilinx_axienet: Introduce dmaengine binding support

[Gaddam, Sarath Babu Naidu]

> -----Original Message-----
> From: Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx>
> Sent: Wednesday, May 17, 2023 8:19 PM
> To: Gaddam, Sarath Babu Naidu
> <sarath.babu.naidu.gaddam@xxxxxxx>; davem@xxxxxxxxxxxxx;
> edumazet@xxxxxxxxxx; kuba@xxxxxxxxxx; pabeni@xxxxxxxxxx;
> robh+dt@xxxxxxxxxx; krzysztof.kozlowski+dt@xxxxxxxxxx
> Cc: linux@xxxxxxxxxxxxxxx; Simek, Michal <michal.simek@xxxxxxx>;
> Pandey, Radhey Shyam <radhey.shyam.pandey@xxxxxxx>;
> netdev@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; linux-arm-
> kernel@xxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; Sarangi,
> Anirudha <anirudha.sarangi@xxxxxxx>; Katakam, Harini
> <harini.katakam@xxxxxxx>; git (AMD-Xilinx) <git@xxxxxxx>
> Subject: Re: [PATCH net-next V3 1/3] dt-bindings: net: xilinx_axienet:
> Introduce dmaengine binding support
> 
> On 17/05/2023 14:06, Gaddam, Sarath Babu Naidu wrote:
> >>>>> +  dma-names:
> >>>>> +    items:
> >>>>> +      - const: tx_chan0
> >>>>
> >>>> tx
> >>>>
> >>>>> +      - const: rx_chan0
> >>>>
> >>>> rx
> >>>
> >>> We want to support more channels in the future, currently we
> support
> >>> AXI DMA which has only one tx and rx channel. In future we want to
> >>> extend support for multichannel DMA (MCDMA) which has 16 TX and
> >>> 16 RX channels. To uniquely identify each channel, we are using chan
> >>> suffix. Depending on the usecase AXI ethernet driver can request any
> >>> combination of multichannel DMA  channels.
> >>>
> >>> dma-names = tx_chan0, tx_chan1, rx_chan0, rx_chan1;
> >>>
> >>> will update the commit message with same.
> >>
> >> I expect the binding to be complete, otherwise you get comments like
> this.
> >> Add missing parts to the binding and resend.
> >
> > Binding is complete for current supported DMA (single channel).  We
> > will extend when we add MCDMA.
> 
> What doe sit mean "current supported DMA"? By driver? or by hardware?
> If the former, then how does it matter for the bindings?
> 
> If the latter, then your hardware is going to change? Then you will have
> different set of compatibles and then can use different names.
> 
> >
> > We will describe the reason for using channel suffix in the
> > description as below.
> >
> >    dma-names:
> >       items:
> >         - const: tx_chan0
> >         - const: rx_chan0
> >      description: |
> >            Chan suffix is used for identifying each channel uniquely.
> >            Current DMA has only one Tx and Rx channel but it will be
> >            extended to support for multichannel DMA (MCDMA) which
> >            has 16 TX and 16 RX channels. Depending on the usecase AXI
> >            ethernet driver can request any combination of multichannel
> >            DMA  channels.
> 
> No, because I don't understand what is "will be extended". Bindings
> should be complete. If they are going to be extended, it means they are
> not complete. If they cannot be complete, which happens, please provide
> a reason. There was no reason so far, except your claim it is complete.

We will re-spin another series with complete bindings including MCDMA
support.

Thanks,
Sarath