ACPI platform MSI, interrupt producer/consumer and its example mbi-gen

From: Hanjun Guo <[email protected]>

With the platform msi domain created, we can set up the msi domain for a platform device when it's probed.

This patch introduces acpi_configure_msi_domain(), which retrieves the domain from iort and set it to platform device.

As some platform devices such as an irqchip needs the msi irqdomain to be the interrupt parent domain, we need to get irqdomain before platform device is probed.

Cc Marc Zyngier <[email protected]>

Cc Greg KH <[email protected]>

Cc Thomas Gleixner <[email protected]>

Cc Bjorn Helgaas <[email protected]>

Cc Lorenzo Pieralisi <[email protected]>

Cc Tomasz Nowicki <[email protected]>

Signed-off-by Hanjun Guo <[email protected]>

---
 drivers/acpi/arm64/iort.c   |  5 ++++-
 drivers/base/platform-msi.c | 15 ++++++++++++++-
 drivers/base/platform.c     |  2 ++
 include/linux/msi.h         |  1 +
 4 files changed, 21 insertions(+), 2 deletions(-)

drivers/acpi/arm64/iort.c +4 -1

index 13a1905..bccd3cc 100644
@@ -478,6 +478,7 @@ struct irq_domain *iort_get_device_domain(struct device *dev, u32 req_id)
 {
 	struct fwnode_handle *handle;
 	int its_id;
+	enum irq_domain_bus_token bus_token;
 
 	if (iort_dev_find_its_id(dev, req_id, 0, &its_id))
 		return NULL;

@@ -486,7 +487,9 @@ struct irq_domain *iort_get_device_domain(struct device *dev, u32 req_id)
 	if (!handle)
 		return NULL;
 
-	return irq_find_matching_fwnode(handle, DOMAIN_BUS_PCI_MSI);
+	bus_token = dev_is_pci(dev) ?
+			DOMAIN_BUS_PCI_MSI : DOMAIN_BUS_PLATFORM_MSI;
+	return irq_find_matching_fwnode(handle, bus_token);
 }
 
 static int __get_pci_rid(struct pci_dev *pdev, u16 alias, void *data)

drivers/base/platform-msi.c +1 -1

index 279e539..f6eae18 100644
@@ -17,8 +17,8 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
+#include <linux/acpi_iort.h>
 #include <linux/device.h>
-#include <linux/idr.h>
 #include <linux/irq.h>
 #include <linux/irqdomain.h>
 #include <linux/msi.h>

drivers/base/platform.c +1 -0

index 6482d47..ea01a37 100644
@@ -24,6 +24,7 @@
 #include <linux/pm_domain.h>
 #include <linux/idr.h>
 #include <linux/acpi.h>
+#include <linux/msi.h>
 #include <linux/clk/clk-conf.h>
 #include <linux/limits.h>
 #include <linux/property.h>

