> > This is what I meant FWIW:
> >
> > diff --git a/include/linux/phy.h b/include/linux/phy.h
> > index 7addde5d14c0..829bd57b8794 100644
> > --- a/include/linux/phy.h
> > +++ b/include/linux/phy.h
> > @@ -1206,10 +1206,13 @@ static inline int phy_read(struct phy_device *phydev, u32 regnum)
> > #define phy_read_poll_timeout(phydev, regnum, val, cond, sleep_us, \
> > timeout_us, sleep_before_read) \
> > ({ \
> > - int __ret = read_poll_timeout(phy_read, val, val < 0 || (cond), \
> > + int __ret, __val; \
> > + \
> > + __ret = read_poll_timeout(phy_read, __val, __val < 0 || (cond), \
> > sleep_us, timeout_us, sleep_before_read, phydev, regnum); \
> > - if (val < 0) \
> > - __ret = val; \
> > + val = __val;
This results in the sign being discarded if val is unsigned. Yes, the
test is remove, which i assume will stop Smatch complaining, but it is
still broken.
> > + if (__val < 0) \
> > + __ret = __val; \
> > if (__ret) \
> > phydev_err(phydev, "%s failed: %d\n", __func__, __ret); \
> > __ret; \
> > I tried enabling -Wtype-limits but it's _very_ noisy :(
This is a no go until GENMASK gets fixed :-(
However, if that is fixed, we might be able to turn it on. But it will
then trigger with this fix.
So i still think a BUILD_BUG_ON is a better fix. Help developers get
the code correct, rather than work around them getting it wrong.
I also wonder if this needs to go down a level. Dan, how often do you
see similar problems with the lower level read_poll_timeout() and
friends?
Andrew
