Re: [PATCH] Bluetooth: hci_h5: implement CRC data integrity

[Luiz Augusto von Dentz]

Hi Javier,

On Sun, Aug 31, 2025 at 10:20 PM Javier Nieto <jgnieto@xxxxxxxxxxxxxxx> wrote:
>
> Dear Paul,
>
> Thanks for the review!
>
> On Wed, Aug 27, 2025 at 12:56:50PM +0200, Paul Menzel wrote:
>
> > Any btmon trace?
>
> The presence of CRC is limited to the H5 layer, so it is not visible on
> btmon. However, I did advertise and connect to a few devices while
> running btmon and everything worked and looked as normal. I also ensured
> that CRC was being used by adding temporary debugging prints.
>
> > I´d add the above to the proper commit message.
>
> Should I resubmit the patch as v2?
>
> > >   static u8 h5_cfg_field(struct h5 *h5)
> > >   {
> > > -   /* Sliding window size (first 3 bits) */
> > > -   return h5->tx_win & 0x07;
> > > +   /* Sliding window size (first 3 bits) and CRC request (fifth bit). */
> > > +   return (h5->tx_win & 0x07) | 0x10;
> >
> > Could a macro be defined for the CRC request bit?
>
> I thought about this, but decided against it since 0x10 is only used
> here and in one other place. Also, the existing code does not define a
> macro for the window size bits 0x07. I am not opposed to adding it if
> someone feels strongly about it though.
>
> > The diff looks good. Feel free to carry:
> >
> > Reviewed-by: Paul Menzel <pmenzel@xxxxxxxxxxxxx>
> >
> >
> > Kind regards,
> >
> > Paul
>
> I see that my patch fails a few test cases because it fails to link
> crc-ccitt. Do you know whether this is a problem with my patch or the
> test environment and where the code for the tests is found?

Yeah, this seems really a problem you are introducing:

ld: vmlinux.o: in function `h5_rx_crc':
hci_h5.c:(.text+0xd938d9): undefined reference to `crc_ccitt'
ld: vmlinux.o: in function `h5_prepare_pkt':
hci_h5.c:(.text+0xd94798): undefined reference to `crc_ccitt'
ld: hci_h5.c:(.text+0xd947a6): undefined reference to `crc_ccitt'

I guess we are missing something like select CRC_CCITT on Kconfig.

> Thanks again for your feedback.
>
> Javier



-- 
Luiz Augusto von Dentz