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. Rob
