[PATCH 16/30] ide: Avoid preprocessor for CONFIG_LBA48

Mattijs Korpershoek mkorpershoek at baylibre.com
Tue Mar 28 16:18:54 CEST 2023


On mar., mars 28, 2023 at 08:07, Simon Glass <sjg at chromium.org> wrote:

> Use IS_ENABLED() instead for all conditions. Add the 'lba48' flag into
> struct blk_desc always, since it uses very little space. Use a bool so
> the meaning is clearer.
>
> Signed-off-by: Simon Glass <sjg at chromium.org>
> ---
>
>  drivers/block/ide.c | 57 ++++++++++++++++-----------------------------
>  include/blk.h       |  4 +---
>  2 files changed, 21 insertions(+), 40 deletions(-)
>
> diff --git a/drivers/block/ide.c b/drivers/block/ide.c
> index 6c5227a5c0e2..45201333c3c5 100644
> --- a/drivers/block/ide.c
> +++ b/drivers/block/ide.c
> @@ -540,11 +540,9 @@ static void atapi_inquiry(struct blk_desc *dev_desc)
>  		((unsigned long) iobuf[5] << 16) +
>  		((unsigned long) iobuf[6] << 8) + ((unsigned long) iobuf[7]);
>  	dev_desc->log2blksz = LOG2(dev_desc->blksz);
> -#ifdef CONFIG_LBA48
> +
>  	/* ATAPI devices cannot use 48bit addressing (ATA/ATAPI v7) */
> -	dev_desc->lba48 = 0;
> -#endif
> -	return;
> +	dev_desc->lba48 = false;
>  }
>  
>  static void ide_ident(struct blk_desc *dev_desc)
> @@ -645,9 +643,9 @@ static void ide_ident(struct blk_desc *dev_desc)
>  			((unsigned long)iop.lba_capacity[0]) |
>  			((unsigned long)iop.lba_capacity[1] << 16);
>  
> -#ifdef CONFIG_LBA48
> -	if (iop.command_set_2 & 0x0400) {	/* LBA 48 support */
> -		dev_desc->lba48 = 1;
> +	if (IS_ENABLED(CONFIG_LBA48) && (iop.command_set_2 & 0x0400)) {
> +		/* LBA 48 support */
> +		dev_desc->lba48 = true;
>  		for (int i = 0; i < 4; i++)
>  			iop.lba48_capacity[i] = be16_to_cpu(iop.lba48_capacity[i]);
>  		dev_desc->lba =
> @@ -656,9 +654,9 @@ static void ide_ident(struct blk_desc *dev_desc)
>  			((unsigned long long)iop.lba48_capacity[2] << 32) |
>  			((unsigned long long)iop.lba48_capacity[3] << 48));
>  	} else {
> -		dev_desc->lba48 = 0;
> +		dev_desc->lba48 = false;
>  	}
> -#endif /* CONFIG_LBA48 */
> +
>  	/* assuming HD */
>  	dev_desc->type = DEV_TYPE_HARDDISK;
>  	dev_desc->blksz = ATA_BLOCKSIZE;
> @@ -795,15 +793,13 @@ static ulong ide_read(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt,
>  	ulong n = 0;
>  	unsigned char c;
>  	unsigned char pwrsave = 0;	/* power save */
> +	bool lba48 = false;
>  
> -#ifdef CONFIG_LBA48
> -	unsigned char lba48 = 0;
> -
> -	if (blknr & 0x0000fffff0000000ULL) {
> +	if (IS_ENABLED(CONFIG_LBA48) && (blknr & 0x0000fffff0000000ULL)) {
>  		/* more than 28 bits used, use 48bit mode */
> -		lba48 = 1;
> +		lba48 = true;
>  	}
> -#endif
> +
>  	debug("ide_read dev %d start " LBAF ", blocks " LBAF " buffer at %lX\n",
>  	      device, blknr, blkcnt, (ulong) buffer);
>  
> @@ -845,8 +841,7 @@ static ulong ide_read(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt,
>  			printf("IDE read: device %d not ready\n", device);
>  			break;
>  		}
> -#ifdef CONFIG_LBA48
> -		if (lba48) {
> +		if (IS_ENABLED(CONFIG_LBA48) && lba48) {
>  			/* write high bits */
>  			ide_outb(device, ATA_SECT_CNT, 0);
>  			ide_outb(device, ATA_LBA_LOW, (blknr >> 24) & 0xFF);
> @@ -858,21 +853,17 @@ static ulong ide_read(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt,
>  			ide_outb(device, ATA_LBA_HIGH, 0);
>  #endif
>  		}
> -#endif
>  		ide_outb(device, ATA_SECT_CNT, 1);
>  		ide_outb(device, ATA_LBA_LOW, (blknr >> 0) & 0xFF);
>  		ide_outb(device, ATA_LBA_MID, (blknr >> 8) & 0xFF);
>  		ide_outb(device, ATA_LBA_HIGH, (blknr >> 16) & 0xFF);
>  
> -#ifdef CONFIG_LBA48
> -		if (lba48) {
> +		if (IS_ENABLED(CONFIG_LBA48) && lba48) {
>  			ide_outb(device, ATA_DEV_HD,
>  				 ATA_LBA | ATA_DEVICE(device));
>  			ide_outb(device, ATA_COMMAND, ATA_CMD_PIO_READ_EXT);
>  
> -		} else
> -#endif
> -		{
> +		} else {
>  			ide_outb(device, ATA_DEV_HD, ATA_LBA |
>  				 ATA_DEVICE(device) | ((blknr >> 24) & 0xF));
>  			ide_outb(device, ATA_COMMAND, ATA_CMD_PIO_READ);
> @@ -914,15 +905,12 @@ static ulong ide_write(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt,
>  	int device = block_dev->devnum;
>  	ulong n = 0;
>  	unsigned char c;
> +	bool lba48 = false;
>  
> -#ifdef CONFIG_LBA48
> -	unsigned char lba48 = 0;
> -
> -	if (blknr & 0x0000fffff0000000ULL) {
> +	if (IS_ENABLED(CONFIG_LBA48) && (blknr & 0x0000fffff0000000ULL)) {
>  		/* more than 28 bits used, use 48bit mode */
> -		lba48 = 1;
> +		lba48 = true;
>  	}
> -#endif
>  
>  	/* Select device
>  	 */
> @@ -935,8 +923,7 @@ static ulong ide_write(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt,
>  			printf("IDE read: device %d not ready\n", device);
>  			goto WR_OUT;
>  		}
> -#ifdef CONFIG_LBA48
> -		if (lba48) {
> +		if (IS_ENABLED(CONFIG_LBA48) && lba48) {
>  			/* write high bits */
>  			ide_outb(device, ATA_SECT_CNT, 0);
>  			ide_outb(device, ATA_LBA_LOW, (blknr >> 24) & 0xFF);
> @@ -948,21 +935,17 @@ static ulong ide_write(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt,
>  			ide_outb(device, ATA_LBA_HIGH, 0);
>  #endif
>  		}
> -#endif
>  		ide_outb(device, ATA_SECT_CNT, 1);
>  		ide_outb(device, ATA_LBA_LOW, (blknr >> 0) & 0xFF);
>  		ide_outb(device, ATA_LBA_MID, (blknr >> 8) & 0xFF);
>  		ide_outb(device, ATA_LBA_HIGH, (blknr >> 16) & 0xFF);
>  
> -#ifdef CONFIG_LBA48
> -		if (lba48) {
> +		if (IS_ENABLED(CONFIG_LBA48) && lba48) {
>  			ide_outb(device, ATA_DEV_HD,
>  				 ATA_LBA | ATA_DEVICE(device));
>  			ide_outb(device, ATA_COMMAND, ATA_CMD_PIO_WRITE_EXT);
>  
> -		} else
> -#endif
> -		{
> +		} else {
>  			ide_outb(device, ATA_DEV_HD, ATA_LBA |
>  				 ATA_DEVICE(device) | ((blknr >> 24) & 0xF));
>  			ide_outb(device, ATA_COMMAND, ATA_CMD_PIO_WRITE);
> diff --git a/include/blk.h b/include/blk.h
> index 871922dcde07..2c9c7985a885 100644
> --- a/include/blk.h
> +++ b/include/blk.h
> @@ -62,10 +62,8 @@ struct blk_desc {
>  	unsigned char	hwpart;		/* HW partition, e.g. for eMMC */
>  	unsigned char	type;		/* device type */
>  	unsigned char	removable;	/* removable device */
> -#ifdef CONFIG_LBA48
>  	/* device can use 48bit addr (ATA/ATAPI v7) */
> -	unsigned char	lba48;
> -#endif
> +	bool	lba48;

nitpick Is there a reason for having dropped this comment?
/* device can use 48bit addr (ATA/ATAPI v7) */

In any case:

Reviewed-by: Mattijs Korpershoek <mkorpershoek at baylibre.com>

>  	unsigned char	atapi;		/* Use ATAPI protocol */
>  	lbaint_t	lba;		/* number of blocks */
>  	unsigned long	blksz;		/* block size */
> -- 
> 2.40.0.348.gf938b09366-goog


More information about the U-Boot mailing list