linux-kernel - Re: [PATCH v11 2/8] mfd: bd70528: Support ROHM bd70528 PMIC

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]

Date:   Thu, 4 Apr 2019 10:24:49 +0300
From:   Matti Vaittinen <matti.vaittinen@...rohmeurope.com>
To:     Lee Jones <lee.jones@...aro.org>
Cc:     "alexandre.belloni@...tlin.com" <alexandre.belloni@...tlin.com>,
        "robh+dt@...nel.org" <robh+dt@...nel.org>,
        "mazziesaccount@...il.com" <mazziesaccount@...il.com>,
        "mturquette@...libre.com" <mturquette@...libre.com>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "linux-pm@...r.kernel.org" <linux-pm@...r.kernel.org>,
        "sre@...nel.org" <sre@...nel.org>,
        "linus.walleij@...aro.org" <linus.walleij@...aro.org>,
        "sboyd@...nel.org" <sboyd@...nel.org>,
        "linux-gpio@...r.kernel.org" <linux-gpio@...r.kernel.org>,
        "a.zummo@...ertech.it" <a.zummo@...ertech.it>,
        "broonie@...nel.org" <broonie@...nel.org>,
        "Mutanen, Mikko" <Mikko.Mutanen@...rohmeurope.com>,
        "mark.rutland@....com" <mark.rutland@....com>,
        "linux-watchdog@...r.kernel.org" <linux-watchdog@...r.kernel.org>,
        "linux@...ck-us.net" <linux@...ck-us.net>,
        "lgirdwood@...il.com" <lgirdwood@...il.com>,
        "bgolaszewski@...libre.com" <bgolaszewski@...libre.com>,
        "wim@...ux-watchdog.org" <wim@...ux-watchdog.org>,
        "linux-clk@...r.kernel.org" <linux-clk@...r.kernel.org>,
        "linux-rtc@...r.kernel.org" <linux-rtc@...r.kernel.org>,
        "Haikola, Heikki" <Heikki.Haikola@...rohmeurope.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v11 2/8] mfd: bd70528: Support ROHM bd70528 PMIC - core

On Thu, Apr 04, 2019 at 07:54:52AM +0100, Lee Jones wrote:
> On Thu, 04 Apr 2019, Vaittinen, Matti wrote:
> 
> > Actually, now that I thik of it the right way to do this would have
> > been the function pointer in parent data as was done in original patch
> > set. HW-colleagues tend to re-use HW blocks, and we like to re-use our
> > drivers. If the next PMIC from ROHM uses same RTC block but does not
> > provide watchdog - then it is cleanest solution to fall back to
> > function pointer and leave it to NULL when there is no WDT or when WDT
> > is unused. Another option is to export dummy function - which is not so
> > nice.
> 
> I think the converse is true.
> 
> Pointers to functions outside of a subsystem API context are generally
> horrible.  It's much nicer to call a function which can be easily
> stubbed out in a header file based on a Kconfig option.  It's how most
> kernel APIs work.

I hate to admit but I see your point. This nicely solves any issues in
syncronizing the startup for driver providing function pointer and for
driver using it.

> > Additional benefit from function pointer would have been that the
> > function pointer can be only used by drivers which have acces to it.
> > This exported function is globally visible. The WDT disable/enable is
> > very specific procedure and it actually would be nicer design to not
> > have it visible globally. It is not intended to be used by anything
> > else besides the WDT and RTC here.
> 
> Why would anything else try to use it?
> 
> There are 1000's of exported functions in the kernel.  If it's
> properly namespaced a consumer would have to purposely call it, which
> if they really wanted to, they could do anyway.  I don't really see
> your point.

I could still argue on this. It _is_ less obvous that an exported function
is not meant to be publicly used than it is for function pointers. But
as you say, this is not a strong enough point to see the trouble in
synchronizing the WDT/RTC startup.

> > But I can't say there will be PMIC with same RTC and no WDT coming from
> > ROHM. Also, I am not terribly excited about the option of changing this
> > back to function-pointer as I already removed the pointer from parent
> > data and this changed parent data is already adapted to all sub drivers
> > - so this is all just babbling. Maybe it is just my huge ego shouting
> > there - 'I was right, I must have the final say'.
> 
> No, a call-back function would be a back-step.

You are probably right.

> Ego or no ego, you're wrong. =:-D

I'd rephrase that as "It's not that I am wrong, but you are right." =)

> > As a side note, I already did submit v12 with other styling fixes but
> > which left the WDT function in MFD. If you still see the WDT functions
> > should be exported from WDT - then please ignore the v12. I'll do v13
> > at the afternoon (my time, which is only a bit after noon your time I
> > guess) which will export these functions from WDT. (Well, I had to try
> > once more :D)
> 
> Please keep the WDT code in the WDT driver.  Create a little stub for
> the cases where the WDT driver is not enabled - job done.

Yes Sir.

Br,
	Matti Vaittinen

-- 
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND

~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~