All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] staging/comedi: fix sparse warning: shift too big
@ 2014-05-02 14:57 Brilliantov Kirill Vladimirovich
  2014-05-02 16:49 ` Ian Abbott
  2014-05-03 23:58 ` Greg Kroah-Hartman
  0 siblings, 2 replies; 6+ messages in thread
From: Brilliantov Kirill Vladimirovich @ 2014-05-02 14:57 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Ian Abbott, H Hartley Sweeten, linux-kernel,
	Brilliantov Kirill Vladimirovich

Signed-off-by: Brilliantov Kirill Vladimirovich <brilliantov@inbox.ru>
---
 drivers/staging/comedi/drivers/ni_stc.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/comedi/drivers/ni_stc.h b/drivers/staging/comedi/drivers/ni_stc.h
index f0630b78..197b568 100644
--- a/drivers/staging/comedi/drivers/ni_stc.h
+++ b/drivers/staging/comedi/drivers/ni_stc.h
@@ -717,7 +717,7 @@ enum AI_AO_Select_Bits {
 static inline unsigned ni_stc_dma_channel_select_bitfield(unsigned channel)
 {
 	if (channel < 4)
-		return 1 << channel;
+		return 1 << (channel & 0x3);
 	if (channel == 4)
 		return 0x3;
 	if (channel == 5)
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] staging/comedi: fix sparse warning: shift too big
  2014-05-02 14:57 [PATCH] staging/comedi: fix sparse warning: shift too big Brilliantov Kirill Vladimirovich
@ 2014-05-02 16:49 ` Ian Abbott
  2014-05-02 18:28   ` Hartley Sweeten
  2014-05-03 23:58 ` Greg Kroah-Hartman
  1 sibling, 1 reply; 6+ messages in thread
From: Ian Abbott @ 2014-05-02 16:49 UTC (permalink / raw)
  To: Brilliantov Kirill Vladimirovich, Greg Kroah-Hartman
  Cc: H Hartley Sweeten, linux-kernel

On 2014-05-02 15:57, Brilliantov Kirill Vladimirovich wrote:
> Signed-off-by: Brilliantov Kirill Vladimirovich <brilliantov@inbox.ru>
> ---
>   drivers/staging/comedi/drivers/ni_stc.h | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/comedi/drivers/ni_stc.h b/drivers/staging/comedi/drivers/ni_stc.h
> index f0630b78..197b568 100644
> --- a/drivers/staging/comedi/drivers/ni_stc.h
> +++ b/drivers/staging/comedi/drivers/ni_stc.h
> @@ -717,7 +717,7 @@ enum AI_AO_Select_Bits {
>   static inline unsigned ni_stc_dma_channel_select_bitfield(unsigned channel)
>   {
>   	if (channel < 4)
> -		return 1 << channel;
> +		return 1 << (channel & 0x3);
>   	if (channel == 4)
>   		return 0x3;
>   	if (channel == 5)
>

The warning seems a bit spurious!

-- 
-=( Ian Abbott @ MEV Ltd.    E-mail: <abbotti@mev.co.uk>        )=-
-=( Tel: +44 (0)161 477 1898   FAX: +44 (0)161 718 3587         )=-

^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [PATCH] staging/comedi: fix sparse warning: shift too big
  2014-05-02 16:49 ` Ian Abbott
@ 2014-05-02 18:28   ` Hartley Sweeten
  2014-05-02 20:24     ` Ian Abbott
  0 siblings, 1 reply; 6+ messages in thread
From: Hartley Sweeten @ 2014-05-02 18:28 UTC (permalink / raw)
  To: Ian Abbott, Brilliantov Kirill Vladimirovich, Greg Kroah-Hartman
  Cc: linux-kernel

On Friday, May 02, 2014 9:50 AM, Ian Abbott wrote:
> On 2014-05-02 15:57, Brilliantov Kirill Vladimirovich wrote:
>> Signed-off-by: Brilliantov Kirill Vladimirovich <brilliantov@inbox.ru>
>> ---
>>   drivers/staging/comedi/drivers/ni_stc.h | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/staging/comedi/drivers/ni_stc.h b/drivers/staging/comedi/drivers/ni_stc.h
>> index f0630b78..197b568 100644
>> --- a/drivers/staging/comedi/drivers/ni_stc.h
>> +++ b/drivers/staging/comedi/drivers/ni_stc.h
>> @@ -717,7 +717,7 @@ enum AI_AO_Select_Bits {
>>   static inline unsigned ni_stc_dma_channel_select_bitfield(unsigned channel)
>>   {
>>   	if (channel < 4)
>> -		return 1 << channel;
>> +		return 1 << (channel & 0x3);
>>   	if (channel == 4)
>>   		return 0x3;
>>   	if (channel == 5)
>>
>
> The warning seems a bit spurious!

The whole mite.c driver and it's users are all a bit of a mess.

I did find the information about the "AI AO Select Register" in the National
Instruments PCI E Series manual:

-----

The AI AO Select Register contains 8 bits that control the logical DMA selection for the
analog input and analog output resources. The contents of this register are cleared upon power
up and after a reset condition.
	Address:	base address + 0x09
	Type:		write-only
	Word size:	8-bit

	Bit	Name		Description
	7-4	Reserved	Reserved-Always write 0 to these bits (for PCI-6032E
				and PCI-6033E only).
	7-4	Output<D..A>	Analog Output Logical Channel D through A-These four
				bits select the logical channels of the MITE to be used by
				the analog output. You can only set one of these bits at a
				time (except for the PCI-6032E and PCI-6033E).
	3-0	Input<D..A>	Analog Input Logical Channel D through A-These four
				bits select the logical channels to be used by the analog
				input. You can only set one of these bits at a time.

-----

Based on this the ni_stc_dma_channel_select_bitfield() function appears to be
incorrect. The "channel"s should only be in the range 0 to 3 and result in a returned
bitfield of:

	channel	bitfield	Description
	0		0x01	Analog Input or Output Channel A
	1		0x02 	Analog Input or Output Channel B
	2		0x04 	Analog Input or Output Channel C
	3		0x08 	Analog Input or Output Channel D

Just my two cents...

Hartley


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] staging/comedi: fix sparse warning: shift too big
  2014-05-02 18:28   ` Hartley Sweeten
@ 2014-05-02 20:24     ` Ian Abbott
  0 siblings, 0 replies; 6+ messages in thread
From: Ian Abbott @ 2014-05-02 20:24 UTC (permalink / raw)
  To: Hartley Sweeten, Brilliantov Kirill Vladimirovich, Greg Kroah-Hartman
  Cc: linux-kernel

On 02/05/14 19:28, Hartley Sweeten wrote:
> On Friday, May 02, 2014 9:50 AM, Ian Abbott wrote:
>> On 2014-05-02 15:57, Brilliantov Kirill Vladimirovich wrote:
>>> Signed-off-by: Brilliantov Kirill Vladimirovich <brilliantov@inbox.ru>
>>> ---
>>>    drivers/staging/comedi/drivers/ni_stc.h | 2 +-
>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/staging/comedi/drivers/ni_stc.h b/drivers/staging/comedi/drivers/ni_stc.h
>>> index f0630b78..197b568 100644
>>> --- a/drivers/staging/comedi/drivers/ni_stc.h
>>> +++ b/drivers/staging/comedi/drivers/ni_stc.h
>>> @@ -717,7 +717,7 @@ enum AI_AO_Select_Bits {
>>>    static inline unsigned ni_stc_dma_channel_select_bitfield(unsigned channel)
>>>    {
>>>    	if (channel < 4)
>>> -		return 1 << channel;
>>> +		return 1 << (channel & 0x3);
>>>    	if (channel == 4)
>>>    		return 0x3;
>>>    	if (channel == 5)
>>>
>>
>> The warning seems a bit spurious!
>
> The whole mite.c driver and it's users are all a bit of a mess.
>
> I did find the information about the "AI AO Select Register" in the National
> Instruments PCI E Series manual:
>
> -----
>
> The AI AO Select Register contains 8 bits that control the logical DMA selection for the
> analog input and analog output resources. The contents of this register are cleared upon power
> up and after a reset condition.
> 	Address:	base address + 0x09
> 	Type:		write-only
> 	Word size:	8-bit
>
> 	Bit	Name		Description
> 	7-4	Reserved	Reserved-Always write 0 to these bits (for PCI-6032E
> 				and PCI-6033E only).
> 	7-4	Output<D..A>	Analog Output Logical Channel D through A-These four
> 				bits select the logical channels of the MITE to be used by
> 				the analog output. You can only set one of these bits at a
> 				time (except for the PCI-6032E and PCI-6033E).
> 	3-0	Input<D..A>	Analog Input Logical Channel D through A-These four
> 				bits select the logical channels to be used by the analog
> 				input. You can only set one of these bits at a time.
>
> -----
>
> Based on this the ni_stc_dma_channel_select_bitfield() function appears to be
> incorrect. The "channel"s should only be in the range 0 to 3 and result in a returned
> bitfield of:
>
> 	channel	bitfield	Description
> 	0		0x01	Analog Input or Output Channel A
> 	1		0x02 	Analog Input or Output Channel B
> 	2		0x04 	Analog Input or Output Channel C
> 	3		0x08 	Analog Input or Output Channel D
>
> Just my two cents...

Apparently, the M-series cards have six DMA channels - two more than the 
E-series cards.  They must have decided to keep the existing bitfield 
encoding for the first four channels for backwards compatibility, and 
use special bitfield values for the extra channels.

-- 
-=( Ian Abbott @ MEV Ltd.    E-mail: <abbotti@mev.co.uk>        )=-
-=( Tel: +44 (0)161 477 1898   FAX: +44 (0)161 718 3587         )=-

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] staging/comedi: fix sparse warning: shift too big
  2014-05-02 14:57 [PATCH] staging/comedi: fix sparse warning: shift too big Brilliantov Kirill Vladimirovich
  2014-05-02 16:49 ` Ian Abbott
@ 2014-05-03 23:58 ` Greg Kroah-Hartman
  2014-05-04  7:46   ` Brilliantov Kirill Vladimirovich
  1 sibling, 1 reply; 6+ messages in thread
From: Greg Kroah-Hartman @ 2014-05-03 23:58 UTC (permalink / raw)
  To: Brilliantov Kirill Vladimirovich
  Cc: Ian Abbott, H Hartley Sweeten, linux-kernel

On Fri, May 02, 2014 at 06:57:04PM +0400, Brilliantov Kirill Vladimirovich wrote:
> Signed-off-by: Brilliantov Kirill Vladimirovich <brilliantov@inbox.ru>
> ---
>  drivers/staging/comedi/drivers/ni_stc.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/comedi/drivers/ni_stc.h b/drivers/staging/comedi/drivers/ni_stc.h
> index f0630b78..197b568 100644
> --- a/drivers/staging/comedi/drivers/ni_stc.h
> +++ b/drivers/staging/comedi/drivers/ni_stc.h
> @@ -717,7 +717,7 @@ enum AI_AO_Select_Bits {
>  static inline unsigned ni_stc_dma_channel_select_bitfield(unsigned channel)
>  {
>  	if (channel < 4)
> -		return 1 << channel;
> +		return 1 << (channel & 0x3);

That change does nothing, so it's not worth applying, don't you agree?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] staging/comedi: fix sparse warning: shift too big
  2014-05-03 23:58 ` Greg Kroah-Hartman
@ 2014-05-04  7:46   ` Brilliantov Kirill Vladimirovich
  0 siblings, 0 replies; 6+ messages in thread
From: Brilliantov Kirill Vladimirovich @ 2014-05-04  7:46 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Ian Abbott, H Hartley Sweeten, linux-kernel

On 2014-05-03 19:58:38, Greg Kroah-Hartman wrote:
> 
> That change does nothing, so it's not worth applying, don't you agree?
> 

You right, this patch is not very useful.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2014-05-04  7:46 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-05-02 14:57 [PATCH] staging/comedi: fix sparse warning: shift too big Brilliantov Kirill Vladimirovich
2014-05-02 16:49 ` Ian Abbott
2014-05-02 18:28   ` Hartley Sweeten
2014-05-02 20:24     ` Ian Abbott
2014-05-03 23:58 ` Greg Kroah-Hartman
2014-05-04  7:46   ` Brilliantov Kirill Vladimirovich

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.