Hello Rob,
On 3/6/2025 3:44 AM, Rob Herring wrote:
On Wed, Mar 5, 2025 at 3:43 PM Vankar, Chintan <c-vankar@xxxxxx> wrote:
Hello Rob,
On 3/5/2025 2:10 AM, Rob Herring wrote:
On Tue, Mar 4, 2025 at 1:03 PM Vankar, Chintan <c-vankar@xxxxxx> wrote:
Hello Rob,
On 3/4/2025 9:09 PM, Rob Herring wrote:
On Tue, Mar 04, 2025 at 03:53:05PM +0530, Chintan Vankar wrote:
DT-binding of reg-mux is defined in such a way that one need to provide
register offset and mask in a "mux-reg-masks" property and corresponding
register value in "idle-states" property. This constraint forces to define
these values in such a way that "mux-reg-masks" and "idle-states" must be
in sync with each other. This implementation would be more complex if
specific register or set of registers need to be configured which has
large memory space. Introduce a new property "mux-reg-masks-state" which
allow to specify offset, mask and value as a tuple in a single property.
Maybe in hindsight that would have been better, but having 2 ways to
specify the same thing that we have to maintain forever is not an
improvement.
No one is making you use this binding. If you have a large number of
muxes, then maybe you should use a specific binding.
Thank you for reviewing the patch. The reason behind choosing mux
subsystem is working and implementation of mmio driver. As we can see
that implementing this new property in mux-controller is almost
identical to mmio driver, and it would make it easier to define and
extend mux-controller's functionality. If we introduce the new driver
than that would be most likely a clone of mmio driver.
I'm talking about the binding, not the driver. They are independent.
Generic drivers are great. I love them. Generic bindings, not so much.
Let me know if implementation would be accepted by adding a new
compatible for it.
Adding a new compatible to the mmio driver? Certainly. That happens
all the time.
I also didn't say don't use this binding as-is. That's fine too.
Can you please review the following binding:
oneOf:
- required: [ mux-reg-masks ]
- required: [ mux-reg-masks-state ]
allOf:
- if:
required:
- mux-reg-masks-state
then:
properties:
idle-states: false
required:
- compatible
- '#mux-control-cells'
I think it won't disturb the current bindings and keep backward
compatibility with existing implementation.
Wasn't that the case before? There's nothing really different here.
No, the binding before was not considering "mux-reg-masks" as required
property which was breaking actual dt-binding. In this binding, one of
the properties from "mux-reg-masks" or "mux-reg-masks-state" is required
which keeps the binding as it is unless someone wants to use
"mux-reg-masks-state".
Regards,
Chintan.
Rob
