[PATCH v3 0/2] irqchip/mbigen: fix the IO remap problem in mbigen driver.

[PATCH v3 0/2] irqchip/mbigen: fix the IO remap problem in mbigen driver.

[MaJun]

From: Ma Jun <majun258@huawei.com>

This patch set is used to fix the problem of remap a set of registers
repeatedly in current mbigen driver.

Changes in v3:
--- Change the log to make more detail description about
	the IO remap problem.

Changes in v2:
--- Change the mbigen device node definition as Mark suggested.
--- Change the mbigen driver based on the new mbigen dts structure.

Ma Jun (2):
  irqchip/mbigen:Change the mbigen node definition in dt binding file
  irqchip/mbigen:Change the mbigen driver based on the new mbigen node definition

 .../interrupt-controller/hisilicon,mbigen-v2.txt   |   22 +++++++++++---
 drivers/irqchip/irq-mbigen.c                       |   30 ++++++++++++++------
 2 files changed, 38 insertions(+), 14 deletions(-)

[PATCH v3 1/2] irqchip/mbigen:Change the mbigen node definition in dt binding file

[MaJun]

From: Ma Jun <majun258@huawei.com>

For mbigen module, there is a special case that more than one mbigen
device nodes use the same reg definition in DTS when these devices
exist in the same mbigen hardware module.

In current mbigen driver, these mbigen devices definition as below:
mbigen_dev1:intc_dev1 {
	...
	reg = <0x0 0xc0080000 0x0 0x10000>;
	...
};

mbigen_dev2:intc_dev2 {
	...
	reg = <0x0 0xc0080000 0x0 0x10000>;
	...
};

On this case, devm_ioremap_resource() returns fail with info
"can't request region for resource" because of memory region check.

To fix this problem, the mbigen node definition and
structure are changed as Mark Rutland suggested in v1 patch[1].

