linux-kernel - Re: [PATCH 3/3] omap: add hwspinlock 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: <20101020155117.GE7285@angua.secretlab.ca>
Date:	Wed, 20 Oct 2010 09:51:17 -0600
From:	Grant Likely <grant.likely@...retlab.ca>
To:	Ohad Ben-Cohen <ohad@...ery.com>
Cc:	Kevin Hilman <khilman@...prootsystems.com>,
	Ryan Mallon <ryan@...ewatersys.com>,
	linux-omap@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-arm-kernel@...ts.infradead.org, akpm@...ux-foundation.org,
	Greg KH <greg@...ah.com>, Tony Lindgren <tony@...mide.com>,
	Benoit Cousson <b-cousson@...com>,
	Hari Kanigeri <h-kanigeri2@...com>, Suman Anna <s-anna@...com>
Subject: Re: [PATCH 3/3] omap: add hwspinlock device

On Wed, Oct 20, 2010 at 04:09:22PM +0200, Ohad Ben-Cohen wrote:
> On Wed, Oct 20, 2010 at 1:12 AM, Grant Likely <grant.likely@...retlab.ca> wrote:
> > On Tue, Oct 19, 2010 at 3:02 PM, Ohad Ben-Cohen <ohad@...ery.com> wrote:
> ...
> >> i2c-omap, which is subsys_initcall (the I2C bus is shared between the
> >> A9 and the M3 on some OMAP4 boards).
> ...
> > Man. this is getting ugly.  I think we need to discuss how to solve
> > this at the Plumbers micro-conference. It kind of fits in with the
> > whole embedded (ab)use of the device model topic anyway. Actually,
> > this particular case isn't bad, but the moving of i2c and spi busses
> > to an earlier initcall is just band-aiding the real problem of driver
> > probe order dependencies.
> 
> +1

:-)

> 
> On Wed, Oct 20, 2010 at 1:53 AM, Kevin Hilman
> <khilman@...prootsystems.com> wrote:
> > Rather than moving towards having more drivers have to be built in (and
> > depend on their probe order) we need to be moving towards building all
> > these drivers as modules, including omap-i2c.
> 
> +1

-1. Like Ryan points out below, the problem isn't modules vs.
built-in, it is drivers depending on implicit init order which sort of
works, but is fragile and will break in subtle ways. It needs to work
in both cases, and the only solution to fix the dependency issue.

I completely agree that more drivers need to support modules, but
that is an orthogonal issue.

I suspect that in most cases the driver model already provides the
needed functionality via the parent->child relationships, and a lot of
the problems can be removed by getting the driver of the parent device
to register the children.

It gets more complex (like in this case) when a single device needs
multiple devices to be initialized before it is probed. I can think of
a couple of ways to solve this. One option is for such drivers to
explicitly look for its dependant devices and defer .probe() work
until they appear. I hacked up some code for this a while back, but I
never pursued it through to completion[1].

Another option is to defer even registering the dependent devices
until the prerequisites are all probed.  This could potentially be
done in board support code by using a bus notifier to look for the
critical device.

[1]http://forum.soft32.com/linux/RFC-drivers-base-Add-bus_register_notifier_alldev-variant-ftopict478522.html

> 
> This whole thing is a mess, and today it's being solved in the wrong,
> non-scalable and error-prone way.
> 
> The question is whether we want to gate hwspinlock until this issue is solved ?

No, don't gate hwspinlock.  I don't see any reason to defer it for
this issue.

> On Wed, Oct 20, 2010 at 3:20 AM, Ryan Mallon <ryan@...ewatersys.com> wrote:
> > The issue of probe order still needs to be resolved for those of us who
> > do want all the drivers built into the kernel.

+1

> 
> What about doing something similar to the way suspend/resume and the
> device hierarchy interact ?
> 
> device_resume waits for its parent to be resumed before waking up the
> device - this sounds similar to what ->probe() should do: wait for its
> device dependency to probe first (so in this case, i2c-omap should
> wait for hwspinlock).
> 
> Conversely, device_suspend waits for all its children to be suspended
> before continuing, which sounds just like what ->remove() should do
> (so again, in this case, the hwspinlock device should wait for all its
> users to be removed before bailing).

This works for the simple hierarchy case, but it doesn't help the
multi-dependency case.

g.
--
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/