Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S966244AbbD2OJi (ORCPT ); Wed, 29 Apr 2015 10:09:38 -0400 Received: from mail-ob0-f176.google.com ([209.85.214.176]:34083 "EHLO mail-ob0-f176.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S966122AbbD2OJg (ORCPT ); Wed, 29 Apr 2015 10:09:36 -0400 MIME-Version: 1.0 In-Reply-To: <20150429125703.GB11757@leverpostej> References: <1430146811-29862-1-git-send-email-geert+renesas@glider.be> <20150427155427.GB16191@leverpostej> <20150427171502.GD16191@leverpostej> <20150429125703.GB11757@leverpostej> Date: Wed, 29 Apr 2015 16:09:35 +0200 X-Google-Sender-Auth: zX-OYj30A6MI6kZScZrDDXuAMBs Message-ID: Subject: Re: [PATCH] ARM: gic: Document Power and Clock Domain optional properties From: Geert Uytterhoeven To: Mark Rutland Cc: Geert Uytterhoeven , Rob Herring , Pawel Moll , Ian Campbell , Kumar Gala , Marc Zyngier , "devicetree@vger.kernel.org" , "linux-pm@vger.kernel.org" , "linux-arm-kernel@lists.infradead.org" , "linux-sh@vger.kernel.org" , "linux-kernel@vger.kernel.org" , Magnus Damm Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5278 Lines: 144 Hi Mark, On Wed, Apr 29, 2015 at 2:57 PM, Mark Rutland wrote: >> >> >> To preserve DT stability, we would like to add these properties to the >> >> >> affected shmobile dtsi files. >> >> > >> >> > ... which means that they could be wrong, and will get in the way of >> >> > stability rather than aiding it. >> >> >> >> We do know the GIC is part of the power domain, and has a controllable >> >> clock (on the affected SoCs). >> > >> > ... my concern is that the data we place into the DT will be untested >> > given that we don't have software relying on it. If said data is not >> > correct, it is harmful to have, especially for such fundamental >> > properties. >> >> Your statement challenges the viability of Stable DT Requirements, as we >> can thus never write a DTS until the full software implementation has been >> completed ;-) > > I appreciate this is difficult, but I disagree that it's impossible ;) > > If you don't want to do clock management currently, don't describe the > clock controller, have some FW/loader pre-program the clocks, and list > fixed-clocks in the DTB. This DTB should continue to work, but a new > kernel alone won't give you fancy clock management. This is what we > expect in terms of stable DTBs. > > When you add clock controller support, you need a different DT to > describe the clock controller anyway. You can have it nuke the clock Currently we have the clock controller in the DTS. It only lacks a few clocks (like INTC-SYS -> GIC). > configuration at boot time as a test that everything you need is > described. Don't worry, I have early boot code that disables all non-critical clocks. This already caught many bugs (missing clock handling). It also caused a bug, as I missed an interrupt storm due to a disabled clock ;-) >> >> > I'm also concerned that the carving up of clock inputs, power domains, >> >> > and other physical details is implementation-specific. I imagine that >> >> > pretty much every user that will care about this is using GIC-400, so >> >> > could we make this specific to GIC-400? >> >> >> >> I have no idea which GIC version is being used. >> > >> > This is unfortunate. >> > >> >> This is for Renesas R-Mobile APE6 (r8a73a4) and various R-Car Gen2. >> >> According to the DTS, they're compatible with "arm,cortex-a15-gic" and >> >> "arm,cortex-a7-gic", and work with that value. >> > >> > Who put the DT together in the first place? >> >> Magnus (added to CC). >> >> > If it's a multi-cluster SoC then we know that we're not using any >> > built-in distributor... >> >> R-Mobile APE6 has 2 clusters (4xCA15 and 4xCA7). >> R-Car Gen2 has 1 or 2 clusters (2/4xCA15 and/or 2/4xCA7). > > It looks like we should be able to read the GICD_IIDR to figure out what > imlpementation is used. Could you see what GICD_IIDR reports on those > platforms? There's a patch at the end of the email to do so. Thanks! - R-Mobile APE6 (r8a73a4): GICD_IIDR reports 0x0200043b - R-Car Gen M2-W (r8a7791): GICD_IIDR reports 0x0200043b ProductID = 0x02 (GIC-400) Variant = 0x0 Revision = 0x0 Implementer = 0x43b (ARM) So both are GIC-400 (in the mean time I found a reference to GIC-400 in the APE6 docs). I also ran it on a few other SoCs: - SH-Mobile AG5 (sh73a0): GICD_IIDR reports 0x0102043b Implementation version = 0x01 (Cortex-A9 MPCore) Revision number = 0x020 Implementer = 0x43b (ARM) Which GIC is that? - R-Mobile A1 (r8a7740): GICD_IIDR reports 0x0000043b ProductID = 0x00 (GIC-500 or Cortex-A15 MPCore or PL390) Variant = 0x0 Revision = 0x0 Implementer = 0x43b (ARM) Seems like 0x0000043b can mean multiple things. Docs say PL390 r0p0. > diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c > index 7b315e3..02c8bb4 100644 > --- a/drivers/irqchip/irq-gic.c > +++ b/drivers/irqchip/irq-gic.c > @@ -959,6 +959,7 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start, > irq_hw_number_t hwirq_base; > struct gic_chip_data *gic; > int gic_irqs, irq_base, i; > + unsigned long iidr; > > BUG_ON(gic_nr >= MAX_GIC_NR); > > @@ -996,6 +997,9 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start, > gic_set_base_accessor(gic, gic_get_common_base); > } > > + iidr = readl_relaxed(gic_data_dist_base(gic) + GIC_DIST_IIDR); > + pr_info("GICD_IIDR reports 0x%08lx\n", (unsigned long)iidr); No need for the cast. Perhaps just pr_debug("GICD_IIDR reports 0x%08lx\n", readl_relaxed(gic_data_dist_base(gic) + GIC_DIST_IIDR)); ? Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/