diff mbox series

[7/7] arm64: dts: qcom: sm8250: add watchdog device

Message ID 20200604004331.669936-7-dmitry.baryshkov@linaro.org (mailing list archive)
State Superseded
Headers show
Series [1/7] arm64: dts: qcom: pm8009: Add base dts file | expand

Commit Message

Dmitry Baryshkov June 4, 2020, 12:43 a.m. UTC
Add on-SoC watchdog device node.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 arch/arm64/boot/dts/qcom/sm8250.dtsi | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

Manivannan Sadhasivam June 4, 2020, 6:06 a.m. UTC | #1
On Thu, Jun 04, 2020 at 03:43:31AM +0300, Dmitry Baryshkov wrote:
> Add on-SoC watchdog device node.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

Acked-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

Thanks,
Mani

> ---
>  arch/arm64/boot/dts/qcom/sm8250.dtsi | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi b/arch/arm64/boot/dts/qcom/sm8250.dtsi
> index 972d8e04c8a2..f1641c6fe203 100644
> --- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
> @@ -1662,6 +1662,12 @@ config {
>  			};
>  		};
>  
> +		watchdog@17c10000 {
> +			compatible = "qcom,apss-wdt-sm8250", "qcom,kpss-wdt";
> +			reg = <0 0x17c10000 0 0x1000>;
> +			clocks = <&sleep_clk>;
> +		};
> +
>  		timer@17c20000 {
>  			#address-cells = <2>;
>  			#size-cells = <2>;
> -- 
> 2.26.2
>
Vinod Koul June 4, 2020, 10:52 a.m. UTC | #2
On 04-06-20, 03:43, Dmitry Baryshkov wrote:
> Add on-SoC watchdog device node.

Reviewed-by: Vinod Koul <vkoul@kernel.org>
Sai Prakash Ranjan June 4, 2020, 11:07 a.m. UTC | #3
On 2020-06-04 06:13, Dmitry Baryshkov wrote:
> Add on-SoC watchdog device node.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>  arch/arm64/boot/dts/qcom/sm8250.dtsi | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi
> b/arch/arm64/boot/dts/qcom/sm8250.dtsi
> index 972d8e04c8a2..f1641c6fe203 100644
> --- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
> @@ -1662,6 +1662,12 @@ config {
>  			};
>  		};
> 
> +		watchdog@17c10000 {
> +			compatible = "qcom,apss-wdt-sm8250", "qcom,kpss-wdt";

Need to add this compatible to bindings.

Thanks,
Sai
Manivannan Sadhasivam June 4, 2020, 11:35 a.m. UTC | #4
On Thu, Jun 04, 2020 at 04:37:01PM +0530, Sai Prakash Ranjan wrote:
> On 2020-06-04 06:13, Dmitry Baryshkov wrote:
> > Add on-SoC watchdog device node.
> > 
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > ---
> >  arch/arm64/boot/dts/qcom/sm8250.dtsi | 6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi
> > b/arch/arm64/boot/dts/qcom/sm8250.dtsi
> > index 972d8e04c8a2..f1641c6fe203 100644
> > --- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
> > @@ -1662,6 +1662,12 @@ config {
> >  			};
> >  		};
> > 
> > +		watchdog@17c10000 {
> > +			compatible = "qcom,apss-wdt-sm8250", "qcom,kpss-wdt";
> 
> Need to add this compatible to bindings.
> 

I did look into this but the binding says,
"compatible : shall contain only one of the following"

So clearly the fallback is not going to work and there is no need to add a
dedicated compatible in the driver. The binding is not in the YAML format to
be validated but still... Other option is to convert the binding to YAML and
make the compatibles listed as 'enum' and 'const' elements appropriately.

Thanks,
Mani

> Thanks,
> Sai
> -- 
> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
> of Code Aurora Forum, hosted by The Linux Foundation
Sai Prakash Ranjan June 4, 2020, 11:51 a.m. UTC | #5
On 2020-06-04 17:05, Manivannan Sadhasivam wrote:
> On Thu, Jun 04, 2020 at 04:37:01PM +0530, Sai Prakash Ranjan wrote:
>> On 2020-06-04 06:13, Dmitry Baryshkov wrote:
>> > Add on-SoC watchdog device node.
>> >
>> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>> > ---
>> >  arch/arm64/boot/dts/qcom/sm8250.dtsi | 6 ++++++
>> >  1 file changed, 6 insertions(+)
>> >
>> > diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi
>> > b/arch/arm64/boot/dts/qcom/sm8250.dtsi
>> > index 972d8e04c8a2..f1641c6fe203 100644
>> > --- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
>> > +++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
>> > @@ -1662,6 +1662,12 @@ config {
>> >  			};
>> >  		};
>> >
>> > +		watchdog@17c10000 {
>> > +			compatible = "qcom,apss-wdt-sm8250", "qcom,kpss-wdt";
>> 
>> Need to add this compatible to bindings.
>> 
> 
> I did look into this but the binding says,
> "compatible : shall contain only one of the following"
> 
> So clearly the fallback is not going to work and there is no need to 
> add a
> dedicated compatible in the driver. The binding is not in the YAML 
> format to
> be validated but still... Other option is to convert the binding to 
> YAML and
> make the compatibles listed as 'enum' and 'const' elements 
> appropriately.
> 

I already converted the bindings to YAML and added the missing 
compatibles for
other SoCs[1] . I thought it was already merged since Rob has already 
reviewed
them, but seems like it was missed. Bjorn, can you please take it?

[1] https://lore.kernel.org/patchwork/cover/1192757/

Thanks,
Sai
Manivannan Sadhasivam June 4, 2020, 12:22 p.m. UTC | #6
On Thu, Jun 04, 2020 at 05:21:46PM +0530, Sai Prakash Ranjan wrote:
> On 2020-06-04 17:05, Manivannan Sadhasivam wrote:
> > On Thu, Jun 04, 2020 at 04:37:01PM +0530, Sai Prakash Ranjan wrote:
> > > On 2020-06-04 06:13, Dmitry Baryshkov wrote:
> > > > Add on-SoC watchdog device node.
> > > >
> > > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > > > ---
> > > >  arch/arm64/boot/dts/qcom/sm8250.dtsi | 6 ++++++
> > > >  1 file changed, 6 insertions(+)
> > > >
> > > > diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi
> > > > b/arch/arm64/boot/dts/qcom/sm8250.dtsi
> > > > index 972d8e04c8a2..f1641c6fe203 100644
> > > > --- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
> > > > +++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
> > > > @@ -1662,6 +1662,12 @@ config {
> > > >  			};
> > > >  		};
> > > >
> > > > +		watchdog@17c10000 {
> > > > +			compatible = "qcom,apss-wdt-sm8250", "qcom,kpss-wdt";
> > > 
> > > Need to add this compatible to bindings.
> > > 
> > 
> > I did look into this but the binding says,
> > "compatible : shall contain only one of the following"
> > 
> > So clearly the fallback is not going to work and there is no need to add
> > a
> > dedicated compatible in the driver. The binding is not in the YAML
> > format to
> > be validated but still... Other option is to convert the binding to YAML
> > and
> > make the compatibles listed as 'enum' and 'const' elements
> > appropriately.
> > 
> 
> I already converted the bindings to YAML and added the missing compatibles
> for
> other SoCs[1] .

Ah, okay. I didn't find it in linux-next. Anyway, for adding
"qcom,apss-wdt-sm8250" compatible, we need to group the compatibles wisely
using 'enum' and 'const'.

> I thought it was already merged since Rob has already
> reviewed
> them, but seems like it was missed. Bjorn, can you please take it?
> 

Perhaps for v5.9...

Thanks,
Mani

> [1] https://lore.kernel.org/patchwork/cover/1192757/
> 
> Thanks,
> Sai
> 
> -- 
> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
> of Code Aurora Forum, hosted by The Linux Foundation
Sai Prakash Ranjan June 4, 2020, 12:52 p.m. UTC | #7
On 2020-06-04 17:52, Manivannan Sadhasivam wrote:
> On Thu, Jun 04, 2020 at 05:21:46PM +0530, Sai Prakash Ranjan wrote:
>> On 2020-06-04 17:05, Manivannan Sadhasivam wrote:
>> > On Thu, Jun 04, 2020 at 04:37:01PM +0530, Sai Prakash Ranjan wrote:
>> > > On 2020-06-04 06:13, Dmitry Baryshkov wrote:
>> > > > Add on-SoC watchdog device node.
>> > > >
>> > > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>> > > > ---
>> > > >  arch/arm64/boot/dts/qcom/sm8250.dtsi | 6 ++++++
>> > > >  1 file changed, 6 insertions(+)
>> > > >
>> > > > diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi
>> > > > b/arch/arm64/boot/dts/qcom/sm8250.dtsi
>> > > > index 972d8e04c8a2..f1641c6fe203 100644
>> > > > --- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
>> > > > +++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
>> > > > @@ -1662,6 +1662,12 @@ config {
>> > > >  			};
>> > > >  		};
>> > > >
>> > > > +		watchdog@17c10000 {
>> > > > +			compatible = "qcom,apss-wdt-sm8250", "qcom,kpss-wdt";
>> > >
>> > > Need to add this compatible to bindings.
>> > >
>> >
>> > I did look into this but the binding says,
>> > "compatible : shall contain only one of the following"
>> >
>> > So clearly the fallback is not going to work and there is no need to add
>> > a
>> > dedicated compatible in the driver. The binding is not in the YAML
>> > format to
>> > be validated but still... Other option is to convert the binding to YAML
>> > and
>> > make the compatibles listed as 'enum' and 'const' elements
>> > appropriately.
>> >
>> 
>> I already converted the bindings to YAML and added the missing 
>> compatibles
>> for
>> other SoCs[1] .
> 
> Ah, okay. I didn't find it in linux-next. Anyway, for adding
> "qcom,apss-wdt-sm8250" compatible, we need to group the compatibles 
> wisely
> using 'enum' and 'const'.
> 

Yes.

>> I thought it was already merged since Rob has already
>> reviewed
>> them, but seems like it was missed. Bjorn, can you please take it?
>> 
> 
> Perhaps for v5.9...
> 

Sure, no problem.

Thanks,
Sai
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi b/arch/arm64/boot/dts/qcom/sm8250.dtsi
index 972d8e04c8a2..f1641c6fe203 100644
--- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
@@ -1662,6 +1662,12 @@  config {
 			};
 		};
 
+		watchdog@17c10000 {
+			compatible = "qcom,apss-wdt-sm8250", "qcom,kpss-wdt";
+			reg = <0 0x17c10000 0 0x1000>;
+			clocks = <&sleep_clk>;
+		};
+
 		timer@17c20000 {
 			#address-cells = <2>;
 			#size-cells = <2>;