>From ddab0bc46eb538c957357549ae2ba657db3887a3 Mon Sep 17 00:00:00 2001
From: Rene Herman <[email protected]>
Date: Thu, 26 Jun 2008 00:14:08 +0200
Subject: [PATCH] PNP: make the resource type an unsigned long
PnP encodes the resource type directly as its struct resource->flags
value which is an unsigned long. Make it so...
Signed-off-by: Rene Herman <[email protected]>
---
drivers/pnp/base.h | 2 +-
drivers/pnp/resource.c | 4 ++--
include/linux/pnp.h | 2 +-
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/pnp/base.h b/drivers/pnp/base.h
index 9fd7bb9..7cc7bf5 100644
--- a/drivers/pnp/base.h
+++ b/drivers/pnp/base.h
@@ -147,7 +147,7 @@ char *pnp_resource_type_name(struct resource *res);
void dbg_pnp_show_resources(struct pnp_dev *dev, char *desc);
void pnp_free_resources(struct pnp_dev *dev);
-int pnp_resource_type(struct resource *res);
+unsigned long pnp_resource_type(struct resource *res);
struct pnp_resource {
struct list_head list;
diff --git a/drivers/pnp/resource.c b/drivers/pnp/resource.c
index 4cfe3a1..dbae23a 100644
--- a/drivers/pnp/resource.c
+++ b/drivers/pnp/resource.c
@@ -467,14 +467,14 @@ int pnp_check_dma(struct pnp_dev *dev, struct resource *res)
#endif
}
-int pnp_resource_type(struct resource *res)
+unsigned long pnp_resource_type(struct resource *res)
{
return res->flags & (IORESOURCE_IO | IORESOURCE_MEM |
IORESOURCE_IRQ | IORESOURCE_DMA);
}
struct resource *pnp_get_resource(struct pnp_dev *dev,
- unsigned int type, unsigned int num)
+ unsigned long type, unsigned int num)
{
struct pnp_resource *pnp_res;
struct resource *res;
diff --git a/include/linux/pnp.h b/include/linux/pnp.h
index 1ce54b6..97c8022 100644
--- a/include/linux/pnp.h
+++ b/include/linux/pnp.h
@@ -21,7 +21,7 @@ struct pnp_dev;
/*
* Resource Management
*/
-struct resource *pnp_get_resource(struct pnp_dev *, unsigned int, unsigned int);
+struct resource *pnp_get_resource(struct pnp_dev *, unsigned long, unsigned int);
static inline int pnp_resource_valid(struct resource *res)
{
--
1.5.5
Rene Herman wrote:
> Hi Bjorn.
>
> Andrew earlier commented that pci_resourec_flags() returns an unsigned
> long. Had this hanging around a local branch. Useful?
>
-int pnp_resource_type(struct resource *res)
+unsigned long pnp_resource_type(struct resource *res)
{
return res->flags & (IORESOURCE_IO | IORESOURCE_MEM |
IORESOURCE_IRQ | IORESOURCE_DMA);
}
Seems a bit pointless ... either one of those flags is >= 32 bits, in
which case we need u64, or it's not, in which case there is no reason to
burden the output with bits we don't need.
-hpa
On 08-08-08 23:55, H. Peter Anvin wrote:
>> Andrew earlier commented that pci_resourec_flags() returns an unsigned
>> long. Had this hanging around a local branch. Useful?
>>
>
> -int pnp_resource_type(struct resource *res)
> +unsigned long pnp_resource_type(struct resource *res)
> {
> return res->flags & (IORESOURCE_IO | IORESOURCE_MEM |
> IORESOURCE_IRQ | IORESOURCE_DMA);
> }
>
> Seems a bit pointless ... either one of those flags is >= 32 bits, in
> which case we need u64, or it's not, in which case there is no reason to
> burden the output with bits we don't need.
Yes, it's a not a functional patch -- only a type-consistency one. Right
now we're mixing ints (signed ones even) and unsigned longs and while in
this case that's not a functional problem it's messy and inconsistent.
I agree (as Andrew said earlier as well) that the struct resource flags
member should probably just be a u32 but it's not. Changing that would
be a bigger change than just a simple conistency thing.
Rene.
Rene Herman wrote:
>>
>> Seems a bit pointless ... either one of those flags is >= 32 bits, in
>> which case we need u64, or it's not, in which case there is no reason
>> to burden the output with bits we don't need.
>
> Yes, it's a not a functional patch -- only a type-consistency one. Right
> now we're mixing ints (signed ones even) and unsigned longs and while in
> this case that's not a functional problem it's messy and inconsistent.
>
> I agree (as Andrew said earlier as well) that the struct resource flags
> member should probably just be a u32 but it's not. Changing that would
> be a bigger change than just a simple conistency thing.
>
You're going in the wrong direction for consistency. long is different
on 32 and 64 bits, and really should be avoided unless that is intended.
-hpa
On 09-08-08 07:25, H. Peter Anvin wrote:
>>> Seems a bit pointless ... either one of those flags is >= 32 bits, in
>>> which case we need u64, or it's not, in which case there is no reason
>>> to burden the output with bits we don't need.
>>
>> Yes, it's a not a functional patch -- only a type-consistency one.
>> Right now we're mixing ints (signed ones even) and unsigned longs and
>> while in this case that's not a functional problem it's messy and
>> inconsistent.
>>
>> I agree (as Andrew said earlier as well) that the struct resource
>> flags member should probably just be a u32 but it's not. Changing that
>> would be a bigger change than just a simple conistency thing.
>>
>
> You're going in the wrong direction for consistency. long is different
> on 32 and 64 bits, and really should be avoided unless that is intended.
I know and fair enough but changing struct resource is just a bit too
central for my tastes.
<shrug>
Rene.
On Friday 08 August 2008 12:39:45 am Rene Herman wrote:
> Andrew earlier commented that pci_resourec_flags() returns an unsigned
> long. Had this hanging around a local branch. Useful?
I see hpa's point that it makes no functional difference, but I do
think it's worthwhile to make the types match. I don't want to
debug the problem that happens when somebody adds an IORESOURCE_*
flag in the upper bits.
You probably want to send this (and the next one, which is really
a pci_resource_flags() usage despite being in drivers/pnp/quirks.c)
to Andi Kleen.
Acked-by: Bjorn Helgaas <[email protected]>
Bjorn Helgaas wrote:
>
> I see hpa's point that it makes no functional difference, but I do
> think it's worthwhile to make the types match. I don't want to
> debug the problem that happens when somebody adds an IORESOURCE_*
> flag in the upper bits.
>
Then half the platforms break anyway.
We should change this to an explicit type.
-hpa
On Monday 11 August 2008 04:08:18 pm H. Peter Anvin wrote:
> Bjorn Helgaas wrote:
> >
> > I see hpa's point that it makes no functional difference, but I do
> > think it's worthwhile to make the types match. I don't want to
> > debug the problem that happens when somebody adds an IORESOURCE_*
> > flag in the upper bits.
>
> Then half the platforms break anyway.
>
> We should change this to an explicit type.
Are you suggesting a change to the "unsigned long flags" in struct
resource, or a different change to pnp_resource_type()?
The struct resource change sounds reasonable, but I think neither
Rene nor I want to sign up to do that right now.
Whatever we have in struct resource, I think it makes sense to use
the same type everywhere we reference the "flags" element. I think
you're saying "it doesn't matter in this case that we use different
types." But I, as a code reader, would rather not have to spend the
mental energy to convince myself that you're right.
Bjorn
On 11-08-08 23:59, Bjorn Helgaas wrote:
> On Friday 08 August 2008 12:39:45 am Rene Herman wrote:
>> Andrew earlier commented that pci_resourec_flags() returns an unsigned
>> long. Had this hanging around a local branch. Useful?
>
> I see hpa's point that it makes no functional difference, but I do
> think it's worthwhile to make the types match. I don't want to
> debug the problem that happens when somebody adds an IORESOURCE_*
> flag in the upper bits.
>
> You probably want to send this (and the next one, which is really
> a pci_resource_flags() usage despite being in drivers/pnp/quirks.c)
> to Andi Kleen.
>
> Acked-by: Bjorn Helgaas <[email protected]>
Okay, thanks. I see that Andrew picked these up with Andi in CC so I'll
assume things will find their way from there.
Rene.