linux-kernel - Re: [PATCH 2/2] mtd: core: Don't fail mtd_device_parse

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <CZQZXAB2GOCY.12YVJ6CRGG26B@kernel.org>
Date: Mon, 11 Mar 2024 15:38:17 +0100
From: "Michael Walle" <mwalle@...nel.org>
To: "Aapo Vienamo" <aapo.vienamo@...ux.intel.com>, "Miquel Raynal"
 <miquel.raynal@...tlin.com>, "Richard Weinberger" <richard@....at>,
 "Vignesh Raghavendra" <vigneshr@...com>, <linux-mtd@...ts.infradead.org>,
 <linux-kernel@...r.kernel.org>
Cc: "Mika Westerberg" <mika.westerberg@...ux.intel.com>
Subject: Re: [PATCH 2/2] mtd: core: Don't fail mtd_device_parse_register()
 if OTP is unsupported

On Thu Mar 7, 2024 at 2:04 PM CET, Aapo Vienamo wrote:
> Handle the case where -EOPNOTSUPP is returned from OTP driver.
>
> This addresses an issue that occurs with the Intel SPI flash controller,
> which has a limited supported opcode set. Whilst the OTP functionality
> is not available due to this restriction, other parts of the MTD
> functionality of the device are intact. This change allows the driver
> to gracefully handle the restriction by allowing the supported
> functionality to remain available instead of failing the probe
> altogether.
>
> Signed-off-by: Aapo Vienamo <aapo.vienamo@...ux.intel.com>
> Reviewed-by: Mika Westerberg <mika.westerberg@...ux.intel.com>
>
> ---
>  drivers/mtd/mtdcore.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
> index c365c97e7232..1cfc8bb5187d 100644
> --- a/drivers/mtd/mtdcore.c
> +++ b/drivers/mtd/mtdcore.c
> @@ -1054,8 +1054,14 @@ int mtd_device_parse_register(struct mtd_info *mtd, const char * const *types,
>  
>  	mtd_set_dev_defaults(mtd);
>  
> +	/*
> +	 * Don't abort MTD init if OTP functionality is unsupported. The
> +	 * cleanup of the OTP init is contained within mtd_otp_nvmem_add().
> +	 * Omitting goto out here is safe since the cleanup code there
> +	 * should be no-ops.
> +	 */

Only if that's true for both the factory and user OTP area.
Also, you'll print an error message for EOPNOTSUPP, although that is
not really an error. Is that intended? 

>  	ret = mtd_otp_nvmem_add(mtd);
> -	if (ret)
> +	if (ret && ret != -EOPNOTSUPP)

Maybe there is a better way to handle this, like controller
capabilities instead of putting these EOPNOTSUPP checks
everywhere? I'm not sure.

-michael
>  		goto out;
>  
>  	if (IS_ENABLED(CONFIG_MTD_PARTITIONED_MASTER)) {


Download attachment "signature.asc" of type "application/pgp-signature" (253 bytes)