[PATCH v3 0/4] add meson secure watchdog driver

[PATCH v3 0/4] add meson secure watchdog driver

[Xingyu Chen]

The watchdog controller on the Meson-A/C series SoCs is moved to secure world,
We have to call SMC instruction to trap the ATF for watchdog operation. These
operations are different from previous SoCs, so we introduce a new watchdog
driver to support this kind of SoCs.

Changes since v2 at [1]:
- remove useless dependency in Kconfig
- return zero when getting left time value fails

Changes since v1 at [0]:
- add a new dependency in Kconfig
- simplify/add the return operation
- remove useless ping operation when setting the timeout
- fix some return values
- fix the license statement

[0]:https://lore.kernel.org/linux-amlogic/1570874721-36077-1-git-send-email-xingyu.chen@amlogic.com
[1]:https://lore.kernel.org/linux-amlogic/1571387622-35132-1-git-send-email-xingyu.chen@amlogic.com

Xingyu Chen (4):
  firmware: meson_sm: add new SMC ID support for accessing secure
    watchdog
  dt-bindings: watchdog: add new binding for meson secure watchdog
  watchdog: add meson secure watchdog driver
  arm64: dts: a1: add secure watchdog controller

 .../bindings/watchdog/amlogic,meson-sec-wdt.yaml   |  34 ++++
 arch/arm64/boot/dts/amlogic/meson-a1.dtsi          |   6 +
 drivers/firmware/meson/meson_sm.c                  |   1 +
 drivers/watchdog/Kconfig                           |  16 ++
 drivers/watchdog/Makefile                          |   1 +
 drivers/watchdog/meson_sec_wdt.c                   | 187 +++++++++++++++++++++
 include/linux/firmware/meson/meson_sm.h            |   1 +
 7 files changed, 246 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/watchdog/amlogic,meson-sec-wdt.yaml
 create mode 100644 drivers/watchdog/meson_sec_wdt.c

-- 
2.7.4

[PATCH v3 2/4] dt-bindings: watchdog: add new binding for meson secure watchdog

[Xingyu Chen]

The binding targets the Meson-A/C series compatible SoCs, in which the
watchdog registers are in secure world.

Signed-off-by: Xingyu Chen <xingyu.chen@amlogic.com>
---
 .../bindings/watchdog/amlogic,meson-sec-wdt.yaml   | 34 ++++++++++++++++++++++
 1 file changed, 34 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/watchdog/amlogic,meson-sec-wdt.yaml

diff --git a/Documentation/devicetree/bindings/watchdog/amlogic,meson-sec-wdt.yaml b/Documentation/devicetree/bindings/watchdog/amlogic,meson-sec-wdt.yaml
new file mode 100644
index 00000000..0bbc807
--- /dev/null
+++ b/Documentation/devicetree/bindings/watchdog/amlogic,meson-sec-wdt.yaml
@@ -0,0 +1,34 @@
+# SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+# Copyright (c) 2019 Amlogic, Inc
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/power/amlogic,meson-sec-wdt.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Amlogic Meson Secure Watchdog Timer
+
+maintainers:
+  - Xingyu Chen <xingyu.chen@amlogic.com>
+
+description: |+
+  Secure Watchdog Timer used in Meson-A/C series Compatible SoCs
+
+properties:
+  compatible:
+    enum:
+      - amlogic,meson-sec-wdt
+
+  secure-monitor:
+    description: phandle to the secure-monitor node
+    $ref: /schemas/types.yaml#/definitions/phandle
+
+required:
+  - compatible
+  - secure-monitor
+
+examples:
+  - |
+    watchdog {
+          compatible = "amlogic,meson-sec-wdt";
+          secure-monitor = <&sm>;
+    };
-- 
2.7.4

[PATCH v3 4/4] arm64: dts: a1: add secure watchdog controller

[Xingyu Chen]

Enable secure watchdog controller for Meson-A1 SoC

Signed-off-by: Xingyu Chen <xingyu.chen@amlogic.com>
---
 arch/arm64/boot/dts/amlogic/meson-a1.dtsi | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm64/boot/dts/amlogic/meson-a1.dtsi b/arch/arm64/boot/dts/amlogic/meson-a1.dtsi
index 7210ad0..047c323 100644
--- a/arch/arm64/boot/dts/amlogic/meson-a1.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-a1.dtsi
@@ -93,6 +93,12 @@
 				clock-names = "xtal", "pclk", "baud";
 				status = "disabled";
 			};
