All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: Arnd Bergmann <arnd@kernel.org>
Cc: Hans Verkuil <hverkuil-cisco@xs4all.nl>,
	Mauro Carvalho Chehab <mchehab@kernel.org>,
	Arnd Bergmann <arnd@arndb.de>,
	"Lad, Prabhakar" <prabhakar.csengg@gmail.com>,
	Eduardo Valentin <edubezval@gmail.com>,
	Sakari Ailus <sakari.ailus@linux.intel.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Vaibhav Gupta <vaibhavgupta40@gmail.com>,
	Liu Shixin <liushixin2@huawei.com>,
	Jacopo Mondi <jacopo+renesas@jmondi.org>,
	Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	linux-kernel@vger.kernel.org, linux-media@vger.kernel.org,
	linux-staging@lists.linux.dev, Hans Verkuil <hverkuil@xs4all.nl>,
	Christoph Hellwig <hch@lst.de>
Subject: Re: [PATCH v3 6/8] media: atomisp: remove compat_ioctl32 code
Date: Mon, 14 Jun 2021 20:07:59 +0300	[thread overview]
Message-ID: <YMeM73cHgomYQNcs@pendragon.ideasonboard.com> (raw)
In-Reply-To: <20210614103409.3154127-7-arnd@kernel.org>

Hi Arnd,

Thank you for the patch.

On Mon, Jun 14, 2021 at 12:34:07PM +0200, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> This is one of the last remaining users of compat_alloc_user_space()
> and copy_in_user(), which are in the process of getting removed.
> 
> As of commit 57e6b6f2303e ("media: atomisp_fops.c: disable
> atomisp_compat_ioctl32"), nothing in this file is actually getting used
> as the only reference has been stubbed out.
> 
> Remove the entire file -- anyone willing to restore the functionality
> can equally well just look up the contents in the git history if needed.
> 
> Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
> Cc: Hans Verkuil <hverkuil@xs4all.nl>
> Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> Suggested-by: Christoph Hellwig <hch@lst.de>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/staging/media/atomisp/Makefile        |    1 -
>  drivers/staging/media/atomisp/TODO            |    5 +
>  .../atomisp/pci/atomisp_compat_ioctl32.c      | 1202 -----------------
>  .../staging/media/atomisp/pci/atomisp_fops.c  |    8 +-
>  4 files changed, 8 insertions(+), 1208 deletions(-)
>  delete mode 100644 drivers/staging/media/atomisp/pci/atomisp_compat_ioctl32.c
> 
> diff --git a/drivers/staging/media/atomisp/Makefile b/drivers/staging/media/atomisp/Makefile
> index 51498b2e85b8..606b7754fdfd 100644
> --- a/drivers/staging/media/atomisp/Makefile
> +++ b/drivers/staging/media/atomisp/Makefile
> @@ -16,7 +16,6 @@ atomisp-objs += \
>  	pci/atomisp_acc.o \
>  	pci/atomisp_cmd.o \
>  	pci/atomisp_compat_css20.o \
> -	pci/atomisp_compat_ioctl32.o \
>  	pci/atomisp_csi2.o \
>  	pci/atomisp_drvfs.o \
>  	pci/atomisp_file.o \
> diff --git a/drivers/staging/media/atomisp/TODO b/drivers/staging/media/atomisp/TODO
> index 6987bb2d32cf..2d1ef9eb262a 100644
> --- a/drivers/staging/media/atomisp/TODO
> +++ b/drivers/staging/media/atomisp/TODO
> @@ -120,6 +120,11 @@ TODO
>      for this driver until the other work is done, as there will be a lot
>      of code churn until this driver becomes functional again.
>  
> +16. Fix private ioctls to not need a compat_ioctl handler for running
> +    32-bit tasks. The compat code has been removed because of bugs,
> +    and should not be needed for modern drivers. Fixing this properly
> +    unfortunately means an incompatible ABI change.
> +
>  Limitations
>  ===========
>  

[snip]

> diff --git a/drivers/staging/media/atomisp/pci/atomisp_fops.c b/drivers/staging/media/atomisp/pci/atomisp_fops.c
> index 26d05474a035..be58f21ab208 100644
> --- a/drivers/staging/media/atomisp/pci/atomisp_fops.c
> +++ b/drivers/staging/media/atomisp/pci/atomisp_fops.c
> @@ -1283,7 +1283,8 @@ const struct v4l2_file_operations atomisp_fops = {
>  	.unlocked_ioctl = video_ioctl2,
>  #ifdef CONFIG_COMPAT
>  	/*
> -	 * There are problems with this code. Disable this for now.
> +	 * this was removed because of bugs, the interface
> +	 * needs to be made safe for compat tasks instead.
>  	.compat_ioctl32 = atomisp_compat_ioctl32,
>  	 */
>  #endif
> @@ -1297,10 +1298,7 @@ const struct v4l2_file_operations atomisp_file_fops = {
>  	.mmap = atomisp_file_mmap,
>  	.unlocked_ioctl = video_ioctl2,
>  #ifdef CONFIG_COMPAT
> -	/*
> -	 * There are problems with this code. Disable this for now.
> -	.compat_ioctl32 = atomisp_compat_ioctl32,
> -	 */
> +	/* .compat_ioctl32 = atomisp_compat_ioctl32, */
>  #endif
>  	.poll = atomisp_poll,
>  };

-- 
Regards,

Laurent Pinchart

  reply	other threads:[~2021-06-14 17:08 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-14 10:34 [PATCH v3 0/7] media: v4l2: compat ioctl fixes Arnd Bergmann
2021-06-14 10:34 ` [PATCH v3 1/8] media: v4l2-core: ignore native time32 ioctls on 64-bit Arnd Bergmann
2021-06-14 13:24   ` Andy Shevchenko
2021-06-14 16:50   ` Laurent Pinchart
2021-06-14 10:34 ` [PATCH v3 2/8] media: v4l2-core: explicitly clear ioctl input data Arnd Bergmann
2021-06-14 16:56   ` Laurent Pinchart
2021-06-14 10:34 ` [PATCH v3 3/8] media: v4l2-core: fix whitespace damage in video_get_user() Arnd Bergmann
2021-06-14 16:58   ` Laurent Pinchart
2021-06-14 10:34 ` [PATCH v3 4/8] media: subdev: remove VIDIOC_DQEVENT_TIME32 handling Arnd Bergmann
2021-06-14 17:02   ` Laurent Pinchart
2021-06-15  8:43     ` Arnd Bergmann
2021-06-15  8:48       ` Hans Verkuil
2021-06-15  9:30         ` Arnd Bergmann
2021-06-14 10:34 ` [PATCH v3 5/8] media: v4l2-core: return -ENODEV from ioctl when not registered Arnd Bergmann
2021-06-14 17:04   ` Laurent Pinchart
2021-06-14 17:04   ` Laurent Pinchart
2021-06-14 10:34 ` [PATCH v3 6/8] media: atomisp: remove compat_ioctl32 code Arnd Bergmann
2021-06-14 17:07   ` Laurent Pinchart [this message]
2021-06-14 10:34 ` [PATCH v3 7/8] media: subdev: fix compat_ioctl32 Arnd Bergmann
2021-06-14 17:18   ` Laurent Pinchart
2021-06-15  8:26     ` Hans Verkuil
2021-06-15  8:39       ` Arnd Bergmann
2021-06-14 10:34 ` [PATCH v3 8/8] media: subdev: disallow ioctl for saa6588/davinci Arnd Bergmann
2021-06-14 17:21   ` Laurent Pinchart

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=YMeM73cHgomYQNcs@pendragon.ideasonboard.com \
    --to=laurent.pinchart@ideasonboard.com \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=arnd@arndb.de \
    --cc=arnd@kernel.org \
    --cc=edubezval@gmail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=hch@lst.de \
    --cc=hverkuil-cisco@xs4all.nl \
    --cc=hverkuil@xs4all.nl \
    --cc=jacopo+renesas@jmondi.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=linux-staging@lists.linux.dev \
    --cc=liushixin2@huawei.com \
    --cc=mchehab@kernel.org \
    --cc=prabhakar.csengg@gmail.com \
    --cc=sakari.ailus@linux.intel.com \
    --cc=vaibhavgupta40@gmail.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.