Hi Luiz, thanks for taking the time to review my patch.
On Tue, Apr 02, 2024 at 12:06:30PM -0400, Luiz Augusto von Dentz wrote:
> Hi Uri,
>
> On Tue, Apr 2, 2024 at 11:56 AM <me@xxxxxxxxxxxxx> wrote:
> >
> > From: Uri Arev <me@xxxxxxxxxxxxx>
> >
> > This fixes the following CHECKs, WARNINGs, and ERRORs reported in
> > hci_intel.c
> >
> > Reported by checkpatch.pl:
> > -----------
> > hci_intel.c
> > -----------
> > WARNING: Prefer using '"%s...", __func__' to using 'intel_setup', this function's name, in a string
> > + bt_dev_dbg(hdev, "start intel_setup");
> >
> > ERROR: code indent should use tabs where possible
> > + /* Check for supported iBT hardware variants of this firmware$
> >
> > ERROR: code indent should use tabs where possible
> > + * loading method.$
> >
> > ERROR: code indent should use tabs where possible
> > + *$
> >
> > ERROR: code indent should use tabs where possible
> > + * This check has been put in place to ensure correct forward$
> >
> > ERROR: code indent should use tabs where possible
> > + * compatibility options when newer hardware variants come along$
> >
> > ERROR: code indent should use tabs where possible
> > + */$
> >
> > CHECK: No space is necessary after a cast
> > + duration = (unsigned long long) ktime_to_ns(delta) >> 10;
> >
> > CHECK: No space is necessary after a cast
> > + duration = (unsigned long long) ktime_to_ns(delta) >> 10;
> >
> > WARNING: Missing a blank line after declarations
> > + int err = PTR_ERR(intel->rx_skb);
> > + bt_dev_err(hu->hdev, "Frame reassembly failed (%d)", err);
> >
> > Signed-off-by: Uri Arev <me@xxxxxxxxxxxxx>
> > ---
> > drivers/bluetooth/hci_intel.c | 19 ++++++++++---------
> > 1 file changed, 10 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/bluetooth/hci_intel.c b/drivers/bluetooth/hci_intel.c
> > index 78afb9a348e7..44731de1a2d6 100644
> > --- a/drivers/bluetooth/hci_intel.c
> > +++ b/drivers/bluetooth/hci_intel.c
> > @@ -537,7 +537,7 @@ static int intel_setup(struct hci_uart *hu)
> > int speed_change = 0;
> > int err;
> >
> > - bt_dev_dbg(hdev, "start intel_setup");
> > + bt_dev_dbg(hdev, "start %s", __func__);
>
> We probably don't need __func__ either since this is normally calls
> pr_debug which already has support for printing function names if the
> user wants to, so I just leave as bt_dev_dbg(hdev, "");
>
Thanks for the comment, will revise. I still don't know most of the
debugging functions in the kernel, as I am a relatively new hacker.
> >
> > hu->hdev->set_diag = btintel_set_diag;
> > hu->hdev->set_bdaddr = btintel_set_bdaddr;
> > @@ -591,12 +591,12 @@ static int intel_setup(struct hci_uart *hu)
> > return -EINVAL;
> > }
> >
> > - /* Check for supported iBT hardware variants of this firmware
> > - * loading method.
> > - *
> > - * This check has been put in place to ensure correct forward
> > - * compatibility options when newer hardware variants come along.
> > - */
> > + /* Check for supported iBT hardware variants of this firmware
> > + * loading method.
> > + *
> > + * This check has been put in place to ensure correct forward
> > + * compatibility options when newer hardware variants come along.
> > + */
> > switch (ver.hw_variant) {
> > case 0x0b: /* LnP */
> > case 0x0c: /* WsP */
> > @@ -777,7 +777,7 @@ static int intel_setup(struct hci_uart *hu)
> >
> > rettime = ktime_get();
> > delta = ktime_sub(rettime, calltime);
> > - duration = (unsigned long long) ktime_to_ns(delta) >> 10;
> > + duration = (unsigned long long)ktime_to_ns(delta) >> 10;
> >
> > bt_dev_info(hdev, "Firmware loaded in %llu usecs", duration);
> >
> > @@ -822,7 +822,7 @@ static int intel_setup(struct hci_uart *hu)
> >
> > rettime = ktime_get();
> > delta = ktime_sub(rettime, calltime);
> > - duration = (unsigned long long) ktime_to_ns(delta) >> 10;
> > + duration = (unsigned long long)ktime_to_ns(delta) >> 10;
> >
> > bt_dev_info(hdev, "Device booted in %llu usecs", duration);
> >
> > @@ -977,6 +977,7 @@ static int intel_recv(struct hci_uart *hu, const void *data, int count)
> > ARRAY_SIZE(intel_recv_pkts));
> > if (IS_ERR(intel->rx_skb)) {
> > int err = PTR_ERR(intel->rx_skb);
> > +
> > bt_dev_err(hu->hdev, "Frame reassembly failed (%d)", err);
> > intel->rx_skb = NULL;
> > return err;
> > --
> > 2.44.0
> >
Uri Arev
