* [PATCH 0/2]linux-next: fix two bugs in ACPI EC driver @ 2008-09-11 3:17 Kevin Hao 2008-09-11 3:17 ` [PATCH 1/2] fix acpi ec read write bug Kevin Hao 0 siblings, 1 reply; 7+ messages in thread From: Kevin Hao @ 2008-09-11 3:17 UTC (permalink / raw) To: ak, astarikovskiy, linux-kernel, linux-acpi Hi all, These patches fix two bugs in ACPI EC driver. First one fix ec read and write data bug. Second one fix ec EC_FLAGS_GPE_STORM flag always set bug. Thanks, Kevin ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2] fix acpi ec read write bug 2008-09-11 3:17 [PATCH 0/2]linux-next: fix two bugs in ACPI EC driver Kevin Hao @ 2008-09-11 3:17 ` Kevin Hao 2008-09-11 3:17 ` [PATCH 2/2] fix acpi ec set GPE storm flag bug Kevin Hao ` (2 more replies) 0 siblings, 3 replies; 7+ messages in thread From: Kevin Hao @ 2008-09-11 3:17 UTC (permalink / raw) To: ak, astarikovskiy, linux-kernel, linux-acpi Fill in command unit of transaction_data structure, otherwise gpe_transaction will skip read or write instruction. Signed-off-by: Kevin Hao <kexin.hao@windriver.com> --- drivers/acpi/ec.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c index bd3555c..0c65e82 100644 --- a/drivers/acpi/ec.c +++ b/drivers/acpi/ec.c @@ -240,6 +240,7 @@ static int acpi_ec_transaction_unlocked(struct acpi_ec *ec, u8 command, } atomic_set(&ec->irq_count, 0); /* fill in transaction structure */ + ec->t.command = command; ec->t.wdata = wdata; ec->t.wlen = wdata_len; ec->t.rdata = rdata; -- 1.5.6.2.220.g44701 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/2] fix acpi ec set GPE storm flag bug 2008-09-11 3:17 ` [PATCH 1/2] fix acpi ec read write bug Kevin Hao @ 2008-09-11 3:17 ` Kevin Hao 2008-09-11 4:07 ` [PATCH 1/2] fix acpi ec read write bug Zhao Yakui 2008-09-11 6:10 ` Alexey Starikovskiy 2 siblings, 0 replies; 7+ messages in thread From: Kevin Hao @ 2008-09-11 3:17 UTC (permalink / raw) To: ak, astarikovskiy, linux-kernel, linux-acpi The intent of the code was clear, however the missing braces meant that EC_FLAGS_GPE_STORM was incorrectly being set all the time Signed-off-by: Kevin Hao <kexin.hao@windriver.com> --- drivers/acpi/ec.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c index 0c65e82..d8f273d 100644 --- a/drivers/acpi/ec.c +++ b/drivers/acpi/ec.c @@ -268,9 +268,10 @@ end: /* it is safe to enable GPE outside of transaction */ acpi_enable_gpe(NULL, ec->gpe, ACPI_NOT_ISR); } else if (test_bit(EC_FLAGS_GPE_MODE, &ec->flags) && - atomic_read(&ec->irq_count) > ACPI_EC_STORM_THRESHOLD) + atomic_read(&ec->irq_count) > ACPI_EC_STORM_THRESHOLD) { pr_debug(PREFIX "GPE storm detected\n"); set_bit(EC_FLAGS_GPE_STORM, &ec->flags); + } return 0; } -- 1.5.6.2.220.g44701 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] fix acpi ec read write bug 2008-09-11 3:17 ` [PATCH 1/2] fix acpi ec read write bug Kevin Hao 2008-09-11 3:17 ` [PATCH 2/2] fix acpi ec set GPE storm flag bug Kevin Hao @ 2008-09-11 4:07 ` Zhao Yakui 2008-09-11 4:49 ` Kevin Hao 2008-09-11 6:10 ` Alexey Starikovskiy 2 siblings, 1 reply; 7+ messages in thread From: Zhao Yakui @ 2008-09-11 4:07 UTC (permalink / raw) To: Kevin Hao; +Cc: ak, astarikovskiy, linux-kernel, linux-acpi On Thu, 2008-09-11 at 11:17 +0800, Kevin Hao wrote: > Fill in command unit of transaction_data structure, otherwise > gpe_transaction will skip read or write instruction. > > Signed-off-by: Kevin Hao <kexin.hao@windriver.com> > --- > drivers/acpi/ec.c | 1 + > 1 files changed, 1 insertions(+), 0 deletions(-) > > diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c > index bd3555c..0c65e82 100644 > --- a/drivers/acpi/ec.c > +++ b/drivers/acpi/ec.c > @@ -240,6 +240,7 @@ static int acpi_ec_transaction_unlocked(struct acpi_ec *ec, u8 command, > } > atomic_set(&ec->irq_count, 0); > /* fill in transaction structure */ > + ec->t.command = command; It is also OK to add this explicitly. In fact the ec->t.command will be assigned in the function of acpi_ec_write_cmd. Thanks. > ec->t.wdata = wdata; > ec->t.wlen = wdata_len; > ec->t.rdata = rdata; ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] fix acpi ec read write bug 2008-09-11 4:07 ` [PATCH 1/2] fix acpi ec read write bug Zhao Yakui @ 2008-09-11 4:49 ` Kevin Hao 2008-09-11 5:54 ` Zhao Yakui 0 siblings, 1 reply; 7+ messages in thread From: Kevin Hao @ 2008-09-11 4:49 UTC (permalink / raw) To: Zhao Yakui; +Cc: ak, astarikovskiy, linux-kernel, linux-acpi On Thu, 2008-09-11 at 12:07 +0800, Zhao Yakui wrote: > On Thu, 2008-09-11 at 11:17 +0800, Kevin Hao wrote: > > Fill in command unit of transaction_data structure, otherwise > > gpe_transaction will skip read or write instruction. > > > > Signed-off-by: Kevin Hao <kexin.hao@windriver.com> > > --- > > drivers/acpi/ec.c | 1 + > > 1 files changed, 1 insertions(+), 0 deletions(-) > > > > diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c > > index bd3555c..0c65e82 100644 > > --- a/drivers/acpi/ec.c > > +++ b/drivers/acpi/ec.c > > @@ -240,6 +240,7 @@ static int acpi_ec_transaction_unlocked(struct acpi_ec *ec, u8 command, > > } > > atomic_set(&ec->irq_count, 0); > > /* fill in transaction structure */ > > + ec->t.command = command; > It is also OK to add this explicitly. In fact the ec->t.command will be > assigned in the function of acpi_ec_write_cmd. NO, I am using the latest linux-next kernel and I don't found ec->t.command is assigned in acpi_ec_write_cmd function. Thanks, Kevin > > Thanks. > > ec->t.wdata = wdata; > > ec->t.wlen = wdata_len; > > ec->t.rdata = rdata; > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] fix acpi ec read write bug 2008-09-11 4:49 ` Kevin Hao @ 2008-09-11 5:54 ` Zhao Yakui 0 siblings, 0 replies; 7+ messages in thread From: Zhao Yakui @ 2008-09-11 5:54 UTC (permalink / raw) To: Kevin Hao; +Cc: ak, astarikovskiy, linux-kernel, linux-acpi On Thu, 2008-09-11 at 12:49 +0800, Kevin Hao wrote: > On Thu, 2008-09-11 at 12:07 +0800, Zhao Yakui wrote: > > On Thu, 2008-09-11 at 11:17 +0800, Kevin Hao wrote: > > > Fill in command unit of transaction_data structure, otherwise > > > gpe_transaction will skip read or write instruction. > > > > > > Signed-off-by: Kevin Hao <kexin.hao@windriver.com> > > > --- > > > drivers/acpi/ec.c | 1 + > > > 1 files changed, 1 insertions(+), 0 deletions(-) > > > > > > diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c > > > index bd3555c..0c65e82 100644 > > > --- a/drivers/acpi/ec.c > > > +++ b/drivers/acpi/ec.c > > > @@ -240,6 +240,7 @@ static int acpi_ec_transaction_unlocked(struct acpi_ec *ec, u8 command, > > > } > > > atomic_set(&ec->irq_count, 0); > > > /* fill in transaction structure */ > > > + ec->t.command = command; > > It is also OK to add this explicitly. In fact the ec->t.command will be > > assigned in the function of acpi_ec_write_cmd. > > NO, I am using the latest linux-next kernel and I don't found > ec->t.command is assigned in acpi_ec_write_cmd function. static inline void acpi_ec_write_cmd(struct acpi_ec *ec, u8 command) { pr_debug(PREFIX "<--- command = 0x%2.2x\n", command); - outb(command, ec->command_addr); + outb((ec->t.command = command), ec->command_addr); } It is very obscure so that it is not easy to understand. > Thanks, > Kevin > > > > > Thanks. > > > ec->t.wdata = wdata; > > > ec->t.wlen = wdata_len; > > > ec->t.rdata = rdata; > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] fix acpi ec read write bug 2008-09-11 3:17 ` [PATCH 1/2] fix acpi ec read write bug Kevin Hao 2008-09-11 3:17 ` [PATCH 2/2] fix acpi ec set GPE storm flag bug Kevin Hao 2008-09-11 4:07 ` [PATCH 1/2] fix acpi ec read write bug Zhao Yakui @ 2008-09-11 6:10 ` Alexey Starikovskiy 2 siblings, 0 replies; 7+ messages in thread From: Alexey Starikovskiy @ 2008-09-11 6:10 UTC (permalink / raw) To: Kevin Hao; +Cc: ak, linux-kernel, linux-acpi Kevin Hao wrote: > Fill in command unit of transaction_data structure, otherwise > gpe_transaction will skip read or write instruction. > > Signed-off-by: Kevin Hao <kexin.hao@windriver.com> > --- > drivers/acpi/ec.c | 1 + > 1 files changed, 1 insertions(+), 0 deletions(-) > > diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c > index bd3555c..0c65e82 100644 > --- a/drivers/acpi/ec.c > +++ b/drivers/acpi/ec.c > @@ -240,6 +240,7 @@ static int acpi_ec_transaction_unlocked(struct acpi_ec *ec, u8 command, > } > atomic_set(&ec->irq_count, 0); > /* fill in transaction structure */ > + ec->t.command = command; > ec->t.wdata = wdata; > ec->t.wlen = wdata_len; > ec->t.rdata = rdata; NAK Command is filled in acpi_ec_write_command() in order to make window between start of the transaction and validity of the transaction data to the bare minimum. ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2008-09-11 6:10 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2008-09-11 3:17 [PATCH 0/2]linux-next: fix two bugs in ACPI EC driver Kevin Hao 2008-09-11 3:17 ` [PATCH 1/2] fix acpi ec read write bug Kevin Hao 2008-09-11 3:17 ` [PATCH 2/2] fix acpi ec set GPE storm flag bug Kevin Hao 2008-09-11 4:07 ` [PATCH 1/2] fix acpi ec read write bug Zhao Yakui 2008-09-11 4:49 ` Kevin Hao 2008-09-11 5:54 ` Zhao Yakui 2008-09-11 6:10 ` Alexey Starikovskiy
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox