Re: [PATCH 2/2] mtd: core: Don't fail mtd_device_parse_register() if OTP is unsupported

From: "Michael Walle" <mwalle@kernel.org>
To: "Aapo Vienamo" <aapo.vienamo@linux.intel.com>,
	"Miquel Raynal" <miquel.raynal@bootlin.com>,
	"Richard Weinberger" <richard@nod.at>,
	"Vignesh Raghavendra" <vigneshr@ti.com>,
	<linux-mtd@lists.infradead.org>, <linux-kernel@vger.kernel.org>
Cc: "Mika Westerberg" <mika.westerberg@linux.intel.com>
Subject: Re: [PATCH 2/2] mtd: core: Don't fail mtd_device_parse_register() if OTP is unsupported
Date: Mon, 11 Mar 2024 15:38:17 +0100	[thread overview]
Message-ID: <CZQZXAB2GOCY.12YVJ6CRGG26B@kernel.org> (raw)
In-Reply-To: <20240307130418.3131898-3-aapo.vienamo@linux.intel.com>

[-- Attachment #1.1: Type: text/plain, Size: 1862 bytes --]

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@linux.intel.com>
> Reviewed-by: Mika Westerberg <mika.westerberg@linux.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)) {

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 252 bytes --]

[-- Attachment #2: Type: text/plain, Size: 144 bytes --]

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/