RE: [RFC V2 PATCH 2/3] dt-bindings: net: xilinx_axienet: Introduce dmaengine binding support

[Pandey, Radhey Shyam]

> -----Original Message-----
> From: Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx>
> Sent: Wednesday, September 21, 2022 1:15 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; linux@xxxxxxxxxxxxxxx
> Cc: netdev@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; linux-arm-
> kernel@xxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; Simek, Michal
> <michal.simek@xxxxxxx>; Pandey, Radhey Shyam
> <radhey.shyam.pandey@xxxxxxx>; Sarangi, Anirudha
> <anirudha.sarangi@xxxxxxx>; Katakam, Harini
> <harini.katakam@xxxxxxx>; git@xxxxxxxxxx; git (AMD-Xilinx)
> <git@xxxxxxx>
> Subject: Re: [RFC V2 PATCH 2/3] dt-bindings: net: xilinx_axienet: Introduce
> dmaengine binding support
> 
> On 20/09/2022 07:57, Sarath Babu Naidu Gaddam wrote:
> > From: Radhey Shyam Pandey <radhey.shyam.pandey@xxxxxxxxxx>
> >
> > The axiethernet driver will now use dmaengine framework to communicate
> > with dma controller IP instead of built-in dma programming sequence.
> >
> > To request dma transmit and receive channels the axiethernet driver
> > uses generic dmas, dma-names properties. It deprecates
> > axistream-connected property, remove axidma reg and interrupt
> > properties from the ethernet node. Just to highlight that these DT
> > changes are not backward compatible due to major driver
> > restructuring/cleanup done in adopting the dmaengine framework.
> >
> > Signed-off-by: Radhey Shyam Pandey <radhey.shyam.pandey@xxxxxxxxxx>
> > Signed-off-by: Sarath Babu Naidu Gaddam
> > <sarath.babu.naidu.gaddam@xxxxxxx>
> > ---
> > Changes in V2:
> > - None.
> > ---
> >  .../devicetree/bindings/net/xlnx,axiethernet.yaml  |   39 ++++++++++++-----
> ---
> >  1 files changed, 23 insertions(+), 16 deletions(-)
> >
> > diff --git
> > a/Documentation/devicetree/bindings/net/xlnx,axiethernet.yaml
> > b/Documentation/devicetree/bindings/net/xlnx,axiethernet.yaml
> > index 780edf3..1dc1719 100644
> > --- a/Documentation/devicetree/bindings/net/xlnx,axiethernet.yaml
> > +++ b/Documentation/devicetree/bindings/net/xlnx,axiethernet.yaml
> > @@ -14,10 +14,8 @@ description: |
> >    offloading TX/RX checksum calculation off the processor.
> >
> >    Management configuration is done through the AXI interface, while
> > payload is
> > -  sent and received through means of an AXI DMA controller. This
> > driver
> > -  includes the DMA driver code, so this driver is incompatible with
> > AXI DMA
> > -  driver.
> > -
> > +  sent and received through means of an AXI DMA controller using
> > + dmaengine  framework.
> >
> >  allOf:
> >    - $ref: "ethernet-controller.yaml#"
> > @@ -36,19 +34,13 @@ properties:
> >
> >    reg:
> >      description:
> > -      Address and length of the IO space, as well as the address
> > -      and length of the AXI DMA controller IO space, unless
> > -      axistream-connected is specified, in which case the reg
> > -      attribute of the node referenced by it is used.
> > -    maxItems: 2
> > +      Address and length of the IO space.
> > +    maxItems: 1
> >
> >    interrupts:
> >      description:
> > -      Can point to at most 3 interrupts. TX DMA, RX DMA, and optionally
> Ethernet
> > -      core. If axistream-connected is specified, the TX/RX DMA interrupts
> should
> > -      be on that node instead, and only the Ethernet core interrupt is
> optionally
> > -      specified here.
> > -    maxItems: 3
> > +      Ethernet core interrupt.
> > +    maxItems: 1
> >
> >    phy-handle: true
> >
> > @@ -109,6 +101,7 @@ properties:
> >        for the AXI DMA controller used by this device. If this is specified,
> >        the DMA-related resources from that device (DMA registers and DMA
> >        TX/RX interrupts) rather than this one will be used.
> > +    deprecated: true
> >
> >    mdio: true
> >
> > @@ -118,12 +111,24 @@ properties:
> >        and "phy-handle" should point to an external PHY if exists.
> >      $ref: /schemas/types.yaml#/definitions/phandle
> >
> > +  dmas:
> > +    items:
> > +      - description: TX DMA Channel phandle and DMA request line number
> > +      - description: RX DMA Channel phandle and DMA request line
> > + number
> > +
> > +  dma-names:
> > +    items:
> > +      - const: tx_chan0
> > +      - const: rx_chan0
> > +
> >  required:
> >    - compatible
> >    - interrupts
> >    - reg
> >    - xlnx,rxmem
> >    - phy-handle
> > +  - dmas
> > +  - dma-names
> >
> >  additionalProperties: false
> >
> > @@ -132,11 +137,13 @@ examples:
> >      axi_ethernet_eth: ethernet@40c00000 {
> >        compatible = "xlnx,axi-ethernet-1.00.a";
> >        interrupt-parent = <&microblaze_0_axi_intc>;
> > -      interrupts = <2>, <0>, <1>;
> > +      interrupts = <1>;
> 
> This looks like an ABI break. How do you handle old DTS? Oh wait... you do
> not handle it at all.

Yes, this is anticipated ABI break due to major changes in axiethernet
driver while adopting to dmaengine framework. Same is highlighted
in commit description - "DT changes are not backward compatible 
due to major driver restructuring/cleanup done in adopting the 
dmaengine framework". 

Some background - Factor out AXI DMA code into separate driver was
a TODO item (mentioned in driver changelog) and is being done as 
part of this series. The DMA code is removed from axiethernet driver 
and ethernet driver now make use of dmaengine framework to 
communicate with AXIDMA IP.

When DMA code is removed from axiethernet driver there is limitation
to support legacy DMA resources binding. One option is to inform
user to switch to new binding when old DTS is detected? (and at some
point we have to make this transition and remove dma code).
Please let us know if there are any other alternative to consider?