diff mbox

[v2,09/11] Documentation: devicetree: Fix ADI AXI SPDIF specification

Message ID 1406242820-20140-10-git-send-email-afaerber@suse.de (mailing list archive)
State New, archived
Headers show

Commit Message

Andreas Färber July 24, 2014, 11 p.m. UTC
The specification requires compatible = "adi,axi-spdif-1.00.a" but
driver and example and file name indicate "adi,axi-spdif-tx-1.00.a".
Change the specification to match the implementation.

Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 v2: New
 
 Documentation/devicetree/bindings/sound/adi,axi-spdif-tx.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Lars-Peter Clausen July 25, 2014, 7:42 a.m. UTC | #1
On 07/25/2014 01:00 AM, Andreas Färber wrote:
> The specification requires compatible = "adi,axi-spdif-1.00.a" but
> driver and example and file name indicate "adi,axi-spdif-tx-1.00.a".
> Change the specification to match the implementation.
>
> Signed-off-by: Andreas Färber <afaerber@suse.de>

Acked-by: Lars-Peter Clausen <lars@metafoo.de>

Thanks.

> ---
>   v2: New
>
>   Documentation/devicetree/bindings/sound/adi,axi-spdif-tx.txt | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/sound/adi,axi-spdif-tx.txt b/Documentation/devicetree/bindings/sound/adi,axi-spdif-tx.txt
> index 46f3449..4eb7997 100644
> --- a/Documentation/devicetree/bindings/sound/adi,axi-spdif-tx.txt
> +++ b/Documentation/devicetree/bindings/sound/adi,axi-spdif-tx.txt
> @@ -1,7 +1,7 @@
>   ADI AXI-SPDIF controller
>
>   Required properties:
> - - compatible : Must be "adi,axi-spdif-1.00.a"
> + - compatible : Must be "adi,axi-spdif-tx-1.00.a"
>    - reg : Must contain SPDIF core's registers location and length
>    - clocks : Pairs of phandle and specifier referencing the controller's clocks.
>      The controller expects two clocks, the clock used for the AXI interface and
>
Michal Simek July 25, 2014, 8:08 a.m. UTC | #2
Hi Mark,

On 07/25/2014 09:42 AM, Lars-Peter Clausen wrote:
> On 07/25/2014 01:00 AM, Andreas Färber wrote:
>> The specification requires compatible = "adi,axi-spdif-1.00.a" but
>> driver and example and file name indicate "adi,axi-spdif-tx-1.00.a".
>> Change the specification to match the implementation.
>>
>> Signed-off-by: Andreas Färber <afaerber@suse.de>
> 
> Acked-by: Lars-Peter Clausen <lars@metafoo.de>

All these patches have been added through your tree that's why I think
you should take this fix through his tree.

Here is my:
Reviewed-by: Michal Simek <michal.simek@xilinx.com>

Thanks,
Michal
Mark Brown July 25, 2014, 10:18 a.m. UTC | #3
On Fri, Jul 25, 2014 at 10:08:06AM +0200, Michal Simek wrote:

> All these patches have been added through your tree that's why I think
> you should take this fix through his tree.

If someone could send me patches I'll take a look at them.
Andreas Färber July 25, 2014, 10:32 a.m. UTC | #4
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Am 25.07.2014 12:18, schrieb Mark Brown:
> On Fri, Jul 25, 2014 at 10:08:06AM +0200, Michal Simek wrote:
> 
>> All these patches have been added through your tree that's why I
>> think you should take this fix through his tree.
> 
> If someone could send me patches I'll take a look at them.

I used scripts/get_maintainer.pl --nogit-fallback. If you were not
CC'ed and need to be, please submit a MAINTAINERS patch. :)

Maybe this works for you?
https://patchwork.kernel.org/patch/4620191/

Otherwise I can resend with Ab/Rb.

Andreas

- -- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.22 (GNU/Linux)

iQIcBAEBAgAGBQJT0jJLAAoJEPou0S0+fgE/od0QAItNVBWohkpacvB5M+Oh0x5H
40aYXWOPVrAHRfXlh5SsSQyba/2P6q719meJL3FseU7Jj+rbkPnICf3q5KWgN3Vb
iXoO+UQPn/gvredRql9g5eUj0xeh6j3bIAmEYmmme3opg0siKV1Gd6D9vEezjL8x
M3xroJ05EbhjAd1UrgW/xEINAfDExLXbUQhsbls1tzL7WL9Cx4Obi7/KWiAVATMU
RPNQiz8zyVXoJq0i596fAKX+EPaID2ztr26IQtpH+vOJOpRhtQeX+B+HBxTm89yO
mg7ZnklhXFFaAYhXKEm1itMbi8OnCUzDuk5hPklCJF+WgzBbyq29erx53VaRQAij
jrebNU4UB0/si7Gnl/XwvRMDUaVDkF6OaeeSjwKZvk0R6o02lperM51gKOgsDdC9
G9VPb9AoNvbebEaNvonU22gB81RWg1MFkW7XF1Enj0C+xINePG86TXpWNiNUPfdV
QAyYhwrAlePV4tKYqnyDd1d8orPs5TySVt3tuloUTB17IpYXM7OWj7db5so5V6O2
99m2rrcMCP5SxDiuB0LUbKxrBoIhGH2kZ0k476Xb/YOYgpBskOxFLqZ+ptzZDW7K
a3frjJ98QKjb5z9fZ1qZBzbegggAEvXlhpIgcaWAkT1wV3vmZhQjycpDHNWuahsu
ybMjvgNj/YiougCN+7bf
=zr8g
-----END PGP SIGNATURE-----
Mark Brown July 25, 2014, 10:39 a.m. UTC | #5
On Fri, Jul 25, 2014 at 12:32:43PM +0200, Andreas Färber wrote:

