Re: [PATCH 1/3] dt-bindings: dmaengine: Add one new cell to present hardware slave id

[Arnd Bergmann]

On Tue, Feb 12, 2019 at 9:25 AM Baolin Wang <baolin.wang@xxxxxxxxxx> wrote:
> On Fri, 1 Feb 2019 at 19:53, Baolin Wang <baolin.wang@xxxxxxxxxx> wrote:
> > On Thu, 31 Jan 2019 at 00:52, Arnd Bergmann <arnd@xxxxxxxx> wrote:
> > > On Tue, Jan 22, 2019 at 2:21 PM Baolin Wang <baolin.wang@xxxxxxxxxx> wrote:
> > > >
> > > >  Client:
> > > >  DMA clients connected to the Spreadtrum DMA controller must use the format
> > > > -described in the dma.txt file, using a two-cell specifier for each channel.
> > > > -The two cells in order are:
> > > > +described in the dma.txt file, using a three-cell specifier for each channel.
> > > > +The three cells in order are:
> > > >  1. A phandle pointing to the DMA controller.
> > > >  2. The channel id.
> > > > +3. The hardware slave id which is used for clients to trigger DMA engine
> > > > +automatically.
> > >
> > > I notice that this is an incompatible binding change. Is that necessary?
> > > If the current code works, I'd suggest allowing both #dma-cells=<2>
> > > and <3>, and then implementing both in the driver.
> >
> > Yes, this is necessary.
> >
> > Yes, current code can work, but the problem is that the DMA clients
> > must add one property (something like "sprd,slave-id") to specify the
> > slave id. So considering this, we want to change the dma-cells to 2,
> > including dma channel and dma slave id, which can avoid introducing
> > some similar properties for DMA clients.
> >
> > Now there are no DMA clients in mainline for Spreadtrum platform, and
> > we want to upstream our first DMA clients: SPI controller. So no other
> > drivers need to change when we changing dma cells. Thanks.
>
> Do you have any other concerns about this patch set? If not, I think
> Vinod can apply this patch set. Thanks.

Sorry for the late reply. Yes, this makes sense since there are no
existing users then. For the DT changes going through the dmaengine
tree

Acked-by: Arnd Bergmann <arnd@xxxxxxxx>

One more question, to make sure we don't need to edit it again:
Why do you need both a 'channel id' and a 'slave id' here? Is this
a strict hardware requirement for your DMA engine? In many
other designs, only a DMA request line number needs to
be described, and I think this would correspond to what you
call the 'hardware slave id' in your documentation.

Does each request line here correspond to a fixed channel
id as well, or can a channel be shared between multiple
slave devices?

      Arnd