diff mbox series

[01/14] dt-bindings: watchdog: sunxi: fix F1C100s compatible

Message ID 20220307143421.1106209-2-andre.przywara@arm.com
State New
Headers show
Series [01/14] dt-bindings: watchdog: sunxi: fix F1C100s compatible | expand

Commit Message

Andre Przywara March 7, 2022, 2:34 p.m. UTC
The F1C100 series actually features a newer generation watchdog IP, so
the compatible string was wrong.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 .../devicetree/bindings/watchdog/allwinner,sun4i-a10-wdt.yaml   | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Rob Herring (Arm) March 8, 2022, 4:08 p.m. UTC | #1
On Mon, 07 Mar 2022 14:34:08 +0000, Andre Przywara wrote:
> The F1C100 series actually features a newer generation watchdog IP, so
> the compatible string was wrong.
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
>  .../devicetree/bindings/watchdog/allwinner,sun4i-a10-wdt.yaml   | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 

Acked-by: Rob Herring <robh@kernel.org>
Guenter Roeck March 9, 2022, 11:02 p.m. UTC | #2
On Mon, Mar 07, 2022 at 02:34:08PM +0000, Andre Przywara wrote:
> The F1C100 series actually features a newer generation watchdog IP, so
> the compatible string was wrong.
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> Acked-by: Rob Herring <robh@kernel.org>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
>  .../devicetree/bindings/watchdog/allwinner,sun4i-a10-wdt.yaml   | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/watchdog/allwinner,sun4i-a10-wdt.yaml b/Documentation/devicetree/bindings/watchdog/allwinner,sun4i-a10-wdt.yaml
> index 43afa24513b9..d90655418d0e 100644
> --- a/Documentation/devicetree/bindings/watchdog/allwinner,sun4i-a10-wdt.yaml
> +++ b/Documentation/devicetree/bindings/watchdog/allwinner,sun4i-a10-wdt.yaml
> @@ -29,7 +29,7 @@ properties:
>            - const: allwinner,sun6i-a31-wdt
>        - items:
>            - const: allwinner,suniv-f1c100s-wdt
> -          - const: allwinner,sun4i-a10-wdt
> +          - const: allwinner,sun6i-a31-wdt
>        - const: allwinner,sun20i-d1-wdt
>        - items:
>            - const: allwinner,sun20i-d1-wdt-reset
Samuel Holland March 10, 2022, 12:46 a.m. UTC | #3
Hi Andre,

On 3/7/22 8:34 AM, Andre Przywara wrote:
> The F1C100 series actually features a newer generation watchdog IP, so
> the compatible string was wrong.

The F1C100s watchdog seems to be unique in that it uses LOSC/osc32k as its only
clock source instead of HOSC/osc24M. The current binding requires that the first
clock is "hosc", so it seems that the binding needs to be relaxed to allow for
this case.

As long as there's only one clock source available, we don't really care where
it comes from. They are both divided to be approximately 32 kHz. So I don't
think this difference prevents using A31 as a fallback compatible.

> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
>  .../devicetree/bindings/watchdog/allwinner,sun4i-a10-wdt.yaml   | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/watchdog/allwinner,sun4i-a10-wdt.yaml b/Documentation/devicetree/bindings/watchdog/allwinner,sun4i-a10-wdt.yaml
> index 43afa24513b9..d90655418d0e 100644
> --- a/Documentation/devicetree/bindings/watchdog/allwinner,sun4i-a10-wdt.yaml
> +++ b/Documentation/devicetree/bindings/watchdog/allwinner,sun4i-a10-wdt.yaml
> @@ -29,7 +29,7 @@ properties:
>            - const: allwinner,sun6i-a31-wdt
>        - items:
>            - const: allwinner,suniv-f1c100s-wdt
> -          - const: allwinner,sun4i-a10-wdt
> +          - const: allwinner,sun6i-a31-wdt

