| Message ID | 20240509-secvio-v1-1-90fbe2baeda2@nxp.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | soc: imx: secvio: Add secvio support | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/patch-applied | success | |
| robh/dtbs-check | warning | build log |
| robh/dt-meta-schema | success |
On Thu, May 09, 2024 at 02:45:32AM +0200, Vabhav Sharma wrote: > Document the secvio device tree bindings. reduntant sentence. > > The tampers are security feature available on i.MX products and > managed by SNVS block.The tamper goal is to detect the variation ^^ space here > of hardware or physical parameters, which can indicate an attack. > > The SNVS, which provides secure non-volatile storage, allows to > detect some hardware attacks against the SoC.They are connected ^^ space here > to the security-violation ports, which send an alert when an > out-of-range value is detected. > > The "imx-secvio-sc" module is designed to report security violations > and tamper triggering via SCU firmware to the user. > > Add the imx-scu secvio sub node and secvio sub node description. > > Signed-off-by: Franck LENORMAND <franck.lenormand@nxp.com> > Signed-off-by: Vabhav Sharma <vabhav.sharma@nxp.com> > --- > .../bindings/arm/freescale/fsl,scu-secvio.yaml | 35 ++++++++++++++++++++++ > .../devicetree/bindings/firmware/fsl,scu.yaml | 10 +++++++ > 2 files changed, 45 insertions(+) > > diff --git a/Documentation/devicetree/bindings/arm/freescale/fsl,scu-secvio.yaml b/Documentation/devicetree/bindings/arm/freescale/fsl,scu-secvio.yaml > new file mode 100644 > index 000000000000..30dc1e21f903 > --- /dev/null > +++ b/Documentation/devicetree/bindings/arm/freescale/fsl,scu-secvio.yaml > @@ -0,0 +1,35 @@ > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/arm/freescale/fsl,scu-secvio.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: NXP i.MX Security Violation driver Violation detect driver > + > +maintainers: > + - Franck LENORMAND <franck.lenormand@nxp.com> > + > +description: | Needn't "|" > + Receive security violation from the SNVS via the SCU firmware. Allow to > + register notifier for additional processing > + > +properties: > + compatible: > + enum: > + - fsl,imx-sc-secvio > + > + nvmem: > + maxItems: 1 > + any interrupt defined? how do you notify such violation event? > +required: > + - compatible > + - nvmem > + > +additionalProperties: false > + > +examples: > + - | > + secvio { > + compatible = "fsl,imx-sc-secvio"; > + nvmem = <&ocotp>; > + }; > diff --git a/Documentation/devicetree/bindings/firmware/fsl,scu.yaml b/Documentation/devicetree/bindings/firmware/fsl,scu.yaml > index 557e524786c2..b40e127fdc88 100644 > --- a/Documentation/devicetree/bindings/firmware/fsl,scu.yaml > +++ b/Documentation/devicetree/bindings/firmware/fsl,scu.yaml > @@ -129,6 +129,11 @@ properties: > RTC controller provided by the SCU > $ref: /schemas/rtc/fsl,scu-rtc.yaml > > + secvio: > + description: > + Receive security violation from the SNVS via the SCU firmware > + $ref: /schemas/arm/freescale/fsl,scu-secvio.yaml > + > thermal-sensor: > description: > Thermal sensor provided by the SCU > @@ -197,6 +202,11 @@ examples: > compatible = "fsl,imx8qxp-sc-rtc"; > }; > > + secvio { > + compatible = "fsl,imx-sc-secvio"; > + nvmem = <&ocotp>; > + }; > + > keys { > compatible = "fsl,imx8qxp-sc-key", "fsl,imx-sc-key"; > linux,keycodes = <KEY_POWER>; > > -- > 2.25.1 >
On 09/05/2024 02:45, Vabhav Sharma wrote: > Document the secvio device tree bindings. > > The tampers are security feature available on i.MX products and > managed by SNVS block.The tamper goal is to detect the variation > of hardware or physical parameters, which can indicate an attack. > > The SNVS, which provides secure non-volatile storage, allows to > detect some hardware attacks against the SoC.They are connected > to the security-violation ports, which send an alert when an > out-of-range value is detected. > > The "imx-secvio-sc" module is designed to report security violations > and tamper triggering via SCU firmware to the user. > > Add the imx-scu secvio sub node and secvio sub node description. > > Signed-off-by: Franck LENORMAND <franck.lenormand@nxp.com> > Signed-off-by: Vabhav Sharma <vabhav.sharma@nxp.com> > --- That's not v1, right? What changed? Why do we have to guess this? This is thoroughly documented in kernel process so read the documentation before posting. > .../bindings/arm/freescale/fsl,scu-secvio.yaml | 35 ++++++++++++++++++++++ > .../devicetree/bindings/firmware/fsl,scu.yaml | 10 +++++++ > 2 files changed, 45 insertions(+) > > diff --git a/Documentation/devicetree/bindings/arm/freescale/fsl,scu-secvio.yaml b/Documentation/devicetree/bindings/arm/freescale/fsl,scu-secvio.yaml > new file mode 100644 > index 000000000000..30dc1e21f903 > --- /dev/null > +++ b/Documentation/devicetree/bindings/arm/freescale/fsl,scu-secvio.yaml > @@ -0,0 +1,35 @@ > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/arm/freescale/fsl,scu-secvio.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: NXP i.MX Security Violation driver Bindings are for hardware, not drivers. Describe hardware. > + > +maintainers: > + - Franck LENORMAND <franck.lenormand@nxp.com> > + > +description: | Do not need '|' unless you need to preserve formatting. > + Receive security violation from the SNVS via the SCU firmware. Allow to > + register notifier for additional processing Notifier? That's a Linux thing, how does it relate to the hardware? > + > +properties: > + compatible: > + enum: > + - fsl,imx-sc-secvio Missing SoC compatibles. So no, that's just abuse of DT to instantiate driver. NAK. Drop the binding. Best regards, Krzysztof
On 09/05/2024 05:06, Frank Li wrote: > On Thu, May 09, 2024 at 02:45:32AM +0200, Vabhav Sharma wrote: >> Document the secvio device tree bindings. > > reduntant sentence. >> >> The tampers are security feature available on i.MX products and >> managed by SNVS block.The tamper goal is to detect the variation > ^^ space here > >> of hardware or physical parameters, which can indicate an attack. >> >> The SNVS, which provides secure non-volatile storage, allows to >> detect some hardware attacks against the SoC.They are connected > ^^ space here >> to the security-violation ports, which send an alert when an >> out-of-range value is detected. >> >> The "imx-secvio-sc" module is designed to report security violations >> and tamper triggering via SCU firmware to the user. >> >> Add the imx-scu secvio sub node and secvio sub node description. >> >> Signed-off-by: Franck LENORMAND <franck.lenormand@nxp.com> >> Signed-off-by: Vabhav Sharma <vabhav.sharma@nxp.com> >> --- >> .../bindings/arm/freescale/fsl,scu-secvio.yaml | 35 ++++++++++++++++++++++ >> .../devicetree/bindings/firmware/fsl,scu.yaml | 10 +++++++ >> 2 files changed, 45 insertions(+) >> >> diff --git a/Documentation/devicetree/bindings/arm/freescale/fsl,scu-secvio.yaml b/Documentation/devicetree/bindings/arm/freescale/fsl,scu-secvio.yaml >> new file mode 100644 >> index 000000000000..30dc1e21f903 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/arm/freescale/fsl,scu-secvio.yaml >> @@ -0,0 +1,35 @@ >> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/arm/freescale/fsl,scu-secvio.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: NXP i.MX Security Violation driver > > Violation detect driver Bindings are not for drivers. Best regards, Krzysztof
> -----Original Message----- > From: Krzysztof Kozlowski <krzk@kernel.org> > Sent: Thursday, May 9, 2024 11:24 AM > To: Vabhav Sharma <vabhav.sharma@nxp.com>; Rob Herring > <robh@kernel.org>; Krzysztof Kozlowski <krzk+dt@kernel.org>; Conor Dooley > <conor+dt@kernel.org>; Franck Lenormand <franck.lenormand@nxp.com>; > Aisheng Dong <aisheng.dong@nxp.com>; Shawn Guo > <shawnguo@kernel.org>; Sascha Hauer <s.hauer@pengutronix.de>; > Pengutronix Kernel Team <kernel@pengutronix.de>; Fabio Estevam > <festevam@gmail.com>; Peng Fan <peng.fan@nxp.com> > Cc: devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; > imx@lists.linux.dev; linux-arm-kernel@lists.infradead.org; Varun Sethi > <V.Sethi@nxp.com>; Silvano Di Ninno <silvano.dininno@nxp.com>; Pankaj > Gupta <pankaj.gupta@nxp.com>; Frank Li <frank.li@nxp.com>; Daniel Baluta > <daniel.baluta@nxp.com> > Subject: [EXT] Re: [PATCH 1/4] dt-bindings: firmware: secvio: Add device tree > bindings > > Caution: This is an external email. Please take care when clicking links or > opening attachments. When in doubt, report the message using the 'Report > this email' button > > > On 09/05/2024 02:45, Vabhav Sharma wrote: > > Document the secvio device tree bindings. > > > > The tampers are security feature available on i.MX products and > > managed by SNVS block.The tamper goal is to detect the variation of > > hardware or physical parameters, which can indicate an attack. > > > > The SNVS, which provides secure non-volatile storage, allows to detect > > some hardware attacks against the SoC.They are connected to the > > security-violation ports, which send an alert when an out-of-range > > value is detected. > > > > The "imx-secvio-sc" module is designed to report security violations > > and tamper triggering via SCU firmware to the user. > > > > Add the imx-scu secvio sub node and secvio sub node description. > > > > Signed-off-by: Franck LENORMAND <franck.lenormand@nxp.com> > > Signed-off-by: Vabhav Sharma <vabhav.sharma@nxp.com> > > --- > > That's not v1, right? What changed? Why do we have to guess this? Yes, correct this is v2, I will provide changelog details in v3 for v2 and v1 > > This is thoroughly documented in kernel process so read the documentation > before posting. > > > > .../bindings/arm/freescale/fsl,scu-secvio.yaml | 35 > ++++++++++++++++++++++ > > .../devicetree/bindings/firmware/fsl,scu.yaml | 10 +++++++ > > 2 files changed, 45 insertions(+) > > > > diff --git > > a/Documentation/devicetree/bindings/arm/freescale/fsl,scu-secvio.yaml > > b/Documentation/devicetree/bindings/arm/freescale/fsl,scu-secvio.yaml > > new file mode 100644 > > index 000000000000..30dc1e21f903 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/arm/freescale/fsl,scu-secvio.y > > +++ aml > > @@ -0,0 +1,35 @@ > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) %YAML 1.2 > > +--- > > +$id: > > +https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevi > > +cetree.org%2Fschemas%2Farm%2Ffreescale%2Ffsl%2Cscu- > secvio.yaml%23&dat > > > +a=05%7C02%7Cvabhav.sharma%40nxp.com%7Cdea3ecc999444d8c3f7108dc > 6fec67e > > > +b%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C63850830837113 > 8503%7CU > > > +nknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTi > I6Ik1h > > > +aWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=Nl9F3A9%2BTraZboxg3KXT > 35pIPAyYZ > > +51URq1wff7XCmo%3D&reserved=0 > > +$schema: > > +https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevi > > +cetree.org%2Fmeta- > schemas%2Fcore.yaml%23&data=05%7C02%7Cvabhav.sharma > > > +%40nxp.com%7Cdea3ecc999444d8c3f7108dc6fec67eb%7C686ea1d3bc2b4c > 6fa92cd > > > +99c5c301635%7C0%7C0%7C638508308371152796%7CUnknown%7CTWFpb > GZsb3d8eyJW > > > +IjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C > 0%7C > > > +%7C%7C&sdata=dwOG7uVGtO8rccW7vcRwvc2%2B9gyDroIsWnFlXyFxbOM% > 3D&reserve > > +d=0 > > + > > +title: NXP i.MX Security Violation driver > > Bindings are for hardware, not drivers. Describe hardware. Yes, I will use "security violation detection hardware exported through SCU firmware" > > > + > > +maintainers: > > + - Franck LENORMAND <franck.lenormand@nxp.com> > > + > > +description: | > > Do not need '|' unless you need to preserve formatting. Ok > > > + Receive security violation from the SNVS via the SCU firmware. > > + Allow to register notifier for additional processing > > Notifier? That's a Linux thing, how does it relate to the hardware? Violation detected by HW will call driver API to signify the violation. > > > + > > +properties: > > + compatible: > > + enum: > > + - fsl,imx-sc-secvio > > Missing SoC compatibles. Ok, I will use fsl,imx8dxl-sc-secvio > > So no, that's just abuse of DT to instantiate driver. > > NAK. Drop the binding. I will detail the dt binding to describe the real hardware > > Best regards, > Krzysztof
> -----Original Message----- > From: Krzysztof Kozlowski <krzk@kernel.org> > Sent: Thursday, May 9, 2024 11:24 AM > To: Frank Li <frank.li@nxp.com>; Vabhav Sharma <vabhav.sharma@nxp.com> > Cc: Rob Herring <robh@kernel.org>; Krzysztof Kozlowski > <krzk+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>; Franck > Lenormand <franck.lenormand@nxp.com>; Aisheng Dong > <aisheng.dong@nxp.com>; Shawn Guo <shawnguo@kernel.org>; Sascha > Hauer <s.hauer@pengutronix.de>; Pengutronix Kernel Team > <kernel@pengutronix.de>; Fabio Estevam <festevam@gmail.com>; Peng Fan > <peng.fan@nxp.com>; devicetree@vger.kernel.org; linux- > kernel@vger.kernel.org; imx@lists.linux.dev; linux-arm- > kernel@lists.infradead.org; Varun Sethi <V.Sethi@nxp.com>; Silvano Di Ninno > <silvano.dininno@nxp.com>; Pankaj Gupta <pankaj.gupta@nxp.com>; Daniel > Baluta <daniel.baluta@nxp.com> > Subject: [EXT] Re: [PATCH 1/4] dt-bindings: firmware: secvio: Add device tree > bindings > > Caution: This is an external email. Please take care when clicking links or > opening attachments. When in doubt, report the message using the 'Report > this email' button > > > On 09/05/2024 05:06, Frank Li wrote: > > On Thu, May 09, 2024 at 02:45:32AM +0200, Vabhav Sharma wrote: > >> Document the secvio device tree bindings. > > > > reduntant sentence. > >> > >> The tampers are security feature available on i.MX products and > >> managed by SNVS block.The tamper goal is to detect the variation > > ^^ space here > > > >> of hardware or physical parameters, which can indicate an attack. > >> > >> The SNVS, which provides secure non-volatile storage, allows to > >> detect some hardware attacks against the SoC.They are connected > > ^^ space here > >> to the security-violation ports, which send an alert when an > >> out-of-range value is detected. > >> > >> The "imx-secvio-sc" module is designed to report security violations > >> and tamper triggering via SCU firmware to the user. > >> > >> Add the imx-scu secvio sub node and secvio sub node description. > >> > >> Signed-off-by: Franck LENORMAND <franck.lenormand@nxp.com> > >> Signed-off-by: Vabhav Sharma <vabhav.sharma@nxp.com> > >> --- > >> .../bindings/arm/freescale/fsl,scu-secvio.yaml | 35 > ++++++++++++++++++++++ > >> .../devicetree/bindings/firmware/fsl,scu.yaml | 10 +++++++ > >> 2 files changed, 45 insertions(+) > >> > >> diff --git > >> a/Documentation/devicetree/bindings/arm/freescale/fsl,scu-secvio.yaml > >> b/Documentation/devicetree/bindings/arm/freescale/fsl,scu-secvio.yaml > >> new file mode 100644 > >> index 000000000000..30dc1e21f903 > >> --- /dev/null > >> +++ b/Documentation/devicetree/bindings/arm/freescale/fsl,scu-secvio. > >> +++ yaml > >> @@ -0,0 +1,35 @@ > >> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) %YAML 1.2 > >> +--- > >> +$id: > >> +https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdev > >> +icetree.org%2Fschemas%2Farm%2Ffreescale%2Ffsl%2Cscu- > secvio.yaml%23&d > >> > +ata=05%7C02%7Cvabhav.sharma%40nxp.com%7C16a07379ee384ddc18f908 > dc6fec > >> > +75e7%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C63850830857 > 3434788 > >> > +%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIi > LCJBTiI > >> > +6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=MBhqXwhXIQjDb3A > RdYJ4U5EXM > >> +ryEy%2F9m5X6jGuNhHxo%3D&reserved=0 > >> +$schema: > >> +https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdev > >> +icetree.org%2Fmeta- > schemas%2Fcore.yaml%23&data=05%7C02%7Cvabhav.shar > >> > +ma%40nxp.com%7C16a07379ee384ddc18f908dc6fec75e7%7C686ea1d3bc2 > b4c6fa9 > >> > +2cd99c5c301635%7C0%7C0%7C638508308573446476%7CUnknown%7CTWF > pbGZsb3d8 > >> > +eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3 > D%7 > >> > +C0%7C%7C%7C&sdata=m0RzUoVfr%2F2HkLlSOjhTq%2FQX3EM6ZAW7h5hQ > Eidnc1g%3D > >> +&reserved=0 > >> + > >> +title: NXP i.MX Security Violation driver > > > > Violation detect driver > > Bindings are not for drivers. This is security violation detection hardware exported through SCU firmware. I will detail the HW in the binding > > Best regards, > Krzysztof
> -----Original Message----- > From: Frank Li <frank.li@nxp.com> > Sent: Thursday, May 9, 2024 8:37 AM > To: Vabhav Sharma <vabhav.sharma@nxp.com> > Cc: Rob Herring <robh@kernel.org>; Krzysztof Kozlowski > <krzk+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>; Franck > Lenormand <franck.lenormand@nxp.com>; Aisheng Dong > <aisheng.dong@nxp.com>; Shawn Guo <shawnguo@kernel.org>; Sascha > Hauer <s.hauer@pengutronix.de>; Pengutronix Kernel Team > <kernel@pengutronix.de>; Fabio Estevam <festevam@gmail.com>; Peng Fan > <peng.fan@nxp.com>; devicetree@vger.kernel.org; linux- > kernel@vger.kernel.org; imx@lists.linux.dev; linux-arm- > kernel@lists.infradead.org; Varun Sethi <V.Sethi@nxp.com>; Silvano Di Ninno > <silvano.dininno@nxp.com>; Pankaj Gupta <pankaj.gupta@nxp.com>; Daniel > Baluta <daniel.baluta@nxp.com> > Subject: Re: [PATCH 1/4] dt-bindings: firmware: secvio: Add device tree > bindings > > On Thu, May 09, 2024 at 02:45:32AM +0200, Vabhav Sharma wrote: > > Document the secvio device tree bindings. > > reduntant sentence. Ok, I am removing in v3. > > > > The tampers are security feature available on i.MX products and > > managed by SNVS block.The tamper goal is to detect the variation > ^^ space here > > > of hardware or physical parameters, which can indicate an attack. > > > > The SNVS, which provides secure non-volatile storage, allows to detect > > some hardware attacks against the SoC.They are connected > ^^ space here > > to the security-violation ports, which send an alert when an > > out-of-range value is detected. > > > > The "imx-secvio-sc" module is designed to report security violations > > and tamper triggering via SCU firmware to the user. > > > > Add the imx-scu secvio sub node and secvio sub node description. > > > > Signed-off-by: Franck LENORMAND <franck.lenormand@nxp.com> > > Signed-off-by: Vabhav Sharma <vabhav.sharma@nxp.com> > > --- > > .../bindings/arm/freescale/fsl,scu-secvio.yaml | 35 > ++++++++++++++++++++++ > > .../devicetree/bindings/firmware/fsl,scu.yaml | 10 +++++++ > > 2 files changed, 45 insertions(+) > > > > diff --git > > a/Documentation/devicetree/bindings/arm/freescale/fsl,scu-secvio.yaml > > b/Documentation/devicetree/bindings/arm/freescale/fsl,scu-secvio.yaml > > new file mode 100644 > > index 000000000000..30dc1e21f903 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/arm/freescale/fsl,scu-secvio.y > > +++ aml > > @@ -0,0 +1,35 @@ > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) %YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/arm/freescale/fsl,scu-secvio.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: NXP i.MX Security Violation driver > > Violation detect driver Ok > > > + > > +maintainers: > > + - Franck LENORMAND <franck.lenormand@nxp.com> > > + > > +description: | > > Needn't "|" Ok > > > + Receive security violation from the SNVS via the SCU firmware. > > + Allow to register notifier for additional processing > > + > > +properties: > > + compatible: > > + enum: > > + - fsl,imx-sc-secvio > > + > > + nvmem: > > + maxItems: 1 > > + > > any interrupt defined? how do you notify such violation event? Yes, there is security violation interrupt bit in register map of SECVIO HW block with uses RPC call to notify/enable/disable this bit using RPC API exported through SCU firmware > > > +required: > > + - compatible > > + - nvmem > > + > > +additionalProperties: false > > + > > +examples: > > + - | > > + secvio { > > + compatible = "fsl,imx-sc-secvio"; > > + nvmem = <&ocotp>; > > + }; > > diff --git a/Documentation/devicetree/bindings/firmware/fsl,scu.yaml > > b/Documentation/devicetree/bindings/firmware/fsl,scu.yaml > > index 557e524786c2..b40e127fdc88 100644 > > --- a/Documentation/devicetree/bindings/firmware/fsl,scu.yaml > > +++ b/Documentation/devicetree/bindings/firmware/fsl,scu.yaml > > @@ -129,6 +129,11 @@ properties: > > RTC controller provided by the SCU > > $ref: /schemas/rtc/fsl,scu-rtc.yaml > > > > + secvio: > > + description: > > + Receive security violation from the SNVS via the SCU firmware > > + $ref: /schemas/arm/freescale/fsl,scu-secvio.yaml > > + > > thermal-sensor: > > description: > > Thermal sensor provided by the SCU @@ -197,6 +202,11 @@ > > examples: > > compatible = "fsl,imx8qxp-sc-rtc"; > > }; > > > > + secvio { > > + compatible = "fsl,imx-sc-secvio"; > > + nvmem = <&ocotp>; > > + }; > > + > > keys { > > compatible = "fsl,imx8qxp-sc-key", "fsl,imx-sc-key"; > > linux,keycodes = <KEY_POWER>; > > > > -- > > 2.25.1 > >
On 07/06/2024 06:58, Vabhav Sharma wrote: >> >> Missing SoC compatibles. > Ok, I will use fsl,imx8dxl-sc-secvio >> >> So no, that's just abuse of DT to instantiate driver. >> >> NAK. Drop the binding. > I will detail the dt binding to describe the real hardware Still looks like way just to instantiate driver. Why it cannot be part of existing firmware SCU node? Best regards, Krzysztof
Hi Krzysztof > From: Krzysztof Kozlowski <krzk@kernel.org> > Sent: 2024年6月7日 15:08 > > On 07/06/2024 06:58, Vabhav Sharma wrote: > >> > >> Missing SoC compatibles. > > Ok, I will use fsl,imx8dxl-sc-secvio > >> > >> So no, that's just abuse of DT to instantiate driver. > >> > >> NAK. Drop the binding. > > I will detail the dt binding to describe the real hardware > > Still looks like way just to instantiate driver. Why it cannot be part of existing > firmware SCU node? > Technically yes. But SCU case is a little bit complicated as there're many functions and all of them are already added as sub nodes in SCU node for consistency and handling platform difference. I guess some of them, e.g. rtc, could be part of SCU node (reuse) while some couldn't. e.g. pinctrl Do you want us to only make secvio reuse existing SCU node? This might look a bit strange to the existing sub nodes. BTW, even we can reuse SCU node for secvio function, we still need update binding doc to add extra property 'nvmem' for secvio. Please share your suggestions considering above information. e.g. https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/Documentation/devicetree/bindings/firmware/fsl,scu.yaml firmware { system-controller { compatible = "fsl,imx-scu"; .... rtc { compatible = "fsl,imx8qxp-sc-rtc"; }; keys { compatible = "fsl,imx8qxp-sc-key", "fsl,imx-sc-key"; linux,keycodes = <KEY_POWER>; }; watchdog { compatible = "fsl,imx8qxp-sc-wdt", "fsl,imx-sc-wdt"; timeout-sec = <60>; }; thermal-sensor { compatible = "fsl,imx8qxp-sc-thermal", "fsl,imx-sc-thermal"; #thermal-sensor-cells = <1>; }; ..... }; Regards Aisheng > Best regards, > Krzysztof
On 12/06/2024 09:20, Aisheng Dong wrote: > Hi Krzysztof > >> From: Krzysztof Kozlowski <krzk@kernel.org> >> Sent: 2024年6月7日 15:08 >> >> On 07/06/2024 06:58, Vabhav Sharma wrote: >>>> >>>> Missing SoC compatibles. >>> Ok, I will use fsl,imx8dxl-sc-secvio >>>> >>>> So no, that's just abuse of DT to instantiate driver. >>>> >>>> NAK. Drop the binding. >>> I will detail the dt binding to describe the real hardware >> >> Still looks like way just to instantiate driver. Why it cannot be part of existing >> firmware SCU node? >> > > Technically yes. But SCU case is a little bit complicated as there're many > functions and all of them are already added as sub nodes in SCU node > for consistency and handling platform difference. > > I guess some of them, e.g. rtc, could be part of SCU node (reuse) while > some couldn't. e.g. pinctrl > Do you want us to only make secvio reuse existing SCU node? Yes > This might look a bit strange to the existing sub nodes. Nothing strange/unusual to me. > > BTW, even we can reuse SCU node for secvio function, we still need update > binding doc to add extra property 'nvmem' for secvio. Sure. Best regards, Krzysztof
Hi Krzysztof, > From: Krzysztof Kozlowski <krzk@kernel.org> > Sent: 2024年6月13日 14:14 > > On 12/06/2024 09:20, Aisheng Dong wrote: > > Hi Krzysztof > > > >> From: Krzysztof Kozlowski <krzk@kernel.org> > >> Sent: 2024年6月7日 15:08 > >> > >> On 07/06/2024 06:58, Vabhav Sharma wrote: > >>>> > >>>> Missing SoC compatibles. > >>> Ok, I will use fsl,imx8dxl-sc-secvio > >>>> > >>>> So no, that's just abuse of DT to instantiate driver. > >>>> > >>>> NAK. Drop the binding. > >>> I will detail the dt binding to describe the real hardware > >> > >> Still looks like way just to instantiate driver. Why it cannot be > >> part of existing firmware SCU node? > >> > > > > Technically yes. But SCU case is a little bit complicated as there're > > many functions and all of them are already added as sub nodes in SCU > > node for consistency and handling platform difference. > > > > I guess some of them, e.g. rtc, could be part of SCU node (reuse) > > while some couldn't. e.g. pinctrl Do you want us to only make secvio > > reuse existing SCU node? > > Yes > Digging a bit more on the implementation. It seems there will be a 'parent depends on child' issue when reusing the parent SCU node for secvio. Considering the defer probe support and ocotop could be modules, I'm still thinking If any solution. Do you have a good suggestion? e.g. system-controller { compatible = "fsl,imx-scu"; nvmem = <&ocotp>; ... ocotp: ocotp { compatible = "fsl,imx8qxp-scu-ocotp"; #address-cells = <1>; #size-cells = <1>; read-only; }; ... } static int imx_scu_probe(struct platform_device *pdev) { ... ret = imx_secvio_sc_setup(dev); if (ret) dev_warn(dev, "failed to initialize secvio: %d\n", ret); ... return devm_of_platform_populate(dev); } Regards Aisheng > > This might look a bit strange to the existing sub nodes. > > Nothing strange/unusual to me. > > > > > BTW, even we can reuse SCU node for secvio function, we still need > > update binding doc to add extra property 'nvmem' for secvio. > > Sure. > > > > Best regards, > Krzysztof
diff --git a/Documentation/devicetree/bindings/arm/freescale/fsl,scu-secvio.yaml b/Documentation/devicetree/bindings/arm/freescale/fsl,scu-secvio.yaml new file mode 100644 index 000000000000..30dc1e21f903 --- /dev/null +++ b/Documentation/devicetree/bindings/arm/freescale/fsl,scu-secvio.yaml @@ -0,0 +1,35 @@ +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/arm/freescale/fsl,scu-secvio.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: NXP i.MX Security Violation driver + +maintainers: + - Franck LENORMAND <franck.lenormand@nxp.com> + +description: | + Receive security violation from the SNVS via the SCU firmware. Allow to + register notifier for additional processing + +properties: + compatible: + enum: + - fsl,imx-sc-secvio + + nvmem: + maxItems: 1 + +required: + - compatible + - nvmem + +additionalProperties: false + +examples: + - | + secvio { + compatible = "fsl,imx-sc-secvio"; + nvmem = <&ocotp>; + }; diff --git a/Documentation/devicetree/bindings/firmware/fsl,scu.yaml b/Documentation/devicetree/bindings/firmware/fsl,scu.yaml index 557e524786c2..b40e127fdc88 100644 --- a/Documentation/devicetree/bindings/firmware/fsl,scu.yaml +++ b/Documentation/devicetree/bindings/firmware/fsl,scu.yaml @@ -129,6 +129,11 @@ properties: RTC controller provided by the SCU $ref: /schemas/rtc/fsl,scu-rtc.yaml + secvio: + description: + Receive security violation from the SNVS via the SCU firmware + $ref: /schemas/arm/freescale/fsl,scu-secvio.yaml + thermal-sensor: description: Thermal sensor provided by the SCU @@ -197,6 +202,11 @@ examples: compatible = "fsl,imx8qxp-sc-rtc"; }; + secvio { + compatible = "fsl,imx-sc-secvio"; + nvmem = <&ocotp>; + }; + keys { compatible = "fsl,imx8qxp-sc-key", "fsl,imx-sc-key"; linux,keycodes = <KEY_POWER>;