linux-kernel - Re: [PATCH 3/4] dt-bindings: power: reset: Add regmap support to the SYSCON reboot-mode bindings

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20200417074559.wxh3hv6xwh5ndaz4@ubsrv2.baikal.int>
Date:   Fri, 17 Apr 2020 10:45:59 +0300
From:   Sergey Semin <Sergey.Semin@...kalelectronics.ru>
To:     Rob Herring <robh@...nel.org>
CC:     Sebastian Reichel <sre@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Alexey Malahov <Alexey.Malahov@...kalelectronics.ru>,
        Thomas Bogendoerfer <tsbogend@...ha.franken.de>,
        Paul Burton <paulburton@...nel.org>,
        Ralf Baechle <ralf@...ux-mips.org>,
        "open list:THERMAL" <linux-pm@...r.kernel.org>,
        <devicetree@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 3/4] dt-bindings: power: reset: Add regmap support to the
 SYSCON reboot-mode bindings

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@...kalelectronics.ru> 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@...kalelectronics.ru wrote:
> > > > > > > From: Serge Semin <Sergey.Semin@...kalelectronics.ru>
> > > > > > >
> > > > > > > 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