linux-kernel - Re: [PATCH 10/12] bluetooth: hci

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: <a2702615-2946-4ca5-a33b-15da7026b58b@bootlin.com>
Date: Wed, 12 Feb 2025 18:01:10 +0100
From: Alexis Lothoré <alexis.lothore@...tlin.com>
To: Luiz Augusto von Dentz <luiz.dentz@...il.com>
Cc: Marcel Holtmann <marcel@...tmann.org>, Rob Herring <robh@...nel.org>,
 Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
 <conor+dt@...nel.org>, Ajay Singh <ajay.kathat@...rochip.com>,
 Claudiu Beznea <claudiu.beznea@...on.dev>, Kalle Valo <kvalo@...nel.org>,
 "David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
 Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
 Simon Horman <horms@...nel.org>, Nicolas Ferre
 <nicolas.ferre@...rochip.com>,
 Alexandre Belloni <alexandre.belloni@...tlin.com>,
 Marek Vasut <marex@...x.de>, Thomas Petazzoni
 <thomas.petazzoni@...tlin.com>, linux-bluetooth@...r.kernel.org,
 devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-wireless@...r.kernel.org, netdev@...r.kernel.org,
 linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH 10/12] bluetooth: hci_wilc: add wilc hci driver

Hi Luiz,
thanks for the prompt review !

On 2/12/25 17:14, Luiz Augusto von Dentz wrote:

[...]

>> +static int wilc_open(struct hci_uart *hu)
>> +{
>> +       struct wilc_data *wdata;
>> +
>> +       BT_DBG("hci_wilc: open");
> 
> Afaik you don't need to include the function name with the likes of
> pr_debug/BT_DBG, that said you should really be using bt_dev_dbg if
> you have hu->hdev set at this point, and this is valid for all other
> places where BT_DBG could be replaced with bt_dev_dbg.

I observe that BT_DBG does not bring any kind of prefix to the emitted log. But
indeed, bt_dev_dbg looks definitely better for my purpose, I'll update all those
logs with it.

>> +       wdata = kzalloc(sizeof(*wdata), GFP_KERNEL);
>> +       if (!wdata)
>> +               return -ENOMEM;
> 
> Add an empty after something like an if statement to make it clearer
> that it is not under the same scope.

True, that will be fixed.

[...]

> 
> Once you address these comments please fill free to add:
> 
> Reviewed-by: Luiz Augusto von Dentz <luiz.von.dentz@...el.com>

Thanks. I'll delay v2 for a few days to let other people review the series.

Alexis

-- 
Alexis Lothoré, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com