+
+			watchdog {
+				compatible = "amlogic,meson-sec-wdt";
+				secure-monitor = <&sm>;
+				status = "okay";
+			};
 		};
 
 		gic: interrupt-controller@ff901000 {
-- 
2.7.4

Re: [PATCH v3 2/4] dt-bindings: watchdog: add new binding for meson secure watchdog

[Rob Herring]

On Fri, Oct 25, 2019 at 02:13:02PM +0800, Xingyu Chen wrote:
> The binding targets the Meson-A/C series compatible SoCs, in which the
> watchdog registers are in secure world.
> 
> Signed-off-by: Xingyu Chen <xingyu.chen@amlogic.com>
> ---
>  .../bindings/watchdog/amlogic,meson-sec-wdt.yaml   | 34 ++++++++++++++++++++++
>  1 file changed, 34 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/watchdog/amlogic,meson-sec-wdt.yaml
> 
> diff --git a/Documentation/devicetree/bindings/watchdog/amlogic,meson-sec-wdt.yaml b/Documentation/devicetree/bindings/watchdog/amlogic,meson-sec-wdt.yaml
> new file mode 100644
> index 00000000..0bbc807
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/watchdog/amlogic,meson-sec-wdt.yaml
> @@ -0,0 +1,34 @@
> +# SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +# Copyright (c) 2019 Amlogic, Inc
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/power/amlogic,meson-sec-wdt.yaml#"
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> +
> +title: Amlogic Meson Secure Watchdog Timer
> +
> +maintainers:
> +  - Xingyu Chen <xingyu.chen@amlogic.com>
> +
> +description: |+
> +  Secure Watchdog Timer used in Meson-A/C series Compatible SoCs
> +
> +properties:
> +  compatible:
> +    enum:
> +      - amlogic,meson-sec-wdt

If there are no other properties, then you don't need this. Just have 
the secure firmware driver instantiate the watchdog.

> +
> +  secure-monitor:
> +    description: phandle to the secure-monitor node
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +
> +required:
> +  - compatible
> +  - secure-monitor
> +
> +examples:
> +  - |
> +    watchdog {
> +          compatible = "amlogic,meson-sec-wdt";
> +          secure-monitor = <&sm>;
> +    };
> -- 
> 2.7.4
> 

Re: [PATCH v3 2/4] dt-bindings: watchdog: add new binding for meson secure watchdog

[Xingyu Chen]

Hi, Rob

On 2019/10/26 4:30, Rob Herring wrote:
> On Fri, Oct 25, 2019 at 02:13:02PM +0800, Xingyu Chen wrote:
>> The binding targets the Meson-A/C series compatible SoCs, in which the
>> watchdog registers are in secure world.
>>
>> Signed-off-by: Xingyu Chen <xingyu.chen@amlogic.com>
>> ---
>>   .../bindings/watchdog/amlogic,meson-sec-wdt.yaml   | 34 ++++++++++++++++++++++
>>   1 file changed, 34 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/watchdog/amlogic,meson-sec-wdt.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/watchdog/amlogic,meson-sec-wdt.yaml b/Documentation/devicetree/bindings/watchdog/amlogic,meson-sec-wdt.yaml
>> new file mode 100644
>> index 00000000..0bbc807
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/watchdog/amlogic,meson-sec-wdt.yaml
>> @@ -0,0 +1,34 @@
>> +# SPDX-License-Identifier: (GPL-2.0+ OR MIT)
>> +# Copyright (c) 2019 Amlogic, Inc
>> +%YAML 1.2
>> +---
>> +$id: "http://devicetree.org/schemas/power/amlogic,meson-sec-wdt.yaml#"
>> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
>> +
>> +title: Amlogic Meson Secure Watchdog Timer
>> +
>> +maintainers:
>> +  - Xingyu Chen <xingyu.chen@amlogic.com>
>> +
>> +description: |+
>> +  Secure Watchdog Timer used in Meson-A/C series Compatible SoCs
>> +
>> +properties:
>> +  compatible:
>> +    enum:
>> +      - amlogic,meson-sec-wdt
> 
> If there are no other properties, then you don't need this. Just have
> the secure firmware driver instantiate the watchdog.
I'am very sorry i don't understand how to initialize the watchdog driver 
if the compatible property is removed, Could you give me more 
suggestions or examples ？ Thank you very much.

> 
>> +
>> +  secure-monitor:
>> +    description: phandle to the secure-monitor node
>> +    $ref: /schemas/types.yaml#/definitions/phandle
>> +
>> +required:
>> +  - compatible
>> +  - secure-monitor
>> +
>> +examples:
>> +  - |
>> +    watchdog {
>> +          compatible = "amlogic,meson-sec-wdt";
>> +          secure-monitor = <&sm>;
>> +    };
>> -- 
>> 2.7.4
>>
> 
> .
> 

Re: [PATCH v3 2/4] dt-bindings: watchdog: add new binding for meson secure watchdog

[Rob Herring]

On Mon, Oct 28, 2019 at 3:35 AM Xingyu Chen <xingyu.chen@amlogic.com> wrote:
>
> Hi, Rob
>
> On 2019/10/26 4:30, Rob Herring wrote:
> > On Fri, Oct 25, 2019 at 02:13:02PM +0800, Xingyu Chen wrote:
> >> The binding targets the Meson-A/C series compatible SoCs, in which the
> >> watchdog registers are in secure world.
> >>
> >> Signed-off-by: Xingyu Chen <xingyu.chen@amlogic.com>
> >> ---
> >>   .../bindings/watchdog/amlogic,meson-sec-wdt.yaml   | 34 ++++++++++++++++++++++
> >>   1 file changed, 34 insertions(+)
> >>   create mode 100644 Documentation/devicetree/bindings/watchdog/amlogic,meson-sec-wdt.yaml
> >>
> >> diff --git a/Documentation/devicetree/bindings/watchdog/amlogic,meson-sec-wdt.yaml b/Documentation/devicetree/bindings/watchdog/amlogic,meson-sec-wdt.yaml
> >> new file mode 100644
> >> index 00000000..0bbc807
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/watchdog/amlogic,meson-sec-wdt.yaml
> >> @@ -0,0 +1,34 @@
> >> +# SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> >> +# Copyright (c) 2019 Amlogic, Inc
> >> +%YAML 1.2
> >> +---
> >> +$id: "http://devicetree.org/schemas/power/amlogic,meson-sec-wdt.yaml#"
> >> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> >> +
> >> +title: Amlogic Meson Secure Watchdog Timer
> >> +
> >> +maintainers:
> >> +  - Xingyu Chen <xingyu.chen@amlogic.com>
> >> +
> >> +description: |+
> >> +  Secure Watchdog Timer used in Meson-A/C series Compatible SoCs
> >> +
> >> +properties:
> >> +  compatible:
> >> +    enum:
> >> +      - amlogic,meson-sec-wdt
> >
> > If there are no other properties, then you don't need this. Just have
> > the secure firmware driver instantiate the watchdog.
> I'am very sorry i don't understand how to initialize the watchdog driver
> if the compatible property is removed, Could you give me more
> suggestions or examples ？ Thank you very much.

platform_device_register_simple() from the secure firmware driver.

Rob

Re: [PATCH v3 2/4] dt-bindings: watchdog: add new binding for meson secure watchdog

[Xingyu Chen]

Hi,Rob

On 2019/10/30 4:51, Rob Herring wrote:
> On Mon, Oct 28, 2019 at 3:35 AM Xingyu Chen <xingyu.chen@amlogic.com> wrote:
>>
>> Hi, Rob
>>
>> On 2019/10/26 4:30, Rob Herring wrote:
>>> On Fri, Oct 25, 2019 at 02:13:02PM +0800, Xingyu Chen wrote:
>>>> The binding targets the Meson-A/C series compatible SoCs, in which the
>>>> watchdog registers are in secure world.
>>>>
>>>> Signed-off-by: Xingyu Chen <xingyu.chen@amlogic.com>
>>>> ---
>>>>    .../bindings/watchdog/amlogic,meson-sec-wdt.yaml   | 34 ++++++++++++++++++++++
>>>>    1 file changed, 34 insertions(+)
>>>>    create mode 100644 Documentation/devicetree/bindings/watchdog/amlogic,meson-sec-wdt.yaml
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/watchdog/amlogic,meson-sec-wdt.yaml b/Documentation/devicetree/bindings/watchdog/amlogic,meson-sec-wdt.yaml
>>>> new file mode 100644
>>>> index 00000000..0bbc807
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/watchdog/amlogic,meson-sec-wdt.yaml
>>>> @@ -0,0 +1,34 @@
>>>> +# SPDX-License-Identifier: (GPL-2.0+ OR MIT)
>>>> +# Copyright (c) 2019 Amlogic, Inc
>>>> +%YAML 1.2
>>>> +---
>>>> +$id: "http://devicetree.org/schemas/power/amlogic,meson-sec-wdt.yaml#"
>>>> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
>>>> +
>>>> +title: Amlogic Meson Secure Watchdog Timer
>>>> +
>>>> +maintainers:
>>>> +  - Xingyu Chen <xingyu.chen@amlogic.com>
>>>> +
>>>> +description: |+
>>>> +  Secure Watchdog Timer used in Meson-A/C series Compatible SoCs
>>>> +
>>>> +properties:
>>>> +  compatible:
>>>> +    enum:
>>>> +      - amlogic,meson-sec-wdt
>>>
>>> If there are no other properties, then you don't need this. Just have
>>> the secure firmware driver instantiate the watchdog.
>> I'am very sorry i don't understand how to initialize the watchdog driver
>> if the compatible property is removed, Could you give me more
>> suggestions or examples ？ Thank you very much.
> 
> platform_device_register_simple() from the secure firmware driver.
Thanks for your help. The device node of wdt looks useless if I use this 
function to register device. if so, how should I get the pointer to 
secure-monitor in wdt driver ? or should I use directly arm_smccc to 
access the secfw ?
> 
> Rob
> 
> .
> 

Re: [PATCH v3 2/4] dt-bindings: watchdog: add new binding for meson secure watchdog

[Rob Herring]

On Wed, Oct 30, 2019 at 7:59 AM Xingyu Chen <xingyu.chen@amlogic.com> wrote:
>
> Hi,Rob
>
> On 2019/10/30 4:51, Rob Herring wrote:
> > On Mon, Oct 28, 2019 at 3:35 AM Xingyu Chen <xingyu.chen@amlogic.com> wrote:
> >>
> >> Hi, Rob
> >>
> >> On 2019/10/26 4:30, Rob Herring wrote:
> >>> On Fri, Oct 25, 2019 at 02:13:02PM +0800, Xingyu Chen wrote:
> >>>> The binding targets the Meson-A/C series compatible SoCs, in which the
> >>>> watchdog registers are in secure world.
> >>>>
> >>>> Signed-off-by: Xingyu Chen <xingyu.chen@amlogic.com>
> >>>> ---
> >>>>    .../bindings/watchdog/amlogic,meson-sec-wdt.yaml   | 34 ++++++++++++++++++++++
> >>>>    1 file changed, 34 insertions(+)
> >>>>    create mode 100644 Documentation/devicetree/bindings/watchdog/amlogic,meson-sec-wdt.yaml
> >>>>
> >>>> diff --git a/Documentation/devicetree/bindings/watchdog/amlogic,meson-sec-wdt.yaml b/Documentation/devicetree/bindings/watchdog/amlogic,meson-sec-wdt.yaml
> >>>> new file mode 100644
> >>>> index 00000000..0bbc807
> >>>> --- /dev/null
> >>>> +++ b/Documentation/devicetree/bindings/watchdog/amlogic,meson-sec-wdt.yaml
> >>>> @@ -0,0 +1,34 @@
> >>>> +# SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> >>>> +# Copyright (c) 2019 Amlogic, Inc
> >>>> +%YAML 1.2
> >>>> +---
> >>>> +$id: "http://devicetree.org/schemas/power/amlogic,meson-sec-wdt.yaml#"
> >>>> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> >>>> +
> >>>> +title: Amlogic Meson Secure Watchdog Timer
> >>>> +
> >>>> +maintainers:
> >>>> +  - Xingyu Chen <xingyu.chen@amlogic.com>
> >>>> +
> >>>> +description: |+
> >>>> +  Secure Watchdog Timer used in Meson-A/C series Compatible SoCs
> >>>> +
> >>>> +properties:
> >>>> +  compatible:
> >>>> +    enum:
> >>>> +      - amlogic,meson-sec-wdt
> >>>
> >>> If there are no other properties, then you don't need this. Just have
> >>> the secure firmware driver instantiate the watchdog.
> >> I'am very sorry i don't understand how to initialize the watchdog driver
> >> if the compatible property is removed, Could you give me more
> >> suggestions or examples ？ Thank you very much.
> >
> > platform_device_register_simple() from the secure firmware driver.
> Thanks for your help. The device node of wdt looks useless if I use this
> function to register device. if so, how should I get the pointer to
> secure-monitor in wdt driver ? or should I use directly arm_smccc to
> access the secfw ?

You can use of_find_compatible_node(). There should only be one firmware node.

Rob

Re: [PATCH v3 2/4] dt-bindings: watchdog: add new binding for meson secure watchdog

[Xingyu Chen]

Hi, Rob

On 2019/10/30 21:41, Rob Herring wrote:
> On Wed, Oct 30, 2019 at 7:59 AM Xingyu Chen <xingyu.chen@amlogic.com> wrote:
>>
>> Hi,Rob
>>
>> On 2019/10/30 4:51, Rob Herring wrote:
>>> On Mon, Oct 28, 2019 at 3:35 AM Xingyu Chen <xingyu.chen@amlogic.com> wrote:
>>>>
>>>> Hi, Rob
>>>>
>>>> On 2019/10/26 4:30, Rob Herring wrote:
>>>>> On Fri, Oct 25, 2019 at 02:13:02PM +0800, Xingyu Chen wrote:
>>>>>> The binding targets the Meson-A/C series compatible SoCs, in which the
>>>>>> watchdog registers are in secure world.
>>>>>>
>>>>>> Signed-off-by: Xingyu Chen <xingyu.chen@amlogic.com>
>>>>>> ---
>>>>>>     .../bindings/watchdog/amlogic,meson-sec-wdt.yaml   | 34 ++++++++++++++++++++++
>>>>>>     1 file changed, 34 insertions(+)
>>>>>>     create mode 100644 Documentation/devicetree/bindings/watchdog/amlogic,meson-sec-wdt.yaml
>>>>>>
>>>>>> diff --git a/Documentation/devicetree/bindings/watchdog/amlogic,meson-sec-wdt.yaml b/Documentation/devicetree/bindings/watchdog/amlogic,meson-sec-wdt.yaml
>>>>>> new file mode 100644
>>>>>> index 00000000..0bbc807
>>>>>> --- /dev/null
>>>>>> +++ b/Documentation/devicetree/bindings/watchdog/amlogic,meson-sec-wdt.yaml
>>>>>> @@ -0,0 +1,34 @@
>>>>>> +# SPDX-License-Identifier: (GPL-2.0+ OR MIT)
>>>>>> +# Copyright (c) 2019 Amlogic, Inc
>>>>>> +%YAML 1.2
>>>>>> +---
>>>>>> +$id: "http://devicetree.org/schemas/power/amlogic,meson-sec-wdt.yaml#"
>>>>>> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
>>>>>> +
>>>>>> +title: Amlogic Meson Secure Watchdog Timer
>>>>>> +
>>>>>> +maintainers:
>>>>>> +  - Xingyu Chen <xingyu.chen@amlogic.com>
>>>>>> +
>>>>>> +description: |+
>>>>>> +  Secure Watchdog Timer used in Meson-A/C series Compatible SoCs
>>>>>> +
>>>>>> +properties:
>>>>>> +  compatible:
>>>>>> +    enum:
>>>>>> +      - amlogic,meson-sec-wdt
>>>>>
>>>>> If there are no other properties, then you don't need this. Just have
>>>>> the secure firmware driver instantiate the watchdog.
>>>> I'am very sorry i don't understand how to initialize the watchdog driver
>>>> if the compatible property is removed, Could you give me more
>>>> suggestions or examples ？ Thank you very much.
>>>
>>> platform_device_register_simple() from the secure firmware driver.
>> Thanks for your help. The device node of wdt looks useless if I use this
>> function to register device. if so, how should I get the pointer to
>> secure-monitor in wdt driver ? or should I use directly arm_smccc to
>> access the secfw ?
> 
> You can use of_find_compatible_node(). There should only be one firmware node.
Thanks for your answer.

I seem to miss something about registration of watchdog device. The 
secure watchdog driver is used only to A1/C1 compatible SoCs, but is not
support for previous SoCs (Eg: gxl axg).

I have to think about platform difference If I use the 
platform_device_register_simple() to register the wdt device in secure
fw driver, because fw driver is compatible with all known SoCs, but the 
secure wdt driver is only compatible with some SoCs. In other
words, the registered wdt device is useless for gxl or axg.

There is no such problem If I use the DT to describe the wdt device.

> 
> Rob
> 
> .
>