On Thu, Apr 16, 2020 at 04:28:42PM -0500, Rob Herring wrote: > On Thu, Apr 16, 2020 at 10:56:20PM +0300, Sergey Semin wrote: > > Rob, > > Any comment on my suggestion below? > > > > Regards, > > -Sergey > > > > On Tue, Mar 31, 2020 at 10:50:53PM +0300, Sergey Semin wrote: > > > On Wed, Mar 18, 2020 at 05:14:25PM -0600, Rob Herring wrote: > > > > On Fri, Mar 13, 2020 at 7:03 AM Sergey Semin > > > > <Sergey.Semin@xxxxxxxxxxxxxxxxxxxx> wrote: > > > > > > > > > > On Thu, Mar 12, 2020 at 04:14:38PM -0500, Rob Herring wrote: > > > > > > On Fri, Mar 06, 2020 at 04:03:40PM +0300, Sergey.Semin@xxxxxxxxxxxxxxxxxxxx wrote: > > > > > > > From: Serge Semin <Sergey.Semin@xxxxxxxxxxxxxxxxxxxx> > > > > > > > > > > > > > > Optional regmap property will be used to refer to a syscon-controller > > > > > > > having a reboot tolerant register mapped. > > > > > > > > > > > > NAK. It should simply be a child node of the 'syscon-controller'. > > > > > > > > > > Hm, It's dilemma. The driver maintainer said ack, while you disagree.) > > > > > So the code change will be merged while the doc-part won't? Lets discuss then > > > > > to settle the issue. > > > > > > > > > > Why 'syscon-reboot' can be out of syscon-controller node, while > > > > > 'syscon-reboot-mode' can't? > > > > > > > > Look at the history and you will see one was reviewed by DT > > > > maintainers and one wasn't. > > > > > > > > > They both belong to the same usecase: save > > > > > cause id and reboot. So having similar properties-set and declaring their > > > > > nodes someplace nearby is natural. > > > > > > > > Which is what I'm asking for. Where else in the tree does it make > > > > sense to locate the 'syscon-reboot-mode' node? Locate nodes where they > > > > logically belong. > > > > > > > > > According to the driver 'syscon-reboot' > > > > > can't lack the regmap property because it's mandatory, while here you refuse > > > > > to have even optional support. Additionally in most of the cases the > > > > > 'syscon-reboot' nodes aren't declared as a child of a system controller > > > > > node. Why 'syscon-reboot-mode' can't work in a similar way? > > > > > > > > There's plenty of bad or "don't follow current best practice" examples > > > > in the tree for all sorts of things. That is not a reason for doing > > > > something in a new binding or adding to an existing one. > > > > > > > > Rob > > > > > > Alright. I see your point. What about I'd provide a sort of opposite > > > implementation? I could make the "regmap"-phandle reference being optional > > > in the !"syscon-reboot"! driver instead of adding the regmap-property > > > support to the "syscon-reboot-mode" driver. So if regmap property isn't > > > defined in the "syscon-reboot"-compatible node, the driver will try to > > > get a syscon regmap from the parental node as it's done in the > > > "syscon-reboot-mode" driver. > > That seems fine. > > > > Seeing you think that regmap-property-based design is a bad practice in > > > this case, I also could mark the property as deprecated in the "syscon-reboot" > > > dt schema and print a warning from the "syscon-reboot" driver if one is defined. > > Depends on how many platforms will start getting warnings. I think just > marking deprecated is enough. Ok. Thanks. I'll do this in v2. Regards, -Sergey > > Rob
