Re: [bug report] drx: add initial drx-d driver
Em Thu, 14 Dec 2017 22:55:02 +0100 Ralph Metzler escreveu: > Hello Dan Carpenter, > > Dan Carpenter writes: > > Hello Ralph Metzler, > > > > The patch 126f1e618870: "drx: add initial drx-d driver" from Mar 12, > > 2011, leads to the following static checker warning: > > > >drivers/media/dvb-frontends/drxd_hard.c:1305 SC_WaitForReady() > >info: return a literal instead of 'status' > > > > drivers/media/dvb-frontends/drxd_hard.c > > 1298 static int SC_WaitForReady(struct drxd_state *state) > > 1299 { > > 1300 int i; > > 1301 > > 1302 for (i = 0; i < DRXD_MAX_RETRIES; i += 1) { > > 1303 int status = Read16(state, SC_RA_RAM_CMD__A, NULL, > 0); > > 1304 if (status == 0) > > 1305 return status; > > ^ > > The register is set to zero when ready? The answer should obviously be > > yes, but it wouldn't totally surprise me if this function just always > > looped 1000 times... Few of the callers check the return. Anyway, it's > > more clear to just "return 0;" > > > > 1306 } > > 1307 return -1; > >^^ > > -1 is not a proper error code. > > > > 1308 } > > > > regards, > > dan carpenter > > I think I wrote the driver more than 10 years ago and somebody later > submitted it > to the kernel. > > I don't know if there is a anybody still maintaining this. Is it even used > anymore? > I could write a patch but cannot test it (e.g. to see if it really always > loops 1000 times ...) It seems that it is used on this board (besides ngene): EM2880_BOARD_HAUPPAUGE_WINTV_HVR_900_R2 a. k. a.: Hauppauge WinTV HVR 900 (R2) I might have a HVR-900 rev 2 somewhere, but if so, it is not at the usual place. I moved a few times since when I touched at the drxd driver, at the time it was merged upstream. Maybe Michael or someone at Hauppauge could test a patch for it, if they still have this device. Thanks, Mauro
Re: [bug report] drx: add initial drx-d driver
I'm really sorry. This warning showed up in my new warnings pile and I didn't look at the date. :/ Sorry for the noise. regards, dan carpenter
Re: [bug report] drx: add initial drx-d driver
> I think I wrote the driver more than 10 years ago and somebody later > submitted it > to the kernel. I'm pretty sure that was me, or perhaps I was the first person to get it to work with a device upstream - it was so long ago. > I don't know if there is a anybody still maintaining this. Is it even used > anymore? > I could write a patch but cannot test it (e.g. to see if it really always > loops 1000 times ...) Pretty sure the register was a status register that had bits set for pending writes, and when the transfer was complete all the bits would be zero (at which point it should stop polling the register and bail out). I would have to test it to be sure, but I'm 80% confident that in normal operation that loop only does a few iterations. Devin -- Devin J. Heitmueller - Kernel Labs http://www.kernellabs.com
[bug report] drx: add initial drx-d driver
Hello Dan Carpenter, Dan Carpenter writes: > Hello Ralph Metzler, > > The patch 126f1e618870: "drx: add initial drx-d driver" from Mar 12, > 2011, leads to the following static checker warning: > > drivers/media/dvb-frontends/drxd_hard.c:1305 SC_WaitForReady() > info: return a literal instead of 'status' > > drivers/media/dvb-frontends/drxd_hard.c > 1298 static int SC_WaitForReady(struct drxd_state *state) > 1299 { > 1300 int i; > 1301 > 1302 for (i = 0; i < DRXD_MAX_RETRIES; i += 1) { > 1303 int status = Read16(state, SC_RA_RAM_CMD__A, NULL, > 0); > 1304 if (status == 0) > 1305 return status; > ^ > The register is set to zero when ready? The answer should obviously be > yes, but it wouldn't totally surprise me if this function just always > looped 1000 times... Few of the callers check the return. Anyway, it's > more clear to just "return 0;" > > 1306 } > 1307 return -1; >^^ > -1 is not a proper error code. > > 1308 } > > regards, > dan carpenter I think I wrote the driver more than 10 years ago and somebody later submitted it to the kernel. I don't know if there is a anybody still maintaining this. Is it even used anymore? I could write a patch but cannot test it (e.g. to see if it really always loops 1000 times ...) Regards, Ralph Metzler -- --
[bug report] drx: add initial drx-d driver
Hello Ralph Metzler, The patch 126f1e618870: "drx: add initial drx-d driver" from Mar 12, 2011, leads to the following static checker warning: drivers/media/dvb-frontends/drxd_hard.c:1305 SC_WaitForReady() info: return a literal instead of 'status' drivers/media/dvb-frontends/drxd_hard.c 1298 static int SC_WaitForReady(struct drxd_state *state) 1299 { 1300 int i; 1301 1302 for (i = 0; i < DRXD_MAX_RETRIES; i += 1) { 1303 int status = Read16(state, SC_RA_RAM_CMD__A, NULL, 0); 1304 if (status == 0) 1305 return status; ^ The register is set to zero when ready? The answer should obviously be yes, but it wouldn't totally surprise me if this function just always looped 1000 times... Few of the callers check the return. Anyway, it's more clear to just "return 0;" 1306 } 1307 return -1; ^^ -1 is not a proper error code. 1308 } regards, dan carpenter