[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2016-February/403691.html

Signed-off-by: Ma Jun <majun258@huawei.com>
---
 .../interrupt-controller/hisilicon,mbigen-v2.txt   |   22 +++++++++++++++----
 1 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/interrupt-controller/hisilicon,mbigen-v2.txt b/Documentation/devicetree/bindings/interrupt-controller/hisilicon,mbigen-v2.txt
index 720f7c9..3b2f4c4 100644
--- a/Documentation/devicetree/bindings/interrupt-controller/hisilicon,mbigen-v2.txt
+++ b/Documentation/devicetree/bindings/interrupt-controller/hisilicon,mbigen-v2.txt
@@ -21,6 +21,8 @@ Mbigen main node required properties:
 - reg: Specifies the base physical address and size of the Mbigen
   registers.
 
+Mbigen sub node required properties:
+------------------------------------------
 - interrupt controller: Identifies the node as an interrupt controller
 
 - msi-parent: Specifies the MSI controller this mbigen use.
@@ -45,13 +47,23 @@ Mbigen main node required properties:
 
 Examples:
 
-	mbigen_device_gmac:intc {
+	mbigen_chip_dsa {
 			compatible = "hisilicon,mbigen-v2";
 			reg = <0x0 0xc0080000 0x0 0x10000>;
-			interrupt-controller;
-			msi-parent = <&its_dsa 0x40b1c>;
-			num-pins = <9>;
-			#interrupt-cells = <2>;
+
+			mbigen_gmac:intc_gmac {
+				interrupt-controller;
+				msi-parent = <&its_dsa 0x40b1c>;
+				num-pins = <9>;
+				#interrupt-cells = <2>;
+			};
+
+			mbigen_i2c:intc_i2c {
+				interrupt-controller;
+				msi-parent = <&its_dsa 0x40b0e>;
+				num-pins = <2>;
+				#interrupt-cells = <2>;
+			};
 	};
 
 Devices connect to mbigen required properties:
-- 
1.7.1

[PATCH v3 2/2] irqchip/mbigen:Change the mbigen driver based on the new mbigen node definition.

[MaJun]

From: Ma Jun <majun258@huawei.com>

In current mbigen driver, each mbigen device is initialized as a platform device.
When these devices belong to same mbigen hardware module(chip), they use the
same register definition in their device node and caused the problem of registers
remapped repeatedly.

Now, I try to initialize the mbigen module(chip) as a platform device and remap
the register once to fix this problem.

Signed-off-by: Ma Jun <majun258@huawei.com>
---
 drivers/irqchip/irq-mbigen.c |   30 +++++++++++++++++++++---------
 1 files changed, 21 insertions(+), 9 deletions(-)

diff --git a/drivers/irqchip/irq-mbigen.c b/drivers/irqchip/irq-mbigen.c
index 4dd3eb8..4d413bc 100644
--- a/drivers/irqchip/irq-mbigen.c
+++ b/drivers/irqchip/irq-mbigen.c
@@ -242,6 +242,8 @@ static int mbigen_device_probe(struct platform_device *pdev)
 	struct resource *res;
 	struct irq_domain *domain;
 	u32 num_pins;
+	struct platform_device *child_pdev;
+	struct device_node *np;
 
 	mgn_chip = devm_kzalloc(&pdev->dev, sizeof(*mgn_chip), GFP_KERNEL);
 	if (!mgn_chip)
@@ -251,25 +253,35 @@ static int mbigen_device_probe(struct platform_device *pdev)
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	mgn_chip->base = devm_ioremap_resource(&pdev->dev, res);
+
 	if (IS_ERR(mgn_chip->base))
 		return PTR_ERR(mgn_chip->base);
 
-	if (of_property_read_u32(pdev->dev.of_node, "num-pins", &num_pins) < 0) {
-		dev_err(&pdev->dev, "No num-pins property\n");
-		return -EINVAL;
-	}
+	for_each_child_of_node(pdev->dev.of_node, np) {
+		if (!of_property_read_bool(np, "interrupt-controller"))
+			continue;
+
+		child_pdev = of_platform_device_create(np, NULL, platform_bus_type.dev_root);
+		if (IS_ERR(child_pdev))
+			return PTR_ERR(child_pdev);
+
+		if (of_property_read_u32(child_pdev->dev.of_node, "num-pins", &num_pins) < 0) {
+			dev_err(&pdev->dev, "No num-pins property\n");
+			return -EINVAL;
+		}
 
-	domain = platform_msi_create_device_domain(&pdev->dev, num_pins,
+		domain = platform_msi_create_device_domain(&child_pdev->dev, num_pins,
 							mbigen_write_msg,
 							&mbigen_domain_ops,
 							mgn_chip);
 
-	if (!domain)
-		return -ENOMEM;
+		if (!domain)
+			return -ENOMEM;
 
-	platform_set_drvdata(pdev, mgn_chip);
+		dev_info(&child_pdev->dev, "Allocated %d MSIs\n", num_pins);
+	}
 
-	dev_info(&pdev->dev, "Allocated %d MSIs\n", num_pins);
+	platform_set_drvdata(pdev, mgn_chip);
 
 	return 0;
 }
-- 
1.7.1

Re: [PATCH v3 0/2] irqchip/mbigen: fix the IO remap problem in mbigen driver.

[Thomas Gleixner]

On Thu, 17 Mar 2016, MaJun wrote:
> This patch set is used to fix the problem of remap a set of registers
> repeatedly in current mbigen driver.
>
>   irqchip/mbigen:Change the mbigen node definition in dt binding file
>   irqchip/mbigen:Change the mbigen driver based on the new mbigen node definition

These subject lines are still horrible as they tell nothing about the nature
of the change. I fixed them up along with the changelogs and applied them to
irq/urgent. Can you spot the difference?

While at it. The config switch for this driver is horrible. It's just in the
middle of the device driver configs. Why is this configurable by the user at
all? It simply should be selected by arm64 or some subarch configuration
there. Please clean that up.

Thanks,

	tglx

[tip:irq/urgent] irqchip/mbigen: Adjust DT bindings to handle multiple devices in a module

[tip-bot for MaJun]

Commit-ID:  d0e286415dc1f4fea2971d6186b0775c7062575b
Gitweb:     http://git.kernel.org/tip/d0e286415dc1f4fea2971d6186b0775c7062575b
Author:     MaJun <majun258@huawei.com>
AuthorDate: Thu, 17 Mar 2016 16:34:00 +0800
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Mon, 21 Mar 2016 11:24:10 +0100

irqchip/mbigen: Adjust DT bindings to handle multiple devices in a module

A mbigen hardware module can contain more than one device node. These device
nodes contain the same register definition.

mbigen_dev1:intc_dev1 {
	...
	reg = <0x0 0xc0080000 0x0 0x10000>;
	...
};

mbigen_dev2:intc_dev2 {
	...
	reg = <0x0 0xc0080000 0x0 0x10000>;
	...
};

In this case both devices try to request the same resource resulting in a
resource conflict.

To address this problem the devices need to be subnodes of the mbigen hardware
module, which then contains the unique register space.

[ tglx: Massaged changelog ]

Suggested-by: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Ma Jun <majun258@huawei.com>
Cc: jason@lakedaemon.net
Cc: marc.zyngier@arm.com
Cc: Catalin.Marinas@arm.com
Cc: guohanjun@huawei.com
Cc: Will.Deacon@arm.com
Cc: huxinwei@huawei.com
Cc: lizefan@huawei.com
Cc: dingtianhong@huawei.com
Cc: zhaojunhua@hisilicon.com
Cc: liguozhu@hisilicon.com
Cc: linux-arm-kernel@lists.infradead.org
Link: http://lkml.kernel.org/r/20160203111602.GA1234@leverpostej
Link: http://lkml.kernel.org/r/1458203641-17172-2-git-send-email-majun258@huawei.com
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

---
 .../interrupt-controller/hisilicon,mbigen-v2.txt   | 22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/interrupt-controller/hisilicon,mbigen-v2.txt b/Documentation/devicetree/bindings/interrupt-controller/hisilicon,mbigen-v2.txt
index 720f7c9..3b2f4c4 100644
--- a/Documentation/devicetree/bindings/interrupt-controller/hisilicon,mbigen-v2.txt
+++ b/Documentation/devicetree/bindings/interrupt-controller/hisilicon,mbigen-v2.txt
@@ -21,6 +21,8 @@ Mbigen main node required properties:
 - reg: Specifies the base physical address and size of the Mbigen
   registers.
 
+Mbigen sub node required properties:
+------------------------------------------
 - interrupt controller: Identifies the node as an interrupt controller
 
 - msi-parent: Specifies the MSI controller this mbigen use.
@@ -45,13 +47,23 @@ Mbigen main node required properties:
 
 Examples:
 
-	mbigen_device_gmac:intc {
+	mbigen_chip_dsa {
 			compatible = "hisilicon,mbigen-v2";
 			reg = <0x0 0xc0080000 0x0 0x10000>;
-			interrupt-controller;
-			msi-parent = <&its_dsa 0x40b1c>;
-			num-pins = <9>;
-			#interrupt-cells = <2>;
+
+			mbigen_gmac:intc_gmac {
+				interrupt-controller;
+				msi-parent = <&its_dsa 0x40b1c>;
+				num-pins = <9>;
+				#interrupt-cells = <2>;
+			};
+
+			mbigen_i2c:intc_i2c {
+				interrupt-controller;
+				msi-parent = <&its_dsa 0x40b0e>;
+				num-pins = <2>;
+				#interrupt-cells = <2>;
+			};
 	};
 
 Devices connect to mbigen required properties:

[tip:irq/urgent] irqchip/mbigen: Handle multiple device nodes in a mbigen module

[tip-bot for MaJun]

Commit-ID:  ed2a1002d25ccdb6606c8ccb608524118bd30614
Gitweb:     http://git.kernel.org/tip/ed2a1002d25ccdb6606c8ccb608524118bd30614
Author:     MaJun <majun258@huawei.com>
AuthorDate: Thu, 17 Mar 2016 16:34:01 +0800
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Mon, 21 Mar 2016 11:24:11 +0100

irqchip/mbigen: Handle multiple device nodes in a mbigen module

Each mbigen device is represented as a independent platform device. If the
devices belong to the same mbigen hardware module, then the register space for
these devices is the same. That leads to a resource conflict.

The solution for this is to represent the mbigen module as a platform device
and make the mbigen devices subdevices of that. The register space is
associated to the mbigen module and therefor the resource conflict is avoided.

[ tglx: Massaged changelog, cleaned up the code and removed the silly printk ]

Signed-off-by: Ma Jun <majun258@huawei.com>
Cc: mark.rutland@arm.com
Cc: jason@lakedaemon.net
Cc: marc.zyngier@arm.com
Cc: Catalin.Marinas@arm.com
Cc: guohanjun@huawei.com
Cc: Will.Deacon@arm.com
Cc: huxinwei@huawei.com
Cc: lizefan@huawei.com
Cc: dingtianhong@huawei.com
Cc: zhaojunhua@hisilicon.com
Cc: liguozhu@hisilicon.com
Cc: linux-arm-kernel@lists.infradead.org
Link: http://lkml.kernel.org/r/1458203641-17172-3-git-send-email-majun258@huawei.com
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 drivers/irqchip/irq-mbigen.c | 38 ++++++++++++++++++++++++--------------
 1 file changed, 24 insertions(+), 14 deletions(-)

diff --git a/drivers/irqchip/irq-mbigen.c b/drivers/irqchip/irq-mbigen.c
index 4dd3eb8..d67baa2 100644
--- a/drivers/irqchip/irq-mbigen.c
+++ b/drivers/irqchip/irq-mbigen.c
@@ -239,8 +239,11 @@ static struct irq_domain_ops mbigen_domain_ops = {
 static int mbigen_device_probe(struct platform_device *pdev)
 {
 	struct mbigen_device *mgn_chip;
-	struct resource *res;
+	struct platform_device *child;
 	struct irq_domain *domain;
+	struct device_node *np;
+	struct device *parent;
+	struct resource *res;
 	u32 num_pins;
 
 	mgn_chip = devm_kzalloc(&pdev->dev, sizeof(*mgn_chip), GFP_KERNEL);
@@ -254,23 +257,30 @@ static int mbigen_device_probe(struct platform_device *pdev)
 	if (IS_ERR(mgn_chip->base))
 		return PTR_ERR(mgn_chip->base);
 
-	if (of_property_read_u32(pdev->dev.of_node, "num-pins", &num_pins) < 0) {
-		dev_err(&pdev->dev, "No num-pins property\n");
-		return -EINVAL;
-	}
+	for_each_child_of_node(pdev->dev.of_node, np) {
+		if (!of_property_read_bool(np, "interrupt-controller"))
+			continue;
 
-	domain = platform_msi_create_device_domain(&pdev->dev, num_pins,
-							mbigen_write_msg,
-							&mbigen_domain_ops,
-							mgn_chip);
+		parent = platform_bus_type.dev_root;
+		child = of_platform_device_create(np, NULL, parent);
+		if (IS_ERR(child))
+			return PTR_ERR(child);
 
-	if (!domain)
-		return -ENOMEM;
+		if (of_property_read_u32(child->dev.of_node, "num-pins",
+					 &num_pins) < 0) {
+			dev_err(&pdev->dev, "No num-pins property\n");
+			return -EINVAL;
+		}
+
+		domain = platform_msi_create_device_domain(&child->dev, num_pins,
+							   mbigen_write_msg,
+							   &mbigen_domain_ops,
+							   mgn_chip);
+		if (!domain)
+			return -ENOMEM;
+	}
 
 	platform_set_drvdata(pdev, mgn_chip);
-
-	dev_info(&pdev->dev, "Allocated %d MSIs\n", num_pins);
-
 	return 0;
 }
 

Re: [PATCH v3 0/2] irqchip/mbigen: fix the IO remap problem in mbigen driver.

[majun (F)]

在 2016/3/21 18:29, Thomas Gleixner 写道:
> On Thu, 17 Mar 2016, MaJun wrote:
>> This patch set is used to fix the problem of remap a set of registers
>> repeatedly in current mbigen driver.
>>
>>   irqchip/mbigen:Change the mbigen node definition in dt binding file
>>   irqchip/mbigen:Change the mbigen driver based on the new mbigen node definition
> 
> These subject lines are still horrible as they tell nothing about the nature
> of the change. I fixed them up along with the changelogs and applied them to
> irq/urgent. Can you spot the difference?

Yes, after you changing, the subject and change log clearly show why we need to
do this change.
I have learned a lot from you :)

> 
> While at it. The config switch for this driver is horrible. It's just in the
> middle of the device driver configs. Why is this configurable by the user at
> all? It simply should be selected by arm64 or some subarch configuration
> there. Please clean that up.
> 
will do.

Thanks!
MaJun

> Thanks,
> 
> 	tglx
> 
> .
> 

Re: [PATCH v3 0/2] irqchip/mbigen: fix the IO remap problem in mbigen driver.

[Marc Zyngier]

On 22/03/16 03:10, majun (F) wrote:
> 
> 
> 在 2016/3/21 18:29, Thomas Gleixner 写道:
>> On Thu, 17 Mar 2016, MaJun wrote:
>>> This patch set is used to fix the problem of remap a set of registers
>>> repeatedly in current mbigen driver.
>>>
>>>   irqchip/mbigen:Change the mbigen node definition in dt binding file
>>>   irqchip/mbigen:Change the mbigen driver based on the new mbigen node definition
>>
>> These subject lines are still horrible as they tell nothing about the nature
>> of the change. I fixed them up along with the changelogs and applied them to
>> irq/urgent. Can you spot the difference?
> 
> Yes, after you changing, the subject and change log clearly show why we need to
> do this change.
> I have learned a lot from you :)
> 
>>
>> While at it. The config switch for this driver is horrible. It's just in the
>> middle of the device driver configs. Why is this configurable by the user at
>> all? It simply should be selected by arm64 or some subarch configuration
>> there. Please clean that up.
>>
> will do.

So what's the status of this? Do you still plan to respin it?

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

Re: [PATCH v3 0/2] irqchip/mbigen: fix the IO remap problem in mbigen driver.

[majun (F)]

Hi Marc:

在 2016/5/5 22:49, Marc Zyngier 写道:
> On 22/03/16 03:10, majun (F) wrote:
>>
>>
>> 在 2016/3/21 18:29, Thomas Gleixner 写道:
>>> On Thu, 17 Mar 2016, MaJun wrote:
>>>> This patch set is used to fix the problem of remap a set of registers
>>>> repeatedly in current mbigen driver.
>>>>
>>>>   irqchip/mbigen:Change the mbigen node definition in dt binding file
>>>>   irqchip/mbigen:Change the mbigen driver based on the new mbigen node definition
>>>
>>> These subject lines are still horrible as they tell nothing about the nature
>>> of the change. I fixed them up along with the changelogs and applied them to
>>> irq/urgent. Can you spot the difference?
>>
>> Yes, after you changing, the subject and change log clearly show why we need to
>> do this change.
>> I have learned a lot from you :)
>>
>>>
>>> While at it. The config switch for this driver is horrible. It's just in the
>>> middle of the device driver configs. Why is this configurable by the user at
>>> all? It simply should be selected by arm64 or some subarch configuration
>>> there. Please clean that up.
>>>
>> will do.
> 
> So what's the status of this? Do you still plan to respin it?
> 

This patch set had been ACK by Thomas.
If you mean the mbigen driver config switch problem, the patch for this problem also
had been ACK by thomas.
please see the link:
https://lkml.org/lkml/2016/3/23/76

Thanks
Ma Jun

> Thanks,
> 
> 	M.
> 

Re: [PATCH v3 0/2] irqchip/mbigen: fix the IO remap problem in mbigen driver.

[Marc Zyngier]

On 06/05/16 02:12, majun (F) wrote:
> Hi Marc:
> 
> 在 2016/5/5 22:49, Marc Zyngier 写道:
>> On 22/03/16 03:10, majun (F) wrote:
>>>
>>>
>>> 在 2016/3/21 18:29, Thomas Gleixner 写道:
>>>> On Thu, 17 Mar 2016, MaJun wrote:
>>>>> This patch set is used to fix the problem of remap a set of registers
>>>>> repeatedly in current mbigen driver.
>>>>>
>>>>>   irqchip/mbigen:Change the mbigen node definition in dt binding file
>>>>>   irqchip/mbigen:Change the mbigen driver based on the new mbigen node definition
>>>>
>>>> These subject lines are still horrible as they tell nothing about the nature
>>>> of the change. I fixed them up along with the changelogs and applied them to
>>>> irq/urgent. Can you spot the difference?
>>>
>>> Yes, after you changing, the subject and change log clearly show why we need to
>>> do this change.
>>> I have learned a lot from you :)
>>>
>>>>
>>>> While at it. The config switch for this driver is horrible. It's just in the
>>>> middle of the device driver configs. Why is this configurable by the user at
>>>> all? It simply should be selected by arm64 or some subarch configuration
>>>> there. Please clean that up.
>>>>
>>> will do.
>>
>> So what's the status of this? Do you still plan to respin it?
>>
> 
> This patch set had been ACK by Thomas.

