linux-kernel - Re: [PATCH 5/8] watchdog: bindings: Provide ST bindings for ST's LPC Watchdog device

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <2894129.AmAkgPJPNf@wuerfel>
Date:	Thu, 18 Dec 2014 09:46:56 +0100
From:	Arnd Bergmann <arnd@...db.de>
To:	linux-arm-kernel@...ts.infradead.org
Cc:	Lee Jones <lee.jones@...aro.org>, a.zummo@...ertech.it,
	kernel@...inux.com, rtc-linux@...glegroups.com,
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
	wim@...ana.be, linux@...ck-us.net, linux-watchdog@...r.kernel.org
Subject: Re: [PATCH 5/8] watchdog: bindings: Provide ST bindings for ST's LPC Watchdog device

On Thursday 18 December 2014 08:13:34 Lee Jones wrote:
> On Wed, 17 Dec 2014, Arnd Bergmann wrote:
> 
> > On Wednesday 17 December 2014 16:45:24 Lee Jones wrote:
> > > +- compatible   : Must be one of: "st,stih407-lpc" "st,stih416-lpc"
> > > +                                 "st,stih415-lpc" "st,stid127-lpc"
> > > +- reg          : LPC registers base address + size
> > > +- interrupts    : LPC interrupt line number and associated flags
> > > +- clocks       : Clock used by LPC device (See: ../clock/clock-bindings.txt)
> > > +- st,lpc-mode  : The LPC can run either one of two modes ST_LPC_MODE_RTC [0] or
> > > +                 ST_LPC_MODE_WDT [1].  One (and only one) mode must be
> > > +                 selected.
> > > 
> > 
> > I'm glad you got it to work with two drivers for the same device.
> > 
> > With this binding, I'm still a bit unhappy about the st,lpc-mode property,
> > in particular since you rely on a shared include file for something that
> > can only be set in one way or another and always has to be present.
> > 
> > Why not just use a boolean property that enforces one mode when present
> > and another mode when absent?
> 
> There is nothing stopping me from doing that, and it was a
> consideration.  I concluded that this method would be more explicit
> however.  Both when describing our choices in DT and at a functional
> level within each of the drivers.
> 
> Let me know if you fundamentally disagree and I can fix-up.

I generally don't like  header files that define interfaces between C code
and DT nodes. There are cases where it's the least ugly solution, but I don't
think this is one of them.

If you want to be more explicit about the modes, how about having one
boolean property per mode? That would also allow devices that could be
driven in either mode, e.g. if you have only one instance of this device.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/