IIO: ADC: add stm32 DFSDM support for PDM microphone
Hello Arnaud Pouliquen,
The patch eca949800d2d: "IIO: ADC: add stm32 DFSDM support for PDM microphone" from Jan 10, 2018, leads to the following static checker warning:
drivers/iio/adc/stm32-dfsdm-adc.c:555 stm32_dfsdm_audio_dma_buffer_done() warn: potential shift truncation. '0xffffff00 (0,256-4294967040) << 8'
drivers/iio/adc/stm32-dfsdm-adc.c
551 while (available >= indio_dev->scan_bytes) {
552 u32 *buffer = (u32 *)&adc->rx_buf[adc->bufi];
553
554 /* Mask 8 LSB that contains the channel ID */
555 *buffer = (*buffer & 0xFFFFFF00) << 8;
Is this shift, correct? The comment just mentions that we're masking out the low bits.
556 available -= indio_dev->scan_bytes;
557 adc->bufi += indio_dev->scan_bytes;
558 if (adc->bufi >= adc->buf_sz) { 559 if (adc->cb)
560 adc->cb(&adc->rx_buf[old_pos],
561 adc->buf_sz - old_pos, adc->cb_priv);
562 adc->bufi = 0;
563 old_pos = 0;
564 }
565 }
566 if (adc->cb)
567 adc->cb(&adc->rx_buf[old_pos], adc->bufi - old_pos,
568 adc->cb_priv);
569 }
regards,
dan carpenter
Hello Dan
Quoted 10 lines · Dan Carpenter
Hello Arnaud Pouliquen,
The patch
eca949800d2d: "IIO: ADC: add stm32 DFSDM support for PDM microphone" from Jan 10, 2018, leads to the following static checker warning:drivers/iio/adc/stm32-dfsdm-adc.c:555 stm32_dfsdm_audio_dma_buffer_done() warn: potential shift truncation. '0xffffff00 (0,256-4294967040) << 8'
drivers/iio/adc/stm32-dfsdm-adc.c
Quoted 8 lines
551 while (available >= indio_dev->scan_bytes) { 552 u32 *buffer = (u32 *)&adc->rx_buf[adc->bufi]; 553 554 /* Mask 8 LSB that contains the channel ID */ 555 *buffer = (*buffer & 0xFFFFFF00) << 8;Is this shift, correct? The comment just mentions that we're masking out the low bits.
Yes mask and shift are not correlated.
The Mask suppresses the 8 LSB that contain ID of the channel scanned The shift is used for the 24 to 32 bits sample conversion.
Regards,
Arnaud
Quoted 19 lines
556 available -= indio_dev->scan_bytes; 557 adc->bufi += indio_dev->scan_bytes; 558 if (adc->bufi >= adc->buf_sz) { 559 if (adc->cb) 560 adc->cb(&adc->rx_buf[old_pos], 561 adc->buf_sz - old_pos, adc->cb_priv); 562 adc->bufi = 0; 563 old_pos = 0; 564 } 565 } 566 if (adc->cb) 567 adc->cb(&adc->rx_buf[old_pos], adc->bufi - old_pos, 568 adc->cb_priv); 569 }regards,
dan carpenter
Quoted 19 lines · Arnaud Pouliquen
Hello Dan
Quoted 16 lines · Dan Carpenter
Hello Arnaud Pouliquen,
The patch
eca949800d2d: "IIO: ADC: add stm32 DFSDM support for PDM microphone" from Jan 10, 2018, leads to the following static checker warning:drivers/iio/adc/stm32-dfsdm-adc.c:555 stm32_dfsdm_audio_dma_buffer_done() warn: potential shift truncation. '0xffffff00 (0,256-4294967040) << 8'
drivers/iio/adc/stm32-dfsdm-adc.c551 while (available >= indio_dev->scan_bytes) { 552 u32 *buffer = (u32 *)&adc->rx_buf[adc->bufi]; 553 554 /* Mask 8 LSB that contains the channel ID */ 555 *buffer = (*buffer & 0xFFFFFF00) << 8;
Quoted 6 lines
Quoted 2 lines
Is this shift, correct? The comment just mentions that we're masking out the low bits.
Yes mask and shift are not correlated.
The Mask suppresses the 8 LSB that contain ID of the channel scanned The shift is used for the 24 to 32 bits sample conversion.
If you wanted to silence the static checker warning you could change the mask to:
*buffer = (*buffer & 0x00FFFF00) << 8;
regards,
dan carpenter
Quoted 27 lines · Dan Carpenter
Quoted 19 lines · Arnaud Pouliquen
Hello Dan
Quoted 16 lines · Dan Carpenter
Hello Arnaud Pouliquen,
The patch
eca949800d2d: "IIO: ADC: add stm32 DFSDM support for PDM microphone" from Jan 10, 2018, leads to the following static checker warning:drivers/iio/adc/stm32-dfsdm-adc.c:555 stm32_dfsdm_audio_dma_buffer_done() warn: potential shift truncation. '0xffffff00 (0,256-4294967040) << 8'
drivers/iio/adc/stm32-dfsdm-adc.c551 while (available >= indio_dev->scan_bytes) { 552 u32 *buffer = (u32 *)&adc->rx_buf[adc->bufi]; 553 554 /* Mask 8 LSB that contains the channel ID */ 555 *buffer = (*buffer & 0xFFFFFF00) << 8;Quoted 6 lines
Quoted 2 lines
Is this shift, correct? The comment just mentions that we're masking out the low bits.
Yes mask and shift are not correlated.
The Mask suppresses the 8 LSB that contain ID of the channel scanned The shift is used for the 24 to 32 bits sample conversion.
Quoted 4 lines
If you wanted to silence the static checker warning you could change the mask to:
*buffer = (*buffer & 0x00FFFF00) << 8;
Thanks for the proposal, fill free to send the patch, otherwise i will add it in next patch series.
For my personal information which static checker warns on this line? Regards
Arnaud
Quoted 4 lines
regards,
dan carpenter
Quoted 30 lines · Arnaud Pouliquen
Quoted 27 lines · Dan Carpenter
Quoted 25 lines · Arnaud Pouliquen
Hello Dan
Quoted 10 lines · Dan Carpenter
Hello Arnaud Pouliquen,
The patch
eca949800d2d: "IIO: ADC: add stm32 DFSDM support for PDM microphone" from Jan 10, 2018, leads to the following static checker warning:drivers/iio/adc/stm32-dfsdm-adc.c:555 stm32_dfsdm_audio_dma_buffer_done() warn: potential shift truncation. '0xffffff00 (0,256-4294967040) << 8'
drivers/iio/adc/stm32-dfsdm-adc.cQuoted 8 lines
551 while (available >= indio_dev->scan_bytes) { 552 u32 *buffer = (u32 *)&adc->rx_buf[adc->bufi]; 553 554 /* Mask 8 LSB that contains the channel ID */ 555 *buffer = (*buffer & 0xFFFFFF00) << 8;Is this shift, correct? The comment just mentions that we're masking out the low bits.
Yes mask and shift are not correlated.
The Mask suppresses the 8 LSB that contain ID of the channel scanned The shift is used for the 24 to 32 bits sample conversion.
Quoted 8 lines
Quoted 4 lines
If you wanted to silence the static checker warning you could change the mask to:
*buffer = (*buffer & 0x00FFFF00) << 8;Thanks for the proposal, fill free to send the patch, otherwise i will add it in next patch series.
For my personal information which static checker warns on this line?
It's some Smatch stuff that has too many false positives so I review it but haven't published it.
regards,
dan carpenter