Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1741297 > unrolled thread

Re: [RFT PATCH v2] gpiolib: allow gpio irqchip to map irqs dynamically

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [RFT PATCH v2] gpiolib: allow gpio irqchip to map irqs  dynamically Mika Westerberg <mika.westerberg@linux.intel.com> - 2017-09-28 10:40 +0200
    Re: [RFT PATCH v2] gpiolib: allow gpio irqchip to map irqs  dynamically Grygorii Strashko <grygorii.strashko@ti.com> - 2017-09-28 16:10 +0200
      Re: [RFT PATCH v2] gpiolib: allow gpio irqchip to map irqs  dynamically Mika Westerberg <mika.westerberg@linux.intel.com> - 2017-09-28 16:40 +0200

#1741297 — Re: [RFT PATCH v2] gpiolib: allow gpio irqchip to map irqs dynamically

On Fri, Jul 21, 2017 at 11:49:00AM -0500, Grygorii Strashko wrote:
> Now IRQ mappings are always created for all (allowed) GPIOs in gpiochip in
> gpiochip_irqchip_add_key() which goes against the idea of SPARSE_IRQ and,
> as result, leads to:
>  - increasing of memory consumption for IRQ descriptors most of which will
> never ever be used (espessially on platform with a high number of GPIOs).
> (sizeof(struct irq_desc) == 256 on my tested platforms)
>  - imposibility to use GPIO irqchip APIs by gpio drivers when HW implements
> GPIO IRQ functionality as IRQ crossbar/router which has only limited
> number of IRQ outputs (example from [1], all GPIOs can be mapped on only 8
> IRQs).
> 
> Hence, remove static IRQ mapping code from gpiochip_irqchip_add_key() and
> instead replace irq_find_mapping() with irq_create_mapping() in
> gpiochip_to_irq(). Also add additional gpiochip_irqchip_irq_valid() calls
> in gpiochip_to_irq() and gpiochip_irq_map().
> 
> After this change gpio2irq mapping will happen the following way when GPIO
> irqchip APIs are used by gpio driver:
>  - IRQ mappings will be created statically if driver passes first_irq>0
> vlaue in gpiochip_irqchip_add_key().
>  - IRQ mappings will be created dynamically from gpio_to_irq() or
> of_irq_get().
> 
> Tested on am335x-evm and dra72-evm-revc.
> - dra72-evm-revc: number of created irq mappings decreased from 402 -> 135
>   Mem savings 267*256 = 68352 (66kB)
> - am335x-evm: number of created irq mappings decreased from 188 -> 63
>   Mem savings 125*256 = 32000 (31kB)
> 
> [1] https://lkml.org/lkml/2017/6/15/428
> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>

Hi Grygorii,

It looks like dc749a09ea5e41 ("gpiolib: allow gpio irqchip to map irqs
dynamically") broke the quirk we added for some Intel Cherryview
systems [1]. Basically after this keyboard of those systems stopped
working again.

Any idea what might be wrong?

I've CC'd Chris Gorman who reported the issue.

[1] https://bugzilla.kernel.org/show_bug.cgi?id=194945

[toc] | [next] | [standalone]

#1741577

On 09/28/2017 03:33 AM, Mika Westerberg wrote:
> On Fri, Jul 21, 2017 at 11:49:00AM -0500, Grygorii Strashko wrote:
>> Now IRQ mappings are always created for all (allowed) GPIOs in gpiochip in
>> gpiochip_irqchip_add_key() which goes against the idea of SPARSE_IRQ and,
>> as result, leads to:
>>   - increasing of memory consumption for IRQ descriptors most of which will
>> never ever be used (espessially on platform with a high number of GPIOs).
>> (sizeof(struct irq_desc) == 256 on my tested platforms)
>>   - imposibility to use GPIO irqchip APIs by gpio drivers when HW implements
>> GPIO IRQ functionality as IRQ crossbar/router which has only limited
>> number of IRQ outputs (example from [1], all GPIOs can be mapped on only 8
>> IRQs).
>>
>> Hence, remove static IRQ mapping code from gpiochip_irqchip_add_key() and
>> instead replace irq_find_mapping() with irq_create_mapping() in
>> gpiochip_to_irq(). Also add additional gpiochip_irqchip_irq_valid() calls
>> in gpiochip_to_irq() and gpiochip_irq_map().
>>
>> After this change gpio2irq mapping will happen the following way when GPIO
>> irqchip APIs are used by gpio driver:
>>   - IRQ mappings will be created statically if driver passes first_irq>0
>> vlaue in gpiochip_irqchip_add_key().
>>   - IRQ mappings will be created dynamically from gpio_to_irq() or
>> of_irq_get().
>>
>> Tested on am335x-evm and dra72-evm-revc.
>> - dra72-evm-revc: number of created irq mappings decreased from 402 -> 135
>>    Mem savings 267*256 = 68352 (66kB)
>> - am335x-evm: number of created irq mappings decreased from 188 -> 63
>>    Mem savings 125*256 = 32000 (31kB)
>>
>> [1] https://lkml.org/lkml/2017/6/15/428
>> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
> 
> Hi Grygorii,
> 
> It looks like dc749a09ea5e41 ("gpiolib: allow gpio irqchip to map irqs
> dynamically") broke the quirk we added for some Intel Cherryview
> systems [1]. Basically after this keyboard of those systems stopped
> working again.
> 
> Any idea what might be wrong?

Yes. kernel do not map all GPIO IRQs by default now.
I've red related discussion and, as per my understanding (correct me if i'm wrong),
on this platform all GPIO IRQs have to be mapped on fixed range of Linux IRQs.
In this case, It's reasonable to specify first_irq parameter in gpiochip_irqchip_add()
which should force IRQ mapping creation.

> 
> I've CC'd Chris Gorman who reported the issue.
> 
> [1] https://bugzilla.kernel.org/show_bug.cgi?id=194945
> 



-- 
regards,
-grygorii

[toc] | [prev] | [next] | [standalone]

#1741602

On Thu, Sep 28, 2017 at 09:02:12AM -0500, Grygorii Strashko wrote:
> 
> 
> On 09/28/2017 03:33 AM, Mika Westerberg wrote:
> > On Fri, Jul 21, 2017 at 11:49:00AM -0500, Grygorii Strashko wrote:
> >> Now IRQ mappings are always created for all (allowed) GPIOs in gpiochip in
> >> gpiochip_irqchip_add_key() which goes against the idea of SPARSE_IRQ and,
> >> as result, leads to:
> >>   - increasing of memory consumption for IRQ descriptors most of which will
> >> never ever be used (espessially on platform with a high number of GPIOs).
> >> (sizeof(struct irq_desc) == 256 on my tested platforms)
> >>   - imposibility to use GPIO irqchip APIs by gpio drivers when HW implements
> >> GPIO IRQ functionality as IRQ crossbar/router which has only limited
> >> number of IRQ outputs (example from [1], all GPIOs can be mapped on only 8
> >> IRQs).
> >>
> >> Hence, remove static IRQ mapping code from gpiochip_irqchip_add_key() and
> >> instead replace irq_find_mapping() with irq_create_mapping() in
> >> gpiochip_to_irq(). Also add additional gpiochip_irqchip_irq_valid() calls
> >> in gpiochip_to_irq() and gpiochip_irq_map().
> >>
> >> After this change gpio2irq mapping will happen the following way when GPIO
> >> irqchip APIs are used by gpio driver:
> >>   - IRQ mappings will be created statically if driver passes first_irq>0
> >> vlaue in gpiochip_irqchip_add_key().
> >>   - IRQ mappings will be created dynamically from gpio_to_irq() or
> >> of_irq_get().
> >>
> >> Tested on am335x-evm and dra72-evm-revc.
> >> - dra72-evm-revc: number of created irq mappings decreased from 402 -> 135
> >>    Mem savings 267*256 = 68352 (66kB)
> >> - am335x-evm: number of created irq mappings decreased from 188 -> 63
> >>    Mem savings 125*256 = 32000 (31kB)
> >>
> >> [1] https://lkml.org/lkml/2017/6/15/428
> >> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
> > 
> > Hi Grygorii,
> > 
> > It looks like dc749a09ea5e41 ("gpiolib: allow gpio irqchip to map irqs
> > dynamically") broke the quirk we added for some Intel Cherryview
> > systems [1]. Basically after this keyboard of those systems stopped
> > working again.
> > 
> > Any idea what might be wrong?
> 
> Yes. kernel do not map all GPIO IRQs by default now.
> I've red related discussion and, as per my understanding (correct me if i'm wrong),
> on this platform all GPIO IRQs have to be mapped on fixed range of Linux IRQs.
> In this case, It's reasonable to specify first_irq parameter in gpiochip_irqchip_add()
> which should force IRQ mapping creation.

So can you propose what value I should pass then? It was working just
fine until this change.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web