This can be combined with the enum of other compatibles that fall back to
allwinner,sun6i-a31-wdt (earlier in the file).

Regards,
Samuel

>        - const: allwinner,sun20i-d1-wdt
>        - items:
>            - const: allwinner,sun20i-d1-wdt-reset
>
Andre Przywara March 14, 2022, 5:39 p.m. UTC | #4
On Wed, 9 Mar 2022 18:46:46 -0600
Samuel Holland <samuel@sholland.org> wrote:

> Hi Andre,
> 
> On 3/7/22 8:34 AM, Andre Przywara wrote:
> > The F1C100 series actually features a newer generation watchdog IP, so
> > the compatible string was wrong.  
> 
> The F1C100s watchdog seems to be unique in that it uses LOSC/osc32k as its only
> clock source instead of HOSC/osc24M. The current binding requires that the first
> clock is "hosc", so it seems that the binding needs to be relaxed to allow for
> this case.
> 
> As long as there's only one clock source available, we don't really care where
> it comes from. They are both divided to be approximately 32 kHz. So I don't
> think this difference prevents using A31 as a fallback compatible.

Right, these were roughly my findings as well, but I should have
written them down, at least in the commit message.

So shall the binding be explicit:
1) Most SoCs required exactly one clock, the 24 MHz HOSC.
2) The F1C100s requires exactly one clock, the 32KHz LOSC.
3) R329/D1 require two clocks with clock-names?

Or do you want to collapse 1) and 2) into one relaxed case? Still not
entirely sure what "LOSC / 32 KHz" means for the F1C100 (32768 or 32000
Hz), or where it really comes from, but it does not seem to matter.

FreeBSD, Xen and U-Boot don't care about clocks at all, and Linux
always uses the first clock and just enables it, so we should be good
either way.

> 
> > Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> > ---
> >  .../devicetree/bindings/watchdog/allwinner,sun4i-a10-wdt.yaml   | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/watchdog/allwinner,sun4i-a10-wdt.yaml b/Documentation/devicetree/bindings/watchdog/allwinner,sun4i-a10-wdt.yaml
> > index 43afa24513b9..d90655418d0e 100644
> > --- a/Documentation/devicetree/bindings/watchdog/allwinner,sun4i-a10-wdt.yaml
> > +++ b/Documentation/devicetree/bindings/watchdog/allwinner,sun4i-a10-wdt.yaml
> > @@ -29,7 +29,7 @@ properties:
> >            - const: allwinner,sun6i-a31-wdt
> >        - items:
> >            - const: allwinner,suniv-f1c100s-wdt
> > -          - const: allwinner,sun4i-a10-wdt
> > +          - const: allwinner,sun6i-a31-wdt  
> 
> This can be combined with the enum of other compatibles that fall back to
> allwinner,sun6i-a31-wdt (earlier in the file).

Oh, right, I missed that.

Cheers,
Andre

> 
> Regards,
> Samuel
> 
> >        - const: allwinner,sun20i-d1-wdt
> >        - items:
> >            - const: allwinner,sun20i-d1-wdt-reset
> >   
> 
>
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/watchdog/allwinner,sun4i-a10-wdt.yaml b/Documentation/devicetree/bindings/watchdog/allwinner,sun4i-a10-wdt.yaml
index 43afa24513b9..d90655418d0e 100644
--- a/Documentation/devicetree/bindings/watchdog/allwinner,sun4i-a10-wdt.yaml
+++ b/Documentation/devicetree/bindings/watchdog/allwinner,sun4i-a10-wdt.yaml
@@ -29,7 +29,7 @@  properties:
           - const: allwinner,sun6i-a31-wdt
       - items:
           - const: allwinner,suniv-f1c100s-wdt
-          - const: allwinner,sun4i-a10-wdt
+          - const: allwinner,sun6i-a31-wdt
       - const: allwinner,sun20i-d1-wdt
       - items:
           - const: allwinner,sun20i-d1-wdt-reset