Linux-ACPI Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] resource: Prevent irqresource_disabled() from erasing flags
@ 2021-04-08 10:37 Angela Czubak
  2021-04-08 18:03 ` Rafael J. Wysocki
  0 siblings, 1 reply; 2+ messages in thread
From: Angela Czubak @ 2021-04-08 10:37 UTC (permalink / raw)
  To: rafael.j.wysocki
  Cc: akpm, mika.westerberg, john.garry, linux-acpi, upstream, dtor,
	Angela Czubak

Do not overwrite flags as it leads to erasing triggering and polarity
information which might be useful in case of hard-coded interrupts.
This way the information can be read later on even though mapping to
APIC domain failed.

Signed-off-by: Angela Czubak <acz@semihalf.com>
---
Some Chromebooks use hard-coded interrupts in their ACPI tables.
This is an excerpt as dumped on Relm:

...
            Name (_HID, "ELAN0001")  // _HID: Hardware ID
            Name (_DDN, "Elan Touchscreen ")  // _DDN: DOS Device Name
            Name (_UID, 0x05)  // _UID: Unique ID
            Name (ISTP, Zero)
            Method (_CRS, 0, NotSerialized)  // _CRS: Current Resource Settings
            {
                Name (BUF0, ResourceTemplate ()
                {
                    I2cSerialBusV2 (0x0010, ControllerInitiated, 0x00061A80,
                        AddressingMode7Bit, "\\_SB.I2C1", 
                        0x00, ResourceConsumer, , Exclusive,
                        )
                    Interrupt (ResourceConsumer, Edge, ActiveLow, Exclusive, ,, )
                    {
                        0x000000B8,
                    }
                })
                Return (BUF0) /* \_SB_.I2C1.ETSA._CRS.BUF0 */ 
            }
...

This interrupt is hard-coded to 0xB8 = 184 which is too high to be mapped
to IO-APIC, so no triggering information is propagated as acpi_register_gsi()
fails and irqresource_disabled() is issued, which leads to erasing triggering
and polarity information.
If that function added its flags instead of overwriting them the correct IRQ
type would be set even for the hard-coded interrupts, which allows device driver
to retrieve it.
This patch was originaly sent to linux-kernel@vger.kernel.org.
I am resending it to linux-acpi@vger.kernel.org as per request of
Rafael J. Wysocki to gather more visibility.
Please let us know if you see possible issues with not erasing the flags or this
modification seems acceptable.
Best Regards,
Angela

 include/linux/ioport.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/ioport.h b/include/linux/ioport.h
index 55de385c839cf..647744d8514e0 100644
--- a/include/linux/ioport.h
+++ b/include/linux/ioport.h
@@ -331,7 +331,7 @@ static inline void irqresource_disabled(struct resource *res, u32 irq)
 {
 	res->start = irq;
 	res->end = irq;
-	res->flags = IORESOURCE_IRQ | IORESOURCE_DISABLED | IORESOURCE_UNSET;
+	res->flags |= IORESOURCE_IRQ | IORESOURCE_DISABLED | IORESOURCE_UNSET;
 }
 
 extern struct address_space *iomem_get_mapping(void);
-- 
2.17.1


^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH] resource: Prevent irqresource_disabled() from erasing flags
  2021-04-08 10:37 [PATCH] resource: Prevent irqresource_disabled() from erasing flags Angela Czubak
@ 2021-04-08 18:03 ` Rafael J. Wysocki
  0 siblings, 0 replies; 2+ messages in thread
From: Rafael J. Wysocki @ 2021-04-08 18:03 UTC (permalink / raw)
  To: Angela Czubak
  Cc: Rafael Wysocki, Andrew Morton, Mika Westerberg, John Garry,
	ACPI Devel Maling List, upstream, Dmitry Torokhov

On Thu, Apr 8, 2021 at 12:38 PM Angela Czubak <acz@semihalf.com> wrote:
>
> Do not overwrite flags as it leads to erasing triggering and polarity
> information which might be useful in case of hard-coded interrupts.
> This way the information can be read later on even though mapping to
> APIC domain failed.
>
> Signed-off-by: Angela Czubak <acz@semihalf.com>
> ---
> Some Chromebooks use hard-coded interrupts in their ACPI tables.
> This is an excerpt as dumped on Relm:
>
> ...
>             Name (_HID, "ELAN0001")  // _HID: Hardware ID
>             Name (_DDN, "Elan Touchscreen ")  // _DDN: DOS Device Name
>             Name (_UID, 0x05)  // _UID: Unique ID
>             Name (ISTP, Zero)
>             Method (_CRS, 0, NotSerialized)  // _CRS: Current Resource Settings
>             {
>                 Name (BUF0, ResourceTemplate ()
>                 {
>                     I2cSerialBusV2 (0x0010, ControllerInitiated, 0x00061A80,
>                         AddressingMode7Bit, "\\_SB.I2C1",
>                         0x00, ResourceConsumer, , Exclusive,
>                         )
>                     Interrupt (ResourceConsumer, Edge, ActiveLow, Exclusive, ,, )
>                     {
>                         0x000000B8,
>                     }
>                 })
>                 Return (BUF0) /* \_SB_.I2C1.ETSA._CRS.BUF0 */
>             }
> ...
>
> This interrupt is hard-coded to 0xB8 = 184 which is too high to be mapped
> to IO-APIC, so no triggering information is propagated as acpi_register_gsi()
> fails and irqresource_disabled() is issued, which leads to erasing triggering
> and polarity information.
> If that function added its flags instead of overwriting them the correct IRQ
> type would be set even for the hard-coded interrupts, which allows device driver
> to retrieve it.
> This patch was originaly sent to linux-kernel@vger.kernel.org.
> I am resending it to linux-acpi@vger.kernel.org as per request of
> Rafael J. Wysocki to gather more visibility.
> Please let us know if you see possible issues with not erasing the flags or this
> modification seems acceptable.

I've applied the patch as 5.13 material, thanks!

>  include/linux/ioport.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/linux/ioport.h b/include/linux/ioport.h
> index 55de385c839cf..647744d8514e0 100644
> --- a/include/linux/ioport.h
> +++ b/include/linux/ioport.h
> @@ -331,7 +331,7 @@ static inline void irqresource_disabled(struct resource *res, u32 irq)
>  {
>         res->start = irq;
>         res->end = irq;
> -       res->flags = IORESOURCE_IRQ | IORESOURCE_DISABLED | IORESOURCE_UNSET;
> +       res->flags |= IORESOURCE_IRQ | IORESOURCE_DISABLED | IORESOURCE_UNSET;
>  }
>
>  extern struct address_space *iomem_get_mapping(void);
> --
> 2.17.1
>

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, back to index

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-08 10:37 [PATCH] resource: Prevent irqresource_disabled() from erasing flags Angela Czubak
2021-04-08 18:03 ` Rafael J. Wysocki

Linux-ACPI Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-acpi/0 linux-acpi/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-acpi linux-acpi/ https://lore.kernel.org/linux-acpi \
		linux-acpi@vger.kernel.org
	public-inbox-index linux-acpi

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-acpi


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git