Re: [bug report] mmc: tmio-mmc: add support for 32bit data port

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Jun 29, 2017 at 04:02:11PM +0000, Chris Brandt wrote:
> Personally, I think a buffer of raw data should be a 'u8 *', not a 
> 'unsigned short *'.

That works, or you could make it a void pointer.  (void *)foo + x and
(u8 *)foo + x are equivalent in kernel code.

> And, the parameter name "count" should really be something like "size" 
> or "length" (but that is still confusing because the passed array 'buf' 
> was designated as an array of "unsigned short *", so what would 'size' 
> really mean??)
> 
> Anyway, that is the API that's in place today, so that's what I needed 
> to work with.

You can change it.  This is the kernel so you can change anything if it
makes sense.

> 
> 
> > 
> >    417                  else
> >    418                          sd_ctrl_write32_rep(host, CTL_SD_DATA_PORT,
> > (u32 *)buf,
> >    419                                              count >> 2);
> >    420
> >    421                  /* if count was multiple of 4 */
> >    422                  if (!(count & 0x3))
> >    423                          return;
> >    424
> >    425                  buf8 = (u8 *)(buf + (count >> 2));
> > 
> > 
> > And this looks like the bug...  buf is a u16 pointer but we're adding
> > "count / sizeof(u32)" when we should be adding "count / (sizeof(*buf))".
> 
> I agree this math does not work out to what I wanted because of the 
> differences in data types.
> 
> Since I simply wanted to get a pointer to where I left off in the 
> buffer, maybe the most clear thing to do is just cast buf and do simple math:
> 
>    buf8 = (u8 *)buf + (count & ~3);
> 
> 
> Do you agree?

Sure.  Unless you want to round up?

regards,
dan carpenter




[Index of Archives]     [Linux Samsung SOC]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux