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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.