ACPI/EC: Cleanup debugging messages and coding styles.
cover ACPI/EC: Cleanup debugging messages and coding styles.
This patchset contains trivial cleanup code to make it easier to search in the EC debugging messages.
It also includes a patch to correct coding style issues that detected by recent scripts/checkpatch.pl.
No functional changes. This patchset can only make remote debugging more efficient.
Lv Zheng (5):
ACPI/EC: Add CPU ID to debugging messages. ACPI/EC: Enhance the logs to apply to QR_EC transactions. ACPI/EC: Add detailed command/query debugging information. ACPI/EC: Refine event/query debugging messages. ACPI/EC: Cleanup coding style.
drivers/acpi/ec.c | 109 ++++++++++++++++++++++++++++++++++------------------- 1 file changed, 70 insertions(+), 39 deletions(-)
Signature · 1 line
1.7.10
PATCH 1/5 ACPI/EC: Add CPU ID to debugging messages.
This patch adds CPU ID to the context entries' debugging output. no functional changes.
--- drivers/acpi/ec.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/acpi/ec.c +2 -1
@@ -181,7 +181,8 @@ static bool advance_transaction(struct acpi_ec *ec)u8 status;bool wakeup = false; -pr_debug ("===== %s =====\n" ,in_interrupt () ?"IRQ" :"TASK" ); +pr_debug ("===== %s (%d) =====\n" , +in_interrupt () ?"IRQ" :"TASK" ,smp_processor_id ()); status =acpi_ec_read_status (ec); t = ec->curr;if (!t)
Signature · 1 line
1.7.10
PATCH 2/5 ACPI/EC: Enhance the logs to apply to QR_EC transactions.
In reply to Lv ZhengCurrently some logs are applied to new transactions, but QR_EC transactions are not included. This patch merges the code path to make the logs also applying to the QR_EC transactions.
--- drivers/acpi/ec.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
drivers/acpi/ec.c +3 -3
@@ -303,12 +303,15 @@ static int acpi_ec_transaction_unlocked(struct acpi_ec *ec,spin_lock_irqsave (&ec->lock, tmp);/* following two actions should be kept atomic */ ec->curr = t; +pr_debug ("transaction start (cmd=0x%02x, addr=0x%02x)\n" , + t->command, t->wdata ? t->wdata[0 ] :0 );start_transaction (ec);spin_unlock_irqrestore (&ec->lock, tmp); ret =ec_poll (ec);spin_lock_irqsave (&ec->lock, tmp);if (ec->curr->command ==ACPI_EC_COMMAND_QUERY )clear_bit (EC_FLAGS_QUERY_PENDING , &ec->flags); +pr_debug ("transaction end\n" ); ec->curr =NULL ;spin_unlock_irqrestore (&ec->lock, tmp);return ret;
@@ -334,8 +337,6 @@ static int acpi_ec_transaction(struct acpi_ec *ec, struct transaction *t) goto unlock; } } -pr_debug ("transaction start (cmd=0x%02x, addr=0x%02x)\n" , - t->command, t->wdata ? t->wdata[0 ] :0 );/* disable GPE during transaction if storm is detected */ if (test_bit (EC_FLAGS_GPE_STORM , &ec->flags)) {/* It has to be disabled, so that it doesn't trigger. */
@@ -356,7 +357,6 @@ static int acpi_ec_transaction(struct acpi_ec *ec, struct transaction *t) t->irq_count);set_bit (EC_FLAGS_GPE_STORM , &ec->flags); } -pr_debug ("transaction end\n" );if (ec->global_lock)acpi_release_global_lock (glk); unlock:
Signature · 1 line
1.7.10
PATCH 3/5 ACPI/EC: Add detailed command/query debugging information.
In reply to Lv ZhengDevelopers really don't need to translate EC commands in mind. This patch adds detailed debugging information for the EC commands.
The address can be found in the follow-up sequential EC_DATA(W) accesses, thus this patch also removes some of the redundant address information.
--- drivers/acpi/ec.c | 34 +++++++++++++++++++++++++++++----- 1 file changed, 29 insertions(+), 5 deletions(-)
drivers/acpi/ec.c +29 -5
@@ -164,6 +164,27 @@ static inline void acpi_ec_write_data(struct acpi_ec *ec, u8 data)outb (data, ec->data_addr); } +#ifdef DEBUG +static const char *acpi_ec_cmd_string (u8 cmd) +{ +switch (cmd) { +case 0x80 : +return "RD_EC" ; +case 0x81 : +return "WR_EC" ; +case 0x82 : +return "BE_EC" ; +case 0x83 : +return "BD_EC" ; +case 0x84 : +return "QR_EC" ; + } +return "UNKNOWN" ; +} +#else +#define acpi_ec_cmd_string (cmd) "UNDEF" +#endif +static int ec_transaction_completed (struct acpi_ec *ec) {unsigned long flags;
@@ -199,7 +220,8 @@ static bool advance_transaction(struct acpi_ec *ec)if (t->rlen == t->ri) { t->flags |=ACPI_EC_COMMAND_COMPLETE ;if (t->command ==ACPI_EC_COMMAND_QUERY ) -pr_debug ("hardware QR_EC completion\n" ); +pr_debug ("***** Command(%s) hardware completion *****\n" , +acpi_ec_cmd_string (t->command)); wakeup = true; } }else
@@ -222,7 +244,8 @@ static bool advance_transaction(struct acpi_ec *ec) t->flags |=ACPI_EC_COMMAND_POLL ; t->rdata[t->ri++] =0x00 ; t->flags |=ACPI_EC_COMMAND_COMPLETE ; -pr_debug ("software QR_EC completion\n" ); +pr_debug ("***** Command(%s) software completion *****\n" , +acpi_ec_cmd_string (t->command)); wakeup = true; }else if ((status &ACPI_EC_FLAG_IBF ) ==0 ) {acpi_ec_write_cmd (ec, t->command);
@@ -303,15 +326,16 @@ static int acpi_ec_transaction_unlocked(struct acpi_ec *ec,spin_lock_irqsave (&ec->lock, tmp);/* following two actions should be kept atomic */ ec->curr = t; -pr_debug ("transaction start (cmd=0x%02x, addr=0x%02x)\n" , - t->command, t->wdata ? t->wdata[0 ] :0 ); +pr_debug ("***** Command(%s) started *****\n" , +acpi_ec_cmd_string (t->command));start_transaction (ec);spin_unlock_irqrestore (&ec->lock, tmp); ret =ec_poll (ec);spin_lock_irqsave (&ec->lock, tmp);if (ec->curr->command ==ACPI_EC_COMMAND_QUERY )clear_bit (EC_FLAGS_QUERY_PENDING , &ec->flags); -pr_debug ("transaction end\n" ); +pr_debug ("***** Command(%s) stopped *****\n" , +acpi_ec_cmd_string (t->command)); ec->curr =NULL ;spin_unlock_irqrestore (&ec->lock, tmp);return ret;
Signature · 1 line
1.7.10
PATCH 4/5 ACPI/EC: Refine event/query debugging messages.
In reply to Lv ZhengThis patch refines event/query debugging messages to use a unified format as commands. Developers can clearly find different processes by checking different log seperators. No functional changes.
--- drivers/acpi/ec.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-)
drivers/acpi/ec.c +8 -6
@@ -332,8 +332,10 @@ static int acpi_ec_transaction_unlocked(struct acpi_ec *ec,spin_unlock_irqrestore (&ec->lock, tmp); ret =ec_poll (ec);spin_lock_irqsave (&ec->lock, tmp); -if (ec->curr->command ==ACPI_EC_COMMAND_QUERY ) +if (ec->curr->command ==ACPI_EC_COMMAND_QUERY ) {clear_bit (EC_FLAGS_QUERY_PENDING , &ec->flags); +pr_debug ("***** Event stopped *****\n" ); + }pr_debug ("***** Command(%s) stopped *****\n" ,acpi_ec_cmd_string (t->command)); ec->curr =NULL ;
@@ -617,12 +619,12 @@ static void acpi_ec_run(void *cxt)struct acpi_ec_query_handler *handler = cxt;if (!handler)return ; -pr_debug ("start query execution\n" ); +pr_debug ("##### Query(0x%02x) started #####\n" , handler->query_bit);if (handler->func) handler->func (handler->data);else if (handler->handle)acpi_evaluate_object (handler->handle,NULL ,NULL ,NULL ); -pr_debug ("stop query execution\n" ); +pr_debug ("##### Query(0x%02x) stopped #####\n" , handler->query_bit);kfree (handler); }
@@ -645,8 +647,8 @@ static int acpi_ec_sync_query(struct acpi_ec *ec, u8 *data)if (!copy)return -ENOMEM ;memcpy (copy, handler,sizeof (*copy)); -pr_debug ("push query execution (0x%2x) on queue\n" , - value); +pr_debug ("##### Query(0x%02x) scheduled #####\n" , + handler->query_bit);return acpi_os_execute ((copy->func) ?OSL_NOTIFY_HANDLER :OSL_GPE_HANDLER , acpi_ec_run, copy);
@@ -669,7 +671,7 @@ static int ec_check_sci(struct acpi_ec *ec, u8 state) {if (state &ACPI_EC_FLAG_SCI ) {if (!test_and_set_bit (EC_FLAGS_QUERY_PENDING , &ec->flags)) { -pr_debug ("push gpe query to the queue\n" ); +pr_debug ("***** Event started *****\n" );return acpi_os_execute (OSL_NOTIFY_HANDLER , acpi_ec_gpe_query, ec); }
Signature · 1 line
1.7.10
PATCH 5/5 ACPI/EC: Cleanup coding style.
In reply to Lv ZhengThis patch cleans up the following coding style issues that are detected by scripts/checkpatch.pl:
ERROR: code indent should use tabs where possible
ERROR: "foo * bar" should be "foo *bar"
WARNING: Missing a blank line after declarations
WARNING: EXPORT_SYMBOL(foo); should immediately follow its function/variable
WARNING: void function return statements are not generally useful
WARNING: else is not generally useful after a break or return
WARNING: break is not useful after a goto or return
WARNING: braces {} are not necessary for single statement blocks
WARNING: line over 80 characters
WARNING: msleep < 20ms can sleep for up to 20ms; see Documentation/timers/timers-howto.txt
No functional changes.
--- drivers/acpi/ec.c | 58 ++++++++++++++++++++++++++++------------------------- 1 file changed, 31 insertions(+), 27 deletions(-)
drivers/acpi/ec.c +31 -27
@@ -128,12 +128,13 @@ static int EC_FLAGS_SKIP_DSDT_SCAN; /* Not all BIOS survive early DSDT scan */static int EC_FLAGS_CLEAR_ON_RESUME ;/* Needs acpi_ec_clear() on boot/resume */ /* -------------------------------------------------------------------------- -Transaction Management --------------------------------------------------------------------------- */ +* Transaction Management +* -------------------------------------------------------------------------- */ static inline u8 acpi_ec_read_status (struct acpi_ec *ec) {u8 x =inb (ec->command_addr); + pr_debug("EC_SC(R) = 0x%2.2x " "SCI_EVT=%d BURST=%d CMD=%d IBF=%d OBF=%d\n", x,
@@ -148,6 +149,7 @@ static inline u8 acpi_ec_read_status(struct acpi_ec *ec)static inline u8 acpi_ec_read_data (struct acpi_ec *ec) {u8 x =inb (ec->data_addr); +pr_debug ("EC_DATA(R) = 0x%2.2x\n" , x);return x; }
@@ -189,6 +191,7 @@ static int ec_transaction_completed(struct acpi_ec *ec) {unsigned long flags;int ret =0 ; +spin_lock_irqsave (&ec->lock, flags);if (ec->curr && (ec->curr->flags &ACPI_EC_COMMAND_COMPLETE )) ret =1 ;
@@ -288,6 +291,7 @@ static int ec_poll(struct acpi_ec *ec) {unsigned long flags;int repeat =5 ;/* number of command restarts */ +while (repeat--) {unsigned long delay = jiffies +msecs_to_jiffies (ec_delay);
@@ -320,6 +324,7 @@ static int acpi_ec_transaction_unlocked(struct acpi_ec *ec, {unsigned long tmp;int ret =0 ; +if (EC_FLAGS_MSI )udelay (ACPI_EC_MSI_UDELAY );/* start transaction */
@@ -347,6 +352,7 @@ static int acpi_ec_transaction(struct acpi_ec *ec, struct transaction *t) {int status;u32 glk; +if (!ec || (!t) || (t->wlen && !t->wdata) || (t->rlen && !t->rdata))return -EINVAL ;if (t->rdata)
@@ -374,7 +380,7 @@ static int acpi_ec_transaction(struct acpi_ec *ec, struct transaction *t)/* check if we received SCI during transaction */ ec_check_sci_sync (ec,acpi_ec_read_status (ec));if (test_bit (EC_FLAGS_GPE_STORM , &ec->flags)) { -msleep (1 ); +msleep (20 );/* It is safe to enable the GPE outside of the transaction. */ acpi_enable_gpe (NULL , ec->gpe, TRUE); }else if (t->irq_count > ec_storm_threshold) {
@@ -410,7 +416,7 @@ static int acpi_ec_burst_disable(struct acpi_ec *ec) acpi_ec_transaction(ec, &t) : 0; } -static int acpi_ec_read (struct acpi_ec *ec,u8 address,u8 * data) +static int acpi_ec_read (struct acpi_ec *ec,u8 address,u8 *data) {int result;u8 d;
@@ -446,10 +452,9 @@ int ec_read(u8 addr, u8 *val)if (!err) { *val = temp_data;return 0 ; - }else -return err; + } +return err; } -EXPORT_SYMBOL (ec_read);int ec_write (u8 addr,u8 val)
@@ -463,22 +468,21 @@ int ec_write(u8 addr, u8 val)return err; } -EXPORT_SYMBOL (ec_write);int ec_transaction (u8 command, -const u8 * wdata,unsigned wdata_len, -u8 * rdata,unsigned rdata_len) +const u8 *wdata,unsigned wdata_len, +u8 *rdata,unsigned rdata_len) {struct transaction t = {.command = command, .wdata = wdata, .rdata = rdata, .wlen = wdata_len, .rlen = rdata_len}; +if (!first_ec)return -ENODEV ;return acpi_ec_transaction (first_ec, &t); } -EXPORT_SYMBOL (ec_transaction);/* Get the handle to the EC device */
@@ -488,7 +492,6 @@ acpi_handle ec_get_handle(void)return NULL ;return first_ec->handle; } -EXPORT_SYMBOL (ec_get_handle);/*
@@ -552,13 +555,14 @@ void acpi_ec_unblock_transactions_early(void)clear_bit (EC_FLAGS_BLOCKED , &first_ec->flags); } -static int acpi_ec_query_unlocked (struct acpi_ec *ec,u8 * data) +static int acpi_ec_query_unlocked (struct acpi_ec *ec,u8 *data) {int result;u8 d;struct transaction t = {.command =ACPI_EC_COMMAND_QUERY , .wdata =NULL , .rdata = &d, .wlen =0 , .rlen =1 }; +if (!ec || !data)return -EINVAL ;/*
@@ -584,6 +588,7 @@ int acpi_ec_add_query_handler(struct acpi_ec *ec, u8 query_bit, {struct acpi_ec_query_handler *handler =kzalloc (sizeof (struct acpi_ec_query_handler ),GFP_KERNEL ); +if (!handler)return -ENOMEM ;
@@ -596,12 +601,12 @@ int acpi_ec_add_query_handler(struct acpi_ec *ec, u8 query_bit,mutex_unlock (&ec->mutex);return 0 ; } -EXPORT_SYMBOL_GPL (acpi_ec_add_query_handler);void acpi_ec_remove_query_handler (struct acpi_ec *ec,u8 query_bit) {struct acpi_ec_query_handler *handler, *tmp; +mutex_lock (&ec->mutex);list_for_each_entry_safe (handler, tmp, &ec->list, node) {if (query_bit == handler->query_bit) {
@@ -611,12 +616,12 @@ void acpi_ec_remove_query_handler(struct acpi_ec *ec, u8 query_bit) }mutex_unlock (&ec->mutex); } -EXPORT_SYMBOL_GPL (acpi_ec_remove_query_handler);static void acpi_ec_run (void *cxt) {struct acpi_ec_query_handler *handler = cxt; +if (!handler)return ;pr_debug ("##### Query(0x%02x) started #####\n" , handler->query_bit);
@@ -660,6 +665,7 @@ static int acpi_ec_sync_query(struct acpi_ec *ec, u8 *data)static void acpi_ec_gpe_query (void *ec_cxt) {struct acpi_ec *ec = ec_cxt; +if (!ec)return ;mutex_lock (&ec->mutex);
@@ -694,8 +700,8 @@ static u32 acpi_ec_gpe_handler(acpi_handle gpe_device, }/* -------------------------------------------------------------------------- -Address Space Management --------------------------------------------------------------------------- */ +* Address Space Management +* -------------------------------------------------------------------------- */ static acpi_status acpi_ec_space_handler(u32 function, acpi_physical_address address,
@@ -726,27 +732,26 @@ acpi_ec_space_handler(u32 function, acpi_physical_address address,switch (result) {case -EINVAL :return AE_BAD_PARAMETER ; -break ;case -ENODEV :return AE_NOT_FOUND ; -break ;case -ETIME :return AE_TIME ; -break ;default :return AE_OK ; } }/* -------------------------------------------------------------------------- -Driver Interface --------------------------------------------------------------------------- */ +* Driver Interface +* -------------------------------------------------------------------------- */ +static acpi_status ec_parse_io_ports (struct acpi_resource *resource,void *context);static struct acpi_ec *make_acpi_ec (void ) {struct acpi_ec *ec =kzalloc (sizeof (struct acpi_ec ),GFP_KERNEL ); +if (!ec)return NULL ; ec->flags =1 <<EC_FLAGS_QUERY_PENDING ;
@@ -769,9 +774,8 @@ acpi_ec_register_query_methods(acpi_handle handle, u32 level, status =acpi_get_name (handle,ACPI_SINGLE_NAME , &buffer); -if (ACPI_SUCCESS (status) &&sscanf (node_name,"_Q%x" , &value) ==1 ) { +if (ACPI_SUCCESS (status) &&sscanf (node_name,"_Q%x" , &value) ==1 )acpi_ec_add_query_handler (ec, value, handle,NULL ,NULL ); - }return AE_OK ; }
@@ -780,7 +784,6 @@ ec_parse_device(acpi_handle handle, u32 Level, void *context, void **retval) {acpi_status status;unsigned long long tmp =0 ; -struct acpi_ec *ec = context;/* clear addr values, ec_parse_io_ports depend on it */
@@ -808,6 +811,7 @@ ec_parse_device(acpi_handle handle, u32 Level, void *context, void **retval)static int ec_install_handlers (struct acpi_ec *ec) {acpi_status status; +if (test_bit (EC_FLAGS_HANDLERS_INSTALLED , &ec->flags))return 0 ; status = acpi_install_gpe_handler(NULL, ec->gpe,
@@ -1105,7 +1109,8 @@ int __init acpi_ec_ecdt_probe(void) boot_ec->data_addr = ecdt_ptr->data.address; boot_ec->gpe = ecdt_ptr->gpe; boot_ec->handle =ACPI_ROOT_OBJECT ; -acpi_get_handle (ACPI_ROOT_OBJECT , ecdt_ptr->id, &boot_ec->handle); +acpi_get_handle (ACPI_ROOT_OBJECT , ecdt_ptr->id, + &boot_ec->handle);/* Don't trust ECDT, which comes from ASUSTek */ if (!EC_FLAGS_VALIDATE_ECDT ) goto install;
@@ -1189,6 +1194,5 @@ static void __exit acpi_ec_exit(void) {acpi_bus_unregister_driver (&acpi_ec_driver); -return ; }#endif /* 0 */
Signature · 1 line
1.7.10
Re: [PATCH 5/5] ACPI/EC: Cleanup coding style.
Quoted 80 lines · Lv Zheng
This patch cleans up the following coding style issues that are detected by
scripts/checkpatch.pl:
ERROR: code indent should use tabs where possible
ERROR: "foo * bar" should be "foo *bar"
WARNING: Missing a blank line after declarations
WARNING:EXPORT_SYMBOL(foo); should immediately follow its function/variable
WARNING: void function return statements are not generally useful
WARNING: else is not generally useful after a break or return
WARNING: break is not useful after a goto or return
WARNING: braces {} are not necessary for single statement blocks
WARNING: line over 80 characters
WARNING: msleep < 20ms can sleep for up to 20ms; seeDocumentation/timers/timers-howto.txt
No functional changes.Signed-off-by Lv Zheng <[email protected]>--- drivers/acpi/ec.c | 58 ++++++++++++++++++++++++++++------------------------- 1 file changed, 31 insertions(+), 27 deletions(-)drivers/acpi/ec.c +8
-2@@ -128,12 +128,13 @@ static int EC_FLAGS_SKIP_DSDT_SCAN; /* Not all BIOS survive early DSDT scan */static int EC_FLAGS_CLEAR_ON_RESUME ;/* Needs acpi_ec_clear() on boot/resume */ /* -------------------------------------------------------------------------- -Transaction Management --------------------------------------------------------------------------- */ +* Transaction Management +* -------------------------------------------------------------------------- */ static inline u8 acpi_ec_read_status (struct acpi_ec *ec) {u8 x =inb (ec->command_addr); + pr_debug("EC_SC(R) = 0x%2.2x " "SCI_EVT=%d BURST=%d CMD=%d IBF=%d OBF=%d\n", x,@@ -148,6 +149,7 @@ static inline u8 acpi_ec_read_status(struct acpi_ec *ec)static inline u8 acpi_ec_read_data (struct acpi_ec *ec) {u8 x =inb (ec->data_addr); +pr_debug ("EC_DATA(R) = 0x%2.2x\n" , x);return x; }@@ -189,6 +191,7 @@ static int ec_transaction_completed(struct acpi_ec *ec) {unsigned long flags;int ret =0 ; +spin_lock_irqsave (&ec->lock, flags);if (ec->curr && (ec->curr->flags &ACPI_EC_COMMAND_COMPLETE )) ret =1 ;@@ -288,6 +291,7 @@ static int ec_poll(struct acpi_ec *ec) {unsigned long flags;int repeat =5 ;/* number of command restarts */ +while (repeat--) {unsigned long delay = jiffies +msecs_to_jiffies (ec_delay);@@ -320,6 +324,7 @@ static int acpi_ec_transaction_unlocked(struct acpi_ec *ec, {unsigned long tmp;int ret =0 ; +if (EC_FLAGS_MSI )udelay (ACPI_EC_MSI_UDELAY );/* start transaction */ @@ -347,6 +352,7 @@ static int acpi_ec_transaction(struct acpi_ec *ec, struct transaction *t) {int status;u32 glk; +if (!ec || (!t) || (t->wlen && !t->wdata) || (t->rlen && !t->rdata))return -EINVAL ;if (t->rdata)
drivers/acpi/ec.c static int acpi_ec_transaction(struct acpi_ec *ec, struct transaction *t) · line 383
Quoted 5 lines
drivers/acpi/ec.c +1
-1@@ -374,7 +380,7 @@ static int acpi_ec_transaction(struct acpi_ec *ec, struct transaction *t)/* check if we received SCI during transaction */ ec_check_sci_sync (ec,acpi_ec_read_status (ec));if (test_bit (EC_FLAGS_GPE_STORM , &ec->flags)) { -msleep (1 ); +msleep (20 );
This is a functional change, so I've dropped it from the patch (it didn't apply for me anyway). Please send it as a separate patch with an appropriate changelog if you want to make it.
Quoted 207 lines
drivers/acpi/ec.c +22
-24/* It is safe to enable the GPE outside of the transaction. */ acpi_enable_gpe (NULL , ec->gpe, TRUE); }else if (t->irq_count > ec_storm_threshold) {@@ -410,7 +416,7 @@ static int acpi_ec_burst_disable(struct acpi_ec *ec) acpi_ec_transaction(ec, &t) : 0; } -static int acpi_ec_read (struct acpi_ec *ec,u8 address,u8 * data) +static int acpi_ec_read (struct acpi_ec *ec,u8 address,u8 *data) {int result;u8 d;@@ -446,10 +452,9 @@ int ec_read(u8 addr, u8 *val)if (!err) { *val = temp_data;return 0 ; - }else -return err; + } +return err; } -EXPORT_SYMBOL (ec_read);int ec_write (u8 addr,u8 val)@@ -463,22 +468,21 @@ int ec_write(u8 addr, u8 val)return err; } -EXPORT_SYMBOL (ec_write);int ec_transaction (u8 command, -const u8 * wdata,unsigned wdata_len, -u8 * rdata,unsigned rdata_len) +const u8 *wdata,unsigned wdata_len, +u8 *rdata,unsigned rdata_len) {struct transaction t = {.command = command, .wdata = wdata, .rdata = rdata, .wlen = wdata_len, .rlen = rdata_len}; +if (!first_ec)return -ENODEV ;return acpi_ec_transaction (first_ec, &t); } -EXPORT_SYMBOL (ec_transaction);/* Get the handle to the EC device */ @@ -488,7 +492,6 @@ acpi_handle ec_get_handle(void)return NULL ;return first_ec->handle; } -EXPORT_SYMBOL (ec_get_handle);/* @@ -552,13 +555,14 @@ void acpi_ec_unblock_transactions_early(void)clear_bit (EC_FLAGS_BLOCKED , &first_ec->flags); } -static int acpi_ec_query_unlocked (struct acpi_ec *ec,u8 * data) +static int acpi_ec_query_unlocked (struct acpi_ec *ec,u8 *data) {int result;u8 d;struct transaction t = {.command =ACPI_EC_COMMAND_QUERY , .wdata =NULL , .rdata = &d, .wlen =0 , .rlen =1 }; +if (!ec || !data)return -EINVAL ;/* @@ -584,6 +588,7 @@ int acpi_ec_add_query_handler(struct acpi_ec *ec, u8 query_bit, {struct acpi_ec_query_handler *handler =kzalloc (sizeof (struct acpi_ec_query_handler ),GFP_KERNEL ); +if (!handler)return -ENOMEM ;@@ -596,12 +601,12 @@ int acpi_ec_add_query_handler(struct acpi_ec *ec, u8 query_bit,mutex_unlock (&ec->mutex);return 0 ; } -EXPORT_SYMBOL_GPL (acpi_ec_add_query_handler);void acpi_ec_remove_query_handler (struct acpi_ec *ec,u8 query_bit) {struct acpi_ec_query_handler *handler, *tmp; +mutex_lock (&ec->mutex);list_for_each_entry_safe (handler, tmp, &ec->list, node) {if (query_bit == handler->query_bit) {@@ -611,12 +616,12 @@ void acpi_ec_remove_query_handler(struct acpi_ec *ec, u8 query_bit) }mutex_unlock (&ec->mutex); } -EXPORT_SYMBOL_GPL (acpi_ec_remove_query_handler);static void acpi_ec_run (void *cxt) {struct acpi_ec_query_handler *handler = cxt; +if (!handler)return ;pr_debug ("##### Query(0x%02x) started #####\n" , handler->query_bit);@@ -660,6 +665,7 @@ static int acpi_ec_sync_query(struct acpi_ec *ec, u8 *data)static void acpi_ec_gpe_query (void *ec_cxt) {struct acpi_ec *ec = ec_cxt; +if (!ec)return ;mutex_lock (&ec->mutex);@@ -694,8 +700,8 @@ static u32 acpi_ec_gpe_handler(acpi_handle gpe_device, }/* -------------------------------------------------------------------------- -Address Space Management --------------------------------------------------------------------------- */ +* Address Space Management +* -------------------------------------------------------------------------- */ static acpi_status acpi_ec_space_handler(u32 function, acpi_physical_address address,@@ -726,27 +732,26 @@ acpi_ec_space_handler(u32 function, acpi_physical_address address,switch (result) {case -EINVAL :return AE_BAD_PARAMETER ; -break ;case -ENODEV :return AE_NOT_FOUND ; -break ;case -ETIME :return AE_TIME ; -break ;default :return AE_OK ; } }/* -------------------------------------------------------------------------- -Driver Interface --------------------------------------------------------------------------- */ +* Driver Interface +* -------------------------------------------------------------------------- */ +static acpi_status ec_parse_io_ports (struct acpi_resource *resource,void *context);static struct acpi_ec *make_acpi_ec (void ) {struct acpi_ec *ec =kzalloc (sizeof (struct acpi_ec ),GFP_KERNEL ); +if (!ec)return NULL ; ec->flags =1 <<EC_FLAGS_QUERY_PENDING ;@@ -769,9 +774,8 @@ acpi_ec_register_query_methods(acpi_handle handle, u32 level, status =acpi_get_name (handle,ACPI_SINGLE_NAME , &buffer); -if (ACPI_SUCCESS (status) &&sscanf (node_name,"_Q%x" , &value) ==1 ) { +if (ACPI_SUCCESS (status) &&sscanf (node_name,"_Q%x" , &value) ==1 )acpi_ec_add_query_handler (ec, value, handle,NULL ,NULL ); - }return AE_OK ; }@@ -780,7 +784,6 @@ ec_parse_device(acpi_handle handle, u32 Level, void *context, void **retval) {acpi_status status;unsigned long long tmp =0 ; -struct acpi_ec *ec = context;/* clear addr values, ec_parse_io_ports depend on it */ @@ -808,6 +811,7 @@ ec_parse_device(acpi_handle handle, u32 Level, void *context, void **retval)static int ec_install_handlers (struct acpi_ec *ec) {acpi_status status; +if (test_bit (EC_FLAGS_HANDLERS_INSTALLED , &ec->flags))return 0 ; status = acpi_install_gpe_handler(NULL, ec->gpe,@@ -1105,7 +1109,8 @@ int __init acpi_ec_ecdt_probe(void) boot_ec->data_addr = ecdt_ptr->data.address; boot_ec->gpe = ecdt_ptr->gpe; boot_ec->handle =ACPI_ROOT_OBJECT ; -acpi_get_handle (ACPI_ROOT_OBJECT , ecdt_ptr->id, &boot_ec->handle); +acpi_get_handle (ACPI_ROOT_OBJECT , ecdt_ptr->id, + &boot_ec->handle);/* Don't trust ECDT, which comes from ASUSTek */ if (!EC_FLAGS_VALIDATE_ECDT ) goto install;@@ -1189,6 +1194,5 @@ static void __exit acpi_ec_exit(void) {acpi_bus_unregister_driver (&acpi_ec_driver); -return ; }#endif /* 0 */
Signature · 2 lines
I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center.
RE: [PATCH 5/5] ACPI/EC: Cleanup coding style.
Hi, Rafael
Quoted 85 lines
From: [email protected] [mailto:[email protected]] On Behalf Of Rafael J. Wysocki
Sent: Tuesday, October 21, 2014 9:29 PMQuoted 81 lines · Lv Zheng
This patch cleans up the following coding style issues that are detected by
scripts/checkpatch.pl:
ERROR: code indent should use tabs where possible
ERROR: "foo * bar" should be "foo *bar"
WARNING: Missing a blank line after declarations
WARNING:EXPORT_SYMBOL(foo); should immediately follow its function/variable
WARNING: void function return statements are not generally useful
WARNING: else is not generally useful after a break or return
WARNING: break is not useful after a goto or return
WARNING: braces {} are not necessary for single statement blocks
WARNING: line over 80 characters
WARNING: msleep < 20ms can sleep for up to 20ms; seeDocumentation/timers/timers-howto.txt
No functional changes.Signed-off-by Lv Zheng <[email protected]>--- drivers/acpi/ec.c | 58 ++++++++++++++++++++++++++++------------------------- 1 file changed, 31 insertions(+), 27 deletions(-)drivers/acpi/ec.c +8
-2@@ -128,12 +128,13 @@ static int EC_FLAGS_SKIP_DSDT_SCAN; /* Not all BIOS survive early DSDT scan */static int EC_FLAGS_CLEAR_ON_RESUME ;/* Needs acpi_ec_clear() on boot/resume */ /* -------------------------------------------------------------------------- -Transaction Management --------------------------------------------------------------------------- */ +* Transaction Management +* -------------------------------------------------------------------------- */ static inline u8 acpi_ec_read_status (struct acpi_ec *ec) {u8 x =inb (ec->command_addr); + pr_debug("EC_SC(R) = 0x%2.2x " "SCI_EVT=%d BURST=%d CMD=%d IBF=%d OBF=%d\n", x,@@ -148,6 +149,7 @@ static inline u8 acpi_ec_read_status(struct acpi_ec *ec)static inline u8 acpi_ec_read_data (struct acpi_ec *ec) {u8 x =inb (ec->data_addr); +pr_debug ("EC_DATA(R) = 0x%2.2x\n" , x);return x; }@@ -189,6 +191,7 @@ static int ec_transaction_completed(struct acpi_ec *ec) {unsigned long flags;int ret =0 ; +spin_lock_irqsave (&ec->lock, flags);if (ec->curr && (ec->curr->flags &ACPI_EC_COMMAND_COMPLETE )) ret =1 ;@@ -288,6 +291,7 @@ static int ec_poll(struct acpi_ec *ec) {unsigned long flags;int repeat =5 ;/* number of command restarts */ +while (repeat--) {unsigned long delay = jiffies +msecs_to_jiffies (ec_delay);@@ -320,6 +324,7 @@ static int acpi_ec_transaction_unlocked(struct acpi_ec *ec, {unsigned long tmp;int ret =0 ; +if (EC_FLAGS_MSI )udelay (ACPI_EC_MSI_UDELAY );/* start transaction */ @@ -347,6 +352,7 @@ static int acpi_ec_transaction(struct acpi_ec *ec, struct transaction *t) {int status;u32 glk; +if (!ec || (!t) || (t->wlen && !t->wdata) || (t->rlen && !t->rdata))return -EINVAL ;if (t->rdata)@@ -374,7 +380,7 @@ static int acpi_ec_transaction(struct acpi_ec *ec, struct transaction *t)/* check if we received SCI during transaction */
Quoted 8 lines
drivers/acpi/ec.c static int acpi_ec_transaction(struct acpi_ec *ec, struct transaction *t) · line 383
Quoted 4 lines
drivers/acpi/ec.c +1
-1ec_check_sci_sync (ec,acpi_ec_read_status (ec));if (test_bit (EC_FLAGS_GPE_STORM , &ec->flags)) { -msleep (1 ); +msleep (20 );This is a functional change, so I've dropped it from the patch (it didn't apply for me anyway). Please send it as a separate patch with an appropriate changelog if you want to make it.
OK. I'll split this as a separate patch.
This was just detected by scripts/checkpatch.pl as:
WARNING: msleep < 20ms can sleep for up to 20ms; see Documentation/timers/timers-howto.txt So I was thinking this could be merged into this checkpatch.pl cleanup commit.
Thanks and best regards
-Lv
Quoted 216 lines
Quoted 202 lines
drivers/acpi/ec.c +22
-23/* It is safe to enable the GPE outside of the transaction. */ acpi_enable_gpe (NULL , ec->gpe, TRUE); }else if (t->irq_count > ec_storm_threshold) {@@ -410,7 +416,7 @@ static int acpi_ec_burst_disable(struct acpi_ec *ec) acpi_ec_transaction(ec, &t) : 0; } -static int acpi_ec_read (struct acpi_ec *ec,u8 address,u8 * data) +static int acpi_ec_read (struct acpi_ec *ec,u8 address,u8 *data) {int result;u8 d;@@ -446,10 +452,9 @@ int ec_read(u8 addr, u8 *val)if (!err) { *val = temp_data;return 0 ; - }else -return err; + } +return err; } -EXPORT_SYMBOL (ec_read);int ec_write (u8 addr,u8 val)@@ -463,22 +468,21 @@ int ec_write(u8 addr, u8 val)return err; } -EXPORT_SYMBOL (ec_write);int ec_transaction (u8 command, -const u8 * wdata,unsigned wdata_len, -u8 * rdata,unsigned rdata_len) +const u8 *wdata,unsigned wdata_len, +u8 *rdata,unsigned rdata_len) {struct transaction t = {.command = command, .wdata = wdata, .rdata = rdata, .wlen = wdata_len, .rlen = rdata_len}; +if (!first_ec)return -ENODEV ;return acpi_ec_transaction (first_ec, &t); } -EXPORT_SYMBOL (ec_transaction);/* Get the handle to the EC device */ @@ -488,7 +492,6 @@ acpi_handle ec_get_handle(void)return NULL ;return first_ec->handle; } -EXPORT_SYMBOL (ec_get_handle);/* @@ -552,13 +555,14 @@ void acpi_ec_unblock_transactions_early(void)clear_bit (EC_FLAGS_BLOCKED , &first_ec->flags); } -static int acpi_ec_query_unlocked (struct acpi_ec *ec,u8 * data) +static int acpi_ec_query_unlocked (struct acpi_ec *ec,u8 *data) {int result;u8 d;struct transaction t = {.command =ACPI_EC_COMMAND_QUERY , .wdata =NULL , .rdata = &d, .wlen =0 , .rlen =1 }; +if (!ec || !data)return -EINVAL ;/* @@ -584,6 +588,7 @@ int acpi_ec_add_query_handler(struct acpi_ec *ec, u8 query_bit, {struct acpi_ec_query_handler *handler =kzalloc (sizeof (struct acpi_ec_query_handler ),GFP_KERNEL ); +if (!handler)return -ENOMEM ;@@ -596,12 +601,12 @@ int acpi_ec_add_query_handler(struct acpi_ec *ec, u8 query_bit,mutex_unlock (&ec->mutex);return 0 ; } -EXPORT_SYMBOL_GPL (acpi_ec_add_query_handler);void acpi_ec_remove_query_handler (struct acpi_ec *ec,u8 query_bit) {struct acpi_ec_query_handler *handler, *tmp; +mutex_lock (&ec->mutex);list_for_each_entry_safe (handler, tmp, &ec->list, node) {if (query_bit == handler->query_bit) {@@ -611,12 +616,12 @@ void acpi_ec_remove_query_handler(struct acpi_ec *ec, u8 query_bit) }mutex_unlock (&ec->mutex); } -EXPORT_SYMBOL_GPL (acpi_ec_remove_query_handler);static void acpi_ec_run (void *cxt) {struct acpi_ec_query_handler *handler = cxt; +if (!handler)return ;pr_debug ("##### Query(0x%02x) started #####\n" , handler->query_bit);@@ -660,6 +665,7 @@ static int acpi_ec_sync_query(struct acpi_ec *ec, u8 *data)static void acpi_ec_gpe_query (void *ec_cxt) {struct acpi_ec *ec = ec_cxt; +if (!ec)return ;mutex_lock (&ec->mutex);@@ -694,8 +700,8 @@ static u32 acpi_ec_gpe_handler(acpi_handle gpe_device, }/* -------------------------------------------------------------------------- -Address Space Management --------------------------------------------------------------------------- */ +* Address Space Management +* -------------------------------------------------------------------------- */ static acpi_status acpi_ec_space_handler(u32 function, acpi_physical_address address,@@ -726,27 +732,26 @@ acpi_ec_space_handler(u32 function, acpi_physical_address address,switch (result) {case -EINVAL :return AE_BAD_PARAMETER ; -break ;case -ENODEV :return AE_NOT_FOUND ; -break ;case -ETIME :return AE_TIME ; -break ;default :return AE_OK ; } }/* -------------------------------------------------------------------------- -Driver Interface --------------------------------------------------------------------------- */ +* Driver Interface +* -------------------------------------------------------------------------- */ +static acpi_status ec_parse_io_ports (struct acpi_resource *resource,void *context);static struct acpi_ec *make_acpi_ec (void ) {struct acpi_ec *ec =kzalloc (sizeof (struct acpi_ec ),GFP_KERNEL ); +if (!ec)return NULL ; ec->flags =1 <<EC_FLAGS_QUERY_PENDING ;@@ -769,9 +774,8 @@ acpi_ec_register_query_methods(acpi_handle handle, u32 level, status =acpi_get_name (handle,ACPI_SINGLE_NAME , &buffer); -if (ACPI_SUCCESS (status) &&sscanf (node_name,"_Q%x" , &value) ==1 ) { +if (ACPI_SUCCESS (status) &&sscanf (node_name,"_Q%x" , &value) ==1 )acpi_ec_add_query_handler (ec, value, handle,NULL ,NULL ); - }return AE_OK ; }@@ -780,7 +784,6 @@ ec_parse_device(acpi_handle handle, u32 Level, void *context, void **retval) {acpi_status status;unsigned long long tmp =0 ; -struct acpi_ec *ec = context;/* clear addr values, ec_parse_io_ports depend on it */ @@ -808,6 +811,7 @@ ec_parse_device(acpi_handle handle, u32 Level, void *context, void **retval)static int ec_install_handlers (struct acpi_ec *ec) {acpi_status status; +if (test_bit (EC_FLAGS_HANDLERS_INSTALLED , &ec->flags))return 0 ; status = acpi_install_gpe_handler(NULL, ec->gpe,@@ -1105,7 +1109,8 @@ int __init acpi_ec_ecdt_probe(void) boot_ec->data_addr = ecdt_ptr->data.address; boot_ec->gpe = ecdt_ptr->gpe; boot_ec->handle =ACPI_ROOT_OBJECT ; -acpi_get_handle (ACPI_ROOT_OBJECT , ecdt_ptr->id, &boot_ec->handle); +acpi_get_handle (ACPI_ROOT_OBJECT , ecdt_ptr->id, + &boot_ec->handle);/* Don't trust ECDT, which comes from ASUSTek */ if (!EC_FLAGS_VALIDATE_ECDT ) goto install;drivers/acpi/ec.c static void __exit acpi_ec_exit(void) · line 1198
Quoted 6 lines
drivers/acpi/ec.c +0
-1@@ -1189,6 +1194,5 @@ static void __exit acpi_ec_exit(void) {acpi_bus_unregister_driver (&acpi_ec_driver); -return ; }#endif /* 0 */ --
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 5/5] ACPI/EC: Cleanup coding style.
Hi, Rafael
Quoted 99 lines
From: [email protected] [mailto:[email protected]] On Behalf Of Zheng, Lv
Sent: Wednesday, October 22, 2014 1:06 PMHi, Rafael
Quoted 85 lines
From: [email protected] [mailto:[email protected]] On Behalf Of Rafael J. Wysocki
Sent: Tuesday, October 21, 2014 9:29 PMQuoted 81 lines · Lv Zheng
This patch cleans up the following coding style issues that are detected by
scripts/checkpatch.pl:
ERROR: code indent should use tabs where possible
ERROR: "foo * bar" should be "foo *bar"
WARNING: Missing a blank line after declarations
WARNING:EXPORT_SYMBOL(foo); should immediately follow its function/variable
WARNING: void function return statements are not generally useful
WARNING: else is not generally useful after a break or return
WARNING: break is not useful after a goto or return
WARNING: braces {} are not necessary for single statement blocks
WARNING: line over 80 characters
WARNING: msleep < 20ms can sleep for up to 20ms; seeDocumentation/timers/timers-howto.txt
No functional changes.Signed-off-by Lv Zheng <[email protected]>--- drivers/acpi/ec.c | 58 ++++++++++++++++++++++++++++------------------------- 1 file changed, 31 insertions(+), 27 deletions(-)drivers/acpi/ec.c +8
-2@@ -128,12 +128,13 @@ static int EC_FLAGS_SKIP_DSDT_SCAN; /* Not all BIOS survive early DSDT scan */static int EC_FLAGS_CLEAR_ON_RESUME ;/* Needs acpi_ec_clear() on boot/resume */ /* -------------------------------------------------------------------------- -Transaction Management --------------------------------------------------------------------------- */ +* Transaction Management +* -------------------------------------------------------------------------- */ static inline u8 acpi_ec_read_status (struct acpi_ec *ec) {u8 x =inb (ec->command_addr); + pr_debug("EC_SC(R) = 0x%2.2x " "SCI_EVT=%d BURST=%d CMD=%d IBF=%d OBF=%d\n", x,@@ -148,6 +149,7 @@ static inline u8 acpi_ec_read_status(struct acpi_ec *ec)static inline u8 acpi_ec_read_data (struct acpi_ec *ec) {u8 x =inb (ec->data_addr); +pr_debug ("EC_DATA(R) = 0x%2.2x\n" , x);return x; }@@ -189,6 +191,7 @@ static int ec_transaction_completed(struct acpi_ec *ec) {unsigned long flags;int ret =0 ; +spin_lock_irqsave (&ec->lock, flags);if (ec->curr && (ec->curr->flags &ACPI_EC_COMMAND_COMPLETE )) ret =1 ;@@ -288,6 +291,7 @@ static int ec_poll(struct acpi_ec *ec) {unsigned long flags;int repeat =5 ;/* number of command restarts */ +while (repeat--) {unsigned long delay = jiffies +msecs_to_jiffies (ec_delay);@@ -320,6 +324,7 @@ static int acpi_ec_transaction_unlocked(struct acpi_ec *ec, {unsigned long tmp;int ret =0 ; +if (EC_FLAGS_MSI )udelay (ACPI_EC_MSI_UDELAY );/* start transaction */ @@ -347,6 +352,7 @@ static int acpi_ec_transaction(struct acpi_ec *ec, struct transaction *t) {int status;u32 glk; +if (!ec || (!t) || (t->wlen && !t->wdata) || (t->rlen && !t->rdata))return -EINVAL ;if (t->rdata)@@ -374,7 +380,7 @@ static int acpi_ec_transaction(struct acpi_ec *ec, struct transaction *t)/* check if we received SCI during transaction */ Quoted 8 lines
drivers/acpi/ec.c static int acpi_ec_transaction(struct acpi_ec *ec, struct transaction *t) · line 383
Quoted 4 lines
drivers/acpi/ec.c +1
-1ec_check_sci_sync (ec,acpi_ec_read_status (ec));if (test_bit (EC_FLAGS_GPE_STORM , &ec->flags)) { -msleep (1 ); +msleep (20 );This is a functional change, so I've dropped it from the patch (it didn't apply for me anyway). Please send it as a separate patch with an appropriate changelog if you want to make it.
Quoted 1 line
OK. I'll split this as a separate patch.
I saw the rest of the diff blocks are merged.
So since no strict requirement on doing this besides of the checkpatch.pl warning. I'll stop doing this.
Thanks and best regards
-Lv
Quoted 228 lines
This was just detected by
scripts/checkpatch.plas:
WARNING: msleep < 20ms can sleep for up to 20ms; seeDocumentation/timers/timers-howto.txtSo I was thinking this could be merged into this checkpatch.pl cleanup commit.Thanks and best regards
-LvQuoted 203 lines
Quoted 203 lines
drivers/acpi/ec.c +22
-23/* It is safe to enable the GPE outside of the transaction. */ acpi_enable_gpe (NULL , ec->gpe, TRUE); }else if (t->irq_count > ec_storm_threshold) {@@ -410,7 +416,7 @@ static int acpi_ec_burst_disable(struct acpi_ec *ec) acpi_ec_transaction(ec, &t) : 0; } -static int acpi_ec_read (struct acpi_ec *ec,u8 address,u8 * data) +static int acpi_ec_read (struct acpi_ec *ec,u8 address,u8 *data) {int result;u8 d;@@ -446,10 +452,9 @@ int ec_read(u8 addr, u8 *val)if (!err) { *val = temp_data;return 0 ; - }else -return err; + } +return err; } -EXPORT_SYMBOL (ec_read);int ec_write (u8 addr,u8 val)@@ -463,22 +468,21 @@ int ec_write(u8 addr, u8 val)return err; } -EXPORT_SYMBOL (ec_write);int ec_transaction (u8 command, -const u8 * wdata,unsigned wdata_len, -u8 * rdata,unsigned rdata_len) +const u8 *wdata,unsigned wdata_len, +u8 *rdata,unsigned rdata_len) {struct transaction t = {.command = command, .wdata = wdata, .rdata = rdata, .wlen = wdata_len, .rlen = rdata_len}; +if (!first_ec)return -ENODEV ;return acpi_ec_transaction (first_ec, &t); } -EXPORT_SYMBOL (ec_transaction);/* Get the handle to the EC device */ @@ -488,7 +492,6 @@ acpi_handle ec_get_handle(void)return NULL ;return first_ec->handle; } -EXPORT_SYMBOL (ec_get_handle);/* @@ -552,13 +555,14 @@ void acpi_ec_unblock_transactions_early(void)clear_bit (EC_FLAGS_BLOCKED , &first_ec->flags); } -static int acpi_ec_query_unlocked (struct acpi_ec *ec,u8 * data) +static int acpi_ec_query_unlocked (struct acpi_ec *ec,u8 *data) {int result;u8 d;struct transaction t = {.command =ACPI_EC_COMMAND_QUERY , .wdata =NULL , .rdata = &d, .wlen =0 , .rlen =1 }; +if (!ec || !data)return -EINVAL ;/* @@ -584,6 +588,7 @@ int acpi_ec_add_query_handler(struct acpi_ec *ec, u8 query_bit, {struct acpi_ec_query_handler *handler =kzalloc (sizeof (struct acpi_ec_query_handler ),GFP_KERNEL ); +if (!handler)return -ENOMEM ;@@ -596,12 +601,12 @@ int acpi_ec_add_query_handler(struct acpi_ec *ec, u8 query_bit,mutex_unlock (&ec->mutex);return 0 ; } -EXPORT_SYMBOL_GPL (acpi_ec_add_query_handler);void acpi_ec_remove_query_handler (struct acpi_ec *ec,u8 query_bit) {struct acpi_ec_query_handler *handler, *tmp; +mutex_lock (&ec->mutex);list_for_each_entry_safe (handler, tmp, &ec->list, node) {if (query_bit == handler->query_bit) {@@ -611,12 +616,12 @@ void acpi_ec_remove_query_handler(struct acpi_ec *ec, u8 query_bit) }mutex_unlock (&ec->mutex); } -EXPORT_SYMBOL_GPL (acpi_ec_remove_query_handler);static void acpi_ec_run (void *cxt) {struct acpi_ec_query_handler *handler = cxt; +if (!handler)return ;pr_debug ("##### Query(0x%02x) started #####\n" , handler->query_bit);@@ -660,6 +665,7 @@ static int acpi_ec_sync_query(struct acpi_ec *ec, u8 *data)static void acpi_ec_gpe_query (void *ec_cxt) {struct acpi_ec *ec = ec_cxt; +if (!ec)return ;mutex_lock (&ec->mutex);@@ -694,8 +700,8 @@ static u32 acpi_ec_gpe_handler(acpi_handle gpe_device, }/* -------------------------------------------------------------------------- -Address Space Management --------------------------------------------------------------------------- */ +* Address Space Management +* -------------------------------------------------------------------------- */ static acpi_status acpi_ec_space_handler(u32 function, acpi_physical_address address,@@ -726,27 +732,26 @@ acpi_ec_space_handler(u32 function, acpi_physical_address address,switch (result) {case -EINVAL :return AE_BAD_PARAMETER ; -break ;case -ENODEV :return AE_NOT_FOUND ; -break ;case -ETIME :return AE_TIME ; -break ;default :return AE_OK ; } }/* -------------------------------------------------------------------------- -Driver Interface --------------------------------------------------------------------------- */ +* Driver Interface +* -------------------------------------------------------------------------- */ +static acpi_status ec_parse_io_ports (struct acpi_resource *resource,void *context);static struct acpi_ec *make_acpi_ec (void ) {struct acpi_ec *ec =kzalloc (sizeof (struct acpi_ec ),GFP_KERNEL ); +if (!ec)return NULL ; ec->flags =1 <<EC_FLAGS_QUERY_PENDING ;@@ -769,9 +774,8 @@ acpi_ec_register_query_methods(acpi_handle handle, u32 level, status =acpi_get_name (handle,ACPI_SINGLE_NAME , &buffer); -if (ACPI_SUCCESS (status) &&sscanf (node_name,"_Q%x" , &value) ==1 ) { +if (ACPI_SUCCESS (status) &&sscanf (node_name,"_Q%x" , &value) ==1 )acpi_ec_add_query_handler (ec, value, handle,NULL ,NULL ); - }return AE_OK ; }@@ -780,7 +784,6 @@ ec_parse_device(acpi_handle handle, u32 Level, void *context, void **retval) {acpi_status status;unsigned long long tmp =0 ; -struct acpi_ec *ec = context;/* clear addr values, ec_parse_io_ports depend on it */ @@ -808,6 +811,7 @@ ec_parse_device(acpi_handle handle, u32 Level, void *context, void **retval)static int ec_install_handlers (struct acpi_ec *ec) {acpi_status status; +if (test_bit (EC_FLAGS_HANDLERS_INSTALLED , &ec->flags))return 0 ; status = acpi_install_gpe_handler(NULL, ec->gpe,@@ -1105,7 +1109,8 @@ int __init acpi_ec_ecdt_probe(void) boot_ec->data_addr = ecdt_ptr->data.address; boot_ec->gpe = ecdt_ptr->gpe; boot_ec->handle =ACPI_ROOT_OBJECT ; -acpi_get_handle (ACPI_ROOT_OBJECT , ecdt_ptr->id, &boot_ec->handle); +acpi_get_handle (ACPI_ROOT_OBJECT , ecdt_ptr->id, + &boot_ec->handle);/* Don't trust ECDT, which comes from ASUSTek */ if (!EC_FLAGS_VALIDATE_ECDT ) goto install;@@ -1189,6 +1194,5 @@ static void __exit acpi_ec_exit(void) {Quoted 13 lines
drivers/acpi/ec.c static void __exit acpi_ec_exit(void) · line 1198
Quoted 5 lines
drivers/acpi/ec.c +0
-1acpi_bus_unregister_driver(&acpi_ec_driver); - return; }#endif /* 0 */ --
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.htmlN�����r��y���b�X��ǧv�^�){.n�+����{�i�b�{ay�ʇڙ�,j ��f���h���z��w���
���j:+v���w�j�m���� ����zZ+�����ݢj"��!�i
Re: [PATCH 5/5] ACPI/EC: Cleanup coding style.
Quoted 101 lines · Lv
Hi, Rafael
Quoted 99 lines
From: [email protected] [mailto:[email protected]] On Behalf Of Zheng, Lv
Sent: Wednesday, October 22, 2014 1:06 PMHi, Rafael
Quoted 93 lines
From: [email protected] [mailto:[email protected]] On Behalf Of Rafael J. Wysocki
Sent: Tuesday, October 21, 2014 9:29 PMQuoted 80 lines · Lv Zheng
This patch cleans up the following coding style issues that are detected by
scripts/checkpatch.pl:
ERROR: code indent should use tabs where possible
ERROR: "foo * bar" should be "foo *bar"
WARNING: Missing a blank line after declarations
WARNING:EXPORT_SYMBOL(foo); should immediately follow its function/variable
WARNING: void function return statements are not generally useful
WARNING: else is not generally useful after a break or return
WARNING: break is not useful after a goto or return
WARNING: braces {} are not necessary for single statement blocks
WARNING: line over 80 characters
WARNING: msleep < 20ms can sleep for up to 20ms; seeDocumentation/timers/timers-howto.txt
No functional changes.Signed-off-by Lv Zheng <[email protected]>--- drivers/acpi/ec.c | 58 ++++++++++++++++++++++++++++------------------------- 1 file changed, 31 insertions(+), 27 deletions(-)drivers/acpi/ec.c +8
-2@@ -128,12 +128,13 @@ static int EC_FLAGS_SKIP_DSDT_SCAN; /* Not all BIOS survive early DSDT scan */static int EC_FLAGS_CLEAR_ON_RESUME ;/* Needs acpi_ec_clear() on boot/resume */ /* -------------------------------------------------------------------------- -Transaction Management --------------------------------------------------------------------------- */ +* Transaction Management +* -------------------------------------------------------------------------- */ static inline u8 acpi_ec_read_status (struct acpi_ec *ec) {u8 x =inb (ec->command_addr); + pr_debug("EC_SC(R) = 0x%2.2x " "SCI_EVT=%d BURST=%d CMD=%d IBF=%d OBF=%d\n", x,@@ -148,6 +149,7 @@ static inline u8 acpi_ec_read_status(struct acpi_ec *ec)static inline u8 acpi_ec_read_data (struct acpi_ec *ec) {u8 x =inb (ec->data_addr); +pr_debug ("EC_DATA(R) = 0x%2.2x\n" , x);return x; }@@ -189,6 +191,7 @@ static int ec_transaction_completed(struct acpi_ec *ec) {unsigned long flags;int ret =0 ; +spin_lock_irqsave (&ec->lock, flags);if (ec->curr && (ec->curr->flags &ACPI_EC_COMMAND_COMPLETE )) ret =1 ;@@ -288,6 +291,7 @@ static int ec_poll(struct acpi_ec *ec) {unsigned long flags;int repeat =5 ;/* number of command restarts */ +while (repeat--) {unsigned long delay = jiffies +msecs_to_jiffies (ec_delay);@@ -320,6 +324,7 @@ static int acpi_ec_transaction_unlocked(struct acpi_ec *ec, {unsigned long tmp;int ret =0 ; +if (EC_FLAGS_MSI )udelay (ACPI_EC_MSI_UDELAY );/* start transaction */ @@ -347,6 +352,7 @@ static int acpi_ec_transaction(struct acpi_ec *ec, struct transaction *t) {int status;u32 glk; +if (!ec || (!t) || (t->wlen && !t->wdata) || (t->rlen && !t->rdata))return -EINVAL ;if (t->rdata)drivers/acpi/ec.c static int acpi_ec_transaction(struct acpi_ec *ec, struct transaction *t) · line 383
Quoted 5 lines
drivers/acpi/ec.c +1
-1@@ -374,7 +380,7 @@ static int acpi_ec_transaction(struct acpi_ec *ec, struct transaction *t)/* check if we received SCI during transaction */ ec_check_sci_sync (ec,acpi_ec_read_status (ec));if (test_bit (EC_FLAGS_GPE_STORM , &ec->flags)) { -msleep (1 ); +msleep (20 );This is a functional change, so I've dropped it from the patch (it didn't apply for me anyway). Please send it as a separate patch with an appropriate changelog if you want to make it.
Quoted 5 lines
Quoted 1 line
OK. I'll split this as a separate patch.
I saw the rest of the diff blocks are merged.
So since no strict requirement on doing this besides of the checkpatch.pl warning. I'll stop doing this.
OK, thanks.
Signature · 2 lines
I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center.
Rafael J. Wysocki
AppliedQuoted 15 lines · Lv Zheng
This patchset contains trivial cleanup code to make it easier to search in the EC debugging messages.
It also includes a patch to correct coding style issues that detected by recentscripts/checkpatch.pl.No functional changes. This patchset can only make remote debugging more efficient.
Lv Zheng (5):
ACPI/EC: Add CPU ID to debugging messages. ACPI/EC: Enhance the logs to apply toQR_ECtransactions. ACPI/EC: Add detailed command/query debugging information. ACPI/EC: Refine event/query debugging messages. ACPI/EC: Cleanup coding style.
Quoted 2 lines
drivers/acpi/ec.c | 109 ++++++++++++++++++++++++++++++++++------------------- 1 file changed, 70 insertions(+), 39 deletions(-)
Applied, except for one thing (please see the comment on patch [5/5]).
Signature · 2 lines
I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center.