Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1478025
| From | Tomasz Nowicki <tn@semihalf.com> |
|---|---|
| Newsgroups | linux.kernel |
| Subject | Re: [PATCH V10 7/8] irqchip/gicv3-its: Factor out PCI-MSI part that might be reused for ACPI |
| Date | 2016-09-07 09:50 +0200 |
| Message-ID | <seFMl-5cY-13@gated-at.bofh.it> (permalink) |
| References | <sekye-8q9-21@gated-at.bofh.it> <sekye-8q9-45@gated-at.bofh.it> <selkC-ll-17@gated-at.bofh.it> <selND-MN-9@gated-at.bofh.it> <semJH-1kY-11@gated-at.bofh.it> |
| Organization | linux.* mail to news gateway |
On 06.09.2016 13:29, Rafael J. Wysocki wrote:
> On Tue, Sep 6, 2016 at 12:22 PM, Tomasz Nowicki <tn@semihalf.com> wrote:
>> Hi Thomas,
>>
>>
>> On 06.09.2016 11:50, Thomas Gleixner wrote:
>>>
>>> On Tue, 6 Sep 2016, Tomasz Nowicki wrote:
>>>>
>>>> -static int __init its_pci_msi_init(void)
>>>> +static int __init its_pci_msi_init_one(struct fwnode_handle *handle,
>>>> + const char *name)
>>>> {
>>>> - struct device_node *np;
>>>> struct irq_domain *parent;
>>>>
>>>> + parent = irq_find_matching_fwnode(handle, DOMAIN_BUS_NEXUS);
>>>> + if (!parent || !msi_get_domain_info(parent)) {
>>>> + pr_err("%s: Unable to locate ITS domain\n", name);
>>>> + return -ENXIO;
>>>> + }
>>>> +
>>>> + if (!pci_msi_create_irq_domain(handle, &its_pci_msi_domain_info,
>>>> + parent)) {
>>>> + pr_err("%s: Unable to create PCI domain\n", name);
>>>> + return -ENOMEM;
>>>
>>>
>>> So you have error codes here.
>>>
>>>> + }
>>>> +
>>>> + return 0;
>>>> +}
>>>> +
>>>> +static int __init its_pci_of_msi_init(void)
>>>> +{
>>>> + struct device_node *np;
>>>> +
>>>> for (np = of_find_matching_node(NULL, its_device_id); np;
>>>> np = of_find_matching_node(np, its_device_id)) {
>>>> if (!of_property_read_bool(np, "msi-controller"))
>>>> continue;
>>>>
>>>> - parent = irq_find_matching_host(np, DOMAIN_BUS_NEXUS);
>>>> - if (!parent || !msi_get_domain_info(parent)) {
>>>> - pr_err("%s: unable to locate ITS domain\n",
>>>> - np->full_name);
>>>> + if (its_pci_msi_init_one(of_node_to_fwnode(np),
>>>> np->full_name))
>>>> continue;
>>>> - }
>>>> -
>>>> - if (!pci_msi_create_irq_domain(of_node_to_fwnode(np),
>>>> - &its_pci_msi_domain_info,
>>>> - parent)) {
>>>> - pr_err("%s: unable to create PCI domain\n",
>>>> - np->full_name);
>>>> - continue;
>>>> - }
>>>>
>>>> pr_info("PCI/MSI: %s domain created\n", np->full_name);
>>>> }
>>>>
>>>> return 0;
>>>
>>>
>>> And no matter what you return success
>>>
>>>> }
>>>> +
>>>> +static int __init its_pci_msi_init(void)
>>>> +{
>>>> + its_pci_of_msi_init();
>>>
>>>
>>> Which you subsequently ignore here. What's the point?
>>>
>>
>> Well, this patch is meant to refactor the code. It does not change the error
>> handing nor the functionality. However, you have got a point but IMO it
>> should be fixed in separate patch if possible.
>
> But you are refactoring the code, so it would make sense to clean it
> up while at it. Perhaps add one more patch to the series?
>
I have a look at the code again and come to the conclusion the code is
fine now. We iterate over available its nodes and try to probe driver.
There might be an error for one of it (print error message) but we keep
trying for the rest. One of it might be probed successfully. So there is
no reason to escape in half way. Since its_pci_of_msi_init() always
returns 0 I can remove return statement so we would have:
void its_pci_of_msi_init(void) instead.
Thanks,
Tomasz
Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread
[PATCH V10 0/8] Introduce ACPI world to ITS irqchip Tomasz Nowicki <tn@semihalf.com> - 2016-09-06 11:10 +0200
[PATCH V10 6/8] irqchip/gicv3-its: Probe ITS in the ACPI way Tomasz Nowicki <tn@semihalf.com> - 2016-09-06 11:10 +0200
[PATCH V10 1/8] ACPI: I/O Remapping Table (IORT) initial support Tomasz Nowicki <tn@semihalf.com> - 2016-09-06 11:10 +0200
[UPDATE PATCH V10 1/8] ACPI: I/O Remapping Table (IORT) initial support Tomasz Nowicki <tn@semihalf.com> - 2016-09-07 14:00 +0200
Re: [UPDATE PATCH V10 1/8] ACPI: I/O Remapping Table (IORT) initial support Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> - 2016-09-08 16:00 +0200
Re: [UPDATE PATCH V10 1/8] ACPI: I/O Remapping Table (IORT) initial support Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> - 2016-09-09 11:30 +0200
Re: [PATCH V10 1/8] ACPI: I/O Remapping Table (IORT) initial support Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> - 2016-09-09 10:50 +0200
[PATCH V10 2/8] ACPI: Add new IORT functions to support MSI domain handling Tomasz Nowicki <tn@semihalf.com> - 2016-09-06 11:10 +0200
Re: [PATCH V10 2/8] ACPI: Add new IORT functions to support MSI domain handling Tomasz Nowicki <tn@semihalf.com> - 2016-09-06 12:30 +0200
Re: [PATCH V10 2/8] ACPI: Add new IORT functions to support MSI domain handling Tomasz Nowicki <tn@semihalf.com> - 2016-09-07 12:40 +0200
Re: [PATCH V10 2/8] ACPI: Add new IORT functions to support MSI domain handling "Rafael J. Wysocki" <rafael@kernel.org> - 2016-09-07 13:40 +0200
Re: [PATCH V10 2/8] ACPI: Add new IORT functions to support MSI domain handling Thomas Gleixner <tglx@linutronix.de> - 2016-09-06 12:30 +0200
[UPDATE PATCH V10 2/8] ACPI: Add new IORT functions to support MSI domain handling Tomasz Nowicki <tn@semihalf.com> - 2016-09-07 14:00 +0200
Re: [UPDATE PATCH V10 2/8] ACPI: Add new IORT functions to support MSI domain handling Tomasz Nowicki <tn@semihalf.com> - 2016-09-07 15:10 +0200
[PATCH V10 7/8] irqchip/gicv3-its: Factor out PCI-MSI part that might be reused for ACPI Tomasz Nowicki <tn@semihalf.com> - 2016-09-06 11:10 +0200
Re: [PATCH V10 7/8] irqchip/gicv3-its: Factor out PCI-MSI part that might be reused for ACPI Thomas Gleixner <tglx@linutronix.de> - 2016-09-06 12:00 +0200
Re: [PATCH V10 7/8] irqchip/gicv3-its: Factor out PCI-MSI part that might be reused for ACPI Tomasz Nowicki <tn@semihalf.com> - 2016-09-06 12:30 +0200
Re: [PATCH V10 7/8] irqchip/gicv3-its: Factor out PCI-MSI part that might be reused for ACPI "Rafael J. Wysocki" <rafael@kernel.org> - 2016-09-06 13:30 +0200
Re: [PATCH V10 7/8] irqchip/gicv3-its: Factor out PCI-MSI part that might be reused for ACPI Tomasz Nowicki <tn@semihalf.com> - 2016-09-07 09:50 +0200
[PATCH V10 4/8] irqchip/gicv3-its: Cleanup for ITS domain initialization Tomasz Nowicki <tn@semihalf.com> - 2016-09-06 11:20 +0200
[PATCH V10 8/8] irqchip/gicv3-its: Use MADT ITS subtable to do PCI/MSI domain initialization Tomasz Nowicki <tn@semihalf.com> - 2016-09-06 11:20 +0200
[PATCH V10 3/8] PCI/MSI: Setup MSI domain on a per-device basis using IORT ACPI table Tomasz Nowicki <tn@semihalf.com> - 2016-09-06 11:20 +0200
[PATCH V10 5/8] irqchip/gicv3-its: Refactor ITS DT init code to prepare for ACPI Tomasz Nowicki <tn@semihalf.com> - 2016-09-06 11:20 +0200
csiph-web