On Tue, Jan 03, 2017 at 04:30:58PM +0100, Greg KH wrote:
> On Wed, Dec 28, 2016 at 10:08:43PM -0800, Eric S. Stone wrote:
> > The modified functions do explicit endian checking and conversion. The
> > added forced casts fix these sparse warnings:
> > 
> > CHECK   drivers/staging/gdm724x/gdm_endian.c
> > drivers/staging/gdm724x/gdm_endian.c:28:24: warning: incorrect type in 
> > return expression (different base types)
> > drivers/staging/gdm724x/gdm_endian.c:28:24:    expected unsigned short
> > drivers/staging/gdm724x/gdm_endian.c:28:24:    got restricted __le16 
> > [usertype] <noident>
> > drivers/staging/gdm724x/gdm_endian.c:30:24: warning: incorrect type in 
> > return expression (different base types)
> > drivers/staging/gdm724x/gdm_endian.c:30:24:    expected unsigned short
> > drivers/staging/gdm724x/gdm_endian.c:30:24:    got restricted __be16 
> > [usertype] <noident>
> > drivers/staging/gdm724x/gdm_endian.c:36:24: warning: cast to restricted 
> > __le16
> > drivers/staging/gdm724x/gdm_endian.c:38:24: warning: cast to restricted 
> > __be16
> > drivers/staging/gdm724x/gdm_endian.c:44:24: warning: incorrect type in 
> > return expression (different base types)
> > drivers/staging/gdm724x/gdm_endian.c:44:24:    expected unsigned int
> > drivers/staging/gdm724x/gdm_endian.c:44:24:    got restricted __le32 
> > [usertype] <noident>
> > drivers/staging/gdm724x/gdm_endian.c:46:24: warning: incorrect type in 
> > return expression (different base types)
> > drivers/staging/gdm724x/gdm_endian.c:46:24:    expected unsigned int
> > drivers/staging/gdm724x/gdm_endian.c:46:24:    got restricted __be32 
> > [usertype] <noident>
> > drivers/staging/gdm724x/gdm_endian.c:52:24: warning: cast to restricted 
> > __le32
> > drivers/staging/gdm724x/gdm_endian.c:54:24: warning: cast to restricted 
> > __be32
> > 
> > Signed-off-by: Eric S. Stone <esst...@gmail.com>
> > ---
> >  drivers/staging/gdm724x/gdm_endian.c | 16 ++++++++--------
> >  1 file changed, 8 insertions(+), 8 deletions(-)
> > 
> > diff --git a/drivers/staging/gdm724x/gdm_endian.c 
> > b/drivers/staging/gdm724x/gdm_endian.c
> > index d7144e7..00ae7a8 100644
> > --- a/drivers/staging/gdm724x/gdm_endian.c
> > +++ b/drivers/staging/gdm724x/gdm_endian.c
> > @@ -25,31 +25,31 @@ void gdm_set_endian(struct gdm_endian *ed, u8 
> > dev_endian)
> >  u16 gdm_cpu_to_dev16(struct gdm_endian *ed, u16 x)
> >  {
> >     if (ed->dev_ed == ENDIANNESS_LITTLE)
> > -           return cpu_to_le16(x);
> > +           return (__force u16)cpu_to_le16(x);
> >     else
> > -           return cpu_to_be16(x);
> > +           return (__force u16)cpu_to_be16(x);
> 
> That's crazy, look at what you are writing here, does it really make any
> sense?
> 
> Please fix this up properly...
> 
> thanks,
> 
> greg k-h

Thanks for the feedback and sorry for the Crazy. I'll resubmit using
cpu_to_le16s(u16 *) and friends, avoiding casts. I was trying to keep
the code using the same non-mutating converters it was already using,
but it was a bad tradeoff since those all use the __bitwise annotated
types and we need u16/u32 here. So instead:

u16 gdm_cpu_to_dev16(struct gdm_endian *ed, u16 x)
{
        if (ed->dev_ed == ENDIANNESS_LITTLE)
                cpu_to_le16s(&x);
        else
                cpu_to_be16s(&x);
        return x;
}

I also tried to alternatively fix the warnings by normalizing the
driver code to one endiannes (getting rid of u16/u32), but since the
different device models have different endianness, conversions of some
sort remain necessary.

Regards,
-Eric S. Stone
_______________________________________________
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Reply via email to