Messages in this thread | ![/](/images/icornerl.gif) | | Date | Mon, 11 Mar 2024 18:20:52 +0200 | From | Aapo Vienamo <> | Subject | Re: [PATCH 2/2] mtd: core: Don't fail mtd_device_parse_register() if OTP is unsupported |
| |
Hi Michael
On Mon, Mar 11, 2024 at 03:38:17PM +0100, Michael Walle wrote: > On Thu Mar 7, 2024 at 2:04 PM CET, Aapo Vienamo wrote: > > Handle the case where -EOPNOTSUPP is returned from OTP driver. > > + /* > > + * 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.
I'm not sure I follow. I'm not seeing a path in mtd_otp_nvmem_add() that would result in either mtd->otp_user_nvmem or mtd->otp_factor_nvmem needing to be cleaned up by the caller, if an error is returned, if that's what you are referring to.
> Also, you'll print an error message for EOPNOTSUPP, although that is > not really an error. Is that intended?
Well, when we hit this, the functionality of the SPI memory itself is degraded in the sense that the OTP functionality is not available. What would you suggest?
> > > 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.
Trying to come up with clear semantics for a capabilities flag to solve this is difficult. The issue is that on the SPI controller side, the limitation stems from the really strict set of opcodes that are allowed. For example, we already hit an error with the 0x35 (read configuration register) not being on the set of allowed opcodes. While this instruction is used by the OTP code, it's not a strictly OTP specific operation.
If there was a flag that would signal OTP support, I think it would have be defined as "the controller supports all operations that are performed by the OTP code", which sounds brittle. The other way around would be to have a really fine-grained set of flags that the MTD core would check, but that feels tedious and error prone as well.
Best, Aapo
| ![\](/images/icornerr.gif) |