diff mbox series

[v2,1/2] dt-bindings: arm: stm32: add simple-mfd compatible for tamp node

Message ID 20201021102855.18026-1-a.fatoum@pengutronix.de (mailing list archive)
State New, archived
Headers show
Series [v2,1/2] dt-bindings: arm: stm32: add simple-mfd compatible for tamp node | expand

Commit Message

Ahmad Fatoum Oct. 21, 2020, 10:28 a.m. UTC
The stm32mp1 TAMP (Tamper and backup registers) does tamper detection
and features 32 backup registers that, being in the RTC domain, may
survive even with Vdd switched off.

This makes it suitable for use to communicate a reboot mode from OS
to bootloader via the syscon-reboot-mode binding. Add a "simple-mfd"
to support probing such a child node. The actual reboot mode
node could then be defined in a board.dts or fixed up by the bootloader.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
v1 available here:
https://lore.kernel.org/linux-arm-kernel/20200916142216.25142-1-a.fatoum@pengutronix.de/

v1 -> v2:
 - new patch, rebased on top of
   https://lore.kernel.org/r/20201014125441.2457-1-arnaud.pouliquen@st.com
---
 .../devicetree/bindings/arm/stm32/st,stm32-syscon.yaml       | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

Comments

Rob Herring (Arm) Oct. 26, 2020, 2:36 p.m. UTC | #1
On Wed, Oct 21, 2020 at 12:28:55PM +0200, Ahmad Fatoum wrote:
> The stm32mp1 TAMP (Tamper and backup registers) does tamper detection
> and features 32 backup registers that, being in the RTC domain, may
> survive even with Vdd switched off.
> 
> This makes it suitable for use to communicate a reboot mode from OS
> to bootloader via the syscon-reboot-mode binding. Add a "simple-mfd"
> to support probing such a child node. The actual reboot mode
> node could then be defined in a board.dts or fixed up by the bootloader.

'simple-mfd' implies there is no dependency on the parent node for the 
child (such as the regmap perhaps). Is that the case here?

> 
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
> v1 available here:
> https://lore.kernel.org/linux-arm-kernel/20200916142216.25142-1-a.fatoum@pengutronix.de/
> 
> v1 -> v2:
>  - new patch, rebased on top of
>    https://lore.kernel.org/r/20201014125441.2457-1-arnaud.pouliquen@st.com
> ---
>  .../devicetree/bindings/arm/stm32/st,stm32-syscon.yaml       | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/arm/stm32/st,stm32-syscon.yaml b/Documentation/devicetree/bindings/arm/stm32/st,stm32-syscon.yaml
> index 6634b3e0853e..4684017a42e4 100644
> --- a/Documentation/devicetree/bindings/arm/stm32/st,stm32-syscon.yaml
> +++ b/Documentation/devicetree/bindings/arm/stm32/st,stm32-syscon.yaml
> @@ -19,8 +19,11 @@ properties:
>                - st,stm32mp151-pwr-mcu
>                - st,stm32-syscfg
>                - st,stm32-power-config
> -              - st,stm32-tamp
>            - const: syscon
> +      - items:
> +          - const: st,stm32-tamp
> +          - const: syscon
> +          - const: simple-mfd
>  
>    reg:
>      maxItems: 1
> -- 
> 2.28.0
>
Ahmad Fatoum Oct. 26, 2020, 9:30 p.m. UTC | #2
Hello Rob,

On 10/26/20 3:36 PM, Rob Herring wrote:
> On Wed, Oct 21, 2020 at 12:28:55PM +0200, Ahmad Fatoum wrote:
>> The stm32mp1 TAMP (Tamper and backup registers) does tamper detection
>> and features 32 backup registers that, being in the RTC domain, may
>> survive even with Vdd switched off.
>>
>> This makes it suitable for use to communicate a reboot mode from OS
>> to bootloader via the syscon-reboot-mode binding. Add a "simple-mfd"
>> to support probing such a child node. The actual reboot mode
>> node could then be defined in a board.dts or fixed up by the bootloader.
> 
> 'simple-mfd' implies there is no dependency on the parent node for the 
> child (such as the regmap perhaps). Is that the case here?

No, there's a dependency and the Linux driver does syscon_node_to_regmap
on the device tree node's parent but that's how the syscon-reboot-mode binding
is documented:

  The SYSCON mapped register is retrieved from the
  parental dt-node plus the offset. So the SYSCON reboot-mode node
  should be represented as a sub-node of a "syscon", "simple-mfd" node.

How would you prefer this being done instead?

Cheers,
Ahmad