drivers/base/platform.c +0 -0

 	if (pdevinfo->dma_mask) {
 		/*
 		 * This memory isn't freed when the device is put,

include/linux/msi.h +0 -0

index e8c81fb..1e93a78 100644

Quoted 65 lines · Hanjun Guo

From: Hanjun Guo <[email protected]>

With the platform msi domain created, we can set up the msi domain for a platform device when it's probed.

This patch introduces acpi_configure_msi_domain(), which retrieves the domain from iort and set it to platform device.

As some platform devices such as an irqchip needs the msi irqdomain to be the interrupt parent domain, we need to get irqdomain before platform device is probed.

Cc Marc Zyngier <[email protected]>

Cc Greg KH <[email protected]>

Cc Thomas Gleixner <[email protected]>

Cc Bjorn Helgaas <[email protected]>

Cc Lorenzo Pieralisi <[email protected]>

Cc Tomasz Nowicki <[email protected]>

Signed-off-by Hanjun Guo <[email protected]>

---
  drivers/acpi/arm64/iort.c   |  5 ++++-
  drivers/base/platform-msi.c | 15 ++++++++++++++-
  drivers/base/platform.c     |  2 ++
  include/linux/msi.h         |  1 +
  4 files changed, 21 insertions(+), 2 deletions(-)

drivers/acpi/arm64/iort.c +4 -1

index 13a1905..bccd3cc 100644
@@ -478,6 +478,7 @@ struct irq_domain *iort_get_device_domain(struct device *dev, u32 req_id)
  {
  	struct fwnode_handle *handle;
  	int its_id;
+	enum irq_domain_bus_token bus_token;
 
  	if (iort_dev_find_its_id(dev, req_id, 0, &its_id))
  		return NULL;

@@ -486,7 +487,9 @@ struct irq_domain *iort_get_device_domain(struct device *dev, u32 req_id)
  	if (!handle)
  		return NULL;
 
-	return irq_find_matching_fwnode(handle, DOMAIN_BUS_PCI_MSI);
+	bus_token = dev_is_pci(dev) ?
+			DOMAIN_BUS_PCI_MSI : DOMAIN_BUS_PLATFORM_MSI;
+	return irq_find_matching_fwnode(handle, bus_token);
  }
 
  static int __get_pci_rid(struct pci_dev *pdev, u16 alias, void *data)

drivers/base/platform-msi.c +1 -1

index 279e539..f6eae18 100644
@@ -17,8 +17,8 @@
   * along with this program.  If not, see <http://www.gnu.org/licenses/>.
   */
 
+#include <linux/acpi_iort.h>
  #include <linux/device.h>
-#include <linux/idr.h>
  #include <linux/irq.h>
  #include <linux/irqdomain.h>
  #include <linux/msi.h>

@@ -416,3 +416,16 @@ int platform_msi_domain_alloc(struct irq_domain *domain, unsigned int virq,
 

drivers/base/platform-msi.c int platform_msi_domain_alloc(struct irq_domain *domain, unsigned int virq, · line 424

Quoted 8 lines

drivers/base/platform-msi.c +6 -0

  	return err;
  }
+
+int acpi_configure_msi_domain(struct device *dev)
+{
+	struct irq_domain *d = NULL;
+
+	d = iort_get_device_domain(dev, 0);

This looks completely wrong. Why RID 0? As far as I can see, 0 is not a special value, and could be something else.

drivers/base/platform-msi.c int platform_msi_domain_alloc(struct irq_domain *domain, unsigned int virq, · line 431

Quoted 8 lines

drivers/base/platform-msi.c +7 -0

+	if (d) {
+		dev_set_msi_domain(dev, d);
+		return 0;
+	}
+
+	return -EINVAL;
+}

I really hate this, as the platform MSI code is intentionally free of any firmware reference. This should live in the ACPI code.

Quoted 17 lines

drivers/base/platform.c +1 -0

index 6482d47..ea01a37 100644
@@ -24,6 +24,7 @@
  #include <linux/pm_domain.h>
  #include <linux/idr.h>
  #include <linux/acpi.h>
+#include <linux/msi.h>
  #include <linux/clk/clk-conf.h>
  #include <linux/limits.h>
  #include <linux/property.h>

@@ -500,6 +501,7 @@ struct platform_device *platform_device_register_full(
  	pdev->dev.parent = pdevinfo->parent;
  	pdev->dev.fwnode = pdevinfo->fwnode;
 

drivers/base/platform.c struct platform_device *platform_device_register_full( · line 504

Quoted 1 line

drivers/base/platform.c +1 -0

+	acpi_configure_msi_domain(&pdev->dev);

It feels odd to put this in the generic code, while you could perfectly put the call into acpi_platform.c and keep the firmware stuff away from the generic code.

Hi Marc,

Thanks for your review, reply inline.

Quoted 66 lines · Marc Zyngier

Quoted 65 lines · Hanjun Guo

From: Hanjun Guo <[email protected]>

With the platform msi domain created, we can set up the msi domain for a platform device when it's probed.

This patch introduces acpi_configure_msi_domain(), which retrieves the domain from iort and set it to platform device.

As some platform devices such as an irqchip needs the msi irqdomain to be the interrupt parent domain, we need to get irqdomain before platform device is probed.

Cc Marc Zyngier <[email protected]>

Cc Greg KH <[email protected]>

Cc Thomas Gleixner <[email protected]>

Cc Bjorn Helgaas <[email protected]>

Cc Lorenzo Pieralisi <[email protected]>

Cc Tomasz Nowicki <[email protected]>

Signed-off-by Hanjun Guo <[email protected]>

---
  drivers/acpi/arm64/iort.c   |  5 ++++-
  drivers/base/platform-msi.c | 15 ++++++++++++++-
  drivers/base/platform.c     |  2 ++
  include/linux/msi.h         |  1 +
  4 files changed, 21 insertions(+), 2 deletions(-)

drivers/acpi/arm64/iort.c +4 -1

index 13a1905..bccd3cc 100644
@@ -478,6 +478,7 @@ struct irq_domain *iort_get_device_domain(struct device *dev, u32 req_id)
  {
  	struct fwnode_handle *handle;
  	int its_id;
+	enum irq_domain_bus_token bus_token;
 
  	if (iort_dev_find_its_id(dev, req_id, 0, &its_id))
  		return NULL;

@@ -486,7 +487,9 @@ struct irq_domain *iort_get_device_domain(struct device *dev, u32 req_id)
  	if (!handle)
  		return NULL;
 
-	return irq_find_matching_fwnode(handle, DOMAIN_BUS_PCI_MSI);
+	bus_token = dev_is_pci(dev) ?
+			DOMAIN_BUS_PCI_MSI : DOMAIN_BUS_PLATFORM_MSI;
+	return irq_find_matching_fwnode(handle, bus_token);
  }
 
  static int __get_pci_rid(struct pci_dev *pdev, u16 alias, void *data)

drivers/base/platform-msi.c +1 -1

index 279e539..f6eae18 100644
@@ -17,8 +17,8 @@
   * along with this program.  If not, see <http://www.gnu.org/licenses/>.
   */
 
+#include <linux/acpi_iort.h>
  #include <linux/device.h>
-#include <linux/idr.h>
  #include <linux/irq.h>
  #include <linux/irqdomain.h>
  #include <linux/msi.h>

@@ -416,3 +416,16 @@ int platform_msi_domain_alloc(struct irq_domain *domain, unsigned int virq,
 

Quoted 11 lines

drivers/base/platform-msi.c int platform_msi_domain_alloc(struct irq_domain *domain, unsigned int virq, · line 424

Quoted 8 lines

drivers/base/platform-msi.c +6 -0

  	return err;
  }
+
+int acpi_configure_msi_domain(struct device *dev)
+{
+	struct irq_domain *d = NULL;
+
+	d = iort_get_device_domain(dev, 0);

This looks completely wrong. Why RID 0? As far as I can see, 0 is not a special value, and could be something else.

You are right. I tried to reuse the API of get irqdomain in IORT for PCI devices, but for platform device, we don't have req id in named component, so I just pass 0 here, I think I need to prepare another API for platform devices.

Quoted 11 lines

drivers/base/platform-msi.c int platform_msi_domain_alloc(struct irq_domain *domain, unsigned int virq, · line 431

Quoted 8 lines

drivers/base/platform-msi.c +7 -0

+	if (d) {
+		dev_set_msi_domain(dev, d);
+		return 0;
+	}
+
+	return -EINVAL;
+}

I really hate this, as the platform MSI code is intentionally free of any firmware reference. This should live in the ACPI code.

Will do, I think locate it in iort.c is better.

Quoted 17 lines

Quoted 17 lines

drivers/base/platform.c +1 -0

index 6482d47..ea01a37 100644
@@ -24,6 +24,7 @@
  #include <linux/pm_domain.h>
  #include <linux/idr.h>
  #include <linux/acpi.h>
+#include <linux/msi.h>
  #include <linux/clk/clk-conf.h>
  #include <linux/limits.h>
  #include <linux/property.h>

@@ -500,6 +501,7 @@ struct platform_device *platform_device_register_full(
  	pdev->dev.parent = pdevinfo->parent;
  	pdev->dev.fwnode = pdevinfo->fwnode;
 

Quoted 5 lines

drivers/base/platform.c struct platform_device *platform_device_register_full( · line 504

Quoted 1 line

drivers/base/platform.c +1 -0

+	acpi_configure_msi_domain(&pdev->dev);

It feels odd to put this in the generic code, while you could perfectly put the call into acpi_platform.c and keep the firmware stuff away from the generic code.

My feeling is the same, I'm still trying to find a new way to do it, but I can't simply put that in acpi_platform.c, because

acpi_create_platform_device()

    platform_device_register_full()
	platform_device_alloc()  --> dev is alloced
         ...
         dev.fwnode  is set
	(I get the msi domain by the fwnode in acpi_configure_msi_domain)
         ...
         platform_device_add()  --> which the device is probed.

For devices like irqchip which needs the dev->msi_domain to be set before it's really probed, because it needs the msi domain to be the parent domain.

If I call the function in acpi_create_platform_device() before platform_device_register_full(), we just can't set dev's msi domain, but if call it after platform_device_register_full(), the irqchip like mbigen will not get its parent domain...

DT is using another API for platform device probe, so has no problems like I said above, any suggestions to do it right in ACPI?

Quoted 1 line · Hanjun Guo

Hi Marc,

Quoted 38 lines

Quoted 16 lines

Quoted 16 lines

drivers/base/platform.c +1 -0

index 6482d47..ea01a37 100644
@@ -24,6 +24,7 @@
  #include <linux/pm_domain.h>
  #include <linux/idr.h>
  #include <linux/acpi.h>
+#include <linux/msi.h>
  #include <linux/clk/clk-conf.h>
  #include <linux/limits.h>
  #include <linux/property.h>

@@ -500,6 +501,7 @@ struct platform_device *platform_device_register_full(
  	pdev->dev.parent = pdevinfo->parent;
  	pdev->dev.fwnode = pdevinfo->fwnode;
 

Quoted 5 lines

drivers/base/platform.c struct platform_device *platform_device_register_full( · line 504

Quoted 1 line

drivers/base/platform.c +1 -0

+	acpi_configure_msi_domain(&pdev->dev);

It feels odd to put this in the generic code, while you could perfectly put the call into acpi_platform.c and keep the firmware stuff away from the generic code.

My feeling is the same, I'm still trying to find a new way to do it, but I can't simply put that in acpi_platform.c, because

acpi_create_platform_device()

    platform_device_register_full()
	platform_device_alloc()  --> dev is alloced
         ...
         dev.fwnode  is set
	(I get the msi domain by the fwnode in acpi_configure_msi_domain)
         ...
         platform_device_add()  --> which the device is probed.

For devices like irqchip which needs the dev->msi_domain to be set before it's really probed, because it needs the msi domain to be the parent domain.

Quoted 8 lines

If I call the function in acpi_create_platform_device() before platform_device_register_full(), we just can't set dev's msi domain, but if call it after platform_device_register_full(), the irqchip like mbigen will not get its parent domain...

DT is using another API for platform device probe, so has no problems like I said above, any suggestions to do it right in ACPI?

How about having something that's completely generic and solves the problem once and for all? Something like this:

drivers/base/platform.c +3 -0

index 6482d47..6f0f90b 100644
@@ -533,6 +533,9 @@ struct platform_device *platform_device_register_full(
 			goto err;
 	}
 
+	if (pdevinfo->pre_add_cb)
+		pdevinfo->pre_add_cb(&pdev->dev);
+
 	ret = platform_device_add(pdev);
 	if (ret) {
 err:

include/linux/platform_device.h +1 -0

index 98c2a7c..44ea133 100644
@@ -74,6 +74,7 @@ struct platform_device_info {
 		u64 dma_mask;
 
 		struct property_entry *properties;
+		void (*pre_add_cb)(struct device *);
 };
 extern struct platform_device *platform_device_register_full(
 		const struct platform_device_info *pdevinfo);

Plug pre_add_cb with your ACPI callback where you can do all the processing you want before the device is actually added.

From: Hanjun Guo <[email protected]>

With the preparation of platform msi support and interrupt producer in DSDT, we can add mbigen ACPI support now.

We are using _PRS methd to indicate number of irq pins instead of num_pins in DT.

For mbi-gen,

    Device(MBI0) {
          Name(_HID, "HISI0152")
          Name(_UID, Zero)
          Name(_CRS, ResourceTemplate() {
                  Memory32Fixed(ReadWrite, 0xa0080000, 0x10000)
          })

          Name (_PRS, ResourceTemplate() {
		  Interrupt(ResourceProducer,...) {12,14,....}
          })

    }

For devices,

   Device(COM0) {
          Name(_HID, "ACPIIDxx")
          Name(_UID, Zero)
          Name(_CRS, ResourceTemplate() {
                 Memory32Fixed(ReadWrite, 0xb0030000, 0x10000)
		 Interrupt(ResourceConsumer,..., "\_SB.MBI0") {12}
          })
    }

With the helpe of platform msi and interrupt producer, then devices will get the virq from mbi-gen's irqdomain.

Cc Marc Zyngier <[email protected]>

Cc Thomas Gleixner <[email protected]>

Cc Ma Jun <[email protected]>

Signed-off-by Hanjun Guo <[email protected]>

---
 drivers/irqchip/irq-mbigen.c | 70 ++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 67 insertions(+), 3 deletions(-)

drivers/irqchip/irq-mbigen.c +64 -3

index 9484ea0..ca6add1 100644
@@ -16,6 +16,7 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
+#include <linux/acpi.h>
 #include <linux/interrupt.h>
 #include <linux/irqchip.h>
 #include <linux/module.h>

@@ -180,7 +181,7 @@ static int mbigen_domain_translate(struct irq_domain *d,
 				    unsigned long *hwirq,
 				    unsigned int *type)
 {
-	if (is_of_node(fwspec->fwnode)) {
+	if (is_of_node(fwspec->fwnode) || is_acpi_device_node(fwspec->fwnode)) {
 		if (fwspec->param_count != 2)
 			return -EINVAL;
 

@@ -271,6 +272,54 @@ static int mbigen_of_create_domain(struct platform_device *pdev,
 	return 0;
 }
 
+#ifdef CONFIG_ACPI
+static acpi_status mbigen_acpi_process_resource(struct acpi_resource *ares,
+					     void *context)
+{
+	struct acpi_resource_extended_irq *ext_irq;
+	u32 *num_irqs = context;
+
+	switch (ares->type) {
+	case ACPI_RESOURCE_TYPE_EXTENDED_IRQ:
+		ext_irq = &ares->data.extended_irq;
+		*num_irqs += ext_irq->interrupt_count;
+		break;
+	default:
+		break;
+	}
+
+	return AE_OK;
+}
+
+static int mbigen_acpi_create_domain(struct platform_device *pdev,
+				     struct mbigen_device *mgn_chip)
+{
+	struct irq_domain *domain;
+	u32 num_msis = 0;
+	acpi_status status;
+
+	status = acpi_walk_resources(ACPI_HANDLE(&pdev->dev), METHOD_NAME__PRS,
+				     mbigen_acpi_process_resource, &num_msis);
+        if (ACPI_FAILURE(status) || num_msis == 0)
+		return -EINVAL;
+
+	domain = platform_msi_create_device_domain(&pdev->dev, num_msis,
+						   mbigen_write_msg,
+						   &mbigen_domain_ops,
+						   mgn_chip);
+	if (!domain)
+		return -ENOMEM;
+
+	return 0;
+}
+#else
+static int mbigen_acpi_create_domain(struct platform_device *pdev,
+				     struct mbigen_device *mgn_chip)
+{
+	return -ENODEV;
+}
+#endif
+
 static int mbigen_device_probe(struct platform_device *pdev)
 {
 	struct mbigen_device *mgn_chip;

@@ -288,9 +337,17 @@ static int mbigen_device_probe(struct platform_device *pdev)
 	if (IS_ERR(mgn_chip->base))
 		return PTR_ERR(mgn_chip->base);
 
-	err = mbigen_of_create_domain(pdev, mgn_chip);
-	if (err)
+	if (IS_ENABLED(CONFIG_OF) && pdev->dev.of_node)
+		err = mbigen_of_create_domain(pdev, mgn_chip);
+	else if (ACPI_COMPANION(&pdev->dev))
+		err = mbigen_acpi_create_domain(pdev, mgn_chip);
+	else
+		err = -EINVAL;
+
+	if (err) {
+		dev_err(&pdev->dev, "Failed to create mbi-gen@%p irqdomain", mgn_chip->base);
 		return err;
+	}
 
 	platform_set_drvdata(pdev, mgn_chip);
 	return 0;

@@ -302,10 +359,17 @@ static const struct of_device_id mbigen_of_match[] = {
 };
 MODULE_DEVICE_TABLE(of, mbigen_of_match);
 
+static const struct acpi_device_id mbigen_acpi_match[] = {
+        { "HISI0152", 0 },
+	{}
+};

Quoted 9 lines · Hanjun Guo

From: Hanjun Guo <[email protected]>

With the preparation of platform msi support and interrupt producer in DSDT, we can add mbigen ACPI support now.

We are using _PRS methd to indicate number of irq pins instead of num_pins in DT.

For mbi-gen,

Quoted 10 lines

    Device(MBI0) {
          Name(_HID, "HISI0152")
          Name(_UID, Zero)
          Name(_CRS, ResourceTemplate() {
                  Memory32Fixed(ReadWrite, 0xa0080000, 0x10000)
          })

          Name (_PRS, ResourceTemplate() {
		  Interrupt(ResourceProducer,...) {12,14,....}
          })

Since I know next to nothing about all of this, I'm going to play the village idiot. What makes it legal to use _PRS as a way to describe the interrupts that are exposed by MBI0? Looking at the 6.0 spec, I do not see why the interrupts would be put there instead of _CRS, and why you'd have a _PRS at all.

Also, are you going to exhaustively describe all the possible interrupts via this method? Knowing that the mbigen can expose thousands of interrupts, I find it slightly mad. Can't you express a range?

From: Hanjun Guo <[email protected]>

mbigen is an irqchip and it needs to be probed before devices, same logic is used for SMMU and etc., let's use arch_initcall instead of platform init for mbigen.

Cc Marc Zyngier <[email protected]>

Cc Thomas Gleixner <[email protected]>

Cc Ma Jun <[email protected]>

Signed-off-by Hanjun Guo <[email protected]>

---
 drivers/irqchip/irq-mbigen.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

drivers/irqchip/irq-mbigen.c +0 -0

index ca6add1..3a33de6 100644
@@ -374,7 +374,11 @@ static struct platform_driver mbigen_platform_driver = {
 	.probe			= mbigen_device_probe,
 };

Quoted 29 lines · Hanjun Guo

From: Hanjun Guo <[email protected]>

mbigen is an irqchip and it needs to be probed before devices, same logic is used for SMMU and etc., let's use arch_initcall instead of platform init for mbigen.

Cc Marc Zyngier <[email protected]>

Cc Thomas Gleixner <[email protected]>

Cc Ma Jun <[email protected]>

Signed-off-by Hanjun Guo <[email protected]>

---
 drivers/irqchip/irq-mbigen.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

drivers/irqchip/irq-mbigen.c +5 -1

index ca6add1..3a33de6 100644
@@ -374,7 +374,11 @@ static struct platform_driver mbigen_platform_driver = {
 	.probe			= mbigen_device_probe,
 };
 
-module_platform_driver(mbigen_platform_driver);
+static __init int mbigen_init(void)
+{
+	return platform_driver_register(&mbigen_platform_driver);
+}
+arch_initcall(mbigen_init);
 

drivers/irqchip/irq-mbigen.c static struct platform_driver mbigen_platform_driver = { · line 384

Quoted 3 lines

drivers/irqchip/irq-mbigen.c +0 -0

 MODULE_AUTHOR("Jun Ma <[email protected]>");
 MODULE_AUTHOR("Yun Wu <[email protected]>");

I've already NACKed such a patch in the past, and I will carry on NACKing it until all the other avenues (like properly tracking device dependencies) have been explored and have been proven not to be fit for purpose.

Rafael had proposed something around this subject last year, and I've

been repeatedly advising you to work with him and others to make it happen. You can choose to ignore this advise, but that doesn't make this patch an acceptable approach.

Quoted 30 lines · Marc Zyngier

Quoted 29 lines · Hanjun Guo

From: Hanjun Guo <[email protected]>

mbigen is an irqchip and it needs to be probed before devices, same logic is used for SMMU and etc., let's use arch_initcall instead of platform init for mbigen.

Cc Marc Zyngier <[email protected]>

Cc Thomas Gleixner <[email protected]>

Cc Ma Jun <[email protected]>

Signed-off-by Hanjun Guo <[email protected]>

---
 drivers/irqchip/irq-mbigen.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

drivers/irqchip/irq-mbigen.c +5 -1

index ca6add1..3a33de6 100644
@@ -374,7 +374,11 @@ static struct platform_driver mbigen_platform_driver = {
 	.probe			= mbigen_device_probe,
 };
 
-module_platform_driver(mbigen_platform_driver);
+static __init int mbigen_init(void)
+{
+	return platform_driver_register(&mbigen_platform_driver);
+}
+arch_initcall(mbigen_init);
 

Quoted 8 lines

drivers/irqchip/irq-mbigen.c static struct platform_driver mbigen_platform_driver = { · line 384

Quoted 3 lines

drivers/irqchip/irq-mbigen.c +0 -0

 MODULE_AUTHOR("Jun Ma <[email protected]>");
 MODULE_AUTHOR("Yun Wu <[email protected]>");

I've already NACKed such a patch in the past, and I will carry on NACKing it until all the other avenues (like properly tracking device dependencies) have been explored and have been proven not to be fit for purpose.

I'd happy to work on this to make progress.

Quoted 2 lines

Rafael had proposed something around this subject last year, and I've

Sorry, obviously I missed something, could you give me the link about Rafael's patches? I will pull back and test it on our hardware.

Quoted 4 lines

been repeatedly advising you to work with him and others to make it happen. You can choose to ignore this advise, but that doesn't make this patch an acceptable approach.

OK, I will drop this patch in next version, and work on the generic solution instead.

Quoted 41 lines · Hanjun Guo

Quoted 30 lines · Marc Zyngier

Quoted 29 lines · Hanjun Guo

From: Hanjun Guo <[email protected]>

mbigen is an irqchip and it needs to be probed before devices, same logic is used for SMMU and etc., let's use arch_initcall instead of platform init for mbigen.

Cc Marc Zyngier <[email protected]>

Cc Thomas Gleixner <[email protected]>

Cc Ma Jun <[email protected]>

Signed-off-by Hanjun Guo <[email protected]>

---
 drivers/irqchip/irq-mbigen.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

drivers/irqchip/irq-mbigen.c +5 -1

index ca6add1..3a33de6 100644
@@ -374,7 +374,11 @@ static struct platform_driver mbigen_platform_driver = {
 	.probe			= mbigen_device_probe,
 };
 
-module_platform_driver(mbigen_platform_driver);
+static __init int mbigen_init(void)
+{
+	return platform_driver_register(&mbigen_platform_driver);
+}
+arch_initcall(mbigen_init);
 

Quoted 7 lines

drivers/irqchip/irq-mbigen.c static struct platform_driver mbigen_platform_driver = { · line 384

Quoted 3 lines

drivers/irqchip/irq-mbigen.c +0 -0

 MODULE_AUTHOR("Jun Ma <[email protected]>");
 MODULE_AUTHOR("Yun Wu <[email protected]>");

I've already NACKed such a patch in the past, and I will carry on NACKing it until all the other avenues (like properly tracking device dependencies) have been explored and have been proven not to be fit for purpose.

I'd happy to work on this to make progress.

Quoted 1 line

Rafael had proposed something around this subject last year, and I've

Quoted 2 lines

Sorry, obviously I missed something, could you give me the link about Rafael's patches? I will pull back and test it on our hardware.

Please see this discussion from almost a year ago:

https://patchwork.kernel.org/patch/7407401/

in which I give the existing pointers and explain why I'm pushing back on things like this patch.