Messages in this thread | | | Date | Wed, 08 Mar 2023 19:12:32 +0100 | From | Rafał Miłecki <> | Subject | Re: [PATCH V2] nvmem: add explicit config option to read OF fixed cells |
| |
On 2023-03-08 19:06, Miquel Raynal wrote: > Hi Rafał, > > rafal@milecki.pl wrote on Wed, 08 Mar 2023 17:55:46 +0100: > >> On 2023-03-08 17:34, Miquel Raynal wrote: >> > Hi Rafał, >> > >> > zajec5@gmail.com wrote on Fri, 24 Feb 2023 08:29:03 +0100: >> > >> >> From: Rafał Miłecki <rafal@milecki.pl> >> >> >> NVMEM subsystem looks for fixed NVMEM cells (specified in DT) by >> >> default. This behaviour made sense in early days before adding support >> >> for dynamic cells. >> >> >> With every new supported NVMEM device with dynamic cells current >> >> behaviour becomes non-optimal. It results in unneeded iterating over >> DT >> >> nodes and may result in false discovery of cells (depending on used DT >> >> properties). >> >> >> This behaviour has actually caused a problem already with the MTD >> >> subsystem. MTD subpartitions were incorrectly treated as NVMEM cells. >> > >> > That's true, but I expect this to be really MTD specific. >> > >> > A concrete proposal below. >> > >> >> Also with upcoming support for NVMEM layouts no new binding or driver >> >> should support fixed cells defined in device node. >> > >> > I'm not sure I agree with this statement. We are not preventing new >> > binding/driver to use fixed cells, or...? We offer a new way to expose >> > nvmem cells with another way than "fixed-offset" and "fixed-size" OF >> > nodes. >> >> From what I understood all new NVMEM bindings should have cells >> defined >> in the nvmem-layout { } node. That's what I mean by saying they should >> not be defined in device node (but its "nvmem-layout" instead). > > Layouts are just another possibility, either you user the nvmem-cells > compatible and produce nvmem cells with fixed OF nodes, or you use the > nvmem-layout container. I don't think all new bindings should have > cells in layouts. It depends if the content is static or not. > >> >> Solve this by modifying drivers for bindings that support specifying >> >> fixed NVMEM cells in DT. Make them explicitly tell NVMEM subsystem to >> >> read cells from DT. >> >> >> It wasn't clear (to me) if rtc and w1 code actually uses fixed cells. >> I >> >> enabled them to don't risk any breakage. >> >> >> Signed-off-by: Rafał Miłecki <rafal@milecki.pl> >> >> [for drivers/nvmem/meson-{efuse,mx-efuse}.c] >> >> Acked-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com> >> >> --- >> >> V2: Fix stm32-romem.c typo breaking its compilation >> >> Pick Martin's Acked-by >> >> Add paragraph about layouts deprecating use_fixed_of_cells >> >> --- >> >> drivers/mtd/mtdcore.c | 2 ++ >> >> drivers/nvmem/apple-efuses.c | 1 + >> >> drivers/nvmem/core.c | 8 +++++--- >> >> drivers/nvmem/imx-ocotp-scu.c | 1 + >> >> drivers/nvmem/imx-ocotp.c | 1 + >> >> drivers/nvmem/meson-efuse.c | 1 + >> >> drivers/nvmem/meson-mx-efuse.c | 1 + >> >> drivers/nvmem/microchip-otpc.c | 1 + >> >> drivers/nvmem/mtk-efuse.c | 1 + >> >> drivers/nvmem/qcom-spmi-sdam.c | 1 + >> >> drivers/nvmem/qfprom.c | 1 + >> >> drivers/nvmem/rave-sp-eeprom.c | 1 + >> >> drivers/nvmem/rockchip-efuse.c | 1 + >> >> drivers/nvmem/sc27xx-efuse.c | 1 + >> >> drivers/nvmem/sprd-efuse.c | 1 + >> >> drivers/nvmem/stm32-romem.c | 1 + >> >> drivers/nvmem/sunplus-ocotp.c | 1 + >> >> drivers/nvmem/sunxi_sid.c | 1 + >> >> drivers/nvmem/uniphier-efuse.c | 1 + >> >> drivers/nvmem/zynqmp_nvmem.c | 1 + >> >> drivers/rtc/nvmem.c | 1 + >> >> drivers/w1/slaves/w1_ds250x.c | 1 + >> >> include/linux/nvmem-provider.h | 2 ++ >> >> 23 files changed, 29 insertions(+), 3 deletions(-) >> >> >> diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c >> >> index 0feacb9fbdac..1bb479c0f758 100644 >> >> --- a/drivers/mtd/mtdcore.c >> >> +++ b/drivers/mtd/mtdcore.c >> >> @@ -523,6 +523,7 @@ static int mtd_nvmem_add(struct mtd_info *mtd) >> >> config.dev = &mtd->dev; >> >> config.name = dev_name(&mtd->dev); >> >> config.owner = THIS_MODULE; >> >> + config.use_fixed_of_cells = of_device_is_compatible(node, >> "nvmem-cells"); >> > >> > I am wondering how mtd specific this is? For me all OF nodes containing >> > the nvmem-cells compatible should be treated as cells providers and >> > populate nvmem cells as for each children. >> > >> > Why don't we just check for this compatible to be present? in >> > nvmem_add_cells_from_of() ? And if not we just skip the operation. >> > >> > This way we still follow the bindings (even though using nvmem-cells in >> > the compatible property to require cells population was a mistake in >> > the first place, as discussed in the devlink thread recently) but there >> > is no need for a per-driver config option? >> >> This isn't mtd specific. Please check this patch for all occurrences >> of >> the: >> use_fixed_of_cells = true >> >> The very first one: drivers/nvmem/apple-efuses.c driver for the >> "apple,efuses" binding. That binding supports fixed OF cells, see: >> Documentation/devicetree/bindings/nvmem/apple,efuses.yaml > > I'm saying: based on what has been enforced so far, I would expect all > fixed cell providers to come with nvmem-cells as compatible, no? > > If that's the case we could use that as a common denominator?
Sorry, I don't get it. Have you checked Documentation/devicetree/bindings/nvmem/apple,efuses.yaml ?
It's a NVMEM provied binding with fixed cells that doesn't use nvmem-cells as compatible. There are many more.
| |