svn commit: r328257 - in head/sys: arm/broadcom/bcm2835 dts/arm modules

[Warner Losh]

On Mon, Jan 22, 2018 at 4:54 PM, John Baldwin <jhb at freebsd.org> wrote:

> On Monday, January 22, 2018 11:20:40 PM Poul-Henning Kamp wrote:
> > --------
> > In message <CANCZdfpq2QoG4EAj0VW2FF=4VXRv-qQKFfJTJerWH9YOwVoVBA at mail.
> gmail.com>
> > , Warner Losh writes:
> >
> > >I think even bootverbose it's insane. Or I don't understand what the
> real
> > >ask is.
> >
> > What I think would be a good idea, is when a driver is kldloaded
> > the user will be told if there were hardware present which it could
> > drive (ie: probe() matches), but for some reason and somehow,
> > it as marked as unavailable/conflicting/disabled, so attach() didn't.
> >
> > Today you load the device driver and if the hardware is "status=disabled"
> > in the FDT, nothing happens, and you get no information about why
> > nothing happened, leaving the user to guess...
> >
> > I've seen people resort to kldload /boot/kernel/*.ko in an
> > attempt to find out if were using the right device driver.
> >
> > I suspect moving the boilerplate
> >
> >       if (!ofw_bus_status_okay(dev))
> >               return (ENXIO);
> >
> > To the attach() function would do it, but we could avoid that
> > boilerplate in all the drivers, if it was done one level up with
> > a consistent device_printf() and not calling attach() at all.
> >
> > What I don't know is how noisy that would be in practice ?
>
> I think the better solution to this problem is the devmatch stuff Warner
> is already working on where one can say "for this given device info
> (e.g. PCI IDs or FDT properties), which drivers support this").  There
> is enough of that already existing now in HEAD I think it would be better
> to extend it to support whatever input mode it needs to handle disabled
> devices as input to run through its existing matching engine.
>

I'm actually thinking the right thing is to remove the checks everywhere.

We then create  a couple of new simplebus methods:

int ok_for_state(device_t dev, const char *state)

which returns 0 if the device should be enabled in this state, or an errno
for why not. It's called on boot and if it returns true, the device will be
attached. Defaults to something akin to ofw_bus_status_okay. Devices can
override this for whatever reasons they may need to.

int change_state(device_t dev, const char *old_state, const char *new_state)

which tells the device we're entering a new state. The device is expected
to cope. The default is to call ok_for_state and return that result. By
default, if there's an error in the new state, the device's detach routine
will be called. Devices are free to override this.

This will allow us to do things like load overlays at runtime that change
the status of devices (and maybe pinmux items) and have an ordered way to
cope. It will also allow us to implement a policy of 'all devices must be
cool with the new state or the overlay is rejected'.

If we do that, then devmatch will work like everywhere else. Otherwise
you'll have the frustrating situation where you kldload foo, see nothing
and have devmatch tell you to load foo...

Warner