Re: [RFC PATCH v2] irqchip: add support for SMP irq router

From: Marc Gonzalez <marc_gonzalez@sigmadesigns.com>
To: Sebastian Frias <sf84@laposte.net>
Cc: Marc Zyngier <marc.zyngier@arm.com>,
	Jason Cooper <jason@lakedaemon.net>,
	Thomas Gleixner <tglx@linutronix.de>,
	LKML <linux-kernel@vger.kernel.org>,
	Thibaud Cornic <thibaud_cornic@sigmadesigns.com>,
	Mason <slash.tmp@free.fr>, Mans Rullgard <mans@mansr.com>
Subject: Re: [RFC PATCH v2] irqchip: add support for SMP irq router
Date: Wed, 20 Jul 2016 11:35:00 +0200	[thread overview]
Message-ID: <578F45C4.2080005@sigmadesigns.com> (raw)
In-Reply-To: <578E37C9.8090907@laposte.net>

On 19/07/2016 16:23, Sebastian Frias wrote:
> 

I don't think there's supposed to be an empty line at the beginning of
the patch. I don't know whether 'git am' knows to zap the spurious line.

> 3) The file is called 'irq-tango_v4.c' but I think it should match the
> compatible string, so I was thinking of renaming:
> irq-tango_v4.c => irq-sigma_smp_irqrouter.c
> irq-tango_v4.h => irq-sigma_smp_irqrouter.h
> 
> What do you think?

Why is the file called irq-tango_v4? Why v4?
This is the second incarnation of a tango IRQ controller (that I know of).
Why not -v2 then?

(Could you please not mix dash and underscore? Use one or the other consistently.)

> 5) checkpatch.pl reports a few warnings:
> 
> - WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
> I have not changed the file because I would like to be settled on the naming
> first (see point 2 above)

I now have a regex pattern that matches "tango" in MAINTAINERS.
However, checkpatch only checks the current patch, not the existing
MAINTAINERS file.

>  .../interrupt-controller/sigma,smp,irqrouter.txt   |  135 ++

We've managed to use "tango" everywhere consistently (so far), please let's not
introduce the "smp" terminology at this point, it will only confuse matters.
(I use "smp87xy" to name specific boards, tango for the architecture/family.)

I found only one occurrence of the "a,b,c" syntax

$ find Documentation/devicetree/bindings -name "*,*,*"
Documentation/devicetree/bindings/net/wireless/ti,wlcore,spi.txt

Maybe we need to discuss this with the devicetree folks.

Note: Mans created
Documentation/devicetree/bindings/interrupt-controller/sigma,smp8642-intc.txt
for the v1 intc driver.

> +Required properties:
> +- compatible: Should be "sigma,smp,irqrouter".

You said in a previous message that you wanted to use a generic name.

AFAIU, the current DT trend is the opposite: to use the name of the first
board (chronologically) that used that HW incarnation.
So that would be "sigma,smp87xx-intc"
(We can fill the xx when the weenies in marketing finally decide to
announce the chip.)

The generic names you cited, like xhci, are actually naming a protocol,
which many controllers implement (because it is standard).

> +/*
> + * Copyright (C) 2014 Sebastian Frias <sf84@laposte.net>

Having discussed this topic with Thibaud, I think it is better to use
"official" email addresses for kernel submissions.

Regards.