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

[Javier Nieto <jgnieto@cs.stanford.edu>]

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@molgen.mpg.de>
> 
> 
> 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?

Thanks again for your feedback.

Javier