From: Mattijs Korpershoek <mkorpershoek@baylibre.com>
To: Simon Glass <sjg@chromium.org>,
U-Boot Mailing List <u-boot@lists.denx.de>
Cc: Bin Meng <bmeng.cn@gmail.com>, Simon Glass <sjg@chromium.org>,
Heinrich Schuchardt <xypron.glpk@gmx.de>
Subject: Re: [PATCH 16/30] ide: Avoid preprocessor for CONFIG_LBA48
Date: Tue, 28 Mar 2023 16:18:54 +0200 [thread overview]
Message-ID: <87mt3x0xip.fsf@baylibre.com> (raw)
In-Reply-To: <20230328080702.16.Ic19d3ab1094a129854187823d74d6bc6a7004944@changeid>
On mar., mars 28, 2023 at 08:07, Simon Glass <sjg@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@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@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
next prev parent reply other threads:[~2023-03-28 14:19 UTC|newest]
Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-03-27 19:06 [PATCH 00/30] ide: Clean up code and fix a few bugs Simon Glass
2023-03-27 19:06 ` [PATCH 01/30] ide: Move ATA_CURR_BASE to C file Simon Glass
2023-03-27 19:06 ` [PATCH 02/30] ide: Use mdelay() for long delays Simon Glass
2023-03-27 19:06 ` [PATCH 03/30] ide: Drop CONFIG_START_IDE Simon Glass
2023-03-27 19:06 ` [PATCH 04/30] ide: Drop init for not using BLK Simon Glass
2023-03-27 19:06 ` [PATCH 05/30] ide: Move ide_init() into probing Simon Glass
2023-03-27 19:06 ` [PATCH 06/30] ide: Drop ide_device_present() Simon Glass
2023-03-27 19:06 ` [PATCH 08/30] ide: Drop weak functions Simon Glass
2023-03-27 19:06 ` [PATCH 09/30] ide: Create a prototype for ide_set_reset() Simon Glass
2023-03-27 19:06 ` [PATCH 10/30] ide: Correct use of ATAPI Simon Glass
2023-03-28 14:23 ` Mattijs Korpershoek
2023-03-27 19:06 ` [PATCH 11/30] ide: Make function static Simon Glass
2023-03-27 19:06 ` [PATCH 12/30] ide: Change the retries variable Simon Glass
2023-03-27 19:07 ` [PATCH 13/30] ide: Refactor confusing loop code Simon Glass
2023-03-27 19:07 ` [PATCH 14/30] ide: Simplify success condition Simon Glass
2023-03-27 19:07 ` [PATCH 15/30] ide: Avoid preprocessor for CONFIG_ATAPI Simon Glass
2023-03-27 19:07 ` [PATCH 16/30] ide: Avoid preprocessor for CONFIG_LBA48 Simon Glass
2023-03-28 14:18 ` Mattijs Korpershoek [this message]
2023-04-19 1:46 ` Simon Glass
2023-03-27 19:07 ` [PATCH 17/30] ide: Move bus init into a function Simon Glass
2023-03-27 19:07 ` [PATCH 18/30] ide: Make ide_bus_ok[] a local variable Simon Glass
2023-03-27 19:07 ` [PATCH 19/30] ide: Move setting of vendor strings into ide_probe() Simon Glass
2023-03-27 19:07 ` [PATCH 20/30] ide: Move ide_init() entirely within ide_probe() Simon Glass
2023-03-27 19:07 ` [PATCH 21/30] ide: Combine the two loops in ide_probe() Simon Glass
2023-03-27 19:07 ` [PATCH 22/30] ide: Use desc consistently for struct blk_desc Simon Glass
2023-03-27 19:07 ` [PATCH 23/30] ide: Make ide_ident() return an error code Simon Glass
2023-03-27 19:07 ` [PATCH 24/30] ide: Move all blk_desc init into ide_ident() Simon Glass
2023-03-27 19:07 ` [PATCH 25/30] ide: Use a single local blk_desc for ide_ident() Simon Glass
2023-03-27 19:07 ` [PATCH 26/30] ide: Correct LBA setting Simon Glass
2023-03-27 19:07 ` [PATCH 27/30] ide: Tidy up ide_reset() Simon Glass
2023-03-27 19:07 ` [PATCH 28/30] ide: Convert to use log_debug() Simon Glass
2023-03-27 19:07 ` [PATCH 29/30] ide: Simplify expressions and hex values Simon Glass
2023-03-27 19:07 ` [PATCH 30/30] ide: Make use of U-Boot types Simon Glass
2023-04-25 15:36 ` [PATCH 00/30] ide: Clean up code and fix a few bugs Tom Rini
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=87mt3x0xip.fsf@baylibre.com \
--to=mkorpershoek@baylibre.com \
--cc=bmeng.cn@gmail.com \
--cc=sjg@chromium.org \
--cc=u-boot@lists.denx.de \
--cc=xypron.glpk@gmx.de \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox