* [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
[parent not found: <20210614155437.3979771-2-sean.anderson@seco.com>]
[parent not found: <47b37414-6587-0792-201b-e255feeee9c9@lucaceresoli.net>]
[parent not found: <3174eed5-1078-68c4-4d98-95c448cd0940@seco.com>]
* 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 read-only IMAP folder(s) and NNTP newsgroup(s).