Re: [PATCH] Custom IORESOURCE Class

Re: [PATCH] Custom IORESOURCE Class

[Greg KH]

On Mon, Aug 08, 2005 at 11:11:45AM -0700, Matthew Gilbert wrote:
> Below is a patch that adds an additional resource class to the platform 
> resource types. This is to support additional resources that need to be passed
> to drivers without overloading the existing specific types. In my case, I need
> to send clock information to the driver to enable power management. 
> 
> Signed-off-by: Matthew Gilbert <mgilbert@mvista.com>

Hm, you do realize that Pat's no longer the driver core maintainer?  :)

Anyway, Russell and Adam, any objections to this patch?

thanks,

greg k-h

> diff -uprN -X dontdiff linux-2.6.12.3.orig/drivers/base/platform.c linux-2.6.12.3/drivers/base/platform.c
> --- linux-2.6.12.3.orig/drivers/base/platform.c	2005-07-15 14:18:57.000000000 -0700
> +++ linux-2.6.12.3/drivers/base/platform.c	2005-08-04 10:54:59.706802768 -0700
> @@ -37,7 +37,7 @@ platform_get_resource(struct platform_de
>  		struct resource *r = &dev->resource[i];
>  
>  		if ((r->flags & (IORESOURCE_IO|IORESOURCE_MEM|
> -				 IORESOURCE_IRQ|IORESOURCE_DMA))
> +				 IORESOURCE_IRQ|IORESOURCE_DMA|IORESOURCE_OTHER))
>  		    == type)
>  			if (num-- == 0)
>  				return r;
> @@ -73,7 +73,7 @@ platform_get_resource_byname(struct plat
>  		struct resource *r = &dev->resource[i];
>  
>  		if ((r->flags & (IORESOURCE_IO|IORESOURCE_MEM|
> -				 IORESOURCE_IRQ|IORESOURCE_DMA)) == type)
> +				 IORESOURCE_IRQ|IORESOURCE_DMA|IORESOURCE_OTHER)) == type)
>  			if (!strcmp(r->name, name))
>  				return r;
>  	}
> diff -uprN -X dontdiff linux-2.6.12.3.orig/include/linux/ioport.h linux-2.6.12.3/include/linux/ioport.h
> --- linux-2.6.12.3.orig/include/linux/ioport.h	2005-07-15 14:18:57.000000000 -0700
> +++ linux-2.6.12.3/include/linux/ioport.h	2005-08-04 10:31:14.099528016 -0700
> @@ -35,13 +35,14 @@ struct resource_list {
>  #define IORESOURCE_MEM		0x00000200
>  #define IORESOURCE_IRQ		0x00000400
>  #define IORESOURCE_DMA		0x00000800
> +#define IORESOURCE_OTHER	0x00001000
>  
> -#define IORESOURCE_PREFETCH	0x00001000	/* No side effects */
> -#define IORESOURCE_READONLY	0x00002000
> -#define IORESOURCE_CACHEABLE	0x00004000
> -#define IORESOURCE_RANGELENGTH	0x00008000
> -#define IORESOURCE_SHADOWABLE	0x00010000
> -#define IORESOURCE_BUS_HAS_VGA	0x00080000
> +#define IORESOURCE_PREFETCH	0x00010000	/* No side effects */
> +#define IORESOURCE_READONLY	0x00020000
> +#define IORESOURCE_CACHEABLE	0x00040000
> +#define IORESOURCE_RANGELENGTH	0x00080000
> +#define IORESOURCE_SHADOWABLE	0x00100000
> +#define IORESOURCE_BUS_HAS_VGA	0x00800000
>  
>  #define IORESOURCE_DISABLED	0x10000000
>  #define IORESOURCE_UNSET	0x20000000
> 
> 

[PATCH] Custom IORESOURCE Class

[Matthew Gilbert]

Below is a patch that adds an additional resource class to the platform 
resource types. This is to support additional resources that need to be passed
to drivers without overloading the existing specific types. In my case, I need
to send clock information to the driver to enable power management. 

Signed-off-by: Matthew Gilbert <mgilbert@mvista.com>

diff -uprN -X dontdiff linux-2.6.12.3.orig/drivers/base/platform.c linux-2.6.12.3/drivers/base/platform.c
--- linux-2.6.12.3.orig/drivers/base/platform.c	2005-07-15 14:18:57.000000000 -0700
+++ linux-2.6.12.3/drivers/base/platform.c	2005-08-04 10:54:59.706802768 -0700
@@ -37,7 +37,7 @@ platform_get_resource(struct platform_de
 		struct resource *r = &dev->resource[i];
 
 		if ((r->flags & (IORESOURCE_IO|IORESOURCE_MEM|
-				 IORESOURCE_IRQ|IORESOURCE_DMA))
+				 IORESOURCE_IRQ|IORESOURCE_DMA|IORESOURCE_OTHER))
 		    == type)
 			if (num-- == 0)
 				return r;
