From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id A7978C76196 for ; Tue, 28 Mar 2023 14:19:06 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 1856E85BB7; Tue, 28 Mar 2023 16:19:03 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=baylibre.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (2048-bit key; unprotected) header.d=baylibre-com.20210112.gappssmtp.com header.i=@baylibre-com.20210112.gappssmtp.com header.b="4hjtjQiB"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id DB76C85C6D; Tue, 28 Mar 2023 16:18:59 +0200 (CEST) Received: from mail-wm1-x336.google.com (mail-wm1-x336.google.com [IPv6:2a00:1450:4864:20::336]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id A3E0485BB7 for ; Tue, 28 Mar 2023 16:18:56 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=baylibre.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=mkorpershoek@baylibre.com Received: by mail-wm1-x336.google.com with SMTP id n19so7075788wms.0 for ; Tue, 28 Mar 2023 07:18:56 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=baylibre-com.20210112.gappssmtp.com; s=20210112; t=1680013136; h=mime-version:message-id:date:references:in-reply-to:subject:cc:to :from:from:to:cc:subject:date:message-id:reply-to; bh=aGqFb9HxQPkFA8j665Y3UwHgzx2Hwjsuqx2bU2vdCfI=; b=4hjtjQiBd3nxJUJFaxgBceIv2+o2ii+LqC8qWLO1cLe4JUtBM2neP2t+y0fc2tm0B8 tDMdw9GALbcwBdREYBR4r9xaiYteMgq7E+6IFiJHU6lko3hskpv+WLg4cNbBqx5kHlWZ ST5e4Z5sFPFYnCUjhy5hVozg93oVTD0hEUvPuKUTWKFPWZib4NCPJUjZrD5HVVcV2LWA vBM5czOkCs8qvIfOX357bBxptYSULo0OJtgrbGR7+oYdsx/AsK5y/Ugt2miYhAq/7+Mg QbAdcWXH/ERh4GJExPs+BUOiQIjpQkZpy9xKQY6cXgW3Xa+74HJxno9J7Jim+90E1Lui KE6A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1680013136; h=mime-version:message-id:date:references:in-reply-to:subject:cc:to :from:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=aGqFb9HxQPkFA8j665Y3UwHgzx2Hwjsuqx2bU2vdCfI=; b=2XnQv35Qydi+XNGiISllJjlxFoGUUPGexxQlrNNR3i0hPoo8wJuGJAM3BOADUTDon+ sDZHDzqg6Iol4ZpzNVdcuJWmwrFBSFdRzNizp+nMo92OqYX2TwKM7qCUUsWMoUoVMstU RPpi7FqR9dkTt3Sp99Gjc/cZBvyQ0wyYN2zOhldNN8MaZb5wxyZrzDA2eMdkEedOJShd EpJzdQGBMhTEs4UVAyRSBZRbFg+vLCyFuwGJQ5k0CXsUpEG/972Ano2aYu+4O/ZWFnuR HW5wWyGsc3jFYjSt3clQg1KUt7MHF+ZK+ZQXPCXugwQZAQn+gNkCzEGCNzbVP7rguR3d iMsw== X-Gm-Message-State: AO0yUKVpPLJdOZtXR7GgE8dqAWnyjcgibmJ0rJeJYTIpbrPpK/Kht7rA 5LyoDJwlr0jaVPqQGFzdEquv7Q== X-Google-Smtp-Source: AK7set87AhfsExWetuggqJA+gnzCpd+So3YUzvgQK746V4SWtkEeFHTIcA0RE3s3/smBPEFDWo4y4w== X-Received: by 2002:a7b:c850:0:b0:3ed:29db:cb80 with SMTP id c16-20020a7bc850000000b003ed29dbcb80mr12118631wml.18.1680013136027; Tue, 28 Mar 2023 07:18:56 -0700 (PDT) Received: from localhost ([82.66.159.240]) by smtp.gmail.com with ESMTPSA id hg26-20020a05600c539a00b003ef5b285f65sm12317777wmb.46.2023.03.28.07.18.55 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 28 Mar 2023 07:18:55 -0700 (PDT) From: Mattijs Korpershoek To: Simon Glass , U-Boot Mailing List Cc: Bin Meng , Simon Glass , Heinrich Schuchardt Subject: Re: [PATCH 16/30] ide: Avoid preprocessor for CONFIG_LBA48 In-Reply-To: <20230328080702.16.Ic19d3ab1094a129854187823d74d6bc6a7004944@changeid> References: <20230327190717.1796230-1-sjg@chromium.org> <20230328080702.16.Ic19d3ab1094a129854187823d74d6bc6a7004944@changeid> Date: Tue, 28 Mar 2023 16:18:54 +0200 Message-ID: <87mt3x0xip.fsf@baylibre.com> MIME-Version: 1.0 Content-Type: text/plain X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.39 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.8 at phobos.denx.de X-Virus-Status: Clean On mar., mars 28, 2023 at 08:07, Simon Glass 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 > --- > > 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 > unsigned char atapi; /* Use ATAPI protocol */ > lbaint_t lba; /* number of blocks */ > unsigned long blksz; /* block size */ > -- > 2.40.0.348.gf938b09366-goog