| Message ID | 20240927141606.66826-2-hdegoede@redhat.com |
|---|---|
| State | Accepted |
| Commit | 63539defee17bf0cbd8e24078cf103efee9c6633 |
| Headers | show |
| Series | [1/4] ACPI: resource: Remove duplicate Asus E1504GAB IRQ override | expand |
Dear Hans, Thank you for your patch. Am 27.09.24 um 16:16 schrieb Hans de Goede: > Like other Asus Vivobooks, the Asus Vivobook Go E1404GA has a DSDT > describing IRQ 1 as ActiveLow, while the kernel overrides to Edge_High. > > $ sudo dmesg | grep DMI:.*BIOS > [ 0.000000] DMI: ASUSTeK COMPUTER INC. Vivobook Go E1404GA_E1404GA/E1404GA, BIOS E1404GA.302 08/23/2023 > $ sudo cp /sys/firmware/acpi/tables/DSDT dsdt.dat > $ iasl -d dsdt.dat > $ grep -A 30 PS2K dsdt.dsl | grep IRQ -A 1 > IRQ (Level, ActiveLow, Exclusive, ) > {1} > > There already is an entry in the irq1_level_low_skip_override[] DMI match > table for the "E1404GAB", change this to match on "E1404GA" to cover > the E1404GA model as well (DMI_MATCH() does a substring match). Ah, good to know. Thank you for fixing it. > Reported-by: Paul Menzel <pmenzel@molgen.mpg.de> > Closes: https://bugzilla.kernel.org/show_bug.cgi?id=219224 > Cc: Tamim Khan <tamim@fusetak.com> > Cc: stable@vger.kernel.org > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > --- > Note this patch replaces Paul Menzel's patch which added a new entry > for the "E1404GA", instead of loosening the "E1404GAB" match: > https://lore.kernel.org/linux-acpi/20240911081612.3931-1-pmenzel@molgen.mpg.de/ > --- > drivers/acpi/resource.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c > index 1ff251fd1901..dfe108e2ccde 100644 > --- a/drivers/acpi/resource.c > +++ b/drivers/acpi/resource.c > @@ -504,10 +504,10 @@ static const struct dmi_system_id irq1_level_low_skip_override[] = { > }, > }, > { > - /* Asus Vivobook Go E1404GAB */ > + /* Asus Vivobook Go E1404GA* */ I guess people are going to grep for the model, if something does not work, so maybe the known ones should listed. I know it’s not optimal, as the comments are very likely be incomplete, but it’s better than than not having it listed, in my opinion. > .matches = { > DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."), > - DMI_MATCH(DMI_BOARD_NAME, "E1404GAB"), > + DMI_MATCH(DMI_BOARD_NAME, "E1404GA"), > }, > }, > { Kind regards, Paul
Hi, On 30-Sep-24 12:42 PM, Paul Menzel wrote: > Dear Hans, > > > Thank you for your patch. > > Am 27.09.24 um 16:16 schrieb Hans de Goede: >> Like other Asus Vivobooks, the Asus Vivobook Go E1404GA has a DSDT >> describing IRQ 1 as ActiveLow, while the kernel overrides to Edge_High. >> >> $ sudo dmesg | grep DMI:.*BIOS >> [ 0.000000] DMI: ASUSTeK COMPUTER INC. Vivobook Go E1404GA_E1404GA/E1404GA, BIOS E1404GA.302 08/23/2023 >> $ sudo cp /sys/firmware/acpi/tables/DSDT dsdt.dat >> $ iasl -d dsdt.dat >> $ grep -A 30 PS2K dsdt.dsl | grep IRQ -A 1 >> IRQ (Level, ActiveLow, Exclusive, ) >> {1} >> >> There already is an entry in the irq1_level_low_skip_override[] DMI match >> table for the "E1404GAB", change this to match on "E1404GA" to cover >> the E1404GA model as well (DMI_MATCH() does a substring match). > > Ah, good to know. Thank you for fixing it. > >> Reported-by: Paul Menzel <pmenzel@molgen.mpg.de> >> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=219224 >> Cc: Tamim Khan <tamim@fusetak.com> >> Cc: stable@vger.kernel.org >> Signed-off-by: Hans de Goede <hdegoede@redhat.com> >> --- >> Note this patch replaces Paul Menzel's patch which added a new entry >> for the "E1404GA", instead of loosening the "E1404GAB" match: >> https://lore.kernel.org/linux-acpi/20240911081612.3931-1-pmenzel@molgen.mpg.de/ >> --- >> drivers/acpi/resource.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c >> index 1ff251fd1901..dfe108e2ccde 100644 >> --- a/drivers/acpi/resource.c >> +++ b/drivers/acpi/resource.c >> @@ -504,10 +504,10 @@ static const struct dmi_system_id irq1_level_low_skip_override[] = { >> }, >> }, >> { >> - /* Asus Vivobook Go E1404GAB */ >> + /* Asus Vivobook Go E1404GA* */ > > I guess people are going to grep for the model, if something does not work, so maybe the known ones should listed. I know it’s not optimal, as the comments are very likely be incomplete, but it’s better than than not having it listed, in my opinion. That is a valid point, OTOH I don't think we want to take patches later just to update the comment if more models show up. I guess we could change the comment to: /* Asus Vivobook Go E1404GA / E1404GAB */ Rafael any preference from you here ? (1) Regards, Hans 1) Other then coming up with a better fix which does not require this quirks at all ... > >> .matches = { >> DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."), >> - DMI_MATCH(DMI_BOARD_NAME, "E1404GAB"), >> + DMI_MATCH(DMI_BOARD_NAME, "E1404GA"), >> }, >> }, >> { > > > Kind regards, > > Paul >
On Mon, Sep 30, 2024 at 12:56 PM Hans de Goede <hdegoede@redhat.com> wrote: > > Hi, > > On 30-Sep-24 12:42 PM, Paul Menzel wrote: > > Dear Hans, > > > > > > Thank you for your patch. > > > > Am 27.09.24 um 16:16 schrieb Hans de Goede: > >> Like other Asus Vivobooks, the Asus Vivobook Go E1404GA has a DSDT > >> describing IRQ 1 as ActiveLow, while the kernel overrides to Edge_High. > >> > >> $ sudo dmesg | grep DMI:.*BIOS > >> [ 0.000000] DMI: ASUSTeK COMPUTER INC. Vivobook Go E1404GA_E1404GA/E1404GA, BIOS E1404GA.302 08/23/2023 > >> $ sudo cp /sys/firmware/acpi/tables/DSDT dsdt.dat > >> $ iasl -d dsdt.dat > >> $ grep -A 30 PS2K dsdt.dsl | grep IRQ -A 1 > >> IRQ (Level, ActiveLow, Exclusive, ) > >> {1} > >> > >> There already is an entry in the irq1_level_low_skip_override[] DMI match > >> table for the "E1404GAB", change this to match on "E1404GA" to cover > >> the E1404GA model as well (DMI_MATCH() does a substring match). > > > > Ah, good to know. Thank you for fixing it. > > > >> Reported-by: Paul Menzel <pmenzel@molgen.mpg.de> > >> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=219224 > >> Cc: Tamim Khan <tamim@fusetak.com> > >> Cc: stable@vger.kernel.org > >> Signed-off-by: Hans de Goede <hdegoede@redhat.com> > >> --- > >> Note this patch replaces Paul Menzel's patch which added a new entry > >> for the "E1404GA", instead of loosening the "E1404GAB" match: > >> https://lore.kernel.org/linux-acpi/20240911081612.3931-1-pmenzel@molgen.mpg.de/ > >> --- > >> drivers/acpi/resource.c | 4 ++-- > >> 1 file changed, 2 insertions(+), 2 deletions(-) > >> > >> diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c > >> index 1ff251fd1901..dfe108e2ccde 100644 > >> --- a/drivers/acpi/resource.c > >> +++ b/drivers/acpi/resource.c > >> @@ -504,10 +504,10 @@ static const struct dmi_system_id irq1_level_low_skip_override[] = { > >> }, > >> }, > >> { > >> - /* Asus Vivobook Go E1404GAB */ > >> + /* Asus Vivobook Go E1404GA* */ > > > > I guess people are going to grep for the model, if something does not work, so maybe the known ones should listed. I know it’s not optimal, as the comments are very likely be incomplete, but it’s better than than not having it listed, in my opinion. > > That is a valid point, OTOH I don't think we want to take patches later just to update > the comment if more models show up. > > I guess we could change the comment to: > > /* Asus Vivobook Go E1404GA / E1404GAB */ > > Rafael any preference from you here ? (1) Not really. > Regards, > > Hans > > > > > > 1) Other then coming up with a better fix which does not require this quirks at all ... Yeah.
Hi, On 30-Sep-24 2:18 PM, Rafael J. Wysocki wrote: > On Mon, Sep 30, 2024 at 12:56 PM Hans de Goede <hdegoede@redhat.com> wrote: >> >> Hi, >> >> On 30-Sep-24 12:42 PM, Paul Menzel wrote: >>> Dear Hans, >>> >>> >>> Thank you for your patch. >>> >>> Am 27.09.24 um 16:16 schrieb Hans de Goede: >>>> Like other Asus Vivobooks, the Asus Vivobook Go E1404GA has a DSDT >>>> describing IRQ 1 as ActiveLow, while the kernel overrides to Edge_High. >>>> >>>> $ sudo dmesg | grep DMI:.*BIOS >>>> [ 0.000000] DMI: ASUSTeK COMPUTER INC. Vivobook Go E1404GA_E1404GA/E1404GA, BIOS E1404GA.302 08/23/2023 >>>> $ sudo cp /sys/firmware/acpi/tables/DSDT dsdt.dat >>>> $ iasl -d dsdt.dat >>>> $ grep -A 30 PS2K dsdt.dsl | grep IRQ -A 1 >>>> IRQ (Level, ActiveLow, Exclusive, ) >>>> {1} >>>> >>>> There already is an entry in the irq1_level_low_skip_override[] DMI match >>>> table for the "E1404GAB", change this to match on "E1404GA" to cover >>>> the E1404GA model as well (DMI_MATCH() does a substring match). >>> >>> Ah, good to know. Thank you for fixing it. >>> >>>> Reported-by: Paul Menzel <pmenzel@molgen.mpg.de> >>>> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=219224 >>>> Cc: Tamim Khan <tamim@fusetak.com> >>>> Cc: stable@vger.kernel.org >>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com> >>>> --- >>>> Note this patch replaces Paul Menzel's patch which added a new entry >>>> for the "E1404GA", instead of loosening the "E1404GAB" match: >>>> https://lore.kernel.org/linux-acpi/20240911081612.3931-1-pmenzel@molgen.mpg.de/ >>>> --- >>>> drivers/acpi/resource.c | 4 ++-- >>>> 1 file changed, 2 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c >>>> index 1ff251fd1901..dfe108e2ccde 100644 >>>> --- a/drivers/acpi/resource.c >>>> +++ b/drivers/acpi/resource.c >>>> @@ -504,10 +504,10 @@ static const struct dmi_system_id irq1_level_low_skip_override[] = { >>>> }, >>>> }, >>>> { >>>> - /* Asus Vivobook Go E1404GAB */ >>>> + /* Asus Vivobook Go E1404GA* */ >>> >>> I guess people are going to grep for the model, if something does not work, so maybe the known ones should listed. I know it’s not optimal, as the comments are very likely be incomplete, but it’s better than than not having it listed, in my opinion. >> >> That is a valid point, OTOH I don't think we want to take patches later just to update >> the comment if more models show up. >> >> I guess we could change the comment to: >> >> /* Asus Vivobook Go E1404GA / E1404GAB */ >> >> Rafael any preference from you here ? (1) > > Not really. Ok, then my vote goes to keeping this as is. So if you're happy with this series please merge it as is. Regards, Hans
On Mon, Sep 30, 2024 at 2:20 PM Hans de Goede <hdegoede@redhat.com> wrote: > > Hi, > > On 30-Sep-24 2:18 PM, Rafael J. Wysocki wrote: > > On Mon, Sep 30, 2024 at 12:56 PM Hans de Goede <hdegoede@redhat.com> wrote: > >> > >> Hi, > >> > >> On 30-Sep-24 12:42 PM, Paul Menzel wrote: > >>> Dear Hans, > >>> > >>> > >>> Thank you for your patch. > >>> > >>> Am 27.09.24 um 16:16 schrieb Hans de Goede: > >>>> Like other Asus Vivobooks, the Asus Vivobook Go E1404GA has a DSDT > >>>> describing IRQ 1 as ActiveLow, while the kernel overrides to Edge_High. > >>>> > >>>> $ sudo dmesg | grep DMI:.*BIOS > >>>> [ 0.000000] DMI: ASUSTeK COMPUTER INC. Vivobook Go E1404GA_E1404GA/E1404GA, BIOS E1404GA.302 08/23/2023 > >>>> $ sudo cp /sys/firmware/acpi/tables/DSDT dsdt.dat > >>>> $ iasl -d dsdt.dat > >>>> $ grep -A 30 PS2K dsdt.dsl | grep IRQ -A 1 > >>>> IRQ (Level, ActiveLow, Exclusive, ) > >>>> {1} > >>>> > >>>> There already is an entry in the irq1_level_low_skip_override[] DMI match > >>>> table for the "E1404GAB", change this to match on "E1404GA" to cover > >>>> the E1404GA model as well (DMI_MATCH() does a substring match). > >>> > >>> Ah, good to know. Thank you for fixing it. > >>> > >>>> Reported-by: Paul Menzel <pmenzel@molgen.mpg.de> > >>>> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=219224 > >>>> Cc: Tamim Khan <tamim@fusetak.com> > >>>> Cc: stable@vger.kernel.org > >>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com> > >>>> --- > >>>> Note this patch replaces Paul Menzel's patch which added a new entry > >>>> for the "E1404GA", instead of loosening the "E1404GAB" match: > >>>> https://lore.kernel.org/linux-acpi/20240911081612.3931-1-pmenzel@molgen.mpg.de/ > >>>> --- > >>>> drivers/acpi/resource.c | 4 ++-- > >>>> 1 file changed, 2 insertions(+), 2 deletions(-) > >>>> > >>>> diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c > >>>> index 1ff251fd1901..dfe108e2ccde 100644 > >>>> --- a/drivers/acpi/resource.c > >>>> +++ b/drivers/acpi/resource.c > >>>> @@ -504,10 +504,10 @@ static const struct dmi_system_id irq1_level_low_skip_override[] = { > >>>> }, > >>>> }, > >>>> { > >>>> - /* Asus Vivobook Go E1404GAB */ > >>>> + /* Asus Vivobook Go E1404GA* */ > >>> > >>> I guess people are going to grep for the model, if something does not work, so maybe the known ones should listed. I know it’s not optimal, as the comments are very likely be incomplete, but it’s better than than not having it listed, in my opinion. > >> > >> That is a valid point, OTOH I don't think we want to take patches later just to update > >> the comment if more models show up. > >> > >> I guess we could change the comment to: > >> > >> /* Asus Vivobook Go E1404GA / E1404GAB */ > >> > >> Rafael any preference from you here ? (1) > > > > Not really. > > Ok, then my vote goes to keeping this as is. So if you're happy > with this series please merge it as is. Now queued up as 6.12-rc2 material, thanks!
diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c index 1ff251fd1901..dfe108e2ccde 100644 --- a/drivers/acpi/resource.c +++ b/drivers/acpi/resource.c @@ -504,10 +504,10 @@ static const struct dmi_system_id irq1_level_low_skip_override[] = { }, }, { - /* Asus Vivobook Go E1404GAB */ + /* Asus Vivobook Go E1404GA* */ .matches = { DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."), - DMI_MATCH(DMI_BOARD_NAME, "E1404GAB"), + DMI_MATCH(DMI_BOARD_NAME, "E1404GA"), }, }, {
Like other Asus Vivobooks, the Asus Vivobook Go E1404GA has a DSDT describing IRQ 1 as ActiveLow, while the kernel overrides to Edge_High. $ sudo dmesg | grep DMI:.*BIOS [ 0.000000] DMI: ASUSTeK COMPUTER INC. Vivobook Go E1404GA_E1404GA/E1404GA, BIOS E1404GA.302 08/23/2023 $ sudo cp /sys/firmware/acpi/tables/DSDT dsdt.dat $ iasl -d dsdt.dat $ grep -A 30 PS2K dsdt.dsl | grep IRQ -A 1 IRQ (Level, ActiveLow, Exclusive, ) {1} There already is an entry in the irq1_level_low_skip_override[] DMI match table for the "E1404GAB", change this to match on "E1404GA" to cover the E1404GA model as well (DMI_MATCH() does a substring match). Reported-by: Paul Menzel <pmenzel@molgen.mpg.de> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=219224 Cc: Tamim Khan <tamim@fusetak.com> Cc: stable@vger.kernel.org Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- Note this patch replaces Paul Menzel's patch which added a new entry for the "E1404GA", instead of loosening the "E1404GAB" match: https://lore.kernel.org/linux-acpi/20240911081612.3931-1-pmenzel@molgen.mpg.de/ --- drivers/acpi/resource.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)