diff mbox

[RESEND] Documentation: devicetree: Fix Xilinx VDMA specification

Message ID 1415800277-6817-1-git-send-email-afaerber@suse.de (mailing list archive)
State Accepted
Headers show

Commit Message

Andreas Färber Nov. 12, 2014, 1:51 p.m. UTC 
The specification requires xlnx,data-width, but example and driver use
xlnx,datawidth. Change the specification to match the implementation.

Reviewed-by: Michal Simek <michal.simek@xilinx.com>
Fixes: eebeac03db93 ("dma: Add Xilinx Video DMA DT Binding Documentation")
Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 This appears to have fallen through the cracks... resending to dmaengine.

 Documentation/devicetree/bindings/dma/xilinx/xilinx_vdma.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Soren Brinkmann Nov. 12, 2014, 3:57 p.m. UTC | #1 
On Wed, 2014-11-12 at 02:51PM +0100, Andreas Färber wrote:
> The specification requires xlnx,data-width, but example and driver use
> xlnx,datawidth. Change the specification to match the implementation.

Isn't this the wrong way around? The bindings are considered API, so
shouldn't the driver be fixed to match the spec?
Are there already dts files out there using either of these options?

	Soren
--
To unsubscribe from this list: send the line "unsubscribe dmaengine" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Andreas Färber Nov. 12, 2014, 6:03 p.m. UTC | #2 
Am 12.11.2014 um 16:57 schrieb Sören Brinkmann:
> On Wed, 2014-11-12 at 02:51PM +0100, Andreas Färber wrote:
>> The specification requires xlnx,data-width, but example and driver use
>> xlnx,datawidth. Change the specification to match the implementation.
> 
> Isn't this the wrong way around? The bindings are considered API, so
> shouldn't the driver be fixed to match the spec?

In theory, patch review should've never let the two differ... ;)

It's not my driver, so I fixed the perceived inconsistency the least
invasive way; Michal and Srikanth seemed to concur at the time.

https://patchwork.kernel.org/patch/4620261/

> Are there already dts files out there using either of these options?

In upstream, no. microblaze and virtex440 use a
xlnx,include-datawidth-matching-0 property as precedence for the
spelling, whereas there is an fsl,data-width and an unused msix-data-width.

Downstream, yes: Beyond my own patch derived from the Parallella tree,
there's some in the ADI tree. None in the Xilinx tree on quick check.

I haven't encountered any using the documented xlnx,data-width - but
this patch was authored pre 3.17, haven't ran a full Web search again.

Regards,
Andreas
Soren Brinkmann Nov. 12, 2014, 6:26 p.m. UTC | #3 
On Wed, 2014-11-12 at 07:03PM +0100, Andreas Färber wrote:
> Am 12.11.2014 um 16:57 schrieb Sören Brinkmann:
> > On Wed, 2014-11-12 at 02:51PM +0100, Andreas Färber wrote:
> >> The specification requires xlnx,data-width, but example and driver use
> >> xlnx,datawidth. Change the specification to match the implementation.
> > 
> > Isn't this the wrong way around? The bindings are considered API, so
> > shouldn't the driver be fixed to match the spec?
> 
> In theory, patch review should've never let the two differ... ;)
> 
> It's not my driver, so I fixed the perceived inconsistency the least
> invasive way; Michal and Srikanth seemed to concur at the time.
> 
> https://patchwork.kernel.org/patch/4620261/
> 
> > Are there already dts files out there using either of these options?
> 
> In upstream, no. microblaze and virtex440 use a
> xlnx,include-datawidth-matching-0 property as precedence for the
> spelling, whereas there is an fsl,data-width and an unused msix-data-width.
> 
> Downstream, yes: Beyond my own patch derived from the Parallella tree,
> there's some in the ADI tree. None in the Xilinx tree on quick check.
> 
> I haven't encountered any using the documented xlnx,data-width - but
> this patch was authored pre 3.17, haven't ran a full Web search again.

grepping through linux-next shows some usage of xlnx,datawidth, but
only the single hit in  Documentation for xlnx,data-width.
Other than VDMA the other hit seems to be just some DT
documentation which I can't find a driver for...
Anyhow, this patch is probably the best way of fixing this.

Reviewed-by: Soren Brinkmann <soren.brinkmann@xilinx.com>

	Thanks,
	Sören
--
To unsubscribe from this list: send the line "unsubscribe dmaengine" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Vinod Koul Dec. 5, 2014, 4:47 p.m. UTC | #4 
On Wed, Nov 12, 2014 at 10:26:03AM -0800, Sören Brinkmann wrote:
> On Wed, 2014-11-12 at 07:03PM +0100, Andreas Färber wrote:
> > Am 12.11.2014 um 16:57 schrieb Sören Brinkmann:
> > > On Wed, 2014-11-12 at 02:51PM +0100, Andreas Färber wrote:
> > >> The specification requires xlnx,data-width, but example and driver use
> > >> xlnx,datawidth. Change the specification to match the implementation.
> > > 
> > > Isn't this the wrong way around? The bindings are considered API, so
> > > shouldn't the driver be fixed to match the spec?
> > 
> > In theory, patch review should've never let the two differ... ;)
> > 
> > It's not my driver, so I fixed the perceived inconsistency the least
> > invasive way; Michal and Srikanth seemed to concur at the time.
> > 
> > https://patchwork.kernel.org/patch/4620261/
> > 
> > > Are there already dts files out there using either of these options?
> > 
> > In upstream, no. microblaze and virtex440 use a
> > xlnx,include-datawidth-matching-0 property as precedence for the
> > spelling, whereas there is an fsl,data-width and an unused msix-data-width.
> > 
> > Downstream, yes: Beyond my own patch derived from the Parallella tree,
> > there's some in the ADI tree. None in the Xilinx tree on quick check.
> > 
> > I haven't encountered any using the documented xlnx,data-width - but
> > this patch was authored pre 3.17, haven't ran a full Web search again.
> 
> grepping through linux-next shows some usage of xlnx,datawidth, but
> only the single hit in  Documentation for xlnx,data-width.
> Other than VDMA the other hit seems to be just some DT
> documentation which I can't find a driver for...
> Anyhow, this patch is probably the best way of fixing this.
the driver had issues so want applied, I havent seen repot though. 
Will apply this now
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/dma/xilinx/xilinx_vdma.txt b/Documentation/devicetree/bindings/dma/xilinx/xilinx_vdma.txt
index 1405ed071bb4..e4c4d47f8137 100644
--- a/Documentation/devicetree/bindings/dma/xilinx/xilinx_vdma.txt
+++ b/Documentation/devicetree/bindings/dma/xilinx/xilinx_vdma.txt
@@ -25,7 +25,7 @@  Required child node properties:
 - compatible: It should be either "xlnx,axi-vdma-mm2s-channel" or
 	"xlnx,axi-vdma-s2mm-channel".
 - interrupts: Should contain per channel VDMA interrupts.
-- xlnx,data-width: Should contain the stream data width, take values
+- xlnx,datawidth: Should contain the stream data width, take values
 	{32,64...1024}.
 
 Optional child node properties: