bus_.*_resource() and rid

[alex.burlyga.ietf alex.burlyga.ietf]

See  replies in-line.

On Tue, Sep 8, 2015 at 7:37 PM, Pokala, Ravi <rpokala at panasas.com> wrote:
> Hi folks,
>
> I'm modifying a home-grown device driver; this is the first time I've
> played around in device-probe and -attach, so I'm a bit out of my element
> here. This is an LPC device attached to a PCI-ISA bridge (aka the LPC
> controller). It's accessed through IOPORT, and the BIOS sets up enough
> stuff that we can look for it.
>
> One thing that's confusing me is the "rid" which is passed to a bunch of
> the bus_.*_resource() functions. I've looked at bus_set_resource(9),
> bus_alloc_resource(9), and section 10.5 of the Architecture Handbook[1],
> and I'm still confused.
>
> bus_alloc_resource(9) says:
>
>      rid points to a bus specific handle that identifies the resource being
>      allocated.  For ISA this is an index into an array of resources that
> have
>      been setup for this device by either the PnP mechanism, or via the
> hints
>      mechanism. ...
>
>
> But there's no indication as to how we get that index. One possibility is
> that the value passed in doesn't matter; bus_alloc_resource() sets it
> correctly and the caller can use the new value. However, that doesn't
> appear to be the case, since lots of times rid is ignored after the call
> to bus_alloc_resource().

It's a discriminator for the resource type. It's bus specific, so in
case of ISA it's just an integer in a range of 0 to 50(i think).

>
> For the sake of discussion, here are stripped-down versions of our probe
> and attach functions:
>
> xxx_probe_unit(device_t dev)
> {
>         uint32_t ioport_base;
>         uint32_t ioport_size;
>         int rc;
>
>         /* Device identification stuff; details unimportant, but we determine
>          * ioport_base and ioport_size.
>          */
>
>         rc = bus_set_resource(
>             /* dev */ dev,
>             /* type */ SYS_RES_IOPORT,
>             /* rid */ 0,
>             /* start */ ioport_base,
>             /* count */ ioport_size);
> }
>
> Most of those args are obvious, but I have no idea why rid is 0. It looks
> like lots of drivers pass in a rid of 0, and the original author might
> have just shrugged and gone with "convention".

Depends on the device, if it only has one IOPORT then this makes sense.

>
> xxx_attach_unit(device_t dev)
> {
>         struct xxx_softc *sc;
>         int rid;
>         struct resource res;
>
>         sc = device_get_softc(dev);
>
>         /* Device configuration stuff; details unimportant. */
>
>         rid = 0;
>         res = bus_alloc_resource(
>             /* dev */ dev,
>             /* type */ SYS_RES_IOPORT,
>             /* rid */ &rid,
>             /* start */ 0,
>             /* end */ ~0,
>             /* count */ sc->ioport_size,
>             /* flags */ RF_ACTIVE);
>
>         /* save stuff for use with bus_space_{read,write}_1() */
>         sc->iobase_addr = rman_get_start(res);
>         sc->iobase_bustag = rman_get_bustag(res);
>         sc->iobase_bushandle = rman_get_bushandle(res);
>         sc->dev = dev;
> }
>
> Again, most things are fairly obvious, but I have no idea why rid is 0.
> It's passed by reference to bus_alloc_resource(), but the
> potentially-altered value is never stored or used.

You are attaching a driver to a device, so you are actually allocating
resource for the range you claimed in the probe function with
bus_set_resource. Again since device has only one IOPORT, and ISA does
not change "rid" so there is no need to store "rid" anywhere.

>
> The lazy part of me says to just go with blindly passing in 0, because it
> works. However, the new device I'm adding support for will have multiple
> IOPORT ranges associated with it, so I'm not sure if passing 0 is the
> right thing.

I think you have options here, during probe you can go with just 0 as
"rid" but then during allocation you would need to populate start and
end parameters with appropriate offsets. Or you can do "rid" per
IOPORT and then you can retain
0, ~0 for start end and pass appropriate "rid" during attach phase.

>
> Any help would be appreciated.
>
> Thanks,
>
> Ravi
>
> [1]
> https://www.freebsd.org/doc/en_US.ISO8859-1/books/arch-handbook/isa-driver-
> resources.html
>
> _______________________________________________
> freebsd-hackers at freebsd.org mailing list
> https://lists.freebsd.org/mailman/listinfo/freebsd-hackers
> To unsubscribe, send any mail to "freebsd-hackers-unsubscribe at freebsd.org"