Re: [PATCH V2 19/19] irqchip: add C-SKY irqchip drivers

[Guo Ren]

On Mon, Jul 02, 2018 at 09:27:13PM -0600, Rob Herring wrote:
> Commit message needed.
Ok

> Do you mean "legacy"?
Yes, it's from arch/csky/Kconfig.debug, and I'll correct it in next
version patch.
 
> It would be better to make this run-time so you can support multiple
> platforms in one build. You should be able to determine this from DT.
The CSKY_VECIRQ_LEGACY means when cpu receive the IRQ, it will directly
enter into the exception vector entry indexed by IRQ number. Just some
old SOC need the feature. We reserve it just as we've mentioned in
arch/csky/Kconfig.debug:

config CSKY_VECIRQ_LEGENCY
	bool "Use legency IRQ vector for interrupt, it's for SOC bugfix."
	help
	  It's a deprecated method for arch/csky. Don't use it, unless your
	  SOC has bug.

As we need this config to setup the vector tables in
arch/csky/kernel/traps.c, "determine this from DT" isn't suitable for us.

> > +IRQCHIP_DECLARE(csky_intc_v1, "csky,intc-v1", csky_intc_v1_init);
> 
> DT bindings must be documented. And the vendor prefix must also be
> registered in vendor-prefixes.txt.
Ok, thx for the tips. I'll follow the rules.

> > +IRQCHIP_DECLARE(csky_intc_v2, "csky,intc-v2", csky_intc_v2_init);
> 
> And this one. Use of v1, v2, etc. is generally discouraged unless
> there is some strict versioning behind it. Most bindings use
> implementation specific compatible strings (which typically means the
> SoC name/number as part of it).

> > +IRQCHIP_DECLARE(nationalchip_intc_v1_ave, "nationalchip,intc-v1,ave", intc_init);
> 
> Here too. And your timers as well.
"csky,intc-v1/v2" are used in many SOCs, just like "arm,gic-v2/v3". So may I
change them like these?:
 csky,intc-v1 >>> csky,ck807-intc
                  csky,ck810-intc
                  csky,ck860-intc

 csky,intc-v2 >>> csky,ck860-mpintc

 nationalchip,intc-v1,ave >>> nationalchip,ck610-gx6605s-intc
 
> You'll also need to do cpu bindings as well especially for SMP.
Ok, thx for tips.