Re: [PATCH] usb: ulpi: don't register drivers if bus doesn't exist

[David Cohen]

Hi Greg,

On Tue, May 26, 2015 at 07:41:18PM -0700, Greg KH wrote:
> On Tue, May 26, 2015 at 10:54:01AM -0700, David Cohen wrote:
> > Hi,
> > 
> > On Mon, May 25, 2015 at 07:00:13PM +0200, Bjørn Mork wrote:
> > > Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx> writes:
> > > 
> > > > If there are other bus drivers that do this, I'll go fix them up,
> > > > pointers to those files would be appreciated.
> > > 
> > > git grep -E 'if .*\.p\W' found a couple of interesting candidates you
> > > might want to check out:
> > > 
> > >  drivers/i2c/i2c-core.c: if (unlikely(WARN_ON(!i2c_bus_type.p))) {
> > >  drivers/i2c/i2c-core.c: if (unlikely(WARN_ON(!i2c_bus_type.p)))
> > >  drivers/spmi/spmi.c:    if (WARN_ON(!spmi_bus_type.p))
> > > 
> > > And this looks somewhat suspicious too, but could very well be OK for
> > > all I know (very close to nothing :)
> > > 
> > >  drivers/gpio/gpiolib-sysfs.c:   if (!gpio_class.p) {
> > >  drivers/gpio/gpiolib-sysfs.c:   if (!gpio_class.p)
> > 
> > I think we need a clear statement on how to proceed on this case.
> 
> Don't mess with bus->p.  I can rename it to
> "do_not_touch_this_isnt_for_you" if people think that would make it more
> obvious that a private data structure shouldn't be messed with in any
> way.  Outside of the driver core, you have no knowledge that even if it
> is a pointer, what that means with regards to anything.

I get that, I agree it's ugly and I'm not trying to push it further. If
you follow the thread, you'll see I reviewed and commented a macro would
make more sense than checking the private data directly for the same
reaon you mentioned. But since with Linux development the source code is
part of the documentation, we need a clear state about what's the
correct way to handle it before go telling people "do not do what kernel
is already doing because it's wrong".

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