All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
To: Sakari Ailus <sakari.ailus@linux.intel.com>
Cc: kernel test robot <lkp@intel.com>,
	kbuild-all@lists.01.org, clang-built-linux@googlegroups.com,
	linux-kernel@vger.kernel.org,
	Mauro Carvalho Chehab <mchehab@kernel.org>,
	linux-media@vger.kernel.org
Subject: Re: drivers/media/pci/intel/ipu3/ipu3-cio2.c:163:56: warning: implicit conversion from 'unsigned long' to 'u16' (aka 'unsigned short') changes value from 131072 to 0
Date: Mon, 14 Dec 2020 17:29:55 +0200	[thread overview]
Message-ID: <20201214152955.GH4077@smile.fi.intel.com> (raw)
In-Reply-To: <20201211165614.GC26370@paasikivi.fi.intel.com>

On Fri, Dec 11, 2020 at 06:56:14PM +0200, Sakari Ailus wrote:
> On Mon, Nov 23, 2020 at 12:40:18PM +0200, Andy Shevchenko wrote:
> > On Sat, Nov 21, 2020 at 04:23:05PM +0800, kernel test robot wrote:

...

> > > All warnings (new ones prefixed by >>):
> > > 
> > > >> drivers/media/pci/intel/ipu3/ipu3-cio2.c:163:56: warning: implicit conversion from 'unsigned long' to 'u16' (aka 'unsigned short') changes value from 131072 to 0 [-Wconstant-conversion]
> > >            entry[1].second_entry.num_of_pages = CIO2_LOP_ENTRIES * CIO2_MAX_LOPS;
> > >                                               ~ ~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~
> > >    1 warning generated.
> > 
> > Okay, now we have an interesting case. The IP is quite unlikely be used on
> > ARM64, but my patches made the clear picture about use of PAGE_SIZE here.
> > 
> > So, I see at least the following options to mitigate the above, i.e.:
> >  1/ reduce driver scope to X86
> >  2/ fix the variables to be wider type to be able to hold PAGE_SIZE > 4k
> >  3/ switch to custom PAGE_SIZE / _SHIFT / _MASK and accompanying macros
> > 
> > And I still consider 3/ is silly move because as we see the driver was
> > never assumed to work with big page sizes (besides unsigned short type
> > here, PAGE_SHIFT and PAGE_MASK in the original code was as is and on ARM64
> > they compiled to 0 values w/o warnings, effectively make the driver
> > improperly functioning anyway).
> 
> Apologies for the late answer.
> 
> I think I'd favour the first option. It's not really useful to be able to
> compile this elsewhere; as such the driver doesn't do anything special that
> would make it prone to breakage through changes elsewhere.
> 
> Would you like to send a patch? :-)

Done.

-- 
With Best Regards,
Andy Shevchenko



WARNING: multiple messages have this Message-ID (diff)
From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
To: kbuild-all@lists.01.org
Subject: Re: drivers/media/pci/intel/ipu3/ipu3-cio2.c:163:56: warning: implicit conversion from 'unsigned long' to 'u16' (aka 'unsigned short') changes value from 131072 to 0
Date: Mon, 14 Dec 2020 17:29:55 +0200	[thread overview]
Message-ID: <20201214152955.GH4077@smile.fi.intel.com> (raw)
In-Reply-To: <20201211165614.GC26370@paasikivi.fi.intel.com>

[-- Attachment #1: Type: text/plain, Size: 1814 bytes --]

On Fri, Dec 11, 2020 at 06:56:14PM +0200, Sakari Ailus wrote:
> On Mon, Nov 23, 2020 at 12:40:18PM +0200, Andy Shevchenko wrote:
> > On Sat, Nov 21, 2020 at 04:23:05PM +0800, kernel test robot wrote:

...

> > > All warnings (new ones prefixed by >>):
> > > 
> > > >> drivers/media/pci/intel/ipu3/ipu3-cio2.c:163:56: warning: implicit conversion from 'unsigned long' to 'u16' (aka 'unsigned short') changes value from 131072 to 0 [-Wconstant-conversion]
> > >            entry[1].second_entry.num_of_pages = CIO2_LOP_ENTRIES * CIO2_MAX_LOPS;
> > >                                               ~ ~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~
> > >    1 warning generated.
> > 
> > Okay, now we have an interesting case. The IP is quite unlikely be used on
> > ARM64, but my patches made the clear picture about use of PAGE_SIZE here.
> > 
> > So, I see at least the following options to mitigate the above, i.e.:
> >  1/ reduce driver scope to X86
> >  2/ fix the variables to be wider type to be able to hold PAGE_SIZE > 4k
> >  3/ switch to custom PAGE_SIZE / _SHIFT / _MASK and accompanying macros
> > 
> > And I still consider 3/ is silly move because as we see the driver was
> > never assumed to work with big page sizes (besides unsigned short type
> > here, PAGE_SHIFT and PAGE_MASK in the original code was as is and on ARM64
> > they compiled to 0 values w/o warnings, effectively make the driver
> > improperly functioning anyway).
> 
> Apologies for the late answer.
> 
> I think I'd favour the first option. It's not really useful to be able to
> compile this elsewhere; as such the driver doesn't do anything special that
> would make it prone to breakage through changes elsewhere.
> 
> Would you like to send a patch? :-)

Done.

-- 
With Best Regards,
Andy Shevchenko


  reply	other threads:[~2020-12-14 15:31 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-21  8:23 drivers/media/pci/intel/ipu3/ipu3-cio2.c:163:56: warning: implicit conversion from 'unsigned long' to 'u16' (aka 'unsigned short') changes value from 131072 to 0 kernel test robot
2020-11-21  8:23 ` kernel test robot
2020-11-23 10:40 ` Andy Shevchenko
2020-11-23 10:40   ` Andy Shevchenko
2020-12-11 16:56   ` Sakari Ailus
2020-12-11 16:56     ` Sakari Ailus
2020-12-14 15:29     ` Andy Shevchenko [this message]
2020-12-14 15:29       ` Andy Shevchenko
2021-01-04 23:16 kernel test robot
2021-01-04 23:16 ` kernel test robot
2021-01-04 23:24 ` Sakari Ailus
2021-01-04 23:24   ` Sakari Ailus

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20201214152955.GH4077@smile.fi.intel.com \
    --to=andriy.shevchenko@linux.intel.com \
    --cc=clang-built-linux@googlegroups.com \
    --cc=kbuild-all@lists.01.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=lkp@intel.com \
    --cc=mchehab@kernel.org \
    --cc=sakari.ailus@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.