linux-kernel - Re: [PATCH v10 2/3] DMA: Freescale: Add new 8-channel DMA engine device tree nodes

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 25 Sep 2013 15:35:46 +0800
From:	Hongbo Zhang <hongbo.zhang@...escale.com>
To:	Stephen Warren <swarren@...dotorg.org>
CC:	<rob.herring@...xeda.com>, <pawel.moll@....com>,
	<mark.rutland@....com>, <ian.campbell@...rix.com>,
	<vinod.koul@...el.com>, <djbw@...com>,
	<devicetree@...r.kernel.org>, <linuxppc-dev@...ts.ozlabs.org>,
	<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v10 2/3] DMA: Freescale: Add new 8-channel DMA engine
 device tree nodes

On 09/25/2013 01:31 AM, Stephen Warren wrote:
> On 09/24/2013 04:30 AM, Hongbo Zhang wrote:
>> On 09/24/2013 01:04 AM, Stephen Warren wrote:
>>> On 09/18/2013 04:15 AM, hongbo.zhang@...escale.com wrote:
>>>> From: Hongbo Zhang <hongbo.zhang@...escale.com>
>>>>
>>>> Freescale QorIQ T4 and B4 introduce new 8-channel DMA engines, this
>>>> patch adds
>>>> the device tree nodes for them.
>>>> diff --git a/Documentation/devicetree/bindings/powerpc/fsl/dma.txt
>>>> b/Documentation/devicetree/bindings/powerpc/fsl/dma.txt
>>>> +Required properties:
>>>> +
>>>> +- compatible        : must include "fsl,elo3-dma"
>>>> +- reg               : DMA General Status Registers, i.e. DGSR0 which
>>>> contains
>>>> +                      status for channel 1~4, and DGSR1 for channel 5~8
>>> Is that a single entry, which is large enough to cover both registers,
>>> or a pair of entries, one per register? Reading the text, I might assume
>>> the former, but looking at the examples, it's the latter.
>> My impression is that I cannot tell it is one larger entry or two
>> entries by reading the description text, but the example gives the answer.
>> Is it so important to specify it is only one entry or entries list?
>> I prefer language as concise as possible, especially for the common
>> properties such as reg and interrupt (eg the reg is implicitly offset
>> and length of registers, can be continuous or not), it is difficult or
>> unnecessary or impossible to describe much details, the example can also
>> work as a complementary description, otherwise no need to put an example
>> in the binding document.
> The description of the properties should fully describe them. The
> example is just an example, not a specification of the properties.
>
It is OK for me to update the description like this:
reg:    containing two entries for DMA General Status Registers, i.e. 
DGSR0 which contains + status for channel 1~4, and DGSR1 for channel 5~8

and let me wait one or more days to see if other reviewers/maintainers 
have further comments before I send our another iteration.

By the way, I know maybe it is difficult, but why not introduce a 
document of maintaining rules for the dt binding docs? we have dedicated 
maintainers for this part now. Description language from one submitter 
cannot satisfy every reviewer/maintainer, for a reg property, is it 
necessary to say "offset and length", to say "how many entries", to say 
"register functions and even names"? If there is specific rules (even 
with good examples), it will be convenient for both submitter and 
reviewers. Without rules/guidelines, new submitter would like to follow 
old bad samples.



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/