[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <6396921.lOV4Wx5bFT@steina-w>
Date: Wed, 05 Feb 2025 07:51:23 +0100
From: Alexander Stein <alexander.stein@...tq-group.com>
To: Frank Li <Frank.li@....com>
Cc: Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, Shawn Guo <shawnguo@...nel.org>, Sascha Hauer <s.hauer@...gutronix.de>, Pengutronix Kernel Team <kernel@...gutronix.de>, Fabio Estevam <festevam@...il.com>, Srinivas Kandagatla <srinivas.kandagatla@...aro.org>, devicetree@...r.kernel.org, imx@...ts.linux.dev, linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 3/4] nvmem: imx-ocotp: Support accessing controller for i.MX8M Nano
Am Freitag, 31. Januar 2025, 17:06:23 CET schrieb Frank Li:
> On Fri, Jan 31, 2025 at 02:54:06PM +0100, Alexander Stein wrote:
> > Hi,
> >
> > Am Donnerstag, 30. Januar 2025, 17:42:32 CET schrieb Frank Li:
> > > On Thu, Jan 30, 2025 at 02:01:00PM +0100, Alexander Stein wrote:
> > > > i.MX8M OCOTP supports a specific peripheral or function being fused
> > > > which means disabled, so
> > > > - Introduce disable_fuse for a list of possible fused peripherals.
> > > > - Iterate all nodes to check accessing permission. If not
> > > > allowed to be accessed, detach the node
> > > >
> > > > Signed-off-by: Alexander Stein <alexander.stein@...tq-group.com>
> > > > ---
> > > > drivers/nvmem/Kconfig | 3 ++
> > > > drivers/nvmem/imx-ocotp.c | 105 +++++++++++++++++++++++++++++++++++++-
> > > > 2 files changed, 107 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig
> > > > index 8671b7c974b93..ba5c928cab520 100644
> > > > --- a/drivers/nvmem/Kconfig
> > > > +++ b/drivers/nvmem/Kconfig
> > > > @@ -84,6 +84,9 @@ config NVMEM_IMX_OCOTP
> > > > This driver can also be built as a module. If so, the module
> > > > will be called nvmem-imx-ocotp.
> > > >
> > > > + If built as modules, any other driver relying on this working
> > > > + as access controller also needs to be a module as well.
> > > > +
> > > > config NVMEM_IMX_OCOTP_ELE
> > > > tristate "i.MX On-Chip OTP Controller support"
> > > > depends on ARCH_MXC || COMPILE_TEST
> > > > diff --git a/drivers/nvmem/imx-ocotp.c b/drivers/nvmem/imx-ocotp.c
> > > > index c5086a16450ac..e3ea026a37d0d 100644
> > > > --- a/drivers/nvmem/imx-ocotp.c
> > > > +++ b/drivers/nvmem/imx-ocotp.c
> > > > @@ -23,6 +23,7 @@
> > > > #include <linux/of.h>
> > > > #include <linux/platform_device.h>
> > > > #include <linux/slab.h>
> > > > +#include <dt-bindings/nvmem/fsl,imx8mn-ocotp.h>
> > > >
> > > > #define IMX_OCOTP_OFFSET_B0W0 0x400 /* Offset from base address of the
> > > > * OTP Bank0 Word0
> > > > @@ -91,11 +92,20 @@ struct ocotp_ctrl_reg {
> > > > u32 bm_rel_shadows;
> > > > };
> > > >
> > > > +#define OCOTP_MAX_NUM_GATE_WORDS 4
> > > > +
> > > > +struct disable_fuse {
> > > > + u32 fuse_addr;
> > > > + u32 mask;
> > > > +};
> > > > +
> > > > struct ocotp_params {
> > > > unsigned int nregs;
> > > > unsigned int bank_address_words;
> > > > void (*set_timing)(struct ocotp_priv *priv);
> > > > struct ocotp_ctrl_reg ctrl;
> > > > + u32 num_disables;
> > > > + struct disable_fuse *disables;
> > > > };
> > > >
> > > > static int imx_ocotp_wait_for_busy(struct ocotp_priv *priv, u32 flags)
> > > > @@ -552,11 +562,25 @@ static const struct ocotp_params imx8mm_params = {
> > > > .ctrl = IMX_OCOTP_BM_CTRL_DEFAULT,
> > > > };
> > > >
> > > > +struct disable_fuse imx8mn_disable_fuse[] = {
> > > > + [IMX8MN_OCOTP_M7_DISABLE] = { .fuse_addr = 20, .mask = BIT(8) },
> > > > + [IMX8MN_OCOTP_M7_MPU_DISABLE] = { .fuse_addr = 20, .mask = BIT(9) },
> > > > + [IMX8MN_OCOTP_M7_FPU_DISABLE] = { .fuse_addr = 20, .mask = BIT(10) },
> > > > + [IMX8MN_OCOTP_USB_OTG1_DISABLE] = { .fuse_addr = 20, .mask = BIT(11) },
> > > > + [IMX8MN_OCOTP_GPU3D_DISABLE] = { .fuse_addr = 20, .mask = BIT(24) },
> > > > + [IMX8MN_OCOTP_MIPI_DSI_DISABLE] = { .fuse_addr = 20, .mask = BIT(28) },
> > > > + [IMX8MN_OCOTP_ENET_DISABLE] = { .fuse_addr = 20, .mask = BIT(29) },
> > > > + [IMX8MN_OCOTP_MIPI_CSI_DISABLE] = { .fuse_addr = 20, .mask = BIT(30) },
> > > > + [IMX8MN_OCOTP_ASRC_DISABLE] = { .fuse_addr = 20, .mask = BIT(31) },
> > > > +};
> > >
> > > Can we direct define IMX8MN_OCOTP_M7_DISABLE as BIT(8), so avoid this
> > > map data?
> >
> > This would be possible for imx8mn, but not for imx8mp which uses
> > multiples fuses for disables. This is an excerpt from imx8mp WIP
> > > struct disable_fuse imx8mp_disable_fuse[] = {
> > > [IMX8MP_OCOTP_CAN_DISABLE] = { .fuse_addr = 16, .mask = BIT(28) },
> > > [IMX8MP_OCOTP_CAN_FD_DISABLE] = { .fuse_addr = 16, .mask = BIT(29) },
> > > [IMX8MP_OCOTP_VPU_VC8000E_DISABLE] = { .fuse_addr = 16, .mask = BIT(30) },
> > > [IMX8MP_OCOTP_IMG_ISP1_DISABLE] = { .fuse_addr = 20, .mask = BIT(0) },
> > > [IMX8MP_OCOTP_IMG_ISP2_DISABLE] = { .fuse_addr = 20, .mask = BIT(1) },
> > > [IMX8MP_OCOTP_IMG_DEWARP_DISABLE] = { .fuse_addr = 20, .mask = BIT(2) },
> > > };
> >
> > Notice the fuse_addr of 16 and 20.
>
> Yes, I am not sure if it good idea to encode fuse_addr to IMX8MP_OCOTP_CAN_DISABLE
>
> like
>
> #define IMX8MP_OCOTP_CAN_DISABLE 16 << 16 | BIT(28)
>
> So dt-bindings/nvmem/fsl,imx8mn-ocotp.h can be moved to dts directory.
Mh, I personally don't like encoding offsets into bits. How about using
> '#access-controller-cells = <2>'
and using the defines like this
> #define IMX8MP_OCOTP_CAN_DISABLE 16 0x10000000
DT stays the same:
> access-controllers = <&ocotp IMX8MP_OCOTP_CAN_DISABLE>;
Note: It seems BIT(x) is not usable in DT.
Best regards,
Alexander
--
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/
Powered by blists - more mailing lists