lkml.org 
[lkml]   [2016]   [May]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH V3 16/17] irqchip/gic: Prepare for adding platform driver
From
Date
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

\
 
 \ /
  Last update: 2016-05-06 16:21    [W:0.132 / U:0.076 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site