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

[Radhey Shyam Pandey <radheys@xxxxxxxxxx>]

> -----Original Message-----
> From: Rob Herring <robh@xxxxxxxxxx>
> Sent: Tuesday, April 13, 2021 12:00 AM
> To: Radhey Shyam Pandey <radheys@xxxxxxxxxx>
> Cc: davem@xxxxxxxxxxxxx; kuba@xxxxxxxxxx; Michal Simek
> <michals@xxxxxxxxxx>; vkoul@xxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx;
> netdev@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx; git <git@xxxxxxxxxx>
> Subject: Re: [RFC PATCH 2/3] dt-bindings: net: xilinx_axienet: Introduce
> dmaengine binding support
> 
> On Fri, Apr 09, 2021 at 11:43:21PM +0530, Radhey Shyam Pandey wrote:
> > 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
> 
> Huh, you just added the property and now deprecating?

In the previous patch - we added the 'xlnx,axistream-connected' property
to dmaengine node.  In this patch we are deprecating axiethernet 
axistream-connected property. So instead of custom properties the 
ethernet client will now use generic  dmas, dma-names properties
to communicate with the dmaengine driver.

> 
> > 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.
> 
> Aren't users going to care this isn't a backwards compatible change?

Yes, as it was a major design change for framework adoption and
there was no  option to support legacy usecases with this new approach.

To advertise that changes aren't backward compatible -
Should we introduce new compatibility string and raise a warning
for earlier unsupported versions? 

> 
> >
> > Signed-off-by: Radhey Shyam Pandey <radhey.shyam.pandey@xxxxxxxxxx>
> > ---
> >  .../devicetree/bindings/net/xilinx_axienet.yaml    | 40 +++++++++++++------
> ---
> >  1 file changed, 24 insertions(+), 16 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/net/xilinx_axienet.yaml
> > b/Documentation/devicetree/bindings/net/xilinx_axienet.yaml
> > index 6a00e03e8804..0ea3972fefef 100644
> > --- a/Documentation/devicetree/bindings/net/xilinx_axienet.yaml
> > +++ b/Documentation/devicetree/bindings/net/xilinx_axienet.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,15 +101,29 @@ 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
> >
> > +  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
> >    - reg
> >    - interrupts
> >    - xlnx,rxmem
> >    - phy-handle
> > +  - dmas
> > +  - dma-names
> >
> >  additionalProperties: false
> >
> > @@ -127,11 +133,13 @@ examples:
> >        compatible = "xlnx,axi-ethernet-1.00.a";
> >        device_type = "network";
> >        interrupt-parent = <&microblaze_0_axi_intc>;
> > -      interrupts = <2>, <0>, <1>;
> > +      interrupts = <1>;
> >        clock-names = "s_axi_lite_clk", "axis_clk", "ref_clk", "mgt_clk";
> >        clocks = <&axi_clk>, <&axi_clk>, <&pl_enet_ref_clk>, <&mgt_clk>;
> >        phy-mode = "mii";
> > -      reg = <0x40c00000 0x40000>,<0x50c00000 0x40000>;
> > +      reg = <0x40c00000 0x40000>;
> > +      dmas = <&xilinx_dma 0>, <&xilinx_dma 1>;
> > +      dma-names = "tx_chan0", "rx_chan0";
> 
> Is there a chan1? Typical dma-names are just 'tx' and 'rx'.
> 
> >        xlnx,rxcsum = <0x2>;
> >        xlnx,rxmem = <0x800>;
> >        xlnx,txcsum = <0x2>;
> > --
> > 2.7.4
> >