lkml.org 
[lkml]   [2012]   [Apr]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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@google.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@gmx.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;


\
 
 \ /
  Last update: 2012-04-01 18:09    [W:0.053 / U:23.252 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site