linux-kernel - Re: [PATCH RFC v3 3/4] mux: Add support for reading mux enable state from DT

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Tue, 30 Nov 2021 09:11:57 +0100
From:   Peter Rosin <peda@...ntia.se>
To:     Aswath Govindraju <a-govindraju@...com>
Cc:     Vignesh Raghavendra <vigneshr@...com>, Nishanth Menon <nm@...com>,
        Rob Herring <robh+dt@...nel.org>,
        Wolfgang Grandegger <wg@...ndegger.com>,
        Marc Kleine-Budde <mkl@...gutronix.de>,
        Kishon Vijay Abraham I <kishon@...com>,
        Vinod Koul <vkoul@...nel.org>, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-can@...r.kernel.org,
        linux-phy@...ts.infradead.org
Subject: Re: [PATCH RFC v3 3/4] mux: Add support for reading mux enable state
 from DT

On 2021-11-30 06:58, Aswath Govindraju wrote:
> On 30/11/21 11:14 am, Aswath Govindraju wrote:
>> On 25/11/21 7:22 pm, Peter Rosin wrote:
>>> With the suggested binding in my comment for patch 1/4, you'd need to do
>>> either
>>>
>>> 	ret = of_parse_phandle_with_args(np,
>>> 					 "mux-controls", "#mux-control-cells",
>>> 					 index, &args);
>>>
>>> or
>>>
>>> 	ret = of_parse_phandle_with_args(np,
>>> 					 "mux-states", "#mux-state-cells",
>>> 					 index, &args);
>>>
>>> depending on if the mux_get helper gets a NULL (enable_)state pointer or a "real"
>>> address, and then...
>>>
>>
>>
>> Sorry, while trying to implement the above method, I came across one
>> more question. So, in a given consumer DT node we will be either having
>> only mux-states or mux-controls right? How would we take care of the
>> condition when we would want to set the state of a given line in the
>> controller. Especially when a single mux chip is used by multiple
>> consumers each using a different line. Wouldn't we require both
>> mux-controls and mux-states in that case? So, shouldn't the
>> implementation be such that we need to first read mux-controls and then
>> based whether the enable_state is NULL, we read mux-states?
>>
>> Just to add more clarity, if we go about this method then we would have
>> both mux-controls and mux-states in the consumer DT node when we want to
>> specify the state.
>>
> 
> I now understood the implementation that you described. mux-states will
> be similar to the mux-controls after this patch is applied. mux-states
> can have 2 arguments(mux line and state) whereas mux-controls can have
> only one argument(mux line).
> 
> Sorry, for the inconvenience.

No trouble at all. And thanks for tackling this! I think it can open up
the mux subsystem to more uses.

I'm not sure if the devicetree folks like the concept though, there has
been no comment from that direction yet. Because it does feel a bit
unusual to potentially have both #mux-control-cells and #mux-state-cells
in a single mux provider node, especially when they are as related as
they are. But sharing a mux control between several consumer is perhaps
not the most common usage, so it will probably be the exception to
require both in actual usage. But I think all that will be clearer
when/if we see the actual binding patches.

Cheers,
Peter