Hi Hans,
Thank you for the patch.
On Fri, Feb 23, 2024 at 10:34:32AM +0100, Hans Verkuil wrote:
> If the media device is unregistered, the read/write/ioctl file ops
> returned EIO instead of ENODEV. And open returned ENXIO instead of the
> linux kernel standard of ENODEV.
Are you sure this is right ? Looking at chrdev_open() for instance, it
returns -ENXIO when it can't find a cdev for the requested minor.
Furthermore, the read() 3 manpage documents the ENXIO error as
ENXIO A request was made of a nonexistent device, or the request
was outside the capabilities of the device.
while it doesn't document ENODEV at all.
> Change the error code to ENODEV and document this as well in
> media-func-open.rst.
>
> Signed-off-by: Hans Verkuil <hverkuil-cisco@xxxxxxxxx>
> ---
> .../userspace-api/media/mediactl/media-func-open.rst | 4 ++--
> drivers/media/mc/mc-devnode.c | 10 +++++-----
> 2 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/Documentation/userspace-api/media/mediactl/media-func-open.rst b/Documentation/userspace-api/media/mediactl/media-func-open.rst
> index 24487cb0a308..8c1c7ebefdb1 100644
> --- a/Documentation/userspace-api/media/mediactl/media-func-open.rst
> +++ b/Documentation/userspace-api/media/mediactl/media-func-open.rst
> @@ -61,5 +61,5 @@ ENFILE
> ENOMEM
> Insufficient kernel memory was available.
>
> -ENXIO
> - No device corresponding to this device special file exists.
> +ENODEV
> + Device not found or was removed.
> diff --git a/drivers/media/mc/mc-devnode.c b/drivers/media/mc/mc-devnode.c
> index 7f67825c8757..fbf76e1414de 100644
> --- a/drivers/media/mc/mc-devnode.c
> +++ b/drivers/media/mc/mc-devnode.c
> @@ -75,7 +75,7 @@ static ssize_t media_read(struct file *filp, char __user *buf,
> if (!devnode->fops->read)
> return -EINVAL;
> if (!media_devnode_is_registered(devnode))
> - return -EIO;
> + return -ENODEV;
> return devnode->fops->read(filp, buf, sz, off);
> }
>
> @@ -87,7 +87,7 @@ static ssize_t media_write(struct file *filp, const char __user *buf,
> if (!devnode->fops->write)
> return -EINVAL;
> if (!media_devnode_is_registered(devnode))
> - return -EIO;
> + return -ENODEV;
> return devnode->fops->write(filp, buf, sz, off);
> }
>
> @@ -114,7 +114,7 @@ __media_ioctl(struct file *filp, unsigned int cmd, unsigned long arg,
> return -ENOTTY;
>
> if (!media_devnode_is_registered(devnode))
> - return -EIO;
> + return -ENODEV;
>
> return ioctl_func(filp, cmd, arg);
> }
> @@ -152,11 +152,11 @@ static int media_open(struct inode *inode, struct file *filp)
> */
> mutex_lock(&media_devnode_lock);
> devnode = container_of(inode->i_cdev, struct media_devnode, cdev);
> - /* return ENXIO if the media device has been removed
> + /* return ENODEV if the media device has been removed
> already or if it is not registered anymore. */
> if (!media_devnode_is_registered(devnode)) {
> mutex_unlock(&media_devnode_lock);
> - return -ENXIO;
> + return -ENODEV;
> }
> /* and increase the device refcount */
> get_device(&devnode->dev);
--
Regards,
Laurent Pinchart