> 
>>
>> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>> ---
>> v1 available here:
>> https://lore.kernel.org/linux-arm-kernel/20200916142216.25142-1-a.fatoum@pengutronix.de/
>>
>> v1 -> v2:
>>  - new patch, rebased on top of
>>    https://lore.kernel.org/r/20201014125441.2457-1-arnaud.pouliquen@st.com
>> ---
>>  .../devicetree/bindings/arm/stm32/st,stm32-syscon.yaml       | 5 ++++-
>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/arm/stm32/st,stm32-syscon.yaml b/Documentation/devicetree/bindings/arm/stm32/st,stm32-syscon.yaml
>> index 6634b3e0853e..4684017a42e4 100644
>> --- a/Documentation/devicetree/bindings/arm/stm32/st,stm32-syscon.yaml
>> +++ b/Documentation/devicetree/bindings/arm/stm32/st,stm32-syscon.yaml
>> @@ -19,8 +19,11 @@ properties:
>>                - st,stm32mp151-pwr-mcu
>>                - st,stm32-syscfg
>>                - st,stm32-power-config
>> -              - st,stm32-tamp
>>            - const: syscon
>> +      - items:
>> +          - const: st,stm32-tamp
>> +          - const: syscon
>> +          - const: simple-mfd
>>  
>>    reg:
>>      maxItems: 1
>> -- 
>> 2.28.0
>>
>
Rob Herring (Arm) Oct. 27, 2020, 12:15 p.m. UTC | #3
On Mon, Oct 26, 2020 at 4:30 PM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
>
> Hello Rob,
>
> On 10/26/20 3:36 PM, Rob Herring wrote:
> > On Wed, Oct 21, 2020 at 12:28:55PM +0200, Ahmad Fatoum wrote:
> >> The stm32mp1 TAMP (Tamper and backup registers) does tamper detection
> >> and features 32 backup registers that, being in the RTC domain, may
> >> survive even with Vdd switched off.
> >>
> >> This makes it suitable for use to communicate a reboot mode from OS
> >> to bootloader via the syscon-reboot-mode binding. Add a "simple-mfd"
> >> to support probing such a child node. The actual reboot mode
> >> node could then be defined in a board.dts or fixed up by the bootloader.
> >
> > 'simple-mfd' implies there is no dependency on the parent node for the
> > child (such as the regmap perhaps). Is that the case here?
>
> No, there's a dependency and the Linux driver does syscon_node_to_regmap
> on the device tree node's parent but that's how the syscon-reboot-mode binding
> is documented:
>
>   The SYSCON mapped register is retrieved from the
>   parental dt-node plus the offset. So the SYSCON reboot-mode node
>   should be represented as a sub-node of a "syscon", "simple-mfd" node.
>
> How would you prefer this being done instead?

Well, probably the syscon driver could just probe any children, but
I'm not sure if that would break anyone. So I guess fine as-is.

Reviewed-by: Rob Herring <robh@kernel.org>

Rob
Ahmad Fatoum Nov. 10, 2020, 10:27 a.m. UTC | #4
Hello Alex,

On 10/27/20 1:15 PM, Rob Herring wrote:
> On Mon, Oct 26, 2020 at 4:30 PM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
>>
>> Hello Rob,
>>
>> On 10/26/20 3:36 PM, Rob Herring wrote:
>>> On Wed, Oct 21, 2020 at 12:28:55PM +0200, Ahmad Fatoum wrote:
>>>> The stm32mp1 TAMP (Tamper and backup registers) does tamper detection
>>>> and features 32 backup registers that, being in the RTC domain, may
>>>> survive even with Vdd switched off.
>>>>
>>>> This makes it suitable for use to communicate a reboot mode from OS
>>>> to bootloader via the syscon-reboot-mode binding. Add a "simple-mfd"
>>>> to support probing such a child node. The actual reboot mode
>>>> node could then be defined in a board.dts or fixed up by the bootloader.
>>>
>>> 'simple-mfd' implies there is no dependency on the parent node for the
>>> child (such as the regmap perhaps). Is that the case here?
>>
>> No, there's a dependency and the Linux driver does syscon_node_to_regmap
>> on the device tree node's parent but that's how the syscon-reboot-mode binding
>> is documented:
>>
>>   The SYSCON mapped register is retrieved from the
>>   parental dt-node plus the offset. So the SYSCON reboot-mode node
>>   should be represented as a sub-node of a "syscon", "simple-mfd" node.
>>
>> How would you prefer this being done instead?
> 
> Well, probably the syscon driver could just probe any children, but
> I'm not sure if that would break anyone. So I guess fine as-is.
> 
> Reviewed-by: Rob Herring <robh@kernel.org>

Gentle ping.

> 
> Rob
>
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/arm/stm32/st,stm32-syscon.yaml b/Documentation/devicetree/bindings/arm/stm32/st,stm32-syscon.yaml
index 6634b3e0853e..4684017a42e4 100644
--- a/Documentation/devicetree/bindings/arm/stm32/st,stm32-syscon.yaml
+++ b/Documentation/devicetree/bindings/arm/stm32/st,stm32-syscon.yaml
@@ -19,8 +19,11 @@  properties:
               - st,stm32mp151-pwr-mcu
               - st,stm32-syscfg
               - st,stm32-power-config
-              - st,stm32-tamp
           - const: syscon
+      - items:
+          - const: st,stm32-tamp
+          - const: syscon
+          - const: simple-mfd
 
   reg:
     maxItems: 1