Re: [PATCH v3 2/3] block: add a s390-only biodasdinfo method

[Christoph Hellwig <hch@lst.de>]

On Fri, May 08, 2020 at 03:14:54PM +0200, Stefan Haberland wrote:
> From: Christoph Hellwig <hch@lst.de>
> 
> The IBM partition parser needs to query the DASD driver for details that
> are very s390 specific.  Instead of using ioctl_by_bdev with a fake user
> space pointer just add a s390-specific method to get the information
> directly.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> [sth@linux.ibm.com: remove fop, add gendisk check, export funcion]
> Signed-off-by: Stefan Haberland <sth@linux.ibm.com>

The subject and changelog need updates for your changes.  I think you
should also claim authorship, even if a few bits are originally from me.
Probaby it would make sense to even just merge this into the next patch.

> index fa552f9f1666..6eac7b11c75b 100644
> --- a/drivers/s390/block/dasd_int.h
> +++ b/drivers/s390/block/dasd_int.h
> @@ -845,6 +845,7 @@ void dasd_destroy_partitions(struct dasd_block *);
>  
>  /* externals in dasd_ioctl.c */
>  int  dasd_ioctl(struct block_device *, fmode_t, unsigned int, unsigned long);
> +int dasd_biodasdinfo(struct gendisk *disk, struct dasd_information2_t *info);

I think this needs to go to a public include/linux/ header for the
partitioning code to share the prototype.

> +int dasd_biodasdinfo(struct gendisk *disk, struct dasd_information2_t *info)
> +{
> +	struct dasd_device *base;
> +	int error;
> +
> +	/*
> +	 * we might get called externaly, so check if the gendisk belongs
> +	 * to a DASD by checking the fops pointer
> +	 */
> +	if (disk->fops != &dasd_device_operations)
> +		return -EINVAL;

I think a function comment (e.g. kernel doc) explaining the use case and
this detail might be useful.