Interesting. Somehow, the ARM email server has decided not to deliver
the tip-bot messages, so I didn't see it was already in.

Sorry for the noise.

	M.
-- 
Jazz is not dead. It just smells funny...

Re: [PATCH v3 0/2] irqchip/mbigen: fix the IO remap problem in mbigen driver.

[majun (F)]

Hi Marc:

在 2016/5/6 15:32, Marc Zyngier 写道:
> On 06/05/16 02:12, majun (F) wrote:
>> Hi Marc:
>>
>> 在 2016/5/5 22:49, Marc Zyngier 写道:
>>> On 22/03/16 03:10, majun (F) wrote:
>>>>
>>>>
>>>> 在 2016/3/21 18:29, Thomas Gleixner 写道:
>>>>> On Thu, 17 Mar 2016, MaJun wrote:
>>>>>> This patch set is used to fix the problem of remap a set of registers
>>>>>> repeatedly in current mbigen driver.
>>>>>>
>>>>>>   irqchip/mbigen:Change the mbigen node definition in dt binding file
>>>>>>   irqchip/mbigen:Change the mbigen driver based on the new mbigen node definition
>>>>>
>>>>> These subject lines are still horrible as they tell nothing about the nature
>>>>> of the change. I fixed them up along with the changelogs and applied them to
>>>>> irq/urgent. Can you spot the difference?
>>>>
>>>> Yes, after you changing, the subject and change log clearly show why we need to
>>>> do this change.
>>>> I have learned a lot from you :)
>>>>
>>>>>
>>>>> While at it. The config switch for this driver is horrible. It's just in the
>>>>> middle of the device driver configs. Why is this configurable by the user at
>>>>> all? It simply should be selected by arm64 or some subarch configuration
>>>>> there. Please clean that up.
>>>>>
>>>> will do.
>>>
>>> So what's the status of this? Do you still plan to respin it?
>>>
>>
>> This patch set had been ACK by Thomas.
> 
> Interesting. Somehow, the ARM email server has decided not to deliver
> the tip-bot messages, so I didn't see it was already in.
> 
> Sorry for the noise.

Not at all. Thank you for your attention.
MaJun
Thanks!
> 
> 	M.
>