On 18/03/16 09:13, Geert Uytterhoeven wrote:
> Hi Jon,
>
> On Thu, Mar 17, 2016 at 3:19 PM, Jon Hunter <jonathanh@xxxxxxxxxx> wrote:
>> Commit afbbd2338176 ("irqchip/gic: Document optional Clock and Power
>> Domain properties") documented optional clock and power-dmoain properties
>> for the ARM GIC. Currently, there are no users of these and for the
>> Tegra210 Audio GIC (based upon the GIC-400) there are two clocks, a
>> functional clock and interface clock, that need to be enabled.
>
> The reason that there are no users for this is twofold:
> 1. The GIC driver doesn't have Runtime PM support yet,
> 2. There was no clean way to prevent the GIC's clock from being disabled.
> Due to this, adding the clocks to the DTSes would mean that they will be
> disabled during boot up as unused clocks, leading to a system lock-up.
>
> I had hoped your series would fix part 1. I gave it a try on r8a7791/koelsch,
> but unfortunately it seems the platform driver only supports non-root
> controllers, while the r8a7791 GIC is the primary one...
Can you try making the following change ...
diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index 9e7cf7abf757..2e971e600036 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -1372,7 +1372,7 @@ static int gic_probe(struct platform_device *pdev)
void __iomem *dist_base;
void __iomem *cpu_base;
u32 percpu_offset;
- int ret, irq;
+ int ret, irq = 0;
if (dev->of_node == NULL)
return -EINVAL;
@@ -1393,11 +1393,8 @@ static int gic_probe(struct platform_device *pdev)
if (ret < 0)
goto rpm_disable;
- irq = irq_of_parse_and_map(dev->of_node, 0);
- if (!irq) {
- ret = -EINVAL;
- goto rpm_put;
- }
+ if (of_irq_count(dev->of_node) > 0)
+ irq = irq_of_parse_and_map(dev->of_node, 0);
ret = gic_of_setup(dev->of_node, &dist_base, &cpu_base, &percpu_offset);
if (ret)
@@ -1411,7 +1408,8 @@ static int gic_probe(struct platform_device *pdev)
gic->chip.parent = dev;
- irq_set_chained_handler_and_data(irq, gic_handle_cascade_irq, gic);
+ if (irq)
+ irq_set_chained_handler_and_data(irq, gic_handle_cascade_irq, gic);
pm_runtime_put(dev);
@@ -1424,7 +1422,6 @@ gic_unmap:
iounmap(cpu_base);
irq_dispose:
irq_dispose_mapping(irq);
-rpm_put:
pm_runtime_put_sync(dev);
rpm_disable:
pm_runtime_disable(dev);
> Alternatively, part 2 can to be fixed by "clk: introduce CLK_ENABLE_HAND_OFF
> flag", combined with the clock driver setting the flag when needed.
> Unfortunately that patch is not yet upstream, and not even in -next.
> Note that drivers/clk/renesas/renesas-cpg-mssr.c already handles
> CLK_ENABLE_HAND_OFF if present, and else just ignores the clock.
> So I could already add the clock to r8a7795.dtsi, which uses that driver.
>
> For older SoCs, the module clocks are described in the dtsi, and I would need a
> crude hack to enable CLK_ENABLE_HAND_OFF in the clock driver.
>
>> To allow flexibility, drop the 'clock-names' from the GIC binding and
>> just provide a list of clocks which the driver can parse. It is assumed
>> that any clocks that are listed, need to be enabled in order to access
>> the GIC.
>
> Originally I just wanted to have "clocks", and let the details be handled by
> SoC-specific code. However, Mark Rutland insisted on using the clock naming
> from the GIC TRMs, as the number of clocks and their names depend on the
> GIC variant.
>
> Apparently they also depend on the SoC...
Yes this case is a little different because the GIC is a 2nd level GIC.
Cheers
Jon
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
