Re: [RFC 1/2] dt-bindings: add mmio-based syscon mux controller DT bindings

From: Steve Longerbeam
Date: Thu Apr 13 2017 - 21:04:47 EST




On 04/13/2017 08:48 AM, Philipp Zabel wrote:
This adds device tree binding documentation for mmio-based syscon
multiplexers controlled by a single bitfield in a syscon register
range.

Nice! (you beat me to it, I was about to embark on this myself :)

Looks good to me, just some minor comments below.


Signed-off-by: Philipp Zabel <p.zabel@xxxxxxxxxxxxxx>
---
Documentation/devicetree/bindings/mux/mmio-mux.txt | 56 ++++++++++++++++++++++
1 file changed, 56 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mux/mmio-mux.txt

diff --git a/Documentation/devicetree/bindings/mux/mmio-mux.txt b/Documentation/devicetree/bindings/mux/mmio-mux.txt
new file mode 100644
index 0000000000000..11d96f5d98583
--- /dev/null
+++ b/Documentation/devicetree/bindings/mux/mmio-mux.txt
@@ -0,0 +1,56 @@
+MMIO bitfield-based multiplexer controller bindings
+
+Define a syscon bitfield to be used to control a multiplexer. The parent

I think "Define a register bitfield to be used ..." is more clear.

+device tree node must be a syscon node to provide register access.
+
+Required properties:
+- compatible : "gpio-mux"

Er, "mmio-mux"

+- reg : register base of the register containing the control bitfield
+- bit-mask : bitmask of the control bitfield in the control register
+- bit-shift : bit offset of the control bitfield in the control register
+- #mux-control-cells : <0>
+* Standard mux-controller bindings as decribed in mux-controller.txt
+
+Optional properties:
+- idle-state : if present, the state the mux will have when idle. The
+ special state MUX_IDLE_AS_IS is the default.
+
+The multiplexer state is defined as the value of the bitfield described
+by the reg, bit-mask, and bit-shift properties, accessed through the parent
+syscon.
+
+Example:
+
+ syscon {
+ compatible = "syscon";
+
+ mux: mux-controller@3 {
+ compatible = "mmio-mux";
+ reg = <0x3>;
+ bit-mask = <0x1>;
+ bit-shift = <5>;
+ #mux-control-cells = <0>;
+ };
+ };
+
+ video-mux {

I like this as an example consumer of a mmio-mux, but just
the same some might argue this doesn't really fit here.

Steve

+ compatible = "video-mux";
+ mux-controls = <&mux>;
+
+ ports {
+ /* input 0 */
+ port@0 {
+ reg = <0>;
+ };
+
+ /* input 1 */
+ port@1 {
+ reg = <1>;
+ };
+
+ /* output */
+ port@2 {
+ reg = <2>;
+ };
+ };
+ };