| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Sun, 01 Apr 2012 18:05:52 +0200
From: Witold Szczeponik <Witold.Szczeponik@....net>
To: Bjorn Helgaas <bhelgaas@...gle.com>
CC: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/3] PNP: Simplify setting of resources
On 27/03/12 22:32, Bjorn Helgaas wrote:
[...]
>
> A few minor comments below, but I think this looks good. In the past,
> I've sent PNP patches through linux-acpi and Len's ACPI tree.
>
> Reviewed-by: Bjorn Helgaas<bhelgaas@...gle.com>
>
I will send an updated version of the patch series also to Len and
linux-acpi. Thanks for pointing this out to me!
>> Signed-off-by: Witold Szczeponik<Witold.Szczeponik@....net>
>>
>>
>> Index: linux/drivers/pnp/interface.c
>> ===================================================================
>> --- linux.orig/drivers/pnp/interface.c
>> +++ linux/drivers/pnp/interface.c
>> @@ -298,6 +298,39 @@ static ssize_t pnp_show_current_resource
>> return ret;
>> }
>>
>> +static char *pnp_get_resource_value(char *buf,
>> + unsigned long type,
>> + resource_size_t *start,
>> + resource_size_t *end,
>> + unsigned long *flags)
>> +{
>> + if (start)
>> + *start = 0;
>> + if (end)
>> + *end = 0;
>> + if (flags)
>> + *flags = 0;
>> +
>> + /* TBD: allow for disabled resources */
>> +
>> + buf = skip_spaces(buf);
>> + if (start) {
>> + *start = simple_strtoull(buf,&buf, 0);
>> + if (end) {
>> + buf = skip_spaces(buf);
>> + if (*buf == '-') {
>> + buf = skip_spaces(buf + 1);
>> + *end = simple_strtoull(buf,&buf, 0);
>> + } else
>> + *end = *start;
>> + }
>> + }
>> +
>> + /* TBD: allow for additional flags, e.g., IORESOURCE_WINDOW */
>> +
>> + return buf;
>> +}
>> +
>> static ssize_t pnp_set_current_resources(struct device *dmdev,
>> struct device_attribute *attr,
>> const char *ubuf, size_t count)
>> @@ -305,7 +338,6 @@ static ssize_t pnp_set_current_resources
>> struct pnp_dev *dev = to_pnp_dev(dmdev);
>> char *buf = (void *)ubuf;
>> int retval = 0;
>> - resource_size_t start, end;
>>
>> if (dev->status& PNP_ATTACHED) {
>> retval = -EBUSY;
>> @@ -349,6 +381,10 @@ static ssize_t pnp_set_current_resources
>> goto done;
>> }
>> if (!strnicmp(buf, "set", 3)) {
>> + resource_size_t start;
>> + resource_size_t end;
>> + unsigned long flags;
>> +
>> if (dev->active)
>> goto done;
>> buf += 3;
>> @@ -357,42 +393,42 @@ static ssize_t pnp_set_current_resources
>> while (1) {
>> buf = skip_spaces(buf);
>> if (!strnicmp(buf, "io", 2)) {
>> - buf = skip_spaces(buf + 2);
>> - start = simple_strtoul(buf,&buf, 0);
>> - buf = skip_spaces(buf);
>> - if (*buf == '-') {
>> - buf = skip_spaces(buf + 1);
>> - end = simple_strtoul(buf,&buf, 0);
>> - } else
>> - end = start;
>> - pnp_add_io_resource(dev, start, end, 0);
>> - continue;
>> - }
>> - if (!strnicmp(buf, "mem", 3)) {
>> - buf = skip_spaces(buf + 3);
>> - start = simple_strtoul(buf,&buf, 0);
>> - buf = skip_spaces(buf);
>> - if (*buf == '-') {
>> - buf = skip_spaces(buf + 1);
>> - end = simple_strtoul(buf,&buf, 0);
>> - } else
>> - end = start;
>> - pnp_add_mem_resource(dev, start, end, 0);
>> - continue;
>> - }
>> - if (!strnicmp(buf, "irq", 3)) {
>> - buf = skip_spaces(buf + 3);
>> - start = simple_strtoul(buf,&buf, 0);
>> - pnp_add_irq_resource(dev, start, 0);
>> - continue;
>> - }
>> - if (!strnicmp(buf, "dma", 3)) {
>> - buf = skip_spaces(buf + 3);
>> - start = simple_strtoul(buf,&buf, 0);
>> - pnp_add_dma_resource(dev, start, 0);
>> - continue;
>> - }
>> - break;
>> + buf = pnp_get_resource_value(buf + 2,
>> + IORESOURCE_IO,
>> +&start,
>> +&end,
>> +&flags);
>
> Looks like at least "&end" will fit on the same line as "&start".
Indeed, they do: will change this.
>
>> + pnp_add_io_resource(dev, start, end, flags);
>> + } else if (!strnicmp(buf, "mem", 3)) {
>> + buf = pnp_get_resource_value(buf + 3,
>> + IORESOURCE_MEM,
>> +&start,
>> +&end,
>> +&flags);
>> + pnp_add_mem_resource(dev, start, end, flags);
>> + } else if (!strnicmp(buf, "irq", 3)) {
>> + buf = pnp_get_resource_value(buf + 3,
>> + IORESOURCE_IRQ,
>> +&start,
>> + NULL,
>> +&flags);
>> + pnp_add_irq_resource(dev, start, flags);
>> + } else if (!strnicmp(buf, "dma", 3)) {
>> + buf = pnp_get_resource_value(buf + 3,
>> + IORESOURCE_DMA,
>> +&start,
>> + NULL,
>> +&flags);
>> + pnp_add_dma_resource(dev, start, flags);
>> + } else if (!strnicmp(buf, "bus", 3)) {
>> + buf = pnp_get_resource_value(buf + 3,
>> + IORESOURCE_BUS,
>> +&start,
>> +&end,
>> + NULL);
>> + pnp_add_bus_resource(dev, start, end);
>
> It makes sense to allow a user to set bus number resources just like
> any other resources, but I don't think the rest of PNP supports this
> yet. I think only PNPACPI supports bus number resources at all, so
> when you "activate" the device after setting its resources, we'll call
> pnpacpi_set_resources() and pnpacpi_encode_resources(), but
> pnpacpi_encode_resources() doesn't know how to encode the address16
> descriptor that will probably be used for bus numbers.
>
> I guess that's all right, though. We already have the same situation
> for all the address space descriptors when used for IO or MEM.
>
>> + } else
>> + break;
>> }
>> mutex_unlock(&pnp_res_mutex);
>> goto done;
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists