From: Philipp Zabel <p.zabel@pengutronix.de>
To: Peter Rosin <peda@axentia.se>
Cc: Rob Herring <robh@kernel.org>,
Mark Rutland <mark.rutland@arm.com>,
Sakari Ailus <sakari.ailus@iki.fi>,
Steve Longerbeam <slongerbeam@gmail.com>,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
kernel@pengutronix.de
Subject: Re: [RFC 1/2] dt-bindings: add mmio-based syscon mux controller DT bindings
Date: Thu, 20 Apr 2017 15:03:13 +0200 [thread overview]
Message-ID: <1492693393.2158.71.camel@pengutronix.de> (raw)
In-Reply-To: <3f6e0337-46e1-fcfd-430b-4221e45c6ca8@axentia.se>
On Thu, 2017-04-20 at 13:57 +0200, Peter Rosin wrote:
> On 2017-04-20 10:14, Philipp Zabel wrote:
> > Hi Rob,
> >
> > On Wed, 2017-04-19 at 17:09 -0500, Rob Herring wrote:
> >> On Thu, Apr 13, 2017 at 05:48:11PM +0200, Philipp Zabel wrote:
> >>> This adds device tree binding documentation for mmio-based syscon
> >>> multiplexers controlled by a single bitfield in a syscon register
> >>> range.
> >>>
> >>> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> >>> ---
> >>> 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
> >>> +device tree node must be a syscon node to provide register access.
> >>> +
> >>> +Required properties:
> >>> +- compatible : "gpio-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>;
> >>
> >> This pattern doesn't scale once you have multiple fields @ addr 3. I
> >> also don't really think a node per register field in DT really scales.
> >
> > Thanks, I have been a bit uneasy with the separate per-bitfield mux
> > controller node, so I'm eager to agree. But thit makes me unsure how to
> > best represent the information that is spelled out above.
> >
> >> I think the parent should be declared as a mux controller instead.
> >
> > The syscon node itself should be the mux controller? Would you expect
> > the mmio-mux driver bind to the syscon node, or should the mux framework
> > handle creation of the mux controls in this case (i.e. does the syscon
> > node get a "mmio-mux" added to its compatible list)?
> >
> >> You could encode the mux addr and bit position in the mux cells.
> >
> > What about the bit-mask / bitfield width? Just add a cell for it?
> >
> > gpr: syscon {
> > compatible = "mmio-mux", "syscon", "simple-mfd";
> > #mux-control-cells = <3>;
> >
> > video-mux {
> > compatible = "video-mux";
> > /* register 0x3, bits [6:5] */
> > mux-controls = <&gpr 0x3 5 0x3>;
> >
> > ports {
> > /* ports 0..5 */
> > };
> > };
> > };
> >
> > Or maybe using MSB and LSB would be better to read?
> >
> > video-mux {
> > /* register 0x3, bits [6:5] */
> > mux-control = <&gpr 0x3 6 5>;
> >
> > ports {
> > /* ports 0..5 */
> > };
> > };
>
> Why do you need three values for one register+field? The shift can be
> implied from the mask, if the mask is pre-shifted. I.e. specifying a
> mask of 0x60 in this case. What I'm I missing?
As long as we have <= 32-bit hardware registers, that would work.
The question then is if things like
mux-control = <&gpr 0x04 0x00300000>;
are considered readable/reviewable enough. And what happens when we get
64-bit general purpose registers containing muxes? Also a binding like
this would allow non-contiguous bit masks.
The reason I suggested using bit-shift in the first place was that there
are already other bindings using "bit-shift" or "reg-shift" properties.
regards
Philipp
next prev parent reply other threads:[~2017-04-20 13:03 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-04-13 15:48 [RFC 1/2] dt-bindings: add mmio-based syscon mux controller DT bindings Philipp Zabel
2017-04-13 15:48 ` [RFC 2/2] mux: mmio-based syscon mux controller Philipp Zabel
2017-04-14 1:09 ` Steve Longerbeam
2017-04-19 11:50 ` Philipp Zabel
2017-04-19 11:58 ` Peter Rosin
2017-04-19 15:27 ` Philipp Zabel
2017-04-19 16:23 ` Steve Longerbeam
2017-04-19 16:32 ` Philipp Zabel
2017-04-19 16:42 ` Peter Rosin
2017-04-14 1:03 ` [RFC 1/2] dt-bindings: add mmio-based syscon mux controller DT bindings Steve Longerbeam
2017-04-19 11:47 ` Philipp Zabel
2017-04-18 8:19 ` Philipp Zabel
2017-04-18 10:08 ` Sakari Ailus
2017-04-18 10:34 ` Pavel Machek
2017-04-18 10:55 ` Sakari Ailus
2017-04-18 11:51 ` Pavel Machek
2017-04-18 10:51 ` Philipp Zabel
2017-04-19 22:09 ` Rob Herring
2017-04-20 8:14 ` Philipp Zabel
2017-04-20 11:57 ` Peter Rosin
2017-04-20 13:03 ` Philipp Zabel [this message]
2017-04-20 13:39 ` Rob Herring
2017-04-20 13:32 ` Peter Rosin
2017-04-20 14:13 ` Peter Rosin
2017-04-20 14:50 ` Philipp Zabel
2017-04-20 15:01 ` Peter Rosin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1492693393.2158.71.camel@pengutronix.de \
--to=p.zabel@pengutronix.de \
--cc=devicetree@vger.kernel.org \
--cc=kernel@pengutronix.de \
--cc=linux-kernel@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=peda@axentia.se \
--cc=robh@kernel.org \
--cc=sakari.ailus@iki.fi \
--cc=slongerbeam@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).