Re: [PATCH 02/14] dmaengine: dma-jz4780: Separate chan/ctrl registers

From: Paul Cercueil
Date: Wed Jul 11 2018 - 19:13:29 EST




Le mer. 11 juil. 2018 à 14:16, Vinod <vkoul@xxxxxxxxxx> a écrit :
On 10-07-18, 17:36, Paul Cercueil wrote:

> > @@ -3,7 +3,8 @@
> > Required properties:
> >
> > - compatible: Should be "ingenic,jz4780-dma"
> > -- reg: Should contain the DMA controller registers location and
> > length.
> > +- reg: Should contain the DMA channel registers location and
> > length, followed
> > + by the DMA controller registers location and length.
> > - interrupts: Should contain the interrupt specifier of the DMA
> > controller.
> > - interrupt-parent: Should be the phandle of the interrupt
> > controller that
> > - clocks: Should contain a clock specifier for the JZ4780 PDMA
> > clock.
> > @@ -22,7 +23,8 @@ Example:
> >
> > dma: dma@13420000 {
> > compatible = "ingenic,jz4780-dma";
> > - reg = <0x13420000 0x10000>;
> > + reg = <0x13420000 0x400
> > + 0x13421000 0x40>;
>
> Second should be optional or we break platform which may not have
> updated DT..

See comment below.

> > - jzdma->base = devm_ioremap_resource(dev, res);
> > - if (IS_ERR(jzdma->base))
> > - return PTR_ERR(jzdma->base);
> > + jzdma->chn_base = devm_ioremap_resource(dev, res);
> > + if (IS_ERR(jzdma->chn_base))
> > + return PTR_ERR(jzdma->chn_base);
> > +
> > + res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> > + if (!res) {
> > + dev_err(dev, "failed to get I/O memory\n");
> > + return -EINVAL;
> > + }
>
> okay and this breaks if you happen to get probed on older DT. I think DT
> is treated as ABI so you need to continue support older method while
> finding if DT has split resources

See my response to PrasannaKumar. All the Ingenic-based boards do compile
the devicetree within the kernel, so I think it's still fine to add breaking
changes. I'll wait on @Rob to give his point of view on this, though.

(It's not something hard to change, but I'd like to know what's the policy
in that case. I have other DT-breaking patches to submit)

The policy is that DT is an ABI and should not break :)

Who maintains Ingenic arch. MAINTAINERS doesn't tell me.

Unofficially that would be me :)

Otherwise that would be the MIPS maintainers, Ralf and Paul (Burton).

--
~Vinod