Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 28 Sep 2015 15:01:13 +0000
From:      "Pokala, Ravi" <rpokala@panasas.com>
To:        John Baldwin <jhb@freebsd.org>, Francois Tigeot <ftigeot@wolfpond.org>, "cem@FreeBSD.org" <cem@FreeBSD.org>
Cc:        "freebsd-hackers@freebsd.org" <freebsd-hackers@freebsd.org>
Subject:   Re: bus_.*_resource() and rid
Message-ID:  <D22E9D9D.146A5C%rpokala@panasas.com>
In-Reply-To: <1637146.Rv3dkk0gMi@ralph.baldwin.cx>
References:  <D214E963.145154%rpokala@panasas.com> <1685918.WyYIclYTSg@ralph.baldwin.cx> <D217B20D.1455A1%rpokala@panasas.com> <1637146.Rv3dkk0gMi@ralph.baldwin.cx>

next in thread | previous in thread | raw e-mail | index | archive | help
-----Original Message-----
From: John Baldwin <jhb@freebsd.org>
Date: 2015-09-23, Wednesday at 08:07
To: Ravi Pokala <rpokala@panasas.com>
Cc: "freebsd-hackers@freebsd.org" <freebsd-hackers@freebsd.org>
Subject: Re: bus_.*_resource() and rid

>Yes, you'd have to grumble around with kgdb to see rids.  We might store
>rids in 'struct resouce *' now so we could possibly expose those to
>devinfo.

Survey says "no":

sys/sys/rman.h:
 89 /*
 90  * The public (kernel) view of struct resource
 91  *
 92  * NB: Changing the offset/size/type of existing fields in struct
resource
 93  * NB: breaks the device driver ABI and is strongly FORBIDDEN.
 94  * NB: Appending new fields is probably just misguided.
 95  */
 96=20
 97 struct resource {
 98         struct resource_i       *__r_i;
 99         bus_space_tag_t         r_bustag; /* bus_space tag */
100         bus_space_handle_t      r_bushandle;    /* bus_space handle */
101 };


We *do* keep the rid in (struct resource_i):

sys/kern/subr_rman.c:
  88 struct resource_i {
  89         struct resource         r_r;
  90         TAILQ_ENTRY(resource_i) r_link;
  91         LIST_ENTRY(resource_i)  r_sharelink;
  92         LIST_HEAD(, resource_i) *r_sharehead;
  93         u_long  r_start;        /* index of the first entry in this
resource */
  94         u_long  r_end;          /* index of the last entry
(inclusive) */
  95         u_int   r_flags;
  96         void    *r_virtual;     /* virtual address of this resource */
  97         struct device *r_dev;   /* device which has allocated this
resource */
  98         struct rman *r_rm;      /* resource manager from whence this
came */
  99         int     r_rid;          /* optional rid for this resource. */
 100 };


I have a one-line change to dump_rman() to include the RID along w/ the
start and end address. That at least got me what I was interested in from
ddb.

As for `devinfo', it looks like (struct u_resource) and (struct u_rman)
don't include the RID either:

sys/sys/rman.h:
 67 struct u_resource {
 68         uintptr_t       r_handle;               /* resource uniquifier
*/
 69         uintptr_t       r_parent;               /* parent rman */
 70         uintptr_t       r_device;               /* device owning this
resource */
 71         char            r_devname[RM_TEXTLEN];  /* device name XXX
obsolete */
 72=20
 73         u_long          r_start;                /* offset in resource
space */
 74         u_long          r_size;                 /* size in resource
space */
 75         u_int           r_flags;                /* RF_* flags */
 76 };
 77=20
 78 struct u_rman {
 79         uintptr_t       rm_handle;              /* rman uniquifier */
 80         char            rm_descr[RM_TEXTLEN];   /* rman description */
 81=20
 82         u_long          rm_start;               /* base of managed
region */
 83         u_long          rm_size;                /* size of managed
region */
 84         enum rman_type  rm_type;                /* region type */
 85 };


>Some drivers take over the LPC (e.g. I think some of the smb controllers
>do this) and provide functionality directly in the driver attached to the
>LPC rather than as a child.  In general I think it is cleaner to provide
>these extended functionalities as pseudo-ISA devices that are children of
>the LPC instead.

Yeah, the existing driver I'm extending is being done as an ISA child of
the PCI-ISA bridge. No worries there.

>>I'll look into bus_get_resource(). Oh, look - there's no manpage
>> (share/man/man9/bus_get_resource.9 does not exist, despite being a "see
>> also" entry in bus_set_resource.9). :-S
>
>Time for Someone(tm) to write one. :-/

Thanks to Francois for pointing at Dragonfly's version, and to Conrad for
bringing it in.

----

I ultimately just added a "next_rid" field to the softc, saving the value
after a successful call to bus_set_resource() and then incrementing it.
That way, each subsequent resource I add for an instance of the device
gets a unique value, which appears to be the only constraint for IOPORT
resources. In any case, with that change, both the original and the new
resource are able to be set and allocated.

Thanks all,

Ravi




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?D22E9D9D.146A5C%rpokala>