From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758446AbcEFOKD (ORCPT ); Fri, 6 May 2016 10:10:03 -0400 Received: from hqemgate14.nvidia.com ([216.228.121.143]:13652 "EHLO hqemgate14.nvidia.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757915AbcEFOKA (ORCPT ); Fri, 6 May 2016 10:10:00 -0400 X-PGP-Universal: processed; by hqnvupgp07.nvidia.com on Fri, 06 May 2016 07:08:43 -0700 Subject: Re: [PATCH V3 16/17] irqchip/gic: Prepare for adding platform driver To: Marc Zyngier , Thomas Gleixner , Jason Cooper , Rob Herring , Pawel Moll , Mark Rutland , Ian Campbell , Kumar Gala , Stephen Warren , Thierry Reding References: <1462379130-11742-1-git-send-email-jonathanh@nvidia.com> <1462379130-11742-17-git-send-email-jonathanh@nvidia.com> <572B54F4.2080103@arm.com> CC: Kevin Hilman , Geert Uytterhoeven , Grygorii Strashko , Lars-Peter Clausen , Linus Walleij , , , , From: Jon Hunter Message-ID: <572CA5AF.7080504@nvidia.com> Date: Fri, 6 May 2016 15:09:51 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.7.2 MIME-Version: 1.0 In-Reply-To: <572B54F4.2080103@arm.com> X-Originating-IP: [10.21.132.133] X-ClientProxiedBy: UKMAIL102.nvidia.com (10.26.138.15) To UKMAIL101.nvidia.com (10.26.138.13) Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Marc, On 05/05/16 15:13, Marc Zyngier wrote: [...] > Gahhh. No. Please. Last time we did that, it took 6 months to untangle > the mess people made by adding their own hacks in this structure, > so I definitely want to keep it completely private, forever. Same goes > for the gic_{dist,cpu.pm}_init() functions. OK. > I've had a go at this, and came up with the following patch. I've only > briefly tested it on a host and a VM, so it is likely to break some stuff > somewhere, but you'll get the idea: The gic_chip_data struct is entirely > opaque, allocated by the GIC driver itself, with a few new fields in > it so that it becomes self-contained. This applies on top of your series. > > It should also make it easy to switch to a model where we allocate > the structure dynamically instead of the old static crap. > > Thoughts? Yes I have been doing some testing and with a couple tweaks we can make something like this work. One thing that caught me out was ... > +int gic_of_setup(struct device_node *node, struct device *dev, > + struct gic_chip_data **gicp) > +{ > + struct gic_chip_data *gic; > > - *cpu_base = of_iomap(node, 1); > - if (WARN(!*cpu_base, "unable to map gic cpu registers\n")) { > - iounmap(*dist_base); > - return -ENOMEM; > + if (!node || !gicp) > + return -EINVAL; > + > + if (dev) { > + *gicp = devm_kzalloc(dev, sizeof(*gic), GFP_KERNEL); > + if (!*gicp) > + return -ENOMEM; > } > > - if (of_property_read_u32(node, "cpu-offset", percpu_offset)) > - *percpu_offset = 0; > + gic = *gicp; > + > + gic->raw_dist_base = of_iomap(node, 0); > + if (WARN(!gic->raw_dist_base, "unable to map gic dist registers\n")) > + goto err; > + > + gic->raw_cpu_base = of_iomap(node, 1); > + if (WARN(!gic->raw_cpu_base, "unable to map gic cpu registers\n")) > + goto err; > + > + if (of_property_read_u32(node, "cpu-offset", &gic->percpu_offset)) > + gic->percpu_offset = 0; > > + gic->chip.parent_device = dev; We can't initialise the device here as it gets overwritten in the gic_init_bases. So I have had to re-organise things a bit. Good news is that I have eliminated the call from the platform driver to gic_init_bases so we only have a single call to initialise the GIC. Cheers Jon