lkml.org 
[lkml]   [2016]   [Nov]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH V6 2/3] ACPI: Add support for ResourceSource/IRQ domain mapping
Hey Lorenzo, Hanjun,

On 2016-11-11 08:33, Hanjun Guo wrote:
> Hi Lorenzo,
>
> On 11/11/2016 01:58 AM, Lorenzo Pieralisi wrote:
>> On Thu, Nov 10, 2016 at 10:02:35AM -0500, agustinv@codeaurora.org
>> wrote:
>>> Hey Hanjun,
>>>
>>> On 2016-11-09 21:36, Hanjun Guo wrote:
>>>> Hi Marc, Rafael, Lorenzo,
>>>>
>>>> Since we agreed to add a probe deferral if we failed to get irq
>>>> resources which mirroring the DT does (patch 1 in this patch set),
>>>> I think the last blocker to make things work both for Agustin and
>>>> me [1] is this patch, which makes the interrupt producer and
>>>> consumer
>>>> work in ACPI, we have two different solution for one thing, we'd
>>>> happy
>>>> to work together for one solution, could you give some suggestions
>>>> please?
>>>>
>>>> [1]:
>>>> https://mail-archive.com/linux-kernel@vger.kernel.org/msg1257419.html
>>>>
>>>> Agustin, I have some comments below.
>>>>
>>>> On 2016/10/29 4:48, Agustin Vega-Frias wrote:
>>>>> This allows irqchip drivers to associate an ACPI DSDT device to
>>>>> an IRQ domain and provides support for using the ResourceSource
>>>>> in Extended IRQ Resources to find the domain and map the IRQs
>>>>> specified on that domain.
>>>>>
>>>>> Signed-off-by: Agustin Vega-Frias <agustinv@codeaurora.org>
>>>>> ---
>>>>> drivers/acpi/Makefile | 1 +
>>>>> drivers/acpi/irqdomain.c | 119
>>>>> +++++++++++++++++++++++++++++++++++++++++++++++
>>>>
>>>> Could we just reuse the gsi.c and not introduce a new
>>>> file, probably we can change the gsi.c to irqdomain.c
>>>> or something similar, then reuse the code in gsi.c.
>>>
>>> I was thinking just that after we chatted off-list.
>>
>> Yes, that's a fair point.
>>
>>> I might revisit and see what I come up with given that we already
>>> have
>>> a device argument and we could pass the IRQ source there.
>>
>> I agree with the approach taken by this patch, I do not like much
>> passing around struct acpi_resource_source *source (in particular
>> the dummy struct) I do not think it is needed, I will comment on
>> the code.
>
> thanks for your time to have a look:)
>
>>
>> Hopefully there is not any buggy FW out there that does use the
>> resource source inappropriately otherwise we will notice on x86/ia64
>> (ie you can't blame FW if it breaks the kernel) but I suspect the
>> only way to find out is by trying, the patch has to go through
>> Rafael's
>> review anyway before getting there so it is fine.
>
> I think we can avoid that by not touching the logic that x86/ia64
> already used, but only adding interrupt producer/consumer function.

I looked at this more today and implemented a new patch that I plan to
test over the weekend, but I wanted to let you know the approach I am
pursuing.

On the new patch use of ResourceSource when parsing ACPI Extended IRQ
Resources is conditional on CONFIG_ACPI_GENERIC_GSI. The reason for this
is two fold:

1. Since we wanted to reduce duplication and place the new APIs on the
same source file as acpi_register_gsi, which is already under that
config flag.
2. So the patch does not have effect on platforms not using the generic
GSI support, including x86/ia64.

If support for this is needed outside platforms using the generic GSI
implementation, we can move these APIs out to their own source file
and eliminate the CONFIG_ACPI_GENERIC_GSI conditionality.

I'll send the new patch, hopefully some time tomorrow, but please let
me know if you have concerns with this approach.

Thanks,
Agustin

--
Qualcomm Datacenter Technologies, Inc. on behalf of the Qualcomm
Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a
Linux Foundation Collaborative Project.

\
 
 \ /
  Last update: 2016-11-12 04:01    [W:0.074 / U:0.164 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site