svn commit: r282674 - in head/sys/dev: iicbus ofw
Luiz Otavio O Souza
loos.br at gmail.com
Sun May 10 02:35:35 UTC 2015
On Sat, May 9, 2015 at 7:45 AM, John Baldwin wrote:
> On Saturday, May 09, 2015 03:05:45 AM Luiz Otavio O Souza wrote:
>> Author: loos
>> Date: Sat May 9 03:05:44 2015
>> New Revision: 282674
>> URL: https://svnweb.freebsd.org/changeset/base/282674
>>
>> Log:
>> Handle IRQ resources on iicbus and ofw_iicbus.
>>
>> Based on a patch submitted by Michal Meloun <meloun at miracle.cz>.
>>
>> Modified:
>> head/sys/dev/iicbus/iicbus.c
>> head/sys/dev/iicbus/iicbus.h
>> head/sys/dev/ofw/ofw_iicbus.c
>>
>> Modified: head/sys/dev/iicbus/iicbus.c
>> ==============================================================================
>> --- head/sys/dev/iicbus/iicbus.c Sat May 9 00:48:44 2015 (r282673)
>> +++ head/sys/dev/iicbus/iicbus.c Sat May 9 03:05:44 2015 (r282674)
>> @@ -157,9 +159,9 @@ iicbus_probe_nomatch(device_t bus, devic
>> {
>> struct iicbus_ivar *devi = IICBUS_IVAR(child);
>>
>> - device_printf(bus, "<unknown card>");
>> - printf(" at addr %#x\n", devi->addr);
>> - return;
>> + device_printf(bus, "<unknown card> at addr %#x", devi->addr);
>> + resource_list_print_type(&devi->rl, "irq", SYS_RES_IRQ, "%ld");
>> + printf("\n");
>
> Other bus drivers do not print resources for nomatch devices (or at
> least PCI doesn't). Given that, I'm not sure it makes sense to do
> that here?
That's right, it was an overzealous from my part.
>
>> +static int
>> +iicbus_set_resource(device_t dev, device_t child, int type, int rid,
>> + u_long start, u_long count)
>> +{
>> + struct iicbus_ivar *devi;
>> + struct resource_list_entry *rle;
>> +
>> + devi = IICBUS_IVAR(child);
>> + rle = resource_list_add(&devi->rl, type, rid, start,
>> + start + count - 1, count);
>> + if (rle == NULL)
>> + return (ENXIO);
>> +
>> + return (0);
>> +}
>
> Isn't this the same as bus_generic_rl_set_resource()?
>
>> +
>> +static struct resource *
>> +iicbus_alloc_resource(device_t bus, device_t child, int type, int *rid,
>> + u_long start, u_long end, u_long count, u_int flags)
>> +{
>> + struct resource_list *rl;
>> + struct resource_list_entry *rle;
>> +
>> + /* Only IRQ resources are supported. */
>> + if (type != SYS_RES_IRQ)
>> + return (NULL);
>> +
>> + /*
>> + * Request for the default allocation with a given rid: use resource
>> + * list stored in the local device info.
>> + */
>> + if ((start == 0UL) && (end == ~0UL)) {
>> + rl = BUS_GET_RESOURCE_LIST(bus, child);
>> + if (rl == NULL)
>> + return (NULL);
>> + rle = resource_list_find(rl, type, *rid);
>> + if (rle == NULL) {
>> + if (bootverbose)
>> + device_printf(bus, "no default resources for "
>> + "rid = %d, type = %d\n", *rid, type);
>> + return (NULL);
>> + }
>> + start = rle->start;
>> + end = rle->end;
>> + count = rle->count;
>> + }
>> +
>> + return (bus_generic_alloc_resource(bus, child, type, rid, start, end,
>> + count, flags));
>
> If you are using a resource list, you should be using resource_list_alloc().
> However, I think you can replace this entire method with just
> bus_generic_rl_alloc_resource().
>
>> @@ -297,6 +366,16 @@ static device_method_t iicbus_methods[]
>> DEVMETHOD(device_detach, iicbus_detach),
>>
>> /* bus interface */
>> + DEVMETHOD(bus_setup_intr, bus_generic_setup_intr),
>> + DEVMETHOD(bus_teardown_intr, bus_generic_teardown_intr),
>> + DEVMETHOD(bus_release_resource, bus_generic_release_resource),
>
> After fixing alloc_resource to use resource_list_alloc() (either directly
> or via the generic method), this should be set to
> bus_generic_rl_release_resource().
>
>> Modified: head/sys/dev/ofw/ofw_iicbus.c
>> ==============================================================================
>> --- head/sys/dev/ofw/ofw_iicbus.c Sat May 9 00:48:44 2015 (r282673)
>> +++ head/sys/dev/ofw/ofw_iicbus.c Sat May 9 03:05:44 2015 (r282674)
>> @@ -199,3 +205,12 @@ ofw_iicbus_get_devinfo(device_t bus, dev
>> dinfo = device_get_ivars(dev);
>> return (&dinfo->opd_obdinfo);
>> }
>> +
>> +static struct resource_list *
>> +ofw_iicbus_get_resource_list(device_t bus __unused, device_t child)
>> +{
>> + struct ofw_iicbus_devinfo *devi;
>> +
>> + devi = device_get_ivars(child);
>> + return (&devi->opd_dinfo.rl);
>> +}
>
> I think you don't actually need this since the inherited method should
> already work.
>
> --
> John Baldwin
Yeah, that's all correct, Michal had already warned about a few of these issues.
Fixed in r282702.
Thanks for the review.
Luiz
More information about the svn-src-head
mailing list