@@ -73,7 +73,7 @@ platform_get_resource_byname(struct plat
 		struct resource *r = &dev->resource[i];
 
 		if ((r->flags & (IORESOURCE_IO|IORESOURCE_MEM|
-				 IORESOURCE_IRQ|IORESOURCE_DMA)) == type)
+				 IORESOURCE_IRQ|IORESOURCE_DMA|IORESOURCE_OTHER)) == type)
 			if (!strcmp(r->name, name))
 				return r;
 	}
diff -uprN -X dontdiff linux-2.6.12.3.orig/include/linux/ioport.h linux-2.6.12.3/include/linux/ioport.h
--- linux-2.6.12.3.orig/include/linux/ioport.h	2005-07-15 14:18:57.000000000 -0700
+++ linux-2.6.12.3/include/linux/ioport.h	2005-08-04 10:31:14.099528016 -0700
@@ -35,13 +35,14 @@ struct resource_list {
 #define IORESOURCE_MEM		0x00000200
 #define IORESOURCE_IRQ		0x00000400
 #define IORESOURCE_DMA		0x00000800
+#define IORESOURCE_OTHER	0x00001000
 
-#define IORESOURCE_PREFETCH	0x00001000	/* No side effects */
-#define IORESOURCE_READONLY	0x00002000
-#define IORESOURCE_CACHEABLE	0x00004000
-#define IORESOURCE_RANGELENGTH	0x00008000
-#define IORESOURCE_SHADOWABLE	0x00010000
-#define IORESOURCE_BUS_HAS_VGA	0x00080000
+#define IORESOURCE_PREFETCH	0x00010000	/* No side effects */
+#define IORESOURCE_READONLY	0x00020000
+#define IORESOURCE_CACHEABLE	0x00040000
+#define IORESOURCE_RANGELENGTH	0x00080000
+#define IORESOURCE_SHADOWABLE	0x00100000
+#define IORESOURCE_BUS_HAS_VGA	0x00800000
 
 #define IORESOURCE_DISABLED	0x10000000
 #define IORESOURCE_UNSET	0x20000000

Re: [PATCH] Custom IORESOURCE Class

[Adam Belay]

On Mon, Aug 08, 2005 at 09:00:21AM -0700, Greg KH wrote:
> On Mon, Aug 08, 2005 at 11:11:45AM -0700, Matthew Gilbert wrote:
> > Below is a patch that adds an additional resource class to the platform 
> > resource types. This is to support additional resources that need to be passed
> > to drivers without overloading the existing specific types. In my case, I need
> > to send clock information to the driver to enable power management. 
> > 
> > Signed-off-by: Matthew Gilbert <mgilbert@mvista.com>
> 
> Hm, you do realize that Pat's no longer the driver core maintainer?  :)
> 
> Anyway, Russell and Adam, any objections to this patch?

I'm not sure if I agree with this patch.  "struct resource" is used primarily for
I/O resource assignment.  Although I agree we may need to add new IORESOURCE types,
I'm not sure if clock data belongs here.  I don't think "start" and "end" would be
useful for most platform data.  Could you provide more information about this
specific issue and resource type?  Maybe we could create a new sysfs attribute?

Thanks,
Adam

Re: [PATCH] Custom IORESOURCE Class

[Kumar Gala]

On Aug 8, 2005, at 6:17 PM, Adam Belay wrote:

> On Mon, Aug 08, 2005 at 09:00:21AM -0700, Greg KH wrote:
>
>> On Mon, Aug 08, 2005 at 11:11:45AM -0700, Matthew Gilbert wrote:
>>
>>> Below is a patch that adds an additional resource class to the
>>>
> platform
>
>>> resource types. This is to support additional resources that need to
>>>
> be passed
>
>>> to drivers without overloading the existing specific types. In my
>>>
> case, I need
>
>>> to send clock information to the driver to enable power management.
>>>
>>> Signed-off-by: Matthew Gilbert <mgilbert@mvista.com>
>>>
>>
>> Hm, you do realize that Pat's no longer the driver core maintainer?
>>
> :)
>
>>
>> Anyway, Russell and Adam, any objections to this patch?
>>
>
> I'm not sure if I agree with this patch.  "struct resource" is used
> primarily for
> I/O resource assignment.  Although I agree we may need to add new
> IORESOURCE types,
> I'm not sure if clock data belongs here.  I don't think "start" and
> "end" would be
> useful for most platform data.  Could you provide more information  
> about
> this
> specific issue and resource type?  Maybe we could create a new sysfs
> attribute?

