* [PATCH v2 1/2] dt-bindings: clk: vc5: Add properties for configuring the SD/OE pin
@ 2021-06-14 15:54 Sean Anderson
2021-06-16 15:41 ` Luca Ceresoli
` (3 more replies)
0 siblings, 4 replies; 6+ messages in thread
From: Sean Anderson @ 2021-06-14 15:54 UTC (permalink / raw)
To: linux-clk, Luca Ceresoli
Cc: Michael Turquette, Adam Ford, Stephen Boyd, Sean Anderson,
Rob Herring, devicetree
These properties allow configuring the SD/OE pin as described in the
datasheet.
Signed-off-by: Sean Anderson <sean.anderson@seco.com>
---
Changes in v2:
- Rename idt,sd-active-high to idt,output-enable-active-high
- Add idt,enable-shutdown
.../bindings/clock/idt,versaclock5.yaml | 33 +++++++++++++++++++
1 file changed, 33 insertions(+)
diff --git a/Documentation/devicetree/bindings/clock/idt,versaclock5.yaml b/Documentation/devicetree/bindings/clock/idt,versaclock5.yaml
index 28675b0b80f1..79d67fad5284 100644
--- a/Documentation/devicetree/bindings/clock/idt,versaclock5.yaml
+++ b/Documentation/devicetree/bindings/clock/idt,versaclock5.yaml
@@ -30,6 +30,22 @@ description: |
3 -- OUT3
4 -- OUT4
+ The idt,enable-shutdown and idt,output-enable-active-high properties
+ correspond to the SH and SP bits of the Primary Source and Shutdown
+ Register, respectively. Their behavior is summarized by the following
+ table:
+
+ SH SP SD/OE Output
+ == == ===== ========
+ 0 0 Low Active
+ 0 0 High Inactive
+ 0 1 Low Inactive
+ 0 1 High Active
+ 1 0 Low Active
+ 1 0 High Shutdown
+ 1 1 Low Inactive
+ 1 1 High Shutdown
+
maintainers:
- Luca Ceresoli <luca@lucaceresoli.net>
@@ -64,6 +80,23 @@ properties:
maximum: 22760
description: Optional load capacitor for XTAL1 and XTAL2
+ idt,enable-shutdown:
+ $ref: /schemas/types.yaml#/definitions/flag
+ description: |
+ Enable the shutdown function when the SD/OE pin is high. This
+ corresponds to setting the SH bit of the Primary Source and
+ Shutdown Register. If this property is set, it takes precedence
+ over the usual enable/disable semantics of the SD/OE pin.
+
+ idt,output-enable-active-high:
+ $ref: /schemas/types.yaml#/definitions/flag
+ description: |
+ This enables output when the SD/OE pin is high, and disables
+ output when the SD/OE pin is low. This corresponds to setting the
+ SP bit of the Primary Source and Shutdown Register. If this
+ property is not present, then the SD/OE pin has the opposite
+ polarity (enabled when low, disabled when high).
+
patternProperties:
"^OUT[1-4]$":
type: object
--
2.25.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/2] dt-bindings: clk: vc5: Add properties for configuring the SD/OE pin
2021-06-14 15:54 [PATCH v2 1/2] dt-bindings: clk: vc5: Add properties for configuring the SD/OE pin Sean Anderson
@ 2021-06-16 15:41 ` Luca Ceresoli
2021-06-24 20:37 ` Rob Herring
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Luca Ceresoli @ 2021-06-16 15:41 UTC (permalink / raw)
To: Sean Anderson, linux-clk
Cc: Michael Turquette, Adam Ford, Stephen Boyd, Rob Herring, devicetree
On 14/06/21 17:54, Sean Anderson wrote:
> These properties allow configuring the SD/OE pin as described in the
> datasheet.
>
> Signed-off-by: Sean Anderson <sean.anderson@seco.com>
Reviewed-by: Luca Ceresoli <luca@lucaceresoli.net>
--
Luca
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/2] dt-bindings: clk: vc5: Add properties for configuring the SD/OE pin
2021-06-14 15:54 [PATCH v2 1/2] dt-bindings: clk: vc5: Add properties for configuring the SD/OE pin Sean Anderson
2021-06-16 15:41 ` Luca Ceresoli
@ 2021-06-24 20:37 ` Rob Herring
2021-06-28 2:31 ` Stephen Boyd
[not found] ` <20210614155437.3979771-2-sean.anderson@seco.com>
3 siblings, 0 replies; 6+ messages in thread
From: Rob Herring @ 2021-06-24 20:37 UTC (permalink / raw)
To: Sean Anderson
Cc: devicetree, Luca Ceresoli, linux-clk, Adam Ford, Stephen Boyd,
Michael Turquette
On Mon, 14 Jun 2021 11:54:36 -0400, Sean Anderson wrote:
> These properties allow configuring the SD/OE pin as described in the
> datasheet.
>
> Signed-off-by: Sean Anderson <sean.anderson@seco.com>
> ---
>
> Changes in v2:
> - Rename idt,sd-active-high to idt,output-enable-active-high
> - Add idt,enable-shutdown
>
> .../bindings/clock/idt,versaclock5.yaml | 33 +++++++++++++++++++
> 1 file changed, 33 insertions(+)
>
Acked-by: Rob Herring <robh@kernel.org>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/2] dt-bindings: clk: vc5: Add properties for configuring the SD/OE pin
2021-06-14 15:54 [PATCH v2 1/2] dt-bindings: clk: vc5: Add properties for configuring the SD/OE pin Sean Anderson
2021-06-16 15:41 ` Luca Ceresoli
2021-06-24 20:37 ` Rob Herring
@ 2021-06-28 2:31 ` Stephen Boyd
[not found] ` <20210614155437.3979771-2-sean.anderson@seco.com>
3 siblings, 0 replies; 6+ messages in thread
From: Stephen Boyd @ 2021-06-28 2:31 UTC (permalink / raw)
To: Luca Ceresoli, Sean Anderson, linux-clk
Cc: Michael Turquette, Adam Ford, Sean Anderson, Rob Herring, devicetree
Quoting Sean Anderson (2021-06-14 08:54:36)
> These properties allow configuring the SD/OE pin as described in the
> datasheet.
>
> Signed-off-by: Sean Anderson <sean.anderson@seco.com>
> ---
Applied to clk-next
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 2/2] clk: vc5: Add properties for configuring SD/OE behavior
[not found] ` <3174eed5-1078-68c4-4d98-95c448cd0940@seco.com>
@ 2021-06-29 12:42 ` Geert Uytterhoeven
2021-06-29 15:49 ` Sean Anderson
0 siblings, 1 reply; 6+ messages in thread
From: Geert Uytterhoeven @ 2021-06-29 12:42 UTC (permalink / raw)
To: Sean Anderson
Cc: Luca Ceresoli, linux-clk, Michael Turquette, Adam Ford,
Stephen Boyd, Kieran Bingham, Laurent Pinchart, Linux-Renesas,
Rob Herring,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS
Hi Sean,
On Thu, Jun 17, 2021 at 4:53 PM Sean Anderson <sean.anderson@seco.com> wrote:
> On 6/16/21 11:41 AM, Luca Ceresoli wrote:
> > On 14/06/21 17:54, Sean Anderson wrote:
> >> The SD/OE pin may be configured to enable output when high or low, and
> >> to shutdown the device when high. This behavior is controller by the SH
> >> and SP bits of the Primary Source and Shutdown Register (and to a lesser
> >> extent the OS and OE bits). By default, both bits are 0, but they may
> >> need to be configured differently, depending on the external circuitry
> >> controlling the SD/OE pin.
> >>
> >> Signed-off-by: Sean Anderson <sean.anderson@seco.com>
> >
> > Reviewed-by: Luca Ceresoli <luca@lucaceresoli.net>
> >
> >> @@ -914,6 +915,15 @@ static int vc5_probe(struct i2c_client *client, const struct i2c_device_id *id)
> >> return PTR_ERR(vc5->regmap);
> >> }
> >>
> >> + oe_polarity = of_property_read_bool(client->dev.of_node,
> >> + "idt,output-enable-active-high");
> >> + sd_enable = of_property_read_bool(client->dev.of_node,
> >> + "idt,enable-shutdown");
> >> + regmap_update_bits(vc5->regmap, VC5_PRIM_SRC_SHDN,
> >> + VC5_PRIM_SRC_SHDN_SP | VC5_PRIM_SRC_SHDN_EN_GBL_SHDN,
> >> + (oe_polarity ? VC5_PRIM_SRC_SHDN_SP : 0)
> >> + | (sd_enable ? VC5_PRIM_SRC_SHDN_EN_GBL_SHDN : 0));
> >> +
> >
> > Did you test all combinations?
>
> No. I only tested "idt,output-enable-active-high". Though I also in
> effect tested the default case (both zero) as well.
>
> One potential impact of this patch could be that systems which enabled
> the SP and SH bits via OTP could end up inadvertently disabling them
> because they need to add the appropriate property. Maintaining full
> backwards compatibility would require a tri-state property of some kind.
And that seems to be exactly what's happening for me...
I've just discovered a failure on Renesas Salvator-XS caused by this
patch, which is now commit e26b493f3495e8a2 ("clk: vc5: Add properties
for configuring SD/OE behavior") in clk-next:
[dm:drm_atomic_helper_wait_for_flip_done] *ERROR* [CRTC:76:crtc-3]
flip_done timed out
[drm:drm_crtc_commit_wait] *ERROR* flip_done timed out
[...]
Printing the value of VC5_PRIM_SRC_SHDN before/after the update:
vc5 4-006a: vc5_probe:945: 0x8a
vc5 4-006a: vc5_probe:951: 0x88
My initial bisection failed, as the register contents are retained
across a reset. Hence booting a "good" kernel after a "bad" kernel
still fails, unless the VC5 is power-cycled in between.
So I think we do need separate "idt,output-enable-active-low" and
"idt,disable-shutdown" properties, and not touch the bits if none of
the corresponding properties is present.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 2/2] clk: vc5: Add properties for configuring SD/OE behavior
2021-06-29 12:42 ` [PATCH v2 2/2] clk: vc5: Add properties for configuring SD/OE behavior Geert Uytterhoeven
@ 2021-06-29 15:49 ` Sean Anderson
0 siblings, 0 replies; 6+ messages in thread
From: Sean Anderson @ 2021-06-29 15:49 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Luca Ceresoli, linux-clk, Michael Turquette, Adam Ford,
Stephen Boyd, Kieran Bingham, Laurent Pinchart, Linux-Renesas,
Rob Herring,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS
On 6/29/21 8:42 AM, Geert Uytterhoeven wrote:
> Hi Sean,
>
> On Thu, Jun 17, 2021 at 4:53 PM Sean Anderson <sean.anderson@seco.com> wrote:
>> On 6/16/21 11:41 AM, Luca Ceresoli wrote:
>> > On 14/06/21 17:54, Sean Anderson wrote:
>> >> The SD/OE pin may be configured to enable output when high or low, and
>> >> to shutdown the device when high. This behavior is controller by the SH
>> >> and SP bits of the Primary Source and Shutdown Register (and to a lesser
>> >> extent the OS and OE bits). By default, both bits are 0, but they may
>> >> need to be configured differently, depending on the external circuitry
>> >> controlling the SD/OE pin.
>> >>
>> >> Signed-off-by: Sean Anderson <sean.anderson@seco.com>
>> >
>> > Reviewed-by: Luca Ceresoli <luca@lucaceresoli.net>
>> >
>> >> @@ -914,6 +915,15 @@ static int vc5_probe(struct i2c_client *client, const struct i2c_device_id *id)
>> >> return PTR_ERR(vc5->regmap);
>> >> }
>> >>
>> >> + oe_polarity = of_property_read_bool(client->dev.of_node,
>> >> + "idt,output-enable-active-high");
>> >> + sd_enable = of_property_read_bool(client->dev.of_node,
>> >> + "idt,enable-shutdown");
>> >> + regmap_update_bits(vc5->regmap, VC5_PRIM_SRC_SHDN,
>> >> + VC5_PRIM_SRC_SHDN_SP | VC5_PRIM_SRC_SHDN_EN_GBL_SHDN,
>> >> + (oe_polarity ? VC5_PRIM_SRC_SHDN_SP : 0)
>> >> + | (sd_enable ? VC5_PRIM_SRC_SHDN_EN_GBL_SHDN : 0));
>> >> +
>> >
>> > Did you test all combinations?
>>
>> No. I only tested "idt,output-enable-active-high". Though I also in
>> effect tested the default case (both zero) as well.
>>
>> One potential impact of this patch could be that systems which enabled
>> the SP and SH bits via OTP could end up inadvertently disabling them
>> because they need to add the appropriate property. Maintaining full
>> backwards compatibility would require a tri-state property of some kind.
>
> And that seems to be exactly what's happening for me...
>
> I've just discovered a failure on Renesas Salvator-XS caused by this
> patch, which is now commit e26b493f3495e8a2 ("clk: vc5: Add properties
> for configuring SD/OE behavior") in clk-next:
>
> [dm:drm_atomic_helper_wait_for_flip_done] *ERROR* [CRTC:76:crtc-3]
> flip_done timed out
> [drm:drm_crtc_commit_wait] *ERROR* flip_done timed out
> [...]
>
> Printing the value of VC5_PRIM_SRC_SHDN before/after the update:
>
> vc5 4-006a: vc5_probe:945: 0x8a
> vc5 4-006a: vc5_probe:951: 0x88
>
> My initial bisection failed, as the register contents are retained
> across a reset. Hence booting a "good" kernel after a "bad" kernel
> still fails, unless the VC5 is power-cycled in between.
>
> So I think we do need separate "idt,output-enable-active-low" and
> "idt,disable-shutdown" properties, and not touch the bits if none of
> the corresponding properties is present.
Ok, I've submitted a v3 with these properties added.
--Sean
>
> Gr{oetje,eeting}s,
>
> Geert
>
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
> -- Linus Torvalds
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-06-29 15:50 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-14 15:54 [PATCH v2 1/2] dt-bindings: clk: vc5: Add properties for configuring the SD/OE pin Sean Anderson
2021-06-16 15:41 ` Luca Ceresoli
2021-06-24 20:37 ` Rob Herring
2021-06-28 2:31 ` Stephen Boyd
[not found] ` <20210614155437.3979771-2-sean.anderson@seco.com>
[not found] ` <47b37414-6587-0792-201b-e255feeee9c9@lucaceresoli.net>
[not found] ` <3174eed5-1078-68c4-4d98-95c448cd0940@seco.com>
2021-06-29 12:42 ` [PATCH v2 2/2] clk: vc5: Add properties for configuring SD/OE behavior Geert Uytterhoeven
2021-06-29 15:49 ` Sean Anderson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).