Date:   Fri, 7 Jan 2022 23:50:40 -0800
From:   Luiz Augusto von Dentz <luiz.dentz@...il.com>
To:     Jakub Kicinski <kuba@...nel.org>, "An, Tedd" <tedd.an@...el.com>
Cc:     David Miller <davem@...emloft.net>,
        "linux-bluetooth@...r.kernel.org" <linux-bluetooth@...r.kernel.org>,
        "open list:NETWORKING [GENERAL]" <netdev@...r.kernel.org>
Subject: Re: pull request: bluetooth 2022-01-07

Hi Jakub,

On Fri, Jan 7, 2022 at 6:27 PM Jakub Kicinski <kuba@...nel.org> wrote:
>
> On Fri,  7 Jan 2022 13:09:42 -0800 Luiz Augusto von Dentz wrote:
> > The following changes since commit 710ad98c363a66a0cd8526465426c5c5f8377ee0:
> >
> >   veth: Do not record rx queue hint in veth_xmit (2022-01-06 13:49:54 +0000)
> >
> > are available in the Git repository at:
> >
> >   git://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git tags/for-net-next-2022-01-07
> >
> > for you to fetch changes up to b9f9dbad0bd1c302d357fdd327c398f51f5fc2b1:
> >
> >   Bluetooth: hci_sock: fix endian bug in hci_sock_setsockopt() (2022-01-07 08:41:38 +0100)
> >
> > ----------------------------------------------------------------
> > bluetooth-next pull request for net-next:
> >
> >  - Add support for Foxconn QCA 0xe0d0
> >  - Fix HCI init sequence on MacBook Air 8,1 and 8,2
> >  - Fix Intel firmware loading on legacy ROM devices
>
> A few warnings here that may be worth addressing - in particular this
> one makes me feel that kbuild bot hasn't looked at the patches:
>
> net/bluetooth/hci_sync.c:5143:5: warning: no previous prototype for ‘hci_le_ext_create_conn_sync’ [-Wmissing-prototypes]
>  5143 | int hci_le_ext_create_conn_sync(struct hci_dev *hdev, struct hci_conn *conn,
>       |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~

Interesting that it doesn't show up when I compile it, perhaps we need
to turn on some warnings?

> Also this Fixes tag could be mended:
>
> Commit: 6845667146a2 ("Bluetooth: hci_qca: Fix NULL vs IS_ERR_OR_NULL check in qca_serdev_probe")
>         Fixes tag: Fixes: 77131dfe ("Bluetooth: hci_qca: Replace devm_gpiod_get() with devm_gpiod_get_optional()")
>         Has these problem(s):
>                 - SHA1 should be at least 12 digits long
>                   Can be fixed by setting core.abbrev to 12 (or more) or (for git v2.11
>                   or later) just making sure it is not set (or set to "auto").

Right, I will check with Marcel why we didn't end up fixing up in place.

>
> Would you be able to fix the new warnings and resend the PR or are you
> confident that there isn't much serious breakage here and follow ups
> will be enough?

I think we might want to do the fixup but the one lacking a prototype
I'm afraid was caused by the previous PR, anyway I will try to send a
fix for that over the weekend.

> FWIW to see the new warnings check out net-next, do a allmodconfig build
> with W=1 C=1, pull in your code, reset back to net-next (this will
> "touch" all the files that need rebuilding), do a single threaded build
> and save (2>file) the warnings, pull in your code, do another build
> (2>file2), diff the warnings from the build of just net-next and after
> pull.

Hmm, we might as well do that in our CI then, but isn't that gonna
cause all sorts of warnings in different subsystem/drivers to appear?
I get that the diff should come clean if we do this 2 stage builds
like you suggested but I'm not sure that is the best approach for CI,
what do you think @An, Tedd? I'd guess we could keep our minimal
config to keep building times in check but add a 2 stage build per
patch so we can detect if they produce new warnings.

-- 
Luiz Augusto von Dentz

Powered by blists - more mailing lists