From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753203AbbJOOD6 (ORCPT ); Thu, 15 Oct 2015 10:03:58 -0400 Received: from mail-bl2on0092.outbound.protection.outlook.com ([65.55.169.92]:44043 "EHLO na01-bl2-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1750752AbbJOODz (ORCPT ); Thu, 15 Oct 2015 10:03:55 -0400 Authentication-Results: spf=none (sender IP is 165.204.84.222) smtp.mailfrom=amd.com; apm.com; dkim=none (message not signed) header.d=none;apm.com; dmarc=permerror action=none header.from=amd.com; X-WSS-ID: 0NW9LQD-08-DL9-02 X-M-MSG: Subject: Re: [PATCH V2 6/6] gicv2m: acpi: Introducing GICv2m ACPI support To: Tomasz Nowicki , , , , References: <1444865156-9870-1-git-send-email-Suravee.Suthikulpanit@amd.com> <1444865156-9870-7-git-send-email-Suravee.Suthikulpanit@amd.com> <561F449E.2050803@semihalf.com> CC: Lorenzo Pieralisi , Will Deacon , Catalin Marinas , , , , , , From: Suravee Suthikulanit Message-ID: <561FB238.10509@amd.com> Date: Thu, 15 Oct 2015 09:03:36 -0500 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:38.0) Gecko/20100101 Thunderbird/38.2.0 MIME-Version: 1.0 In-Reply-To: <561F449E.2050803@semihalf.com> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit X-EOPAttributedMessage: 0 X-Forefront-Antispam-Report: CIP:165.204.84.222;CTRY:US;IPV:NLI;EFV:NLI;SFV:NSPM;SFS:(10009020)(6009001)(2980300002)(428002)(479174004)(189002)(164054003)(377454003)(199003)(24454002)(51914003)(97736004)(76176999)(23746002)(86362001)(36756003)(46102003)(50986999)(64126003)(5007970100001)(47776003)(11100500001)(65816999)(92566002)(5008740100001)(65806001)(2950100001)(64706001)(120886001)(5001770100001)(77096005)(105586002)(65956001)(50466002)(33656002)(19580395003)(4001350100001)(106466001)(87936001)(54356999)(2201001)(101416001)(189998001)(83506001);DIR:OUT;SFP:1101;SCL:1;SRVR:CY1PR12MB0854;H:atltwp02.amd.com;FPR:;SPF:None;PTR:InfoDomainNonexistent;MX:1;A:1;LANG:en; X-Microsoft-Exchange-Diagnostics: 1;CY1PR12MB0854;2:+US8BtQ0qznCtaQ6sDNZDOg8wDo1wEXQRRl2NH5gKKP+EyAotPN62S78+sWCc8gObHmdTa9yGAwZ4t4t6zq/2Taz+nj+PUS62d9rObR7tQcKRONtBBVQvDv5iKfKqmQfkPqfRMmIUnv1MHv61rubxYYYO7ySO3HH0FDVARsAVDk=;3:MWskLJi9RebMw0CUQdjLL/NoHU4DWBe5oUh6jwMF3Gs1GqLb30pX8mY8m5yHd1J76qWV4K4h7KHEBcPmYPhtpVT4sMAdRGM7Yhg+lUnO0uoAU2h/BGOaDR6edv772g1c+5wcr63Uqchxo5jZUONclyrAgKMt6WV0IH4xLSiCLfc28i2ZNEjQNtCkXXH0Fjz3jAiLyydxF2TWOUeeRjjykNaS0Cs+3cnGIpwTGrKTrWn3ez5OFc0ADUDVzFUnPwzt;25:1nEo0BDbQbcYiNnX0Dg9xwY3w5ZUktQbGAEW9OTBgG1e4+WrMqTiAFP+Tvz9dgjSicFyLUc4Pa7pL+iRRgsjT7nLsszdMTs8IMvK5ypS47LRkPWS8Tcyh3L1JJEypTV21lwQEmDWPitJHZ+0gPhB2WMM9FPUwZBvJgQawva4w/b5DMK7/SldZ22hC9Db4kNTpLrr6KRvGVdiziNp0xVWb6UEekxNROH9syMEWbmc+Tlg+rTbtt79MaKOGou6SBs7daC6V7C9ikIlZZve+jrbjg== X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:CY1PR12MB0854; X-Microsoft-Exchange-Diagnostics: 1;CY1PR12MB0854;20:gnagl/74Y1o90+RMOp2EnNS3neDPnz/Mxlic05W/GL7fziZC5xWkRH0Dao7dRFYi/937rzAuSUf1Vxrh3Lfnuc8/MOGODReo1ZyyqG0dB6ULKFmhFQI7qQI7ygmm6oYl+NJqK+H9ie9PCpWAxu75Etp2iVA13Z0JcQUqkCJbcwEESrMv2A538uWRTeHNYC2IEcvjyFPeNA3tS/tbN65sDlBsurIC4HJ0o45oGn676min1zcJdfIT1hJv9+D/Ozw7StifIJdgCfGt77Zw9jOBpKfPRb16+h45OafcMLBziNSyjcyESNT1k7uCPJHzM7Qztkqy+gUZ7dUqAv9ReAqzSivy0rawL80Q1zP9JmyH6B6da3v74rFUVS7eHshnxt2rHEnhQypGgtBunHbCmteawwT8ZInIlq8BxkZAYeNbBYdq+vlclDgPbGfbswLO8sv1chOLMXfqSZFZsCTpW1a6vw1i9BlfMRbIbWrO4yM2gcr+00yIK/ERyHmLegx7b+uF;4:Xq/VnuxL1stanq1sCTq7K0ReEn2R/3IxGSHiY0X3Sd1EBiGWVJFDx9KGqHO7NtKUNqb4KLhuLK/5Incgoo7F32Bq5RRZj0wT29nDUJ+9+6Br9uOpa6hBcLHFcGTM4t7fJiR/9gXQjqyYm/HMDV0TuWAv3fKkwDDarE2Zyt2Dg7oK7rFm33Tvqrwvc2j9ztgDHpnoSl2pe407dea7M4UoEq+Zf2MK1Dm2QEDUvEJx+Q7MAOWOEzcsSq90E7jMwF5/6R/RQ+TszssrFHNmKzrvSZ04RpryGuSIrh9M19Adn/gmbTZhcevKHlCKVKGI/shp31WlIS5TmHbV+eZNWUrQINFgKXzZKfTpcm+fW5wMNsY= X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:; X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(601004)(2401047)(5005006)(520078)(8121501046)(3002001);SRVR:CY1PR12MB0854;BCL:0;PCL:0;RULEID:;SRVR:CY1PR12MB0854; X-Forefront-PRVS: 0730093765 X-Microsoft-Exchange-Diagnostics: =?Windows-1252?Q?1;CY1PR12MB0854;23:1dMmOU8qaSprJnfCdMpOdBzTQjPg/udbJ6Wvj?= =?Windows-1252?Q?cRCjMKe4I6K0foWhvBxTWj9awtFIgSmtXQl1Ur9keC20KP1nuX3+C9bH?= =?Windows-1252?Q?X8LLY2wCzshSWHS2tBN3iViKYbfGJ+TCjc2oKrNSJUCyiB+/+7yV8zKm?= =?Windows-1252?Q?xPimssmwaxF9rbk+S80On5b9bw0BPilGjMSbLIOtGosiWXfhJH7CFlEp?= =?Windows-1252?Q?52ohiMxjciPcnG+rU3VHtpC1+g9hGv4RKCapvQDlfcTG0XtjYP/OgW+w?= =?Windows-1252?Q?6y6EVNcvNC6zqTwMcH/dbZVvv0sCxqJSIG73/l02eLdK354V1F/29a38?= =?Windows-1252?Q?KUIBfFqP0CppK4PFb933W17flJBWSVuSLFStAQcakM5qO59oCPIwgGIE?= =?Windows-1252?Q?i2FnTD6kLeo7IWNlWV6IkPTykbX588cN2U0K/DAKfyGLJXgnV2tJBz9X?= =?Windows-1252?Q?VXORykos5rcryZiiyOkoINkSIyIM1P/K4XeZh+1clAOny/TXJPwMq8wN?= =?Windows-1252?Q?QeLNB6g2Zw7T/PeqfZ103ry0F/hlsxzoBjY1Cw50RjFn0VJ/QMNlPAN6?= =?Windows-1252?Q?cGpScJs0n4j4ttyOnKibZ10oGDb1SUdybN1WdxBmVkueHysf8PhLPaNX?= =?Windows-1252?Q?Bkf1+yNk5eCfIf7+Z40GmJXKE//GBxCpUsmqndtXfdv5DL/8dKhuM+4F?= =?Windows-1252?Q?n0T9WiCjB+Nw3G++r6c4w0m/s3x6miYXWmbTEOASuBRTCPsZXG7+td2R?= =?Windows-1252?Q?6X1impCUo7S6nOx++t/AUZcpWp+NB1F2P6zRm/2XZP+Rw6NZHaD8ryOv?= =?Windows-1252?Q?N+TJCjPthfejRP9J4UNOMrRaYNv1QHOR770JlsqkttACFFP+unCSyKUJ?= =?Windows-1252?Q?J8B3sLqSmJNf+mTnLNvyRIaqG3sZ/xrfGCwUkEHudmLlWAm0ggzWpJs6?= =?Windows-1252?Q?MAsb5XKK7QN30T+wn6rXiheh4UX86EQDvWZCxLoYnnsSskVZQUkZ2Nr3?= =?Windows-1252?Q?O0XP7P4+p29IOtR5s5P5NEIzWhHkRtGnpisN32p6hlEh1whg4GzmwnJR?= =?Windows-1252?Q?yjniNd60Nq75rQkdZN47DW7Go1k95EcpfrX9Mh8z0I6Mw9yHBt7ECs1S?= =?Windows-1252?Q?81ANdvwE2Lhc7Ht6UxePyWHDVdPcJ0pkWefwBzLj1SCKjWKeA9V0B4p5?= =?Windows-1252?Q?D0dWK4DBn+eL3xKe67jEBjnLPU1DVZJA3HOROOxYycA7zjC5JNY?= X-Microsoft-Exchange-Diagnostics: 1;CY1PR12MB0854;5:nkOsKUeW5lUvRqcI+5+TovvFuApxe2Szr+2tWZNbBYr+v8RYemL5WXVUZpBUF39FBwFAC0SZ3Yt0D4DsHNN9I89WKmqFKzwH7UbfBVvGtHfmJk25vTX7DNQ6gBpa/9wd3ZwWI+1Is9edZOCzwxCmAg==;24:bn2DO7NLf9V/1MRuUb0rVbs7DbWvEcyfP5ngKzvvc3yaD1gwZWQzKIucWusLj+qs2bZBWD0ZtFl6hro5zKNgHPjumx7Yrm7ceBoOLuQmS7A=;20:JIQFp6WpmeOUWFn4gPYMRXozkE+g3Hokppj5umHk3A4Rgi4hDKCMoDpg+pwx1NFu6KyWI3GqQ2a2/b3k0HQuQ3gh/uPKvD/a3g8J9EKNreZVH2UTdF6QkPwUVCqVrrokXJRtlr1BbaN0ggmjJfhzLgWdbZweoHSNYiyvFaH/VVZkjutjZE8oenqo0WZ8Tr3hsl1I8Nrz82vG2S9XZEZYAt0D31rec/2vDzav7tJoffdowBS9tVuqWOPjv8ApHGtm SpamDiagnosticOutput: 1:23 SpamDiagnosticMetadata: NSPM X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 15 Oct 2015 14:03:51.0706 (UTC) X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=3dd8961f-e488-4e60-8e11-a82d994e183d;Ip=[165.204.84.222];Helo=[atltwp02.amd.com] X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY1PR12MB0854 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Tomasz, Thanks for the feedback. On 10/15/2015 1:15 AM, Tomasz Nowicki wrote: >> [..] >> @@ -138,6 +140,11 @@ static int gicv2m_irq_gic_domain_alloc(struct >> irq_domain *domain, >> fwspec.param[0] = 0; >> fwspec.param[1] = hwirq - 32; >> fwspec.param[2] = IRQ_TYPE_EDGE_RISING; >> + } else if (is_fwnode_irqchip(domain->parent->fwnode)) { >> + fwspec.fwnode = domain->parent->fwnode; >> + fwspec.param_count = 2; >> + fwspec.param[0] = hwirq; >> + fwspec.param[1] = IRQ_TYPE_EDGE_RISING & IRQ_TYPE_SENSE_MASK; > How about just: > fwspec.param[1] = IRQ_TYPE_EDGE_RISING; Right. > >> } else { >> return -EINVAL; >> } >> @@ -255,6 +262,8 @@ static void gicv2m_teardown(void) >> kfree(v2m->bm); >> iounmap(v2m->base); >> of_node_put(to_of_node(v2m->fwnode)); >> + if (is_fwnode_irqchip(v2m->fwnode)) >> + irq_domain_free_fwnode(v2m->fwnode); >> kfree(v2m); >> } >> } >> @@ -359,6 +368,8 @@ static int __init gicv2m_init_one(struct >> fwnode_handle *fwnode, >> >> if (to_of_node(fwnode)) >> name = to_of_node(fwnode)->name; >> + else >> + name = irq_domain_get_irqchip_fwnode_name(fwnode); >> >> pr_info("Frame %s: range[%#lx:%#lx], SPI[%d:%d]\n", name, >> (unsigned long)res->start, (unsigned long)res->end, >> @@ -415,3 +426,86 @@ int __init gicv2m_of_init(struct device_node >> *node, struct irq_domain *parent) >> gicv2m_teardown(); >> return ret; >> } >> + >> +#ifdef CONFIG_ACPI >> +static int acpi_num_msi; >> + >> +static struct fwnode_handle *gicv2m_get_fwnode(struct device *dev) >> +{ >> + struct v2m_data *data; >> + >> + if (WARN_ON(acpi_num_msi <= 0)) >> + return NULL; >> + >> + /* We only return the fwnode of the first MSI frame. */ >> + data = list_first_entry_or_null(&v2m_nodes, >> + struct v2m_data, entry); > This can be one line and still fits within 80 characters. Ok. >> + if (!data) >> + return NULL; >> + >> + return data->fwnode; >> +} >> + >> +static int __init >> +acpi_parse_madt_msi(struct acpi_subtable_header *header, >> + const unsigned long end) >> +{ >> + int ret; >> + struct resource res; >> + u32 spi_start = 0, nr_spis = 0; >> + struct acpi_madt_generic_msi_frame *m; >> + struct fwnode_handle *fwnode = NULL; >> + >> + m = (struct acpi_madt_generic_msi_frame *)header; >> + if (BAD_MADT_ENTRY(m, end)) >> + return -EINVAL; >> + >> + res.start = m->base_address; >> + res.end = m->base_address + 0x1000; >> + >> + if (m->flags & ACPI_MADT_OVERRIDE_SPI_VALUES) { >> + spi_start = m->spi_base; >> + nr_spis = m->spi_count; >> + >> + pr_info("ACPI overriding V2M MSI_TYPER (base:%u, num:%u)\n", >> + spi_start, nr_spis); >> + } >> + >> + fwnode = irq_domain_alloc_fwnode((void *)m->base_address); >> + if (!fwnode) { >> + pr_err("Unable to allocate GICv2m domain token\n"); >> + return -EINVAL; >> + } >> + >> + ret = gicv2m_init_one(fwnode, spi_start, nr_spis, &res); > I case of error, we should call here: > irq_domain_free_fwnode(fwnode); This should have already been handled when returning from the acpi_parse_madt_msi() in gicv2m_teardown() since we would need to iterate through all existing MSI frame to clean up. >> + >> + return ret; >> +} >> + >> +int __init gicv2m_acpi_init(struct irq_domain *parent) >> +{ >> + int ret; >> + >> + if (acpi_num_msi > 0) >> + return 0; >> + >> + acpi_num_msi = >> acpi_table_parse_madt(ACPI_MADT_TYPE_GENERIC_MSI_FRAME, >> + acpi_parse_madt_msi, 0); >> + >> + if (acpi_num_msi <= 0) >> + goto err_out; > If acpi_table_parse_madt return 0, then we don't need to call > gicv2m_teardown(). Instead we can simply return, optionally add some > pr_info. Well, gicv2m_teardown would do nothing, so this is just > cosmetic and up to you. I'd be hesitate to add pr_info here since V2m is optional, we already print information for each frame found. I think I would just leave this one the way it is. >> [..] >> diff --git a/include/linux/irqchip/arm-gic.h >> b/include/linux/irqchip/arm-gic.h >> index bae69e5..7398538 100644 >> --- a/include/linux/irqchip/arm-gic.h >> +++ b/include/linux/irqchip/arm-gic.h >> @@ -108,6 +108,12 @@ void gic_init(unsigned int nr, int start, >> >> int gicv2m_of_init(struct device_node *node, struct irq_domain >> *parent); >> >> +#ifdef CONFIG_ACPI >> +#include > I think, we don't need this include here. You already added it to itq-gic.c Right. Thanks, Suravee