On Tuesday, March 26, 2013 1:29 PM, Ian Abbott wrote:
> On 26/03/2013 18:23, H Hartley Sweeten wrote:
>> There are a number of uint16_t casts used in the #define's of the
>> constant bit field values as well as the calls to DEBIreplace().
>> These cause a number of sparse warnings of the type:
>>
>> warning: cast truncates bits from constant value (ffff1cff becomes 1cff)
>>
>> Remove all of the casts and change the types of the parameters to
>> DEBIreplace from uin16_t to unsigned int. This fixes all the warnings.
>>
>> Mask the values written to the P_DEBICMD register and read from the
>> P_DEBIAD register with 0xffff. These registers are only 16-bits but
>> are accessed with 32-bit instructions.
>
> No, the values written to P_DEBICMD should be 32-bits, consisting of a
> 16-bit address in the lower 16 bits and some control bits in the upper
> 16 bits.
Grr... One of the reasons I hate casts...
I overlooked that DEBI_CMD_RDWORD is actually:
#define DEBI_CMD_SIZE16 (2 << 17)
#define DEBI_CMD_READ 0x00010000
#define DEBI_CMD_RDWORD (DEBI_CMD_READ | DEBI_CMD_SIZE16)
>>
>> Signed-off-by: H Hartley Sweeten <hsweeten@xxxxxxxxxxxxxxxxxxx>
>> Cc: Ian Abbott <abbotti@xxxxxxxxx>
>> Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
>> ---
>> drivers/staging/comedi/drivers/s626.c | 47 ++++++++++++---------------
>> drivers/staging/comedi/drivers/s626.h | 60 +++++++++++++++++------------------
>> 2 files changed, 51 insertions(+), 56 deletions(-)
>>
>> diff --git a/drivers/staging/comedi/drivers/s626.c b/drivers/staging/comedi/drivers/s626.c
>> index 22ced23..33682a3 100644
>> --- a/drivers/staging/comedi/drivers/s626.c
>> +++ b/drivers/staging/comedi/drivers/s626.c
>> @@ -239,20 +239,20 @@ static void DEBIwrite(struct comedi_device *dev, uint16_t addr, uint16_t wdata)
>> * specifies bits that are to be preserved, wdata is new value to be
>> * or'd with the masked original.
>> */
>> -static void DEBIreplace(struct comedi_device *dev, uint16_t addr, uint16_t mask,
>> - uint16_t wdata)
>> +static void DEBIreplace(struct comedi_device *dev, unsigned int addr,
>> + unsigned int mask, unsigned int wdata)
>> {
>> struct s626_private *devpriv = dev->private;
>> unsigned int val;
>>
>> - writel(DEBI_CMD_RDWORD | addr, devpriv->mmio + P_DEBICMD);
>> + writel((DEBI_CMD_RDWORD | addr) & 0xffff, devpriv->mmio + P_DEBICMD);
>
> That's wrong. Should be:
>
> addr &= 0xffff;
> writel(DEBI_CMD_RDWORD | addr, devpriv->mmio + P_DEBICMD);
>
> (two statements, so the truncated addr can be used below...)
You are correct. I'll post a v2 shortly.
Thanks,
Hartley
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
