Re: [PATCH 5/6] toshiba_bluetooth: Change BT status message to debug

[Azael Avalos]

Hi there,

2015-04-28 1:36 GMT-06:00 BjÃrn Mork <bjorn@xxxxxxx>:
> Azael Avalos <coproscefalo@xxxxxxxxx> writes:
>
>> The function toshiba_bluetooth_status s currently printing the status
>> of the device whenever it is queried, but since the introduction of
>> the rfkill poll code, this value will get printed everytime the poll
>> occurs.
>>
>> This patch changes the level of the printed message from info to
>> debug, and also adds a few more debug messages printing the
>> killswitch, plug and power status of the device as well.
>>
>> Signed-off-by: Azael Avalos <coproscefalo@xxxxxxxxx>
>> ---
>>  drivers/platform/x86/toshiba_bluetooth.c | 6 +++++-
>>  1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/platform/x86/toshiba_bluetooth.c b/drivers/platform/x86/toshiba_bluetooth.c
>> index 9867ccd..93b9688 100644
>> --- a/drivers/platform/x86/toshiba_bluetooth.c
>> +++ b/drivers/platform/x86/toshiba_bluetooth.c
>> @@ -99,7 +99,7 @@ static int toshiba_bluetooth_status(acpi_handle handle)
>>               return -ENXIO;
>>       }
>>
>> -     pr_info("Bluetooth status %llu\n", status);
>> +     pr_debug("Bluetooth status %llu\n", status);
>>
>>       return status;
>>  }
>> @@ -157,6 +157,10 @@ static int toshiba_bluetooth_sync_status(struct toshiba_bluetooth_dev *bt_dev)
>>       bt_dev->plugged = (status & BT_PLUGGED_MASK) ? true : false;
>>       bt_dev->powered = (status & BT_POWER_MASK) ? true : false;
>>
>> +     pr_debug("killswitch %d\n", bt_dev->killswitch);
>> +     pr_debug("plugged %d\n", bt_dev->plugged);
>> +     pr_debug("powered %d\n", bt_dev->powered);
>
>
> Those are terribly generic messages.  I don't think I would have guessed
> which device was trying to tell me "powered 1" if I found it in the
> logs...

Well, I was under the impression that what really gets printed is:
toshiba_bluetooth: killswitch 1

and then a siple "dmesg | grep toshiba_bluetooth" would suffice,
but yeah, they are quite non obvious.

>
> How about using e.g dev_dbg() to get a bit more context here?
>
> You might also want to put all three into a single call, so that they
> make a single dynamic debug entry when dynamic debugging is enabled.
>

Alright, will do, I'll just wait on Darren's (or someone else) comments and
send a v2.

> And looking at toshiba_bluetooth_status() I see that all callers have a
> device. How about propagating the device to be able to use the dev_*
> printk's there as well?  Let the device identify itself instead of
> having to guess.
>
>
> BjÃrn


Cheers
Azael



-- 
-- El mundo apesta y vosotros apestais tambien --
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/