> I used scripts/get_maintainer.pl --nogit-fallback. If you were not
> CC'ed and need to be, please submit a MAINTAINERS patch. :)

You need to think about what you're doing when you use get_maintainer,
it gives both false positives and false negatives.

> Maybe this works for you?
> https://patchwork.kernel.org/patch/4620191/

> Otherwise I can resend with Ab/Rb.

Documentation/SubmittingPatches.
Andreas Färber July 28, 2014, 11:43 a.m. UTC | #6
Mark,

Am 25.07.2014 12:39, schrieb Mark Brown:
> On Fri, Jul 25, 2014 at 12:32:43PM +0200, Andreas Färber wrote:
> 
>> I used scripts/get_maintainer.pl --nogit-fallback. If you were not
>> CC'ed and need to be, please submit a MAINTAINERS patch. :)
> 
> You need to think about what you're doing when you use get_maintainer,
> it gives both false positives and false negatives.
> 
>> Maybe this works for you?
>> https://patchwork.kernel.org/patch/4620191/
> 
>> Otherwise I can resend with Ab/Rb.
> 
> Documentation/SubmittingPatches.

Not helpful here, nor is
Documentation/devicetree/bindings/submitting-patches.txt. It's separate
from code changes, sent via git-send-email, has a Sob, went to LKML and
LAKML and DTML. I could add a Fixes: header and CC trivial for this
one-line fix IIUC.
Doesn't tell me which patches I should CC you on in the future though.
Therefore my request to fix the false negative in MAINTAINERS so that me
and other kernel newbies don't run into it again.

I'll simply interpret your reply as "yes, please resend with
broonie@kernel.org CC'ed" then.

Regards,
Andreas
Mark Brown July 28, 2014, 12:20 p.m. UTC | #7
On Mon, Jul 28, 2014 at 01:43:02PM +0200, Andreas Färber wrote:
> Am 25.07.2014 12:39, schrieb Mark Brown:
> > On Fri, Jul 25, 2014 at 12:32:43PM +0200, Andreas Färber wrote:

> >> Maybe this works for you?
> >> https://patchwork.kernel.org/patch/4620191/

> >> Otherwise I can resend with Ab/Rb.

> > Documentation/SubmittingPatches.

> Not helpful here, nor is
> Documentation/devicetree/bindings/submitting-patches.txt. It's separate
> from code changes, sent via git-send-email, has a Sob, went to LKML and
> LAKML and DTML. I could add a Fixes: header and CC trivial for this
> one-line fix IIUC.

It's telling you that the kernel process is to review patches sent via
e-mail, not random links to web appliations - notice the context to
which I'm replying, I'm saying that sending a link to something on the
web isn't a good way of submitting a patch.

> Doesn't tell me which patches I should CC you on in the future though.
> Therefore my request to fix the false negative in MAINTAINERS so that me
> and other kernel newbies don't run into it again.

Feel free to submit patches if you want to see changes.
Andreas Färber July 28, 2014, 12:28 p.m. UTC | #8
Am 28.07.2014 14:20, schrieb Mark Brown:
> On Mon, Jul 28, 2014 at 01:43:02PM +0200, Andreas Färber wrote:
>> Am 25.07.2014 12:39, schrieb Mark Brown:
>>> On Fri, Jul 25, 2014 at 12:32:43PM +0200, Andreas Färber wrote:
> 
>>>> Maybe this works for you?
>>>> https://patchwork.kernel.org/patch/4620191/
> 
>>>> Otherwise I can resend with Ab/Rb.
> 
>>> Documentation/SubmittingPatches.
> 
>> Not helpful here, nor is
>> Documentation/devicetree/bindings/submitting-patches.txt. It's separate
>> from code changes, sent via git-send-email, has a Sob, went to LKML and
>> LAKML and DTML. I could add a Fixes: header and CC trivial for this
>> one-line fix IIUC.
> 
> It's telling you that the kernel process is to review patches sent via
> e-mail, not random links to web appliations - notice the context to
> which I'm replying, I'm saying that sending a link to something on the
> web isn't a good way of submitting a patch.
> 
>> Doesn't tell me which patches I should CC you on in the future though.
>> Therefore my request to fix the false negative in MAINTAINERS so that me
>> and other kernel newbies don't run into it again.
> 
> Feel free to submit patches if you want to see changes.

Dude, this patch *was* submitted and has been reviewed by two people.
Don't pretend I didn't follow the SubmittingPatches workflow.

The link I gave you contains an mbox-format link that can be used with
wget and git-am to *apply* the patch if you apparently missed it on the
mailing lists.

I will *re*-submit it for your convenience.

Andreas
Mark Brown July 28, 2014, 1:44 p.m. UTC | #9
On Mon, Jul 28, 2014 at 02:28:12PM +0200, Andreas Färber wrote:

> Dude, this patch *was* submitted and has been reviewed by two people.
> Don't pretend I didn't follow the SubmittingPatches workflow.

> The link I gave you contains an mbox-format link that can be used with
> wget and git-am to *apply* the patch if you apparently missed it on the
> mailing lists.

> I will *re*-submit it for your convenience.

Which is what I originally requested when I got CCed into the thread -
I'm being grumpy because instead of getting the patch sent to me as
requested I got sent a link to patchwork.
Andreas Färber July 28, 2014, 3:39 p.m. UTC | #10
Am 28.07.2014 15:44, schrieb Mark Brown:
> On Mon, Jul 28, 2014 at 02:28:12PM +0200, Andreas Färber wrote:
> 
>> I will *re*-submit it for your convenience.
> 
> Which is what I originally requested when I got CCed into the thread -
> I'm being grumpy because instead of getting the patch sent to me as
> requested I got sent a link to patchwork.

Hindsight, had you simply replied with "Yes, please." instead of the
non-telling "Documentation/SubmittingPatches.", you would've spared
yourself keystrokes and us four mailing list messages. ;)

Similarly, your statement about false negatives didn't really help in
resolving the not-CC'ed problem. I now know to CC you if I ever have
something for adi,axi-spdif-tx.txt again, but the next person might make
you grumpy again and potentially demotivates new contributors, so
certainly worth fixing.

Regmap, SPI, regulators, touchscreen MAINTAINERS entries seem unrelated.
Should your "SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEMENT (ASoC)"
MAINTAINERS entry get an
F: Documentation/devicetree/bindings/sound/
or is the "ANALOG DEVICES Inc ASOC DRIVERS" entry missing a line
F: sound/soc/adi/
F: Documentation/devicetree/bindings/sound/adi,axi-spdif-tx.txt
(which would then still not CC you)
or is a new MAINTAINERS section needed that CCs specifically Lars-Peter
and you on the latter?

Regards,
Andreas
Mark Brown July 28, 2014, 9:28 p.m. UTC | #11
On Mon, Jul 28, 2014 at 05:39:46PM +0200, Andreas Färber wrote:

> Similarly, your statement about false negatives didn't really help in
> resolving the not-CC'ed problem. I now know to CC you if I ever have
> something for adi,axi-spdif-tx.txt again, but the next person might make
> you grumpy again and potentially demotivates new contributors, so
> certainly worth fixing.

> Regmap, SPI, regulators, touchscreen MAINTAINERS entries seem unrelated.
> Should your "SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEMENT (ASoC)"
> MAINTAINERS entry get an
> F: Documentation/devicetree/bindings/sound/
> or is the "ANALOG DEVICES Inc ASOC DRIVERS" entry missing a line
> F: sound/soc/adi/
> F: Documentation/devicetree/bindings/sound/adi,axi-spdif-tx.txt
> (which would then still not CC you)
> or is a new MAINTAINERS section needed that CCs specifically Lars-Peter
> and you on the latter?

What you're really talking about is making MAINTAINERS complete which is
a very big job; even if only looking at the DT bindings you'd need to go
through every binding and make sure that MAINTAINERS says the same
things as it does for the matching driver code, and where there are
missing entries add them.  In the immediate case both bits are missing.

Really as with so much else it's much better advice to ask people to
look and think about what they're doing; it's the same thing as ensuring
that commit logs match the standard style for the thing being edited
(especially the subject lines) and that if there's people who normally
apply patches and work on the code they're included.
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/sound/adi,axi-spdif-tx.txt b/Documentation/devicetree/bindings/sound/adi,axi-spdif-tx.txt
index 46f3449..4eb7997 100644
--- a/Documentation/devicetree/bindings/sound/adi,axi-spdif-tx.txt
+++ b/Documentation/devicetree/bindings/sound/adi,axi-spdif-tx.txt
@@ -1,7 +1,7 @@ 
 ADI AXI-SPDIF controller
 
 Required properties:
- - compatible : Must be "adi,axi-spdif-1.00.a"
+ - compatible : Must be "adi,axi-spdif-tx-1.00.a"
  - reg : Must contain SPDIF core's registers location and length
  - clocks : Pairs of phandle and specifier referencing the controller's clocks.
    The controller expects two clocks, the clock used for the AXI interface and