re: [media] DVB: add support for the LG2160 ATSC-MH demodulator

2012-05-21 Thread Dan Carpenter 
Hi Michael,

I have a question about e26f2ae4527b: [media] DVB: add support for the
LG2160 ATSC-MH demodulator from Jan 29, 2012.

   122  static int lg216x_write_regs(struct lg216x_state *state,
   123   struct lg216x_reg *regs, int len)
   124  {
   125  int i, ret;
   126  
   127  lg_reg(writing %d registers...\n, len);
   128  
   129  for (i = 0; i  len - 1; i++) {
^^^
Shouldn't this just be i  len?  Why do we skip the last element in the
array?

   130  ret = lg216x_write_reg(state, regs[i].reg, regs[i].val);
   131  if (lg_fail(ret))
   132  return ret;
   133  }
   134  return 0;
   135  }

This function is called like:
ret = lg216x_write_regs(state, lg2160_init, ARRAY_SIZE(lg2160_init));

The last element of the lg2160_init[] array looks useful.

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

Re: [media] DVB: add support for the LG2160 ATSC-MH demodulator

2012-05-21 Thread Michael Krufky 
On Mon, May 21, 2012 at 9:58 AM, Dan Carpenter dan.carpen...@oracle.com wrote:
 Hi Michael,

 I have a question about e26f2ae4527b: [media] DVB: add support for the
 LG2160 ATSC-MH demodulator from Jan 29, 2012.

   122  static int lg216x_write_regs(struct lg216x_state *state,
   123                               struct lg216x_reg *regs, int len)
   124  {
   125          int i, ret;
   126
   127          lg_reg(writing %d registers...\n, len);
   128
   129          for (i = 0; i  len - 1; i++) {
                            ^^^
 Shouldn't this just be i  len?  Why do we skip the last element in the
 array?

   130                  ret = lg216x_write_reg(state, regs[i].reg, 
 regs[i].val);
   131                  if (lg_fail(ret))
   132                          return ret;
   133          }
   134          return 0;
   135  }

 This function is called like:
        ret = lg216x_write_regs(state, lg2160_init, ARRAY_SIZE(lg2160_init));

 The last element of the lg2160_init[] array looks useful.

You're right, Dan - that's a bug -- thanks!

I'll queue up a fix for this.

Best Regards,

Mike Krufky
--
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