| Message ID | 1483363905-2806-7-git-send-email-hanjun.guo@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
On 04/01/17 08:25, Hanjun Guo wrote: > On 2017/1/4 15:29, Tomasz Nowicki wrote: >> On 04.01.2017 08:02, Hanjun Guo wrote: >>> Hi Tomasz, >>> >>> On 2017/1/3 15:41, Tomasz Nowicki wrote: >>>> Hi, >>>> >>>> Can we merge patch 4 & 6 into one patch so that we keep refactoring part >>>> as one piece ? I do not see a reason to keep them separate or have patch >>>> 5 in between. You can refactor what needs to be refactored, add >>>> necessary functions to iort.c and then support ACPI for >>>> irq-gic-v3-its-platform-msi.c >>> >>> There are two functions here, >>> - retrieve the dev id from IORT which was DT based only; >>> >>> - init the platform msi domain from MADT; >>> >>> For each of them split it into two steps, >>> - refactor the code for ACPI later and it's easy for review >>> because wen can easily to figure out it has functional >>> change or not >>> >>> - add ACPI functionality >>> >>> Does it make sense? >> >> It is up to Marc, but personally I prefer: >> 1. Refactor dev id retrieving and init function in one patch and >> highlight no functional changes in changelog >> 2. Crate necessary infrastructure in iort.c >> 3. Then add ACPI support to irq-gic-v3-its-platform-msi.c > > I have no strong preferences, and it's easy to do so as just > need to squash/reorder the patches. > > Marc, Lorenzo, could you give some suggestions here? I think it'd make the reviewing easier to have patches that are semantically grouped together (all the ACPI IORT together, for example). It would help understanding where you're aiming at instead of jumping from irqchip to ACPI and back every other patch... Thanks, M. -- Jazz is not dead. It just smells funny...
diff --git a/drivers/irqchip/irq-gic-v3-its-platform-msi.c b/drivers/irqchip/irq-gic-v3-its-platform-msi.c index 16587a9..ff72704 100644 --- a/drivers/irqchip/irq-gic-v3-its-platform-msi.c +++ b/drivers/irqchip/irq-gic-v3-its-platform-msi.c @@ -84,34 +84,43 @@ static int its_pmsi_prepare(struct irq_domain *domain, struct device *dev, {}, }; -static int __init its_pmsi_init(void) +static int __init its_pmsi_init_one(struct fwnode_handle *fwnode, + const char *name) { - struct device_node *np; struct irq_domain *parent; + parent = irq_find_matching_fwnode(fwnode, DOMAIN_BUS_NEXUS); + if (!parent || !msi_get_domain_info(parent)) { + pr_err("%s: unable to locate ITS domain\n", name); + return -ENXIO; + } + + if (!platform_msi_create_irq_domain(fwnode, &its_pmsi_domain_info, + parent)) { + pr_err("%s: unable to create platform domain\n", name); + return -ENXIO; + } + + pr_info("Platform MSI: %s domain created\n", name); + return 0; +} + +static void __init its_pmsi_of_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); - continue; - } - - if (!platform_msi_create_irq_domain(of_node_to_fwnode(np), - &its_pmsi_domain_info, - parent)) { - pr_err("%s: unable to create platform domain\n", - np->full_name); - continue; - } - - pr_info("Platform MSI: %s domain created\n", np->full_name); + its_pmsi_init_one(of_node_to_fwnode(np), np->full_name); } +} +static int __init its_pmsi_init(void) +{ + its_pmsi_of_init(); return 0; } early_initcall(its_pmsi_init);