| Message ID | 20230530102126.2886766-1-sakari.ailus@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/1] media: tc358746: Address compiler warnings | expand |
Hi Sakari, Hans already sent a patch a few months ago: - https://lore.kernel.org/linux-media/15030a07-3615-fca0-1891-a234dc054b00@xs4all.nl/ It turned out that the compiler had a bug albeit the compiler listed in 'Closes:' is already a gcc-12 and now the warning used is slightly different. I'm not again the patch but we should point out that this patch is only required to make the compiler happy. Regards, Marco On 23-05-30, Sakari Ailus wrote: > Address these compiler warnings by initialising the m_best and p_best > values to 0 and 1 respectively (as latter is used as a divisor): > > drivers/media/i2c/tc358746.c: In function 'tc358746_find_pll_settings': > >> drivers/media/i2c/tc358746.c:817:13: warning: 'p_best' is used uninitialized > [-Wuninitialized] > 817 | u16 p_best, p; > | ^~~~~~ > >> drivers/media/i2c/tc358746.c:816:13: warning: 'm_best' is used uninitialized > [-Wuninitialized] > 816 | u16 m_best, mul; > | ^~~~~~ > > Reported-by: kernel test robot <lkp@intel.com> > Closes: https://lore.kernel.org/oe-kbuild-all/202305301627.fLT3Bkds-lkp@intel.com/ > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> > --- > drivers/media/i2c/tc358746.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/media/i2c/tc358746.c b/drivers/media/i2c/tc358746.c > index ec1a193ba161a..25fbce5cabdaa 100644 > --- a/drivers/media/i2c/tc358746.c > +++ b/drivers/media/i2c/tc358746.c > @@ -813,8 +813,8 @@ static unsigned long tc358746_find_pll_settings(struct tc358746 *tc358746, > u32 min_delta = 0xffffffff; > u16 prediv_max = 17; > u16 prediv_min = 1; > - u16 m_best, mul; > - u16 p_best, p; > + u16 m_best = 0, mul; > + u16 p_best = 1, p; > u8 postdiv; > > if (fout > 1000 * HZ_PER_MHZ) { > -- > 2.30.2 > >
Hi Marco, On Wed, May 31, 2023 at 10:18:10AM +0200, Marco Felsch wrote: > Hi Sakari, > > Hans already sent a patch a few months ago: > - https://lore.kernel.org/linux-media/15030a07-3615-fca0-1891-a234dc054b00@xs4all.nl/ > > It turned out that the compiler had a bug albeit the compiler listed in > 'Closes:' is already a gcc-12 and now the warning used is slightly > different. > > I'm not again the patch but we should point out that this patch is only > required to make the compiler happy. Ack, thanks. I'll drop this then. The condition isn't trivial for a compiler to figure out though, even I'm not quite sure this is the case for all parameter values.
On Thu, Jun 01, 2023 at 08:20:06AM +0000, Sakari Ailus wrote: > Hi Marco, > > On Wed, May 31, 2023 at 10:18:10AM +0200, Marco Felsch wrote: > > Hi Sakari, > > > > Hans already sent a patch a few months ago: > > - https://lore.kernel.org/linux-media/15030a07-3615-fca0-1891-a234dc054b00@xs4all.nl/ > > > > It turned out that the compiler had a bug albeit the compiler listed in > > 'Closes:' is already a gcc-12 and now the warning used is slightly > > different. > > > > I'm not again the patch but we should point out that this patch is only > > required to make the compiler happy. > > Ack, thanks. I'll drop this then. The condition isn't trivial for a > compiler to figure out though, even I'm not quite sure this is the case for > all parameter values. Hans's patch actually assigns p_best to 0 which isn't a best default value for a divisor.
Hi Sakari, On 23-06-01, Sakari Ailus wrote: > On Thu, Jun 01, 2023 at 08:20:06AM +0000, Sakari Ailus wrote: > > Hi Marco, > > > > On Wed, May 31, 2023 at 10:18:10AM +0200, Marco Felsch wrote: > > > Hi Sakari, > > > > > > Hans already sent a patch a few months ago: > > > - https://lore.kernel.org/linux-media/15030a07-3615-fca0-1891-a234dc054b00@xs4all.nl/ > > > > > > It turned out that the compiler had a bug albeit the compiler listed in > > > 'Closes:' is already a gcc-12 and now the warning used is slightly > > > different. > > > > > > I'm not again the patch but we should point out that this patch is only > > > required to make the compiler happy. > > > > Ack, thanks. I'll drop this then. The condition isn't trivial for a > > compiler to figure out though, even I'm not quite sure this is the case for > > all parameter values. I don't want to complain here O:) and as I said we can pick the patch but we should mention that this is a patch for the compiler. > Hans's patch actually assigns p_best to 0 which isn't a best default value > for a divisor. You're right. Regards, Marco
diff --git a/drivers/media/i2c/tc358746.c b/drivers/media/i2c/tc358746.c index ec1a193ba161a..25fbce5cabdaa 100644 --- a/drivers/media/i2c/tc358746.c +++ b/drivers/media/i2c/tc358746.c @@ -813,8 +813,8 @@ static unsigned long tc358746_find_pll_settings(struct tc358746 *tc358746, u32 min_delta = 0xffffffff; u16 prediv_max = 17; u16 prediv_min = 1; - u16 m_best, mul; - u16 p_best, p; + u16 m_best = 0, mul; + u16 p_best = 1, p; u8 postdiv; if (fout > 1000 * HZ_PER_MHZ) {