All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sakari Ailus <sakari.ailus@linux.intel.com>
To: Andy Shevchenko <andriy.shevchenko@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: Fri, 11 Dec 2020 18:56:14 +0200	[thread overview]
Message-ID: <20201211165614.GC26370@paasikivi.fi.intel.com> (raw)
In-Reply-To: <20201123104018.GX4077@smile.fi.intel.com>

Hi Andy,

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:
> > tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> > head:   27bba9c532a8d21050b94224ffd310ad0058c353
> > commit: 7b285f41f7376dc37e7fad1e803995fd39f42848 media: ipu3-cio2: Introduce CIO2_LOP_ENTRIES constant
> > date:   2 months ago
> > config: arm64-randconfig-r031-20201121 (attached as .config)
> > compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project bec968cbb367dd03439c89c1d4ef968ef662d7c0)
> > reproduce (this is a W=1 build):
> >         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> >         chmod +x ~/bin/make.cross
> >         # install arm64 cross compiling tool for clang build
> >         # apt-get install binutils-aarch64-linux-gnu
> >         # https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=7b285f41f7376dc37e7fad1e803995fd39f42848
> >         git remote add linus https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
> >         git fetch --no-tags linus master
> >         git checkout 7b285f41f7376dc37e7fad1e803995fd39f42848
> >         # save the attached .config to linux build tree
> >         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=arm64 
> > 
> > If you fix the issue, kindly add following tag as appropriate
> > Reported-by: kernel test robot <lkp@intel.com>
> > 
> > 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? :-)

-- 
Kind regards,

Sakari Ailus

WARNING: multiple messages have this Message-ID (diff)
From: Sakari Ailus <sakari.ailus@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: Fri, 11 Dec 2020 18:56:14 +0200	[thread overview]
Message-ID: <20201211165614.GC26370@paasikivi.fi.intel.com> (raw)
In-Reply-To: <20201123104018.GX4077@smile.fi.intel.com>

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

Hi Andy,

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:
> > tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> > head:   27bba9c532a8d21050b94224ffd310ad0058c353
> > commit: 7b285f41f7376dc37e7fad1e803995fd39f42848 media: ipu3-cio2: Introduce CIO2_LOP_ENTRIES constant
> > date:   2 months ago
> > config: arm64-randconfig-r031-20201121 (attached as .config)
> > compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project bec968cbb367dd03439c89c1d4ef968ef662d7c0)
> > reproduce (this is a W=1 build):
> >         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> >         chmod +x ~/bin/make.cross
> >         # install arm64 cross compiling tool for clang build
> >         # apt-get install binutils-aarch64-linux-gnu
> >         # https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=7b285f41f7376dc37e7fad1e803995fd39f42848
> >         git remote add linus https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
> >         git fetch --no-tags linus master
> >         git checkout 7b285f41f7376dc37e7fad1e803995fd39f42848
> >         # save the attached .config to linux build tree
> >         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=arm64 
> > 
> > If you fix the issue, kindly add following tag as appropriate
> > Reported-by: kernel test robot <lkp@intel.com>
> > 
> > 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? :-)

-- 
Kind regards,

Sakari Ailus

  reply	other threads:[~2020-12-11 18:11 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 [this message]
2020-12-11 16:56     ` Sakari Ailus
2020-12-14 15:29     ` Andy Shevchenko
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=20201211165614.GC26370@paasikivi.fi.intel.com \
    --to=sakari.ailus@linux.intel.com \
    --cc=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 \
    /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.