This patch series provides exports and config tweaks to allow
the qcom-pdc, pinctrl-msm and qcom-scm drivers to be able to be
configured as permement modules (particularlly useful for the
Android Generic Kernel Image efforts).
Feedback would be appreciated!
thanks
-john
Cc: Andy Gross <[email protected]>
Cc: Bjorn Andersson <[email protected]>
Cc: Joerg Roedel <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Jason Cooper <[email protected]>
Cc: Marc Zyngier <[email protected]>
Cc: Linus Walleij <[email protected]>
Cc: Lina Iyer <[email protected]>
Cc: Saravana Kannan <[email protected]>
Cc: Todd Kjos <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
John Stultz (5):
irq: irqdomain: Export irq_domain_update_bus_token
irq: irqchip: Export irq_chip_retrigger_hierarchy and
irq_chip_set_vcpu_affinity_parent
irqchip: Allow QCOM_PDC to be loadable as a perment module
pinctrl: qcom: Allow pinctrl-msm code to be loadable as a module
firmware: QCOM_SCM: Allow qcom_scm driver to be loadable as a
permenent module
drivers/firmware/Kconfig | 2 +-
drivers/firmware/Makefile | 3 ++-
drivers/firmware/qcom_scm.c | 4 ++++
drivers/iommu/Kconfig | 2 ++
drivers/irqchip/Kconfig | 2 +-
drivers/irqchip/qcom-pdc.c | 30 ++++++++++++++++++++++++++++++
drivers/pinctrl/qcom/Kconfig | 2 +-
drivers/pinctrl/qcom/pinctrl-msm.c | 3 +++
kernel/irq/chip.c | 3 ++-
kernel/irq/irqdomain.c | 1 +
10 files changed, 47 insertions(+), 5 deletions(-)
--
2.17.1
Tweaks to allow pinctrl-msm code to be loadable as a module.
This is needed in order to support having the qcom-scm driver,
which pinctrl-msm calls into, configured as a module.
Cc: Andy Gross <[email protected]>
Cc: Bjorn Andersson <[email protected]>
Cc: Joerg Roedel <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Jason Cooper <[email protected]>
Cc: Marc Zyngier <[email protected]>
Cc: Linus Walleij <[email protected]>
Cc: Lina Iyer <[email protected]>
Cc: Saravana Kannan <[email protected]>
Cc: Todd Kjos <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Signed-off-by: John Stultz <[email protected]>
---
drivers/pinctrl/qcom/Kconfig | 2 +-
drivers/pinctrl/qcom/pinctrl-msm.c | 3 +++
2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/pinctrl/qcom/Kconfig b/drivers/pinctrl/qcom/Kconfig
index ff1ee159dca2..5a7e1bc621e6 100644
--- a/drivers/pinctrl/qcom/Kconfig
+++ b/drivers/pinctrl/qcom/Kconfig
@@ -2,7 +2,7 @@
if (ARCH_QCOM || COMPILE_TEST)
config PINCTRL_MSM
- bool
+ tristate
select PINMUX
select PINCONF
select GENERIC_PINCONF
diff --git a/drivers/pinctrl/qcom/pinctrl-msm.c b/drivers/pinctrl/qcom/pinctrl-msm.c
index 83b7d64bc4c1..54a226f682e9 100644
--- a/drivers/pinctrl/qcom/pinctrl-msm.c
+++ b/drivers/pinctrl/qcom/pinctrl-msm.c
@@ -1355,3 +1355,6 @@ int msm_pinctrl_remove(struct platform_device *pdev)
}
EXPORT_SYMBOL(msm_pinctrl_remove);
+MODULE_DESCRIPTION("Qualcomm Technologies, Inc. pinctrl-msm driver");
+MODULE_LICENSE("GPL v2");
+
--
2.17.1
Add EXPORT_SYMBOL_GPL entries for irq_chip_retrigger_hierarchy()
and irq_chip_set_vcpu_affinity_parent() so that we can allow
drivers like the qcom-pdc driver to be loadable as a module.
Cc: Andy Gross <[email protected]>
Cc: Bjorn Andersson <[email protected]>
Cc: Joerg Roedel <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Jason Cooper <[email protected]>
Cc: Marc Zyngier <[email protected]>
Cc: Linus Walleij <[email protected]>
Cc: Lina Iyer <[email protected]>
Cc: Saravana Kannan <[email protected]>
Cc: Todd Kjos <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Signed-off-by: John Stultz <[email protected]>
---
kernel/irq/chip.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
index 41e7e37a0928..ba6ce66d7ed6 100644
--- a/kernel/irq/chip.c
+++ b/kernel/irq/chip.c
@@ -1478,6 +1478,7 @@ int irq_chip_retrigger_hierarchy(struct irq_data *data)
return 0;
}
+EXPORT_SYMBOL_GPL(irq_chip_retrigger_hierarchy);
/**
* irq_chip_set_vcpu_affinity_parent - Set vcpu affinity on the parent interrupt
@@ -1492,7 +1493,7 @@ int irq_chip_set_vcpu_affinity_parent(struct irq_data *data, void *vcpu_info)
return -ENOSYS;
}
-
+EXPORT_SYMBOL_GPL(irq_chip_set_vcpu_affinity_parent);
/**
* irq_chip_set_wake_parent - Set/reset wake-up on the parent interrupt
* @data: Pointer to interrupt specific data
--
2.17.1
Allow the qcom_scm driver to be loadable as a
permenent module.
Cc: Andy Gross <[email protected]>
Cc: Bjorn Andersson <[email protected]>
Cc: Joerg Roedel <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Jason Cooper <[email protected]>
Cc: Marc Zyngier <[email protected]>
Cc: Linus Walleij <[email protected]>
Cc: Lina Iyer <[email protected]>
Cc: Saravana Kannan <[email protected]>
Cc: Todd Kjos <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Signed-off-by: John Stultz <[email protected]>
---
drivers/firmware/Kconfig | 2 +-
drivers/firmware/Makefile | 3 ++-
drivers/firmware/qcom_scm.c | 4 ++++
drivers/iommu/Kconfig | 2 ++
4 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
index fbd785dd0513..9e533a462bf4 100644
--- a/drivers/firmware/Kconfig
+++ b/drivers/firmware/Kconfig
@@ -236,7 +236,7 @@ config INTEL_STRATIX10_RSU
Say Y here if you want Intel RSU support.
config QCOM_SCM
- bool
+ tristate "Qcom SCM driver"
depends on ARM || ARM64
select RESET_CONTROLLER
diff --git a/drivers/firmware/Makefile b/drivers/firmware/Makefile
index 99510be9f5ed..cf24d674216b 100644
--- a/drivers/firmware/Makefile
+++ b/drivers/firmware/Makefile
@@ -17,7 +17,8 @@ obj-$(CONFIG_ISCSI_IBFT) += iscsi_ibft.o
obj-$(CONFIG_FIRMWARE_MEMMAP) += memmap.o
obj-$(CONFIG_RASPBERRYPI_FIRMWARE) += raspberrypi.o
obj-$(CONFIG_FW_CFG_SYSFS) += qemu_fw_cfg.o
-obj-$(CONFIG_QCOM_SCM) += qcom_scm.o qcom_scm-smc.o qcom_scm-legacy.o
+obj-$(CONFIG_QCOM_SCM) += qcom-scm.o
+qcom-scm-objs += qcom_scm.o qcom_scm-smc.o qcom_scm-legacy.o
obj-$(CONFIG_TI_SCI_PROTOCOL) += ti_sci.o
obj-$(CONFIG_TRUSTED_FOUNDATIONS) += trusted_foundations.o
obj-$(CONFIG_TURRIS_MOX_RWTM) += turris-mox-rwtm.o
diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c
index 0e7233a20f34..b5e88bf66975 100644
--- a/drivers/firmware/qcom_scm.c
+++ b/drivers/firmware/qcom_scm.c
@@ -1155,6 +1155,7 @@ static const struct of_device_id qcom_scm_dt_match[] = {
{ .compatible = "qcom,scm" },
{}
};
+MODULE_DEVICE_TABLE(of, qcom_scm_dt_match);
static struct platform_driver qcom_scm_driver = {
.driver = {
@@ -1170,3 +1171,6 @@ static int __init qcom_scm_init(void)
return platform_driver_register(&qcom_scm_driver);
}
subsys_initcall(qcom_scm_init);
+
+MODULE_DESCRIPTION("Qualcomm Technologies, Inc. SCM driver");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index b510f67dfa49..714893535dd2 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -381,6 +381,7 @@ config SPAPR_TCE_IOMMU
config ARM_SMMU
tristate "ARM Ltd. System MMU (SMMU) Support"
depends on (ARM64 || ARM || (COMPILE_TEST && !GENERIC_ATOMIC64)) && MMU
+ depends on QCOM_SCM || !QCOM_SCM #if QCOM_SCM=m this can't be =y
select IOMMU_API
select IOMMU_IO_PGTABLE_LPAE
select ARM_DMA_USE_IOMMU if ARM
@@ -500,6 +501,7 @@ config QCOM_IOMMU
# Note: iommu drivers cannot (yet?) be built as modules
bool "Qualcomm IOMMU Support"
depends on ARCH_QCOM || (COMPILE_TEST && !GENERIC_ATOMIC64)
+ depends on QCOM_SCM=y
select IOMMU_API
select IOMMU_IO_PGTABLE_LPAE
select ARM_DMA_USE_IOMMU
--
2.17.1
Allows qcom-pdc driver to be loaded as a permenent module
Also, due to the fact that IRQCHIP_DECLARE becomes a no-op when
building as a module, we have to add the platform driver hooks
explicitly.
Thanks to Saravana for his help on pointing out the
IRQCHIP_DECLARE issue and guidance on a solution.
Cc: Andy Gross <[email protected]>
Cc: Bjorn Andersson <[email protected]>
Cc: Joerg Roedel <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Jason Cooper <[email protected]>
Cc: Marc Zyngier <[email protected]>
Cc: Linus Walleij <[email protected]>
Cc: Lina Iyer <[email protected]>
Cc: Saravana Kannan <[email protected]>
Cc: Todd Kjos <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Signed-off-by: John Stultz <[email protected]>
---
drivers/irqchip/Kconfig | 2 +-
drivers/irqchip/qcom-pdc.c | 30 ++++++++++++++++++++++++++++++
2 files changed, 31 insertions(+), 1 deletion(-)
diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index 29fead208cad..12765bed08f9 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -425,7 +425,7 @@ config GOLDFISH_PIC
for Goldfish based virtual platforms.
config QCOM_PDC
- bool "QCOM PDC"
+ tristate "QCOM PDC"
depends on ARCH_QCOM
select IRQ_DOMAIN_HIERARCHY
help
diff --git a/drivers/irqchip/qcom-pdc.c b/drivers/irqchip/qcom-pdc.c
index 6ae9e1f0819d..98d74160afcd 100644
--- a/drivers/irqchip/qcom-pdc.c
+++ b/drivers/irqchip/qcom-pdc.c
@@ -11,7 +11,9 @@
#include <linux/irqdomain.h>
#include <linux/io.h>
#include <linux/kernel.h>
+#include <linux/module.h>
#include <linux/of.h>
+#include <linux/of_irq.h>
#include <linux/of_address.h>
#include <linux/of_device.h>
#include <linux/soc/qcom/irq.h>
@@ -430,4 +432,32 @@ static int qcom_pdc_init(struct device_node *node, struct device_node *parent)
return ret;
}
+#ifdef MODULE
+static int qcom_pdc_probe(struct platform_device *pdev)
+{
+ struct device_node *np = pdev->dev.of_node;
+ struct device_node *parent = of_irq_find_parent(np);
+
+ return qcom_pdc_init(np, parent);
+}
+
+static const struct of_device_id qcom_pdc_match_table[] = {
+ { .compatible = "qcom,pdc" },
+ {}
+};
+MODULE_DEVICE_TABLE(of, qcom_pdc_match_table);
+
+static struct platform_driver qcom_pdc_driver = {
+ .probe = qcom_pdc_probe,
+ .driver = {
+ .name = "qcom-pdc",
+ .of_match_table = qcom_pdc_match_table,
+ },
+};
+module_platform_driver(qcom_pdc_driver);
+#else
IRQCHIP_DECLARE(qcom_pdc, "qcom,pdc", qcom_pdc_init);
+#endif
+
+MODULE_DESCRIPTION("Qualcomm Technologies, Inc. Power Domain Controller");
+MODULE_LICENSE("GPL v2");
--
2.17.1
Add export for irq_domain_update_bus_token() so that
we can allow drivers like the qcom-pdc driver to be
loadable as a module.
Cc: Andy Gross <[email protected]>
Cc: Bjorn Andersson <[email protected]>
Cc: Joerg Roedel <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Jason Cooper <[email protected]>
Cc: Marc Zyngier <[email protected]>
Cc: Linus Walleij <[email protected]>
Cc: Lina Iyer <[email protected]>
Cc: Saravana Kannan <[email protected]>
Cc: Todd Kjos <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Signed-off-by: John Stultz <[email protected]>
---
kernel/irq/irqdomain.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index a4c2c915511d..ca974d965fda 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -281,6 +281,7 @@ void irq_domain_update_bus_token(struct irq_domain *domain,
mutex_unlock(&irq_domain_mutex);
}
+EXPORT_SYMBOL_GPL(irq_domain_update_bus_token);
/**
* irq_domain_add_simple() - Register an irq_domain and optionally map a range of irqs
--
2.17.1
Hi John,
+Will for the SMMU part.
On 2020-06-16 07:13, John Stultz wrote:
> Allow the qcom_scm driver to be loadable as a
> permenent module.
>
> Cc: Andy Gross <[email protected]>
> Cc: Bjorn Andersson <[email protected]>
> Cc: Joerg Roedel <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> Cc: Jason Cooper <[email protected]>
> Cc: Marc Zyngier <[email protected]>
> Cc: Linus Walleij <[email protected]>
> Cc: Lina Iyer <[email protected]>
> Cc: Saravana Kannan <[email protected]>
> Cc: Todd Kjos <[email protected]>
> Cc: Greg Kroah-Hartman <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: John Stultz <[email protected]>
> ---
> drivers/firmware/Kconfig | 2 +-
> drivers/firmware/Makefile | 3 ++-
> drivers/firmware/qcom_scm.c | 4 ++++
> drivers/iommu/Kconfig | 2 ++
> 4 files changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
> index fbd785dd0513..9e533a462bf4 100644
> --- a/drivers/firmware/Kconfig
> +++ b/drivers/firmware/Kconfig
> @@ -236,7 +236,7 @@ config INTEL_STRATIX10_RSU
> Say Y here if you want Intel RSU support.
>
> config QCOM_SCM
> - bool
> + tristate "Qcom SCM driver"
> depends on ARM || ARM64
> select RESET_CONTROLLER
>
> diff --git a/drivers/firmware/Makefile b/drivers/firmware/Makefile
> index 99510be9f5ed..cf24d674216b 100644
> --- a/drivers/firmware/Makefile
> +++ b/drivers/firmware/Makefile
> @@ -17,7 +17,8 @@ obj-$(CONFIG_ISCSI_IBFT) += iscsi_ibft.o
> obj-$(CONFIG_FIRMWARE_MEMMAP) += memmap.o
> obj-$(CONFIG_RASPBERRYPI_FIRMWARE) += raspberrypi.o
> obj-$(CONFIG_FW_CFG_SYSFS) += qemu_fw_cfg.o
> -obj-$(CONFIG_QCOM_SCM) += qcom_scm.o qcom_scm-smc.o qcom_scm-legacy.o
> +obj-$(CONFIG_QCOM_SCM) += qcom-scm.o
> +qcom-scm-objs += qcom_scm.o qcom_scm-smc.o qcom_scm-legacy.o
> obj-$(CONFIG_TI_SCI_PROTOCOL) += ti_sci.o
> obj-$(CONFIG_TRUSTED_FOUNDATIONS) += trusted_foundations.o
> obj-$(CONFIG_TURRIS_MOX_RWTM) += turris-mox-rwtm.o
> diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c
> index 0e7233a20f34..b5e88bf66975 100644
> --- a/drivers/firmware/qcom_scm.c
> +++ b/drivers/firmware/qcom_scm.c
> @@ -1155,6 +1155,7 @@ static const struct of_device_id
> qcom_scm_dt_match[] = {
> { .compatible = "qcom,scm" },
> {}
> };
> +MODULE_DEVICE_TABLE(of, qcom_scm_dt_match);
>
> static struct platform_driver qcom_scm_driver = {
> .driver = {
> @@ -1170,3 +1171,6 @@ static int __init qcom_scm_init(void)
> return platform_driver_register(&qcom_scm_driver);
> }
> subsys_initcall(qcom_scm_init);
> +
> +MODULE_DESCRIPTION("Qualcomm Technologies, Inc. SCM driver");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
> index b510f67dfa49..714893535dd2 100644
> --- a/drivers/iommu/Kconfig
> +++ b/drivers/iommu/Kconfig
> @@ -381,6 +381,7 @@ config SPAPR_TCE_IOMMU
> config ARM_SMMU
> tristate "ARM Ltd. System MMU (SMMU) Support"
> depends on (ARM64 || ARM || (COMPILE_TEST && !GENERIC_ATOMIC64)) &&
> MMU
> + depends on QCOM_SCM || !QCOM_SCM #if QCOM_SCM=m this can't be =y
This looks a bit ugly. Could you explain why we need this at the SMMU
level? I'd have expected the dependency to flow the other way around...
> select IOMMU_API
> select IOMMU_IO_PGTABLE_LPAE
> select ARM_DMA_USE_IOMMU if ARM
> @@ -500,6 +501,7 @@ config QCOM_IOMMU
> # Note: iommu drivers cannot (yet?) be built as modules
> bool "Qualcomm IOMMU Support"
> depends on ARCH_QCOM || (COMPILE_TEST && !GENERIC_ATOMIC64)
> + depends on QCOM_SCM=y
> select IOMMU_API
> select IOMMU_IO_PGTABLE_LPAE
> select ARM_DMA_USE_IOMMU
Thanks,
M.
--
Jazz is not dead. It just smells funny...
Hi,
On 6/16/2020 11:43 AM, John Stultz wrote:
> Allows qcom-pdc driver to be loaded as a permenent module
typo: permanent
> Also, due to the fact that IRQCHIP_DECLARE becomes a no-op when
> building as a module, we have to add the platform driver hooks
> explicitly.
>
> Thanks to Saravana for his help on pointing out the
> IRQCHIP_DECLARE issue and guidance on a solution.
>
> Cc: Andy Gross <[email protected]>
> Cc: Bjorn Andersson <[email protected]>
> Cc: Joerg Roedel <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> Cc: Jason Cooper <[email protected]>
> Cc: Marc Zyngier <[email protected]>
> Cc: Linus Walleij <[email protected]>
> Cc: Lina Iyer <[email protected]>
> Cc: Saravana Kannan <[email protected]>
> Cc: Todd Kjos <[email protected]>
> Cc: Greg Kroah-Hartman <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: John Stultz <[email protected]>
> ---
> drivers/irqchip/Kconfig | 2 +-
> drivers/irqchip/qcom-pdc.c | 30 ++++++++++++++++++++++++++++++
> 2 files changed, 31 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
> index 29fead208cad..12765bed08f9 100644
> --- a/drivers/irqchip/Kconfig
> +++ b/drivers/irqchip/Kconfig
> @@ -425,7 +425,7 @@ config GOLDFISH_PIC
> for Goldfish based virtual platforms.
>
> config QCOM_PDC
> - bool "QCOM PDC"
> + tristate "QCOM PDC"
> depends on ARCH_QCOM
> select IRQ_DOMAIN_HIERARCHY
> help
> diff --git a/drivers/irqchip/qcom-pdc.c b/drivers/irqchip/qcom-pdc.c
> index 6ae9e1f0819d..98d74160afcd 100644
> --- a/drivers/irqchip/qcom-pdc.c
> +++ b/drivers/irqchip/qcom-pdc.c
> @@ -11,7 +11,9 @@
> #include <linux/irqdomain.h>
> #include <linux/io.h>
> #include <linux/kernel.h>
> +#include <linux/module.h>
> #include <linux/of.h>
> +#include <linux/of_irq.h>
please move this include in order after of_device.h
> #include <linux/of_address.h>
> #include <linux/of_device.h>
> #include <linux/soc/qcom/irq.h>
> @@ -430,4 +432,32 @@ static int qcom_pdc_init(struct device_node *node, struct device_node *parent)
> return ret;
> }
>
> +#ifdef MODULE
> +static int qcom_pdc_probe(struct platform_device *pdev)
> +{
> + struct device_node *np = pdev->dev.of_node;
> + struct device_node *parent = of_irq_find_parent(np);
> +
> + return qcom_pdc_init(np, parent);
> +}
> +
> +static const struct of_device_id qcom_pdc_match_table[] = {
> + { .compatible = "qcom,pdc" },
> + {}
> +};
> +MODULE_DEVICE_TABLE(of, qcom_pdc_match_table);
> +
> +static struct platform_driver qcom_pdc_driver = {
> + .probe = qcom_pdc_probe,
> + .driver = {
> + .name = "qcom-pdc",
> + .of_match_table = qcom_pdc_match_table,
can you please set .suppress_bind_attrs = true,
This is to prevent bind/unbind using sysfs. Once irqchip driver module
is loaded, it shouldn't get unbind at runtime.
Thanks,
Maulik
> + },
> +};
> +module_platform_driver(qcom_pdc_driver);
> +#else
> IRQCHIP_DECLARE(qcom_pdc, "qcom,pdc", qcom_pdc_init);
> +#endif
> +
> +MODULE_DESCRIPTION("Qualcomm Technologies, Inc. Power Domain Controller");
> +MODULE_LICENSE("GPL v2");
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
On Tue, Jun 16 2020 at 05:30 -0600, Maulik Shah wrote:
>Hi,
>
>On 6/16/2020 11:43 AM, John Stultz wrote:
>>Allows qcom-pdc driver to be loaded as a permenent module
>
>typo: permanent
>
>>Also, due to the fact that IRQCHIP_DECLARE becomes a no-op when
>>building as a module, we have to add the platform driver hooks
>>explicitly.
>>
>>Thanks to Saravana for his help on pointing out the
>>IRQCHIP_DECLARE issue and guidance on a solution.
>>
>>Cc: Andy Gross <[email protected]>
>>Cc: Bjorn Andersson <[email protected]>
>>Cc: Joerg Roedel <[email protected]>
>>Cc: Thomas Gleixner <[email protected]>
>>Cc: Jason Cooper <[email protected]>
>>Cc: Marc Zyngier <[email protected]>
>>Cc: Linus Walleij <[email protected]>
>>Cc: Lina Iyer <[email protected]>
>>Cc: Saravana Kannan <[email protected]>
>>Cc: Todd Kjos <[email protected]>
>>Cc: Greg Kroah-Hartman <[email protected]>
>>Cc: [email protected]
>>Cc: [email protected]
>>Cc: [email protected]
>>Signed-off-by: John Stultz <[email protected]>
>>---
>> drivers/irqchip/Kconfig | 2 +-
>> drivers/irqchip/qcom-pdc.c | 30 ++++++++++++++++++++++++++++++
>> 2 files changed, 31 insertions(+), 1 deletion(-)
>>
>>diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
>>index 29fead208cad..12765bed08f9 100644
>>--- a/drivers/irqchip/Kconfig
>>+++ b/drivers/irqchip/Kconfig
>>@@ -425,7 +425,7 @@ config GOLDFISH_PIC
>> for Goldfish based virtual platforms.
>> config QCOM_PDC
>>- bool "QCOM PDC"
>>+ tristate "QCOM PDC"
>> depends on ARCH_QCOM
>> select IRQ_DOMAIN_HIERARCHY
>> help
>>diff --git a/drivers/irqchip/qcom-pdc.c b/drivers/irqchip/qcom-pdc.c
>>index 6ae9e1f0819d..98d74160afcd 100644
>>--- a/drivers/irqchip/qcom-pdc.c
>>+++ b/drivers/irqchip/qcom-pdc.c
>>@@ -11,7 +11,9 @@
>> #include <linux/irqdomain.h>
>> #include <linux/io.h>
>> #include <linux/kernel.h>
>>+#include <linux/module.h>
>> #include <linux/of.h>
>>+#include <linux/of_irq.h>
>please move this include in order after of_device.h
>> #include <linux/of_address.h>
>> #include <linux/of_device.h>
>> #include <linux/soc/qcom/irq.h>
>>@@ -430,4 +432,32 @@ static int qcom_pdc_init(struct device_node *node, struct device_node *parent)
>> return ret;
>> }
>>+#ifdef MODULE
>>+static int qcom_pdc_probe(struct platform_device *pdev)
>>+{
>>+ struct device_node *np = pdev->dev.of_node;
>>+ struct device_node *parent = of_irq_find_parent(np);
>>+
>>+ return qcom_pdc_init(np, parent);
>>+}
>>+
>>+static const struct of_device_id qcom_pdc_match_table[] = {
>>+ { .compatible = "qcom,pdc" },
>>+ {}
>>+};
>>+MODULE_DEVICE_TABLE(of, qcom_pdc_match_table);
>>+
>>+static struct platform_driver qcom_pdc_driver = {
>>+ .probe = qcom_pdc_probe,
>>+ .driver = {
>>+ .name = "qcom-pdc",
>>+ .of_match_table = qcom_pdc_match_table,
>
>can you please set .suppress_bind_attrs = true,
>
>This is to prevent bind/unbind using sysfs. Once irqchip driver module
>is loaded, it shouldn't get unbind at runtime.
>
That is a good point. We probably should do that to RPMH RSC driver as well.
>Thanks,
>Maulik
>>+ },
>>+};
>>+module_platform_driver(qcom_pdc_driver);
>>+#else
>> IRQCHIP_DECLARE(qcom_pdc, "qcom,pdc", qcom_pdc_init);
>>+#endif
>>+
>>+MODULE_DESCRIPTION("Qualcomm Technologies, Inc. Power Domain Controller");
>>+MODULE_LICENSE("GPL v2");
>
>--
>QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
>
On Tue, Jun 16, 2020 at 12:55 AM Marc Zyngier <[email protected]> wrote:
> On 2020-06-16 07:13, John Stultz wrote:
> > diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
> > index b510f67dfa49..714893535dd2 100644
> > --- a/drivers/iommu/Kconfig
> > +++ b/drivers/iommu/Kconfig
> > @@ -381,6 +381,7 @@ config SPAPR_TCE_IOMMU
> > config ARM_SMMU
> > tristate "ARM Ltd. System MMU (SMMU) Support"
> > depends on (ARM64 || ARM || (COMPILE_TEST && !GENERIC_ATOMIC64)) &&
> > MMU
> > + depends on QCOM_SCM || !QCOM_SCM #if QCOM_SCM=m this can't be =y
>
> This looks a bit ugly. Could you explain why we need this at the SMMU
> level? I'd have expected the dependency to flow the other way around...
Yea, so the arm-smmu-qcom.c file calls directly into the qcom-scm code
via qcom_scm_qsmmu500_wait_safe_toggle()
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/iommu/arm-smmu-qcom.c?h=v5.8-rc1#n44
So if ARM_SMMU=y and QCOM_SCM=m we get:
drivers/iommu/arm-smmu-qcom.o: In function `qcom_smmu500_reset':
arm-smmu-qcom.c:(.text+0xb4): undefined reference to
`qcom_scm_qsmmu500_wait_safe_toggle'
Do you have a suggestion for an alternative approach?
thanks
-john
On Tue, Jun 16, 2020 at 4:30 AM Maulik Shah <[email protected]> wrote:
>
> Hi,
>
> On 6/16/2020 11:43 AM, John Stultz wrote:
> > Allows qcom-pdc driver to be loaded as a permenent module
>
> typo: permanent
>
> > Also, due to the fact that IRQCHIP_DECLARE becomes a no-op when
> > building as a module, we have to add the platform driver hooks
> > explicitly.
> >
> > Thanks to Saravana for his help on pointing out the
> > IRQCHIP_DECLARE issue and guidance on a solution.
> >
> > Cc: Andy Gross <[email protected]>
> > Cc: Bjorn Andersson <[email protected]>
> > Cc: Joerg Roedel <[email protected]>
> > Cc: Thomas Gleixner <[email protected]>
> > Cc: Jason Cooper <[email protected]>
> > Cc: Marc Zyngier <[email protected]>
> > Cc: Linus Walleij <[email protected]>
> > Cc: Lina Iyer <[email protected]>
> > Cc: Saravana Kannan <[email protected]>
> > Cc: Todd Kjos <[email protected]>
> > Cc: Greg Kroah-Hartman <[email protected]>
> > Cc: [email protected]
> > Cc: [email protected]
> > Cc: [email protected]
> > Signed-off-by: John Stultz <[email protected]>
> > ---
> > drivers/irqchip/Kconfig | 2 +-
> > drivers/irqchip/qcom-pdc.c | 30 ++++++++++++++++++++++++++++++
> > 2 files changed, 31 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
> > index 29fead208cad..12765bed08f9 100644
> > --- a/drivers/irqchip/Kconfig
> > +++ b/drivers/irqchip/Kconfig
> > @@ -425,7 +425,7 @@ config GOLDFISH_PIC
> > for Goldfish based virtual platforms.
> >
> > config QCOM_PDC
> > - bool "QCOM PDC"
> > + tristate "QCOM PDC"
> > depends on ARCH_QCOM
> > select IRQ_DOMAIN_HIERARCHY
> > help
> > diff --git a/drivers/irqchip/qcom-pdc.c b/drivers/irqchip/qcom-pdc.c
> > index 6ae9e1f0819d..98d74160afcd 100644
> > --- a/drivers/irqchip/qcom-pdc.c
> > +++ b/drivers/irqchip/qcom-pdc.c
> > @@ -11,7 +11,9 @@
> > #include <linux/irqdomain.h>
> > #include <linux/io.h>
> > #include <linux/kernel.h>
> > +#include <linux/module.h>
> > #include <linux/of.h>
> > +#include <linux/of_irq.h>
> please move this include in order after of_device.h
> > #include <linux/of_address.h>
> > #include <linux/of_device.h>
> > #include <linux/soc/qcom/irq.h>
> > @@ -430,4 +432,32 @@ static int qcom_pdc_init(struct device_node *node, struct device_node *parent)
> > return ret;
> > }
> >
> > +#ifdef MODULE
> > +static int qcom_pdc_probe(struct platform_device *pdev)
> > +{
> > + struct device_node *np = pdev->dev.of_node;
> > + struct device_node *parent = of_irq_find_parent(np);
> > +
> > + return qcom_pdc_init(np, parent);
> > +}
> > +
> > +static const struct of_device_id qcom_pdc_match_table[] = {
> > + { .compatible = "qcom,pdc" },
> > + {}
> > +};
> > +MODULE_DEVICE_TABLE(of, qcom_pdc_match_table);
> > +
> > +static struct platform_driver qcom_pdc_driver = {
> > + .probe = qcom_pdc_probe,
> > + .driver = {
> > + .name = "qcom-pdc",
> > + .of_match_table = qcom_pdc_match_table,
>
> can you please set .suppress_bind_attrs = true,
>
> This is to prevent bind/unbind using sysfs. Once irqchip driver module
> is loaded, it shouldn't get unbind at runtime.
Thanks, I really appreciate the review! I've made these changes on my
side and they'll be included in v2.
thanks
-john
On Tue, Jun 16, 2020 at 8:13 AM John Stultz <[email protected]> wrote:
> Tweaks to allow pinctrl-msm code to be loadable as a module.
> This is needed in order to support having the qcom-scm driver,
> which pinctrl-msm calls into, configured as a module.
>
> Cc: Andy Gross <[email protected]>
> Cc: Bjorn Andersson <[email protected]>
> Cc: Joerg Roedel <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> Cc: Jason Cooper <[email protected]>
> Cc: Marc Zyngier <[email protected]>
> Cc: Linus Walleij <[email protected]>
> Cc: Lina Iyer <[email protected]>
> Cc: Saravana Kannan <[email protected]>
> Cc: Todd Kjos <[email protected]>
> Cc: Greg Kroah-Hartman <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: John Stultz <[email protected]>
Unless there are dependencies on the irqchip patches I can apply
this if Bjorn is OK with it.
Yours,
Linus Walleij
On Mon 15 Jun 23:13 PDT 2020, John Stultz wrote:
> Tweaks to allow pinctrl-msm code to be loadable as a module.
> This is needed in order to support having the qcom-scm driver,
> which pinctrl-msm calls into, configured as a module.
>
This means that we need a "depends on QCOM_SCM || QCOM_SCM=n" on all
entries in the Kconfig that selects PINCTRL_MSM, or switch PINCTRL_MSM
to be user selectable and make all the others depend on it.
> Cc: Andy Gross <[email protected]>
> Cc: Bjorn Andersson <[email protected]>
> Cc: Joerg Roedel <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> Cc: Jason Cooper <[email protected]>
> Cc: Marc Zyngier <[email protected]>
> Cc: Linus Walleij <[email protected]>
> Cc: Lina Iyer <[email protected]>
> Cc: Saravana Kannan <[email protected]>
> Cc: Todd Kjos <[email protected]>
> Cc: Greg Kroah-Hartman <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: John Stultz <[email protected]>
> ---
> drivers/pinctrl/qcom/Kconfig | 2 +-
> drivers/pinctrl/qcom/pinctrl-msm.c | 3 +++
> 2 files changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/pinctrl/qcom/Kconfig b/drivers/pinctrl/qcom/Kconfig
> index ff1ee159dca2..5a7e1bc621e6 100644
> --- a/drivers/pinctrl/qcom/Kconfig
> +++ b/drivers/pinctrl/qcom/Kconfig
> @@ -2,7 +2,7 @@
> if (ARCH_QCOM || COMPILE_TEST)
>
> config PINCTRL_MSM
> - bool
> + tristate
> select PINMUX
> select PINCONF
> select GENERIC_PINCONF
> diff --git a/drivers/pinctrl/qcom/pinctrl-msm.c b/drivers/pinctrl/qcom/pinctrl-msm.c
> index 83b7d64bc4c1..54a226f682e9 100644
> --- a/drivers/pinctrl/qcom/pinctrl-msm.c
> +++ b/drivers/pinctrl/qcom/pinctrl-msm.c
> @@ -1355,3 +1355,6 @@ int msm_pinctrl_remove(struct platform_device *pdev)
> }
> EXPORT_SYMBOL(msm_pinctrl_remove);
>
> +MODULE_DESCRIPTION("Qualcomm Technologies, Inc. pinctrl-msm driver");
It's the "Qualcomm Technologies, Inc. TLMM driver"
> +MODULE_LICENSE("GPL v2");
> +
Please don't retain my empty line at the end of this file :)
Regards,
Bjorn
> --
> 2.17.1
>
On Sat, Jun 20, 2020 at 11:03 PM Bjorn Andersson
<[email protected]> wrote:
>
> On Mon 15 Jun 23:13 PDT 2020, John Stultz wrote:
>
> > Tweaks to allow pinctrl-msm code to be loadable as a module.
> > This is needed in order to support having the qcom-scm driver,
> > which pinctrl-msm calls into, configured as a module.
> >
>
> This means that we need a "depends on QCOM_SCM || QCOM_SCM=n" on all
> entries in the Kconfig that selects PINCTRL_MSM, or switch PINCTRL_MSM
> to be user selectable and make all the others depend on it.
Oh, good point! I already had to fix that in a different tree, but
forgot to move the fix over to my upstreaming tree.
> >
> > +MODULE_DESCRIPTION("Qualcomm Technologies, Inc. pinctrl-msm driver");
>
> It's the "Qualcomm Technologies, Inc. TLMM driver"
>
> > +MODULE_LICENSE("GPL v2");
> > +
>
> Please don't retain my empty line at the end of this file :)
Done and done. Thanks so much for the review!
-john
On Tue, Jun 16, 2020 at 01:52:32PM -0700, John Stultz wrote:
> On Tue, Jun 16, 2020 at 12:55 AM Marc Zyngier <[email protected]> wrote:
> > On 2020-06-16 07:13, John Stultz wrote:
> > > diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
> > > index b510f67dfa49..714893535dd2 100644
> > > --- a/drivers/iommu/Kconfig
> > > +++ b/drivers/iommu/Kconfig
> > > @@ -381,6 +381,7 @@ config SPAPR_TCE_IOMMU
> > > config ARM_SMMU
> > > tristate "ARM Ltd. System MMU (SMMU) Support"
> > > depends on (ARM64 || ARM || (COMPILE_TEST && !GENERIC_ATOMIC64)) &&
> > > MMU
> > > + depends on QCOM_SCM || !QCOM_SCM #if QCOM_SCM=m this can't be =y
> >
> > This looks a bit ugly. Could you explain why we need this at the SMMU
> > level? I'd have expected the dependency to flow the other way around...
>
> Yea, so the arm-smmu-qcom.c file calls directly into the qcom-scm code
> via qcom_scm_qsmmu500_wait_safe_toggle()
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/iommu/arm-smmu-qcom.c?h=v5.8-rc1#n44
>
> So if ARM_SMMU=y and QCOM_SCM=m we get:
> drivers/iommu/arm-smmu-qcom.o: In function `qcom_smmu500_reset':
> arm-smmu-qcom.c:(.text+0xb4): undefined reference to
> `qcom_scm_qsmmu500_wait_safe_toggle'
>
> Do you have a suggestion for an alternative approach?
Can you use symbol_get() or something like that? How are module dependencies
handled by other drivers?
Will
On 03-07-20, 13:23, Will Deacon wrote:
> On Tue, Jun 16, 2020 at 01:52:32PM -0700, John Stultz wrote:
> > On Tue, Jun 16, 2020 at 12:55 AM Marc Zyngier <[email protected]> wrote:
> > > On 2020-06-16 07:13, John Stultz wrote:
> > > > diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
> > > > index b510f67dfa49..714893535dd2 100644
> > > > --- a/drivers/iommu/Kconfig
> > > > +++ b/drivers/iommu/Kconfig
> > > > @@ -381,6 +381,7 @@ config SPAPR_TCE_IOMMU
> > > > config ARM_SMMU
> > > > tristate "ARM Ltd. System MMU (SMMU) Support"
> > > > depends on (ARM64 || ARM || (COMPILE_TEST && !GENERIC_ATOMIC64)) &&
> > > > MMU
> > > > + depends on QCOM_SCM || !QCOM_SCM #if QCOM_SCM=m this can't be =y
> > >
> > > This looks a bit ugly. Could you explain why we need this at the SMMU
> > > level? I'd have expected the dependency to flow the other way around...
> >
> > Yea, so the arm-smmu-qcom.c file calls directly into the qcom-scm code
> > via qcom_scm_qsmmu500_wait_safe_toggle()
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/iommu/arm-smmu-qcom.c?h=v5.8-rc1#n44
> >
> > So if ARM_SMMU=y and QCOM_SCM=m we get:
> > drivers/iommu/arm-smmu-qcom.o: In function `qcom_smmu500_reset':
> > arm-smmu-qcom.c:(.text+0xb4): undefined reference to
> > `qcom_scm_qsmmu500_wait_safe_toggle'
> >
> > Do you have a suggestion for an alternative approach?
>
> Can you use symbol_get() or something like that? How are module dependencies
> handled by other drivers?
So drivers deal with this by making rules in Kconfig which will prohibit
this case. QCOM_SCM depends on ARM_SMMU with the caveat that if ARM_SMMU
is a module, QCOM_SCM cant be inbuilt.
This can be done by adding below line in Kconfig for QCOM_SCM:
depends on ARM_SMMU || !ARM_SMMU
This is quite prevalent is drivers to ensure dependency like this
Thanks
--
~Vinod