SoC base drivers: SoC helper API
RFC PATCH 1/4 SoC base drivers: SoC helper API
Hello linux-kernel,
soc-core: Helper API for SoC base drivers to register/unregister subdevices.
Signed-off-by Paul Sokolovsky <[email protected]>
arch/arm/Kconfig | 2 + drivers/Makefile | 1 + drivers/soc/soc-core.c | 106 ++++++++++++++++++++++++++++++++++++++++++++++++ drivers/soc/soc-core.h | 30 +++++++++++++ include/linux/ioport.h | 3 + 5 files changed, 142 insertions(+), 0 deletions(-)
arch/arm/Kconfig +2 -0
@@ -997,6 +997,8 @@ source "drivers/mmc/Kconfig" source "drivers/rtc/Kconfig" +source "drivers/soc/Kconfig" + endmenu source "fs/Kconfig"
drivers/Makefile +1 -0
@@ -80,3 +80,4 @@ obj-$(CONFIG_GENERIC_TIME) += clocksource/ obj-$(CONFIG_DMA_ENGINE) += dma/ obj-$(CONFIG_HID) += hid/ obj-$(CONFIG_PPC_PS3) += ps3/ +obj-y += soc/
drivers/soc/soc-core.c new file +106 -0
106 lines not on this page · raw message
drivers/soc/soc-core.h new file +30 -0
@@ -0,0 +1,30 @@ +/* +* drivers/soc/soc-core.h +* +* core SoC support +* Copyright (c) 2006 Ian Molton +* +* This program is free software; you can redistribute it and/or modify +* it under the terms of the GNU General Public License version 2 as +* published by the Free Software Foundation. +* +* This file contains prototypes for the functions in soc-core.c +* +* Created: 2006-11-28 +* +*/ + +struct soc_device_data { +char *name; +struct resource *res; +int num_resources; +void *hwconfig;/* platform_data to pass to the subdevice */ +}; + +struct platform_device *soc_add_devices (struct platform_device *dev, +struct soc_device_data *soc,int n_devs, +struct resource *mem, +int relative_addr_shift,int irq_base); + +void soc_free_devices (struct platform_device *devices,int nr_devs); +
include/linux/ioport.h +3 -0
@@ -57,6 +57,9 @@ struct resource_list {#define IORESOURCE_IRQ_LOWLEVEL (1<<3)#define IORESOURCE_IRQ_SHAREABLE (1<<4) +/* SoC device IRQ specific bits (IORESOURCE_BITS) */ +#define IORESOURCE_IRQ_SOC_SUBDEVICE (1<<5) +/* ISA PnP DMA specific bits (IORESOURCE_BITS) */ #define IORESOURCE_DMA_TYPE_MASK (3<<0)#define IORESOURCE_DMA_8BIT (0<<0)
Signature · 2 lines
Best regards, Paul mailto:[email protected]
Quoted 33 lines · On Tue, 1 May 2007 08:08:27 +0300 Paul Sokolovsky <[email protected]> wrote:
drivers/soc/soc-core.c new file +27
-0@@ -0,0 +1,106 @@ +/* +* drivers/soc/soc-core.c +* +* core SoC support +* Copyright (c) 2006 Ian Molton +* +* This program is free software; you can redistribute it and/or modify +* it under the terms of the GNU General Public License version 2 as +* published by the Free Software Foundation. +* +* This file contains functionality used by many SoC type devices. +* +* Created: 2006-11-28 +* +*/ + +#include <linux/ioport.h> +#include <linux/slab.h> +#include <linux/kernel.h> +#include <linux/platform_device.h> +#include "soc-core.h" + +void soc_free_devices (struct platform_device *devices,int nr_devs) +{ +struct platform_device *dev = devices; +int i; +
drivers/soc/soc-core.c line 37
Quoted 10 lines
drivers/soc/soc-core.c new file +10
-0+for (i =0 ; i < nr_devs; i++) { +struct resource *res = dev->resource; +platform_device_unregister (dev++); +kfree (res); + } +kfree (devices); +} +EXPORT_SYMBOL_GPL (soc_free_devices); + +#define SIGNED_SHIFT (val, shift) ((shift) >= 0 ? ((val) << (shift)) : ((val) >> -(shift)))
This will do bad things with
SIGNED_SHIFT(val, distance++);
So either
a) take a copy of the args inside the macro or, better
b) use a static inline.
It's probably worth putting the result into include/linux/kernel.h as well. It's quite generic. If so, that's a standalone patch, please. Or keep it here if you can't be bothered with that.
Quoted 21 lines
drivers/soc/soc-core.c new file +21
-0+struct platform_device *soc_add_devices (struct platform_device *dev, +struct soc_device_data *soc,int nr_devs, +struct resource *mem, +int relative_addr_shift,int irq_base) +{ +struct platform_device *devices; +int i, r, base; + + devices =kzalloc (nr_devs *sizeof (struct platform_device ),GFP_KERNEL ); +if (!devices) +return NULL ; + +for (i =0 ; i < nr_devs; i++) { +struct platform_device *sdev = &devices[i]; +struct soc_device_data *blk = &soc[i]; +struct resource *res; + + sdev->id =-1 ; + sdev->name = blk->name; + + sdev->dev.parent = &dev->dev;
drivers/soc/soc-core.c line 69
Quoted 10 lines
drivers/soc/soc-core.c new file +10
-0+ sdev->dev.platform_data = (void *)blk->hwconfig; + sdev->num_resources = blk->num_resources; + +/* Allocate space for the subdevice resources */ + res =kzalloc (blk->num_resources *sizeof (struct resource ),GFP_KERNEL ); +if (!res) + goto fail; + +for (r =0 ; r < blk->num_resources ; r++) { + res[r].name = blk->res[r].name;// Fixme - should copy
fix me then ;)
With kstrdup(), presumably.
drivers/soc/soc-core.c line 74
Quoted 5 lines
drivers/soc/soc-core.c new file +5
-0+ +/* Find out base to use */ + base =0 ; +if (blk->res[r].flags &IORESOURCE_MEM ) { + base = mem->start;
mem->start is of type resource_size_t, and `base' should be of that type as well.
drivers/soc/soc-core.c line 84
Quoted 10 lines
drivers/soc/soc-core.c new file +10
-0+ }else if ((blk->res[r].flags &IORESOURCE_IRQ ) && + (blk->res[r].flags &IORESOURCE_IRQ_SOC_SUBDEVICE )) { + base = irq_base; + } + +/* Adjust resource */ +if (blk->res[r].flags &IORESOURCE_MEM ) { + res[r].parent = mem; + res[r].start = base +SIGNED_SHIFT (blk->res[r].start, relative_addr_shift); + res[r].end = base +SIGNED_SHIFT (blk->res[r].end, relative_addr_shift);
Which means that SIGNED_SHIFT really has to be a macro, as it needs to handle resource_size_t's and you don't know what type they have.
Also, things get interesting when working out how to handle right-shift of a negative quantity of unknown type.
Quoted 64 lines
drivers/soc/soc-core.c new file +22
-0+ }else { + res[r].start = base + blk->res[r].start; + res[r].end = base + blk->res[r].end; + } + res[r].flags = blk->res[r].flags; + } + + sdev->resource = res; +if (platform_device_register (sdev)) { +kfree (res); + goto fail; + } + +printk (KERN_INFO "SoC: registering %s\n" , blk->name); + } +return devices; + +fail: +soc_free_devices (devices, i +1 ); +return NULL ; +} +EXPORT_SYMBOL_GPL (soc_add_devices);drivers/soc/soc-core.h new file +30
-0@@ -0,0 +1,30 @@ +/* +* drivers/soc/soc-core.h +* +* core SoC support +* Copyright (c) 2006 Ian Molton +* +* This program is free software; you can redistribute it and/or modify +* it under the terms of the GNU General Public License version 2 as +* published by the Free Software Foundation. +* +* This file contains prototypes for the functions in soc-core.c +* +* Created: 2006-11-28 +* +*/ + +struct soc_device_data { +char *name; +struct resource *res; +int num_resources; +void *hwconfig;/* platform_data to pass to the subdevice */ +}; + +struct platform_device *soc_add_devices (struct platform_device *dev, +struct soc_device_data *soc,int n_devs, +struct resource *mem, +int relative_addr_shift,int irq_base); + +void soc_free_devices (struct platform_device *devices,int nr_devs); +include/linux/ioport.h +0
-0
include/linux/ioport.h struct resource_list { · line 65
Quoted 9 lines
include/linux/ioport.h +3
-0@@ -57,6 +57,9 @@ struct resource_list {#define IORESOURCE_IRQ_LOWLEVEL (1<<3)#define IORESOURCE_IRQ_SHAREABLE (1<<4) +/* SoC device IRQ specific bits (IORESOURCE_BITS) */ +#define IORESOURCE_IRQ_SOC_SUBDEVICE (1<<5) +/* ISA PnP DMA specific bits (IORESOURCE_BITS) */ #define IORESOURCE_DMA_TYPE_MASK (3<<0)#define IORESOURCE_DMA_8BIT (0<<0)
It looks reasonable to me, but I'm not Greg or Russell or anything like that.
Quoted 2 lines · Paul Sokolovsky
soc-core: Helper API for SoC base drivers to register/unregister subdevices.
It would be nice if the code said at some point what SoC stands for. It makes me think of Silicon on Conductor, but I don't think that's right.
Signature · 2 lines
Ben Pfaff http://benpfaff.org
drivers/soc/soc-core.c line 33
Quoted 11 lines · Paul Sokolovsky
drivers/soc/soc-core.c new file +11
-0+void soc_free_devices (struct platform_device *devices,int nr_devs) +{ +struct platform_device *dev = devices; +int i; + +for (i =0 ; i < nr_devs; i++) { +struct resource *res = dev->resource; +platform_device_unregister (dev++); +kfree (res); + } +kfree (devices);
Utterly unsafe. Repeat after me: kfreeing the memory assocated with any kobject is asking for an OOPS. Do not do it.
There's a reason that platform_device_alloc() and friends exist. If you want to unregister platform devices, use those functions. Don't try to invent your own buggy variants.
Signature · 3 lines
Russell King Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/ maintainer of: