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 79A79EB64D9 for ; Thu, 6 Jul 2023 17:52:24 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id A6D2B86139; Thu, 6 Jul 2023 19:52:22 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=konsulko.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (1024-bit key; unprotected) header.d=konsulko.com header.i=@konsulko.com header.b="YbvB2czW"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 2637286313; Thu, 6 Jul 2023 19:52:21 +0200 (CEST) Received: from mail-yb1-xb2c.google.com (mail-yb1-xb2c.google.com [IPv6:2607:f8b0:4864:20::b2c]) (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 541A381FA8 for ; Thu, 6 Jul 2023 19:52:18 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=konsulko.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=trini@konsulko.com Received: by mail-yb1-xb2c.google.com with SMTP id 3f1490d57ef6-c11e2b31b95so1067527276.3 for ; Thu, 06 Jul 2023 10:52:18 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=konsulko.com; s=google; t=1688665937; x=1691257937; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=A5yXlpVUM94h69kvCvcwAvXOj0eaMwZSs7nxI3tTG5U=; b=YbvB2czWAxGYwflOcWYFGCSn+wRJPJQdcyCbH9LbYigk99yFctoNXO4uscBzZLenuI zW/MveGUVg9Q9RFRfJCZPBsFoMisTafsy/i0J1och86KPIAznzOXlMsfxt9uZWLAxatm gAlGNX7PQwW5nKuFJlTieX06jf5f7xA0eMtwk= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1688665937; x=1691257937; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=A5yXlpVUM94h69kvCvcwAvXOj0eaMwZSs7nxI3tTG5U=; b=SAhOceQmT+STevJ5J+r2kB28/nWs0Ptq7NJclcxvWF0FStFoiEo8lps9uRW5+Ytt5l /PEJH74TY9j5cSphCdGLw9R8wDYhogXrD2oedawBk5+4oRRVtvlUqcL2ssk5OzIa7Lfn BWmeQvOVmmfCMpONkQ2kqGZOkZXxLfMdC5Gj+04QgXyl6x/8scPbAlue2IjkKOW6afrq hNObHvkl4G7SKlCHrzRzSyz0IIcD4yIgQcFOinm84m+Fu0IIp3O8qXyT6eXVWpWyczA6 +NCBkrelTnSnJa+gNnxoK/XA0h5ca1rtWWhQIe4HT0nZZeS3TztMXPuE8De4t4H1JDLU R0/g== X-Gm-Message-State: ABy/qLYwLwET25JS/wyl+p4eGbAX11QS8yP4tY/jXt6/efUgyL4ha3H+ BSv1ao/g3M7dog77B5O5qs5tIf7o9/26kt9qcPELtA== X-Google-Smtp-Source: APBJJlHNRV87xoC0HXBv+NBJhcuqe5gtCglQl8qxD1P0iju9pUDqv6/FeRaAGkmrdVNzMfoOFkOgCQ== X-Received: by 2002:a25:348a:0:b0:c41:51cf:f4ed with SMTP id b132-20020a25348a000000b00c4151cff4edmr2096210yba.32.1688665937053; Thu, 06 Jul 2023 10:52:17 -0700 (PDT) Received: from bill-the-cat (2603-6081-7b00-6400-8731-f9e3-ab03-9278.res6.spectrum.com. [2603:6081:7b00:6400:8731:f9e3:ab03:9278]) by smtp.gmail.com with ESMTPSA id g1-20020a25a481000000b00ba773472647sm463481ybi.19.2023.07.06.10.52.16 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 06 Jul 2023 10:52:16 -0700 (PDT) Date: Thu, 6 Jul 2023 13:52:14 -0400 From: Tom Rini To: Pali =?iso-8859-1?Q?Roh=E1r?= Cc: Jaehoon Chung , u-boot@lists.denx.de Subject: Re: [PATCH v2 u-boot] mmc: spl: Make partition choice in default_spl_mmc_emmc_boot_partition() more explicit Message-ID: <20230706175214.GC7930@bill-the-cat> References: <20230413211057.10975-2-pali@kernel.org> <20230706173502.2796-1-pali@kernel.org> <20230706174218.GB7930@bill-the-cat> <20230706174918.iupb2gatj3s7w7jd@pali> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="MFjMC4IsyJPxKUHs" Content-Disposition: inline In-Reply-To: <20230706174918.iupb2gatj3s7w7jd@pali> X-Clacks-Overhead: GNU Terry Pratchett 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 --MFjMC4IsyJPxKUHs Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Jul 06, 2023 at 07:49:18PM +0200, Pali Roh=E1r wrote: > On Thursday 06 July 2023 13:42:18 Tom Rini wrote: > > On Thu, Jul 06, 2023 at 07:35:02PM +0200, Pali Roh=E1r wrote: > > > To make eMMC partition choosing in default_spl_mmc_emmc_boot_partitio= n() > > > function better understandable, rewrite it via explicit switch-case c= ode > > > pattern. > > >=20 > > > Also add a warning when eMMC EXT_CSD[179] register is configured by u= ser to > > > value which is not suitable for eMMC booting and SPL do not know how = to > > > interpret it. > > >=20 > > > Note that when booting from eMMC device via EXT_CSD[179] register is > > > explicitly disabled then SPL still loads and boots from this eMMC dev= ice > > > from User Area partition. This behavior was not changed in this commi= t and > > > should be revisited in the future. > > >=20 > > > Signed-off-by: Pali Roh=E1r > > > --- > > > Changes in v2: > > > * Disable showing warning on sama5d2_xplained due to size restrictions > > > --- > > > This patch depends on another patch: > > > mmc: spl: Add comments for default_spl_mmc_emmc_boot_partition() > > > https://patchwork.ozlabs.org/project/uboot/patch/20230404202805.8523-= 1-pali@kernel.org/ > > > --- > > > common/spl/Kconfig | 7 +++++++ > > > common/spl/spl_mmc.c | 46 ++++++++++++++++++++++++++++++++++++------= -- > > > 2 files changed, 45 insertions(+), 8 deletions(-) > > >=20 > > > diff --git a/common/spl/Kconfig b/common/spl/Kconfig > > > index 865571d4579c..0574d22b3b25 100644 > > > --- a/common/spl/Kconfig > > > +++ b/common/spl/Kconfig > > > @@ -855,6 +855,13 @@ config SPL_MMC_WRITE > > > help > > > Enable write access to MMC and SD Cards in SPL > > > =20 > > > +config SPL_MMC_WARNINGS > > > + bool "Print MMC warnings" > > > + depends on SPL_MMC > > > + default y if !TARGET_SAMA5D2_XPLAINED > > > + help > > > + Print SPL MMC warnings. You can disable this option to reduce SPL= size. > > > + > > > =20 > > > config SPL_MPC8XXX_INIT_DDR > > > bool "Support MPC8XXX DDR init" > > > diff --git a/common/spl/spl_mmc.c b/common/spl/spl_mmc.c > > > index f7a42a11477d..ec424ceded0e 100644 > > > --- a/common/spl/spl_mmc.c > > > +++ b/common/spl/spl_mmc.c > > > @@ -408,15 +408,45 @@ int default_spl_mmc_emmc_boot_partition(struct = mmc *mmc) > > > * > > > * Note: See difference between EXT_CSD_EXTRACT_PARTITION_ACCESS > > > * and EXT_CSD_EXTRACT_BOOT_PART, specially about User area value. > > > - * > > > - * FIXME: When booting from this eMMC device is explicitly > > > - * disabled then we use User area for booting. This is incorrect. > > > - * Probably we should skip this eMMC device and select the next > > > - * one for booting. Or at least throw warning about this fallback. > > > */ > > > - part =3D EXT_CSD_EXTRACT_BOOT_PART(mmc->part_config); > > > - if (part =3D=3D 7) > > > - part =3D 0; > > > + if (mmc->part_config =3D=3D MMCPART_NOAVAILABLE) > > > + part =3D 0; /* If partitions are not supported then we have only U= ser Area partition */ > > > + else { > > > + switch(EXT_CSD_EXTRACT_BOOT_PART(mmc->part_config)) { > > > + case 0: /* Booting from this eMMC device is disabled */ > > > +#ifdef CONFIG_SPL_LIBCOMMON_SUPPORT > > > +#ifdef CONFIG_SPL_MMC_WARNINGS > > > + puts("spl: WARNING: Booting from this eMMC device is disabled in = EXT_CSD[179] register\n"); > > > + puts("spl: WARNING: Continuing anyway and selecting User Area par= tition for booting\n"); > > > +#else > > > + puts("spl: mmc: fallback to user area\n"); > > > +#endif > > > +#endif > > > + /* FIXME: This is incorrect and probably we should select next eM= MC device for booting */ > > > + part =3D 0; > > > + break; > > > + case 1: /* Boot partition 1 is used for booting */ > > > + part =3D 1; > > > + break; > > > + case 2: /* Boot partition 2 is used for booting */ > > > + part =3D 2; > > > + break; > > > + case 7: /* User area is used for booting */ > > > + part =3D 0; > > > + break; > > > + default: /* Other values are reserved */ > > > +#ifdef CONFIG_SPL_LIBCOMMON_SUPPORT > > > +#ifdef CONFIG_SPL_MMC_WARNINGS > > > + puts("spl: WARNING: EXT_CSD[179] register is configured to boot f= rom Reserved value\n"); > > > + puts("spl: WARNING: Selecting User Area partition for booting\n"); > > > +#else > > > + puts("spl: mmc: fallback to user area\n"); > > > +#endif > > > +#endif > > > + part =3D 0; > > > + break; > > > + } > > > + } > > > #endif > >=20 > > Please just use debug() for these messages. >=20 > All other error/warning messages in this file are printed via puts(). > So I'm just following the current style (and I'm really not going to > change all occurrences in this patch). Except for the messages in that file which use debug() they use puts(), yes. Since none of these are fatal messages (you're falling through) please switch them to debug() rather than introduce a new CONFIG symbol, so that if someone is bringing up a platform where this is a problem they'll be able to debug it, but the general case does not increase the binary size of most platforms. I'm not asking you to change anything existing in the file, only what you're adding. --=20 Tom --MFjMC4IsyJPxKUHs Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQGzBAABCgAdFiEEGjx/cOCPqxcHgJu/FHw5/5Y0tywFAmSm/04ACgkQFHw5/5Y0 tyxwRwwAhOiGOtiqrY5V7FwVih2pauTzjKXdZTS0h05cfgF7CZxVQoxpQ+AQZ+pN rqKz53wla/qKFvWGCyt2o46cbbENnKmDzrqnOz6fc90kGQ+4gcq1Jz5J1gqobMcJ DwKhXSlV6ZkKzNwD6uPqoO/SStv4FVgKKZxL0Ljg1m5tQrFFU3cNU8wSclcSBxb1 DmPRWx/kiockUsrwyDLd0fe11c+KFsBvrNIDxXSTsdBBsAW0Qb+FdrjnaV4JlN9g gUiaS2eo3J8+kuZcNzkNoKq0GQTxxP5whcXle/cCE8BjI6Y3OpEnFDPJp3q7ko4U 3XNHCFfNwdY18wTSvL+p1W0AsQ68CQq+KWObbheK6O22Ku8jjjEcehnnmSevrbE1 QRSQfCaNL5xAKKkmJn7Tm8H4/HokQVgwV2oMnhbbwjGTFcrWuz/NcPIXAlrjsNQN Mp/1janBmCm7IOdFN3O6UWOnnWYiLWZF5j106gj4tFe0r9S5PAEf4SnfU7qIsIsS VcJMZMlo =yPdj -----END PGP SIGNATURE----- --MFjMC4IsyJPxKUHs--