Re: [PATCH 1/3] cdrom: remove the unused driver specific disc change ioctl

Re: [PATCH 1/3] cdrom: remove the unused driver specific disc change ioctl

[Phillip Potter]

On Wed, Apr 27, 2022 at 09:24:33AM -0400, Paul Gortmaker wrote:
> This was only used by the ide-cd driver, which went away in
> commit b7fb14d3ac63 ("ide: remove the legacy ide driver")
> so we might as well take advantage of that and get rid of
> this hook as well.
> 
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Jens Axboe <axboe@kernel.dk>
> Cc: Phillip Potter <phil@philpotter.co.uk>
> Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
> ---
>  Documentation/cdrom/cdrom-standard.rst | 10 ----------
>  drivers/cdrom/cdrom.c                  |  8 --------
>  include/linux/cdrom.h                  |  1 -
>  3 files changed, 19 deletions(-)
> 
> diff --git a/Documentation/cdrom/cdrom-standard.rst b/Documentation/cdrom/cdrom-standard.rst
> index 52ea7b6b2fe8..7964fe134277 100644
> --- a/Documentation/cdrom/cdrom-standard.rst
> +++ b/Documentation/cdrom/cdrom-standard.rst
> @@ -218,7 +218,6 @@ current *struct* is::
>  		int (*tray_move)(struct cdrom_device_info *, int);
>  		int (*lock_door)(struct cdrom_device_info *, int);
>  		int (*select_speed)(struct cdrom_device_info *, int);
> -		int (*select_disc)(struct cdrom_device_info *, int);
>  		int (*get_last_session) (struct cdrom_device_info *,
>  					 struct cdrom_multisession *);
>  		int (*get_mcn)(struct cdrom_device_info *, struct cdrom_mcn *);
> @@ -419,15 +418,6 @@ this `auto-selection` capability, the decision should be made on the
>  current disc loaded and the return value should be positive. A negative
>  return value indicates an error.
>  
> -::
> -
> -	int select_disc(struct cdrom_device_info *cdi, int number)
> -
> -If the drive can store multiple discs (a juke-box) this function
> -will perform disc selection. It should return the number of the
> -selected disc on success, a negative value on error. Currently, only
> -the ide-cd driver supports this functionality.
> -
>  ::
>  
>  	int get_last_session(struct cdrom_device_info *cdi,
> diff --git a/drivers/cdrom/cdrom.c b/drivers/cdrom/cdrom.c
> index 2dc9da683a13..0a858bfea849 100644
> --- a/drivers/cdrom/cdrom.c
> +++ b/drivers/cdrom/cdrom.c
> @@ -2443,14 +2443,6 @@ static int cdrom_ioctl_select_disc(struct cdrom_device_info *cdi,
>  			return -EINVAL;
>  	}
>  
> -	/*
> -	 * ->select_disc is a hook to allow a driver-specific way of
> -	 * seleting disc.  However, since there is no equivalent hook for
> -	 * cdrom_slot_status this may not actually be useful...
> -	 */
> -	if (cdi->ops->select_disc)
> -		return cdi->ops->select_disc(cdi, arg);
> -
>  	cd_dbg(CD_CHANGER, "Using generic cdrom_select_disc()\n");
>  	return cdrom_select_disc(cdi, arg);
>  }
> diff --git a/include/linux/cdrom.h b/include/linux/cdrom.h
> index 0a89f111e00e..67caa909e3e6 100644
> --- a/include/linux/cdrom.h
> +++ b/include/linux/cdrom.h
> @@ -77,7 +77,6 @@ struct cdrom_device_ops {
>  	int (*tray_move) (struct cdrom_device_info *, int);
>  	int (*lock_door) (struct cdrom_device_info *, int);
>  	int (*select_speed) (struct cdrom_device_info *, int);
> -	int (*select_disc) (struct cdrom_device_info *, int);
>  	int (*get_last_session) (struct cdrom_device_info *,
>  				 struct cdrom_multisession *);
>  	int (*get_mcn) (struct cdrom_device_info *,
> -- 
> 2.33.0
> 

Hi Paul,

Thanks for the patch, looks good. I'll send onto Jens during the next
merge window.

Signed-off-by: Phillip Potter <phil@philpotter.co.uk>

Regards,
Phil

Re: [PATCH 2/3] cdrom: mark CDROMGETSPINDOWN/CDROMSETSPINDOWN obsolete

[Phillip Potter]

On Wed, Apr 27, 2022 at 09:24:34AM -0400, Paul Gortmaker wrote:
> These were only implemented by the IDE CD driver, which has since
> been removed.  Given that nobody is likely to create new CD/DVD
> hardware (and associated drivers) we can mark these appropriately.
> 
> Cc: Jens Axboe <axboe@kernel.dk>
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Phillip Potter <phil@philpotter.co.uk>
> Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
> ---
>  Documentation/userspace-api/ioctl/cdrom.rst | 6 ++++++
>  include/uapi/linux/cdrom.h                  | 2 +-
>  2 files changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/userspace-api/ioctl/cdrom.rst b/Documentation/userspace-api/ioctl/cdrom.rst
> index 682948fc88a3..2ad91dbebd7c 100644
> --- a/Documentation/userspace-api/ioctl/cdrom.rst
> +++ b/Documentation/userspace-api/ioctl/cdrom.rst
> @@ -718,6 +718,9 @@ CDROMPLAYBLK
>  
>  
>  CDROMGETSPINDOWN
> +	Obsolete, was ide-cd only
> +
> +
>  	usage::
>  
>  	  char spindown;
> @@ -736,6 +739,9 @@ CDROMGETSPINDOWN
>  
>  
>  CDROMSETSPINDOWN
> +	Obsolete, was ide-cd only
> +
> +
>  	usage::
>  
>  	  char spindown
> diff --git a/include/uapi/linux/cdrom.h b/include/uapi/linux/cdrom.h
> index 804ff8d98f71..011e594e4a0d 100644
> --- a/include/uapi/linux/cdrom.h
> +++ b/include/uapi/linux/cdrom.h
> @@ -103,7 +103,7 @@
>  #define CDROMREADALL		0x5318	/* read all 2646 bytes */
>  
>  /* 
> - * These ioctls are (now) only in ide-cd.c for controlling 
> + * These ioctls were only in (now removed) ide-cd.c for controlling
>   * drive spindown time.  They should be implemented in the
>   * Uniform driver, via generic packet commands, GPCMD_MODE_SELECT_10,
>   * GPCMD_MODE_SENSE_10 and the GPMODE_POWER_PAGE...
> -- 
> 2.33.0
> 

Same for this one, looks good, will send on. Many thanks.

Signed-off-by: Phillip Potter <phil@philpotter.co.uk>

Regards,
Phil