On 13/06/16 10:58, Jon Hunter wrote: > Hi Marc, > > On 13/06/16 10:10, Marc Zyngier wrote: >> Hi Jon, >> >> On 07/06/16 16:12, Jon Hunter wrote: >>> Add a platform driver to support non-root GICs that require runtime >>> power-management. Currently, only non-root GICs are supported because >>> the functions, smp_cross_call() and set_handle_irq(), that need to >>> be called for a root controller are located in the __init section and >>> so cannot be called by the platform driver. >>> >>> The GIC platform driver re-uses many functions from the existing GIC >>> driver including some functions to save and restore the GIC context >>> during power transitions. The functions for saving and restoring the >>> GIC context are currently only defined if CONFIG_CPU_PM is enabled and >>> to ensure that these functions are always defined when the platform >>> driver is enabled, a dependency on CONFIG_ARM_GIC_PM (which selects the >>> platform driver) has been added. >>> >>> In order to re-use the private GIC initialisation code, a new public >>> function, gic_of_init_child(), has been added which calls various >>> private functions to initialise the GIC. This is different from the >>> existing gic_of_init() because it only supports non-root GICs (ie. does >>> not call smp_cross_call() is set_handle_irq()) and is not located in >>> the __init section (so can be used by platform drivers). Furthermore, >>> gic_of_init_child() dynamically allocates memory for the GIC chip data >>> which is also different from gic_of_init(). >>> >>> There is no specific suspend handling for GICs registered as platform >>> devices. Non-wakeup interrupts will be disabled by the kernel during >>> late suspend, however, this alone will not power down the GIC if >>> interrupts have been requested and not freed. Therefore, requestors of >>> non-wakeup interrupts will need to free them on entering suspend in >>> order to power-down the GIC. >>> >>> Signed-off-by: Jon Hunter <jonathanh@xxxxxxxxxx> >>> --- >>> drivers/irqchip/Kconfig | 6 ++ >>> drivers/irqchip/Makefile | 1 + >>> drivers/irqchip/irq-gic-pm.c | 184 ++++++++++++++++++++++++++++++++++++++++ >>> drivers/irqchip/irq-gic.c | 38 ++++++++- >>> include/linux/irqchip/arm-gic.h | 6 ++ >>> 5 files changed, 232 insertions(+), 3 deletions(-) >>> create mode 100644 drivers/irqchip/irq-gic-pm.c >>> >>> diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig >>> index fa33c50b0e5a..5495a5ba8039 100644 >>> --- a/drivers/irqchip/Kconfig >>> +++ b/drivers/irqchip/Kconfig >>> @@ -8,6 +8,12 @@ config ARM_GIC >>> select IRQ_DOMAIN_HIERARCHY >>> select MULTI_IRQ_HANDLER >>> >>> +config ARM_GIC_PM >>> + bool >>> + depends on PM >>> + select ARM_GIC >>> + select PM_CLK >>> + >>> config ARM_GIC_MAX_NR >>> int >>> default 2 if ARCH_REALVIEW >>> diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile >>> index 38853a187607..bd0257e0aab6 100644 >>> --- a/drivers/irqchip/Makefile >>> +++ b/drivers/irqchip/Makefile >>> @@ -24,6 +24,7 @@ obj-$(CONFIG_ARCH_SUNXI) += irq-sun4i.o >>> obj-$(CONFIG_ARCH_SUNXI) += irq-sunxi-nmi.o >>> obj-$(CONFIG_ARCH_SPEAR3XX) += spear-shirq.o >>> obj-$(CONFIG_ARM_GIC) += irq-gic.o irq-gic-common.o >>> +obj-$(CONFIG_ARM_GIC_PM) += irq-gic-pm.o >>> obj-$(CONFIG_REALVIEW_DT) += irq-gic-realview.o >>> obj-$(CONFIG_ARM_GIC_V2M) += irq-gic-v2m.o >>> obj-$(CONFIG_ARM_GIC_V3) += irq-gic-v3.o irq-gic-common.o >>> diff --git a/drivers/irqchip/irq-gic-pm.c b/drivers/irqchip/irq-gic-pm.c >>> new file mode 100644 >>> index 000000000000..4cbffba3ff13 >>> --- /dev/null >>> +++ b/drivers/irqchip/irq-gic-pm.c >>> @@ -0,0 +1,184 @@ >>> +/* >>> + * Copyright (C) 2016 NVIDIA CORPORATION, All Rights Reserved. >>> + * >>> + * This program is free software; you can redistribute it and/or modify >>> + * it under the terms of the GNU General Public License version 2 as >>> + * published by the Free Software Foundation. >>> + * >>> + * This program is distributed in the hope that it will be useful, >>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >>> + * GNU General Public License for more details. >>> + * >>> + * You should have received a copy of the GNU General Public License >>> + * along with this program. If not, see <http://www.gnu.org/licenses/>. >>> + */ >>> +#include <linux/module.h> >>> +#include <linux/clk.h> >>> +#include <linux/of_device.h> >>> +#include <linux/of_irq.h> >>> +#include <linux/irqchip/arm-gic.h> >>> +#include <linux/platform_device.h> >>> +#include <linux/pm_clock.h> >>> +#include <linux/pm_runtime.h> >>> +#include <linux/slab.h> >>> + >>> +struct gic_clk_data { >>> + unsigned int num_clocks; >>> + const char *const *clocks; >>> +}; >>> + >>> +static int gic_runtime_resume(struct device *dev) >>> +{ >>> + struct gic_chip_data *gic = dev_get_drvdata(dev); >>> + int ret; >>> + >>> + ret = pm_clk_resume(dev); >>> + if (ret) >>> + return ret; >>> + >>> + /* >>> + * On the very first resume, the pointer to the driver data >>> + * will be NULL and this is intentional, because we do not >>> + * want to restore the GIC on the very first resume. So if >>> + * the pointer is not valid just return. >>> + */ >>> + if (!gic) >>> + return 0; >>> + >>> + gic_dist_restore(gic); >>> + gic_cpu_restore(gic); >>> + >>> + return 0; >>> +} >>> + >>> +static int gic_runtime_suspend(struct device *dev) >>> +{ >>> + struct gic_chip_data *gic = dev_get_drvdata(dev); >>> + >>> + gic_dist_save(gic); >>> + gic_cpu_save(gic); >>> + >>> + return pm_clk_suspend(dev); >>> +} >>> + >>> +static int gic_get_clocks(struct device *dev, const struct gic_clk_data *data) >>> +{ >>> + struct clk *clk; >>> + unsigned int i; >>> + int ret; >>> + >>> + if (!dev || !data) >>> + return -EINVAL; >>> + >>> + ret = pm_clk_create(dev); >>> + if (ret) >>> + return ret; >>> + >>> + for (i = 0; i < data->num_clocks; i++) { >>> + clk = of_clk_get_by_name(dev->of_node, data->clocks[i]); >>> + if (IS_ERR(clk)) { >>> + dev_err(dev, "failed to get clock %s\n", >>> + data->clocks[i]); >>> + ret = PTR_ERR(clk); >>> + goto error; >>> + } >>> + >>> + ret = pm_clk_add_clk(dev, clk); >>> + if (ret) { >>> + dev_err(dev, "failed to add clock at index %d\n", i); >>> + clk_put(clk); >>> + goto error; >>> + } >>> + } >>> + >>> + return 0; >>> + >>> +error: >>> + pm_clk_destroy(dev); >>> + >>> + return ret; >>> +} >>> + >>> +static int gic_probe(struct platform_device *pdev) >>> +{ >>> + struct device *dev = &pdev->dev; >>> + const struct gic_clk_data *data; >>> + struct gic_chip_data *gic; >>> + int ret, irq; >>> + >>> + data = of_device_get_match_data(&pdev->dev); >>> + if (!data) { >>> + dev_err(&pdev->dev, "no device match found\n"); >>> + return -ENODEV; >>> + } >>> + >>> + irq = irq_of_parse_and_map(dev->of_node, 0); >>> + if (!irq) { >>> + dev_err(dev, "no parent interrupt found!\n"); >>> + return -EINVAL; >>> + } >>> + >>> + ret = gic_get_clocks(dev, data); >>> + if (ret) >>> + goto irq_dispose; >>> + >>> + pm_runtime_enable(dev); >>> + >>> + ret = pm_runtime_get_sync(dev); >>> + if (ret < 0) >>> + goto rpm_disable; >>> + >>> + ret = gic_of_init_child(dev, &gic, irq); >>> + if (ret) >>> + goto rpm_put; >>> + >>> + platform_set_drvdata(pdev, gic); >>> + >>> + pm_runtime_put(dev); >>> + >>> + dev_info(dev, "GIC IRQ controller registered\n"); >>> + >>> + return 0; >>> + >>> +rpm_put: >>> + pm_runtime_put_sync(dev); >>> +rpm_disable: >>> + pm_runtime_disable(dev); >>> + pm_clk_destroy(dev); >>> +irq_dispose: >>> + irq_dispose_mapping(irq); >>> + >>> + return ret; >>> +} >>> + >>> +static const struct dev_pm_ops gic_pm_ops = { >>> + SET_RUNTIME_PM_OPS(gic_runtime_suspend, >>> + gic_runtime_resume, NULL) >>> +}; >>> + >>> +static const char * const gic400_clocks[] = { >>> + "clk", >>> +}; >>> + >>> +static const struct gic_clk_data gic400_data = { >>> + .num_clocks = ARRAY_SIZE(gic400_clocks), >>> + .clocks = gic400_clocks, >>> +}; >>> + >>> +static const struct of_device_id gic_match[] = { >>> + { .compatible = "nvidia,tegra210-agic", .data = &gic400_data }, >>> + {}, >>> +}; >>> +MODULE_DEVICE_TABLE(of, gic_match); >>> + >>> +static struct platform_driver gic_driver = { >>> + .probe = gic_probe, >>> + .driver = { >>> + .name = "gic", >>> + .of_match_table = gic_match, >>> + .pm = &gic_pm_ops, >>> + } >>> +}; >>> + >>> +builtin_platform_driver(gic_driver); >>> diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c >>> index 141ea5801784..57423f3b218b 100644 >>> --- a/drivers/irqchip/irq-gic.c >>> +++ b/drivers/irqchip/irq-gic.c >>> @@ -75,7 +75,7 @@ struct gic_chip_data { >>> void __iomem *raw_dist_base; >>> void __iomem *raw_cpu_base; >>> u32 percpu_offset; >>> -#ifdef CONFIG_CPU_PM >>> +#if defined(CONFIG_CPU_PM) || defined(CONFIG_ARM_GIC_PM) >>> u32 saved_spi_enable[DIV_ROUND_UP(1020, 32)]; >>> u32 saved_spi_active[DIV_ROUND_UP(1020, 32)]; >>> u32 saved_spi_conf[DIV_ROUND_UP(1020, 16)]; >>> @@ -528,7 +528,7 @@ int gic_cpu_if_down(unsigned int gic_nr) >>> return 0; >>> } >>> >>> -#ifdef CONFIG_CPU_PM >>> +#if defined(CONFIG_CPU_PM) || defined(CONFIG_ARM_GIC_PM) >>> /* >>> * Saves the GIC distributor registers during suspend or idle. Must be called >>> * with interrupts disabled but before powering down the GIC. After calling >>> @@ -1296,6 +1296,34 @@ error: >>> return -ENOMEM; >>> } >>> >>> +int gic_of_init_child(struct device *dev, struct gic_chip_data **gic, int irq) >> >> I got this: >> >> WARNING: drivers/built-in.o(.text+0x2464): Section mismatch in reference from the function gic_of_init_child() to the function .init.text:gic_of_setup() >> The function gic_of_init_child() references >> the function __init gic_of_setup(). >> This is often because gic_of_init_child lacks a __init >> annotation or the annotation of gic_of_setup is wrong. > > Ugh! A lot of the changes were to avoid these section mismatches. Somehow I missed that one :-( > >> I've fixed it locally by making the function __init. > > So gic_of_init_child() cannot be __init because it is called by the driver. If you do this you should get ... > > WARNING: drivers/irqchip/built-in.o(.text+0x2314): Section mismatch in reference from the function gic_probe() to the function .init.text:gic_of_init_child() > The function gic_probe() references > the function __init gic_of_init_child(). > This is often because gic_probe lacks a __init > annotation or the annotation of gic_of_init_child is wrong. Odd, I'm not getting that one... [digging] OK, that's because ARM_GIC_PM is never selected (and there is no way I can actually select). Do you have some additional patches that selects it for Tegra? > > > The correct fix is ... > > diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c > index 57423f3b218b..1de07eb5839c 100644 > --- a/drivers/irqchip/irq-gic.c > +++ b/drivers/irqchip/irq-gic.c > @@ -1272,7 +1272,7 @@ static bool gic_check_eoimode(struct device_node *node, void __iomem **base) > return true; > } > > -static int __init gic_of_setup(struct gic_chip_data *gic, struct device_node *node) > +static int gic_of_setup(struct gic_chip_data *gic, struct device_node *node) > { > if (!gic || !node) > return -EINVAL; OK, I've applied this locally (no need to resend). Thanks, M. -- Jazz is not dead. It just smells funny... -- 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