I would also like to understand more about what the need is here.  We  
have clock data and such but use platform_data for it.

- kumar

Re: [PATCH] Custom IORESOURCE Class

[Matthew Gilbert]

On Mon, 2005-08-08 at 23:23 -0500, Kumar Gala wrote:
> On Aug 8, 2005, at 6:17 PM, Adam Belay wrote:
> 
> > On Mon, Aug 08, 2005 at 09:00:21AM -0700, Greg KH wrote:
> >
> >> On Mon, Aug 08, 2005 at 11:11:45AM -0700, Matthew Gilbert wrote:
> >>
> >>> Below is a patch that adds an additional resource class to the
> >>>
> > platform
> >
> >>> resource types. This is to support additional resources that need to
> >>>
> > be passed
> >
> >>> to drivers without overloading the existing specific types. In my
> >>>
> > case, I need
> >
> >>> to send clock information to the driver to enable power management.
> >>>
> >>> Signed-off-by: Matthew Gilbert <mgilbert@mvista.com>
> >>>
> >>
> >> Hm, you do realize that Pat's no longer the driver core maintainer?
> >>
> > :)
> >
> >>
> >> Anyway, Russell and Adam, any objections to this patch?
> >>
> >
> > I'm not sure if I agree with this patch.  "struct resource" is used
> > primarily for
> > I/O resource assignment.  Although I agree we may need to add new
> > IORESOURCE types,
> > I'm not sure if clock data belongs here.  I don't think "start" and
> > "end" would be
> > useful for most platform data.  Could you provide more information  
> > about
> > this
> > specific issue and resource type?  Maybe we could create a new sysfs
> > attribute?
> 
> I would also like to understand more about what the need is here.  We  
> have clock data and such but use platform_data for it.

I am using IORESOURCE_MEM to pass in the base addresses of the necessary
clock registers. I also need to pass a fractional divider clk id. The
resource table seemed appropriate because the base addresses and the
divider id are closely related. Its also a great framework for enabling
varying resource lists. Currently I don't use this, but in the future I
may. Its possible in a future board revision there may not be a
fractional divider available. The resource framework makes querying for
the clk id very straight forward as opposed to magic values in a struct
I pass through platform_data. 

It can easily be moved to platform_data (or split between the two) if
that is more appropriate. Thanks for the feedback. _matt