On Sun, Jun 10, 2012 at 09:42:22PM +0200, Ralph Metzler wrote:
> Dan Carpenter writes:
> > Hello Ralph Metzler,
> >
> > The patch 43dd07f758d8: "[media] DRX-K: Initial check-in" from Jul 3,
> > 2011, leads to the following warning:
> > drivers/media/dvb/frontends/drxk_hard.c:2980 ADCSynchronization()
> > warn: suspicious bitop condition
> >
> > 2977 status = read16(state, IQM_AF_CLKNEG__A, &clkNeg);
> > 2978 if (status < 0)
> > 2979 goto error;
> > 2980 if ((clkNeg | IQM_AF_CLKNEG_CLKNEGDATA__M) ==
> > 2981 IQM_AF_CLKNEG_CLKNEGDATA_CLK_ADC_DATA_POS)
> {
> >
> > IQM_AF_CLKNEG_CLKNEGDATA__M is 2.
> > IQM_AF_CLKNEG_CLKNEGDATA_CLK_ADC_DATA_POS is 0.
> > So this condition can never be true.
>
> It seems this should be & instead of |. The mistake was also present in the
> windows driver.
>
Good deal. Do you want me to send a patch, or are you going to
handle it? Could I get a Reported-by cookie?
>
> >
> > 2982 clkNeg &= (~(IQM_AF_CLKNEG_CLKNEGDATA__M));
> > 2983 clkNeg |=
> > 2984
> IQM_AF_CLKNEG_CLKNEGDATA_CLK_ADC_DATA_NEG;
> > 2985 } else {
> > 2986 clkNeg &= (~(IQM_AF_CLKNEG_CLKNEGDATA__M));
> > 2987 clkNeg |=
> > 2988
> IQM_AF_CLKNEG_CLKNEGDATA_CLK_ADC_DATA_POS;
> >
> > clkNeg |= 0; <-- doesn't make much sense to the unenlightened.
> >
> > 2989 }
>
> This is perfectly normal since those defines were automatically created from
> the
> firmware source code. It is better to leave the code as it is. If there ever
> is a firmware update
> and these bits change their values it will be much harder to adjust the
> driver.
>
Sounds good. When I ran my script against the kernel it turns out
that doing x |= FOO; where foo is zero is very normal.
regards,
dan carpenter
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
