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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAGdLNWGJger9MRmd0NQaooiuMPjGJA1rG+xGSJWtF84UEU+CFQ@mail.gmail.com>
Date:	Tue, 28 Apr 2015 09:08:32 -0600
From:	Azael Avalos <coproscefalo@...il.com>
To:	Bjørn Mork <bjorn@...k.no>
Cc:	Darren Hart <dvhart@...radead.org>,
	"platform-driver-x86@...r.kernel.org" 
	<platform-driver-x86@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 5/6] toshiba_bluetooth: Change BT status message to debug

Hi there,

2015-04-28 1:36 GMT-06:00 Bjørn Mork <bjorn@...k.no>:
> Azael Avalos <coproscefalo@...il.com> 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@...il.com>
>> ---
>>  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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/