Hello,
These patches introduce some clean-up code and fix one bug related to the
presence of ipack_driver ops.
The big change is added by [PATCH 2/5] where it adds a better support to device
model in the ipack driver.
All the devices (carrier and mezzanine boards) are ipack_devices and it is
needed to make a relation between an ipack_device and an ipack_driver using
the provided match/probe functions.
Doing that, it is possible to represent the relation between each carrier and
mezzanines plugged to it through sysfs and, also, between each device and its
driver.
These patches apply on top of the previous ones:
http://lkml.indiana.edu/hypermail/linux/kernel/1205.1/02410.html
Thanks,
Sam
Samuel Iglesias Gonsalvez (5):
Staging ipack: fix a few sparse warnings
Staging: ipack: add proper device model into ipack_bus_register.
Staging: ipack: remove board_name and bus_name fields from struct
ipack_device
Staging ipack/bridges/tpci200: remove TPCI200_SHORTNAME constant
Staging: ipack/devices/ipoctal: check for availability of helper
functions in ipoctal_match()
drivers/staging/ipack/bridges/tpci200.c | 225 ++++++++++++++++---------------
drivers/staging/ipack/bridges/tpci200.h | 18 ++-
drivers/staging/ipack/devices/ipoctal.c | 12 +-
drivers/staging/ipack/ipack.c | 68 ++++++++--
drivers/staging/ipack/ipack.h | 61 ++++-----
5 files changed, 220 insertions(+), 164 deletions(-)
--
1.7.10
Signed-off-by: Samuel Iglesias Gonsalvez <[email protected]>
---
drivers/staging/ipack/bridges/tpci200.c | 51 ++++++++++++++++++-------------
drivers/staging/ipack/bridges/tpci200.h | 6 ++--
drivers/staging/ipack/devices/ipoctal.c | 4 +--
drivers/staging/ipack/ipack.h | 2 +-
4 files changed, 35 insertions(+), 28 deletions(-)
diff --git a/drivers/staging/ipack/bridges/tpci200.c b/drivers/staging/ipack/bridges/tpci200.c
index 80cdd9e..4e812a7 100644
--- a/drivers/staging/ipack/bridges/tpci200.c
+++ b/drivers/staging/ipack/bridges/tpci200.c
@@ -16,7 +16,7 @@
#include <linux/module.h>
#include "tpci200.h"
-struct ipack_bus_ops tpci200_bus_ops;
+static struct ipack_bus_ops tpci200_bus_ops;
/* TPCI200 controls registers */
static int control_reg[] = {
@@ -70,35 +70,39 @@ static struct tpci200_board *check_slot(struct ipack_device *dev)
return tpci200;
}
-static inline unsigned char __tpci200_read8(void *address, unsigned long offset)
+static inline unsigned char __tpci200_read8(void __iomem *address,
+ unsigned long offset)
{
return ioread8(address + (offset^1));
}
-static inline unsigned short __tpci200_read16(void *address,
+static inline unsigned short __tpci200_read16(void __iomem *address,
unsigned long offset)
{
return ioread16(address + offset);
}
-static inline unsigned int __tpci200_read32(void *address, unsigned long offset)
+static inline unsigned int __tpci200_read32(void __iomem *address,
+ unsigned long offset)
{
return swahw32(ioread32(address + offset));
}
static inline void __tpci200_write8(unsigned char value,
- void *address, unsigned long offset)
+ void __iomem *address, unsigned long offset)
{
iowrite8(value, address+(offset^1));
}
-static inline void __tpci200_write16(unsigned short value, void *address,
+static inline void __tpci200_write16(unsigned short value,
+ void __iomem *address,
unsigned long offset)
{
iowrite16(value, address+offset);
}
-static inline void __tpci200_write32(unsigned int value, void *address,
+static inline void __tpci200_write32(unsigned int value,
+ void __iomem *address,
unsigned long offset)
{
iowrite32(swahw32(value), address+offset);
@@ -318,7 +322,7 @@ static irqreturn_t tpci200_interrupt(int irq, void *dev_id)
spin_lock_irqsave(&tpci200->info->access_lock, flags);
/* Read status register */
- status_reg = readw((unsigned short *) (tpci200->info->interface_regs +
+ status_reg = readw((void __iomem *) (tpci200->info->interface_regs +
TPCI200_STATUS_REG));
if (status_reg & TPCI200_SLOT_INT_MASK) {
@@ -331,10 +335,10 @@ static irqreturn_t tpci200_interrupt(int irq, void *dev_id)
ret = tpci200->slots[i].irq->handler(tpci200->slots[i].irq->arg);
/* Dummy reads */
- readw((unsigned short *)
+ readw((void __iomem *)
(tpci200->slots[i].dev->io_space.address +
0xC0));
- readw((unsigned short *)
+ readw((void __iomem *)
(tpci200->slots[i].dev->io_space.address +
0xC2));
@@ -349,12 +353,14 @@ static irqreturn_t tpci200_interrupt(int irq, void *dev_id)
pr_info("No registered ISR for slot [%s %d:%d]!. IRQ will be disabled.\n",
TPCI200_SHORTNAME,
tpci200->number, i);
- reg_value = readw((unsigned short *)(tpci200->info->interface_regs +
+ reg_value = readw(
+ (void __iomem *)(tpci200->info->interface_regs +
control_reg[i]));
reg_value &=
~(TPCI200_INT0_EN | TPCI200_INT1_EN);
- writew(reg_value, (unsigned short *)(tpci200->info->interface_regs +
- control_reg[i]));
+ writew(reg_value,
+ (void __iomem *)(tpci200->info->interface_regs +
+ control_reg[i]));
}
}
}
@@ -675,21 +681,21 @@ static int tpci200_register(struct tpci200_board *tpci200)
/* Set all slot physical address space */
for (i = 0; i < TPCI200_NB_SLOT; i++) {
tpci200->slots[i].io_phys.address =
- (void *)ioidint_base +
+ (void __iomem *)ioidint_base +
TPCI200_IO_SPACE_OFF + TPCI200_IO_SPACE_GAP*i;
tpci200->slots[i].io_phys.size = TPCI200_IO_SPACE_SIZE;
tpci200->slots[i].id_phys.address =
- (void *)ioidint_base +
+ (void __iomem *)ioidint_base +
TPCI200_ID_SPACE_OFF + TPCI200_ID_SPACE_GAP*i;
tpci200->slots[i].id_phys.size = TPCI200_ID_SPACE_SIZE;
tpci200->slots[i].mem_phys.address =
- (void *)mem_base + TPCI200_MEM8_GAP*i;
+ (void __iomem *)mem_base + TPCI200_MEM8_GAP*i;
tpci200->slots[i].mem_phys.size = TPCI200_MEM8_SIZE;
writew(slot_ctrl,
- (unsigned short *)(tpci200->info->interface_regs +
+ (void __iomem *)(tpci200->info->interface_regs +
control_reg[i]));
}
@@ -732,7 +738,7 @@ static int __tpci200_request_irq(struct tpci200_board *tpci200,
* clock rate 8 MHz
*/
slot_ctrl = TPCI200_INT0_EN | TPCI200_INT1_EN;
- writew(slot_ctrl, (unsigned short *)(tpci200->info->interface_regs +
+ writew(slot_ctrl, (void __iomem *)(tpci200->info->interface_regs +
control_reg[dev->slot]));
return 0;
@@ -752,8 +758,9 @@ static void __tpci200_free_irq(struct tpci200_board *tpci200,
* clock rate 8 MHz
*/
slot_ctrl = 0;
- writew(slot_ctrl, (unsigned short *)(tpci200->info->interface_regs +
- control_reg[dev->slot]));
+ writew(slot_ctrl,
+ (void __iomem *)(tpci200->info->interface_regs +
+ control_reg[dev->slot]));
}
static int tpci200_free_irq(struct ipack_device *dev)
@@ -878,7 +885,7 @@ static int tpci200_slot_map_space(struct ipack_device *dev,
{
int res;
unsigned int size_to_map;
- void *phys_address;
+ void __iomem *phys_address;
struct ipack_addr_space *virt_addr_space;
struct tpci200_board *tpci200;
@@ -1041,7 +1048,7 @@ static void tpci200_uninstall(struct tpci200_board *tpci200)
kfree(tpci200->slots);
}
-struct ipack_bus_ops tpci200_bus_ops = {
+static struct ipack_bus_ops tpci200_bus_ops = {
.map_space = tpci200_slot_map_space,
.unmap_space = tpci200_slot_unmap_space,
.request_irq = tpci200_request_irq,
diff --git a/drivers/staging/ipack/bridges/tpci200.h b/drivers/staging/ipack/bridges/tpci200.h
index 75801f6..c052107 100644
--- a/drivers/staging/ipack/bridges/tpci200.h
+++ b/drivers/staging/ipack/bridges/tpci200.h
@@ -147,9 +147,9 @@ struct tpci200_slot {
struct tpci200_infos {
struct pci_dev *pdev;
struct pci_device_id *id_table;
- void *interface_regs;
- void *ioidint_space;
- void *mem8_space;
+ void __iomem *interface_regs;
+ void __iomem *ioidint_space;
+ void __iomem *mem8_space;
spinlock_t access_lock;
struct ipack_bus_device drv;
};
diff --git a/drivers/staging/ipack/devices/ipoctal.c b/drivers/staging/ipack/devices/ipoctal.c
index a28c677..c88f391 100644
--- a/drivers/staging/ipack/devices/ipoctal.c
+++ b/drivers/staging/ipack/devices/ipoctal.c
@@ -72,7 +72,7 @@ static inline void ipoctal_write_io_reg(struct ipoctal *ipoctal,
{
unsigned long offset;
- offset = ((void *) dest) - ipoctal->dev->io_space.address;
+ offset = ((void __iomem *) dest) - ipoctal->dev->io_space.address;
ipoctal->dev->ops->write8(ipoctal->dev, IPACK_IO_SPACE, offset, value);
}
@@ -89,7 +89,7 @@ static inline unsigned char ipoctal_read_io_reg(struct ipoctal *ipoctal,
unsigned long offset;
unsigned char value;
- offset = ((void *) src) - ipoctal->dev->io_space.address;
+ offset = ((void __iomem *) src) - ipoctal->dev->io_space.address;
ipoctal->dev->ops->read8(ipoctal->dev, IPACK_IO_SPACE, offset, &value);
return value;
}
diff --git a/drivers/staging/ipack/ipack.h b/drivers/staging/ipack/ipack.h
index 3aa38c5..7f408ad 100644
--- a/drivers/staging/ipack/ipack.h
+++ b/drivers/staging/ipack/ipack.h
@@ -42,7 +42,7 @@ enum ipack_space {
* @size: size of the mapped space
*/
struct ipack_addr_space {
- void *address;
+ void __iomem *address;
unsigned int size;
};
--
1.7.10
When installing the module, there could be a kernel oops due to a dereference of
a NULL pointer.
Signed-off-by: Samuel Iglesias Gonsalvez <[email protected]>
---
drivers/staging/ipack/devices/ipoctal.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/staging/ipack/devices/ipoctal.c b/drivers/staging/ipack/devices/ipoctal.c
index 7e370d3..df42b9b 100644
--- a/drivers/staging/ipack/devices/ipoctal.c
+++ b/drivers/staging/ipack/devices/ipoctal.c
@@ -799,6 +799,9 @@ static int ipoctal_match(struct ipack_device *dev)
int res;
unsigned char board_id;
+ if ((!dev->ops) || (!dev->ops->map_space) || (!dev->ops->unmap_space))
+ return -ENODEV;
+
res = dev->ops->map_space(dev, 0, IPACK_ID_SPACE);
if (res)
return res;
--
1.7.10
Removed board_name and bus_name fields from struct ipack_device that are
completely useless.
Signed-off-by: Samuel Iglesias Gonsalvez <[email protected]>
---
drivers/staging/ipack/bridges/tpci200.c | 31 ++++---------------------------
drivers/staging/ipack/bridges/tpci200.h | 8 ++++----
drivers/staging/ipack/ipack.h | 4 ----
3 files changed, 8 insertions(+), 35 deletions(-)
diff --git a/drivers/staging/ipack/bridges/tpci200.c b/drivers/staging/ipack/bridges/tpci200.c
index c7bfb98..e6bc2cc 100644
--- a/drivers/staging/ipack/bridges/tpci200.c
+++ b/drivers/staging/ipack/bridges/tpci200.c
@@ -372,9 +372,7 @@ static irqreturn_t tpci200_interrupt(int irq, void *dev_id)
#ifdef CONFIG_SYSFS
-static struct ipack_device *tpci200_slot_register(const char *board_name,
- int size,
- unsigned int tpci200_number,
+static struct ipack_device *tpci200_slot_register(unsigned int tpci200_number,
unsigned int slot_position)
{
int found = 0;
@@ -416,17 +414,6 @@ static struct ipack_device *tpci200_slot_register(const char *board_name,
goto err_unlock;
}
- if (size > IPACK_BOARD_NAME_SIZE) {
- pr_warning("Slot [%s %d:%d] name (%s) too long (%d > %d). Will be truncated!\n",
- TPCI200_SHORTNAME, tpci200_number, slot_position,
- board_name, (int)strlen(board_name),
- IPACK_BOARD_NAME_SIZE);
-
- size = IPACK_BOARD_NAME_SIZE;
- }
-
- strncpy(dev->board_name, board_name, size-1);
- dev->board_name[size-1] = '\0';
/*
* Give the same IRQ number as the slot number.
* The TPCI200 has assigned his own two IRQ by PCI bus driver
@@ -461,7 +448,7 @@ static ssize_t tpci200_store_board(struct device *pdev, const char *buf,
if (dev != NULL)
return -EBUSY;
- dev = tpci200_slot_register(buf, count, card->number, slot);
+ dev = tpci200_slot_register(card->number, slot);
if (dev == NULL)
return -ENODEV;
@@ -474,7 +461,7 @@ static ssize_t tpci200_show_board(struct device *pdev, char *buf, int slot)
struct ipack_device *dev = card->slots[slot].dev;
if (dev != NULL)
- return snprintf(buf, PAGE_SIZE, "%s\n", dev->board_name);
+ return snprintf(buf, PAGE_SIZE, "%s\n", dev_name(&dev->dev));
else
return snprintf(buf, PAGE_SIZE, "none\n");
}
@@ -1000,17 +987,7 @@ static int tpci200_request_irq(struct ipack_device *dev, int vector,
slot_irq->vector = vector;
slot_irq->handler = handler;
slot_irq->arg = arg;
- if (dev->board_name) {
- if (strlen(dev->board_name) > IPACK_IRQ_NAME_SIZE) {
- pr_warning("Slot [%s %d:%d] IRQ name too long (%d char > %d char MAX). Will be truncated!\n",
- TPCI200_SHORTNAME, dev->bus_nr, dev->slot,
- (int)strlen(dev->board_name),
- IPACK_IRQ_NAME_SIZE);
- }
- strncpy(slot_irq->name, dev->board_name, IPACK_IRQ_NAME_SIZE-1);
- } else {
- strcpy(slot_irq->name, "Unknown");
- }
+ slot_irq->name = dev_name(&dev->dev);
tpci200->slots[dev->slot].irq = slot_irq;
res = __tpci200_request_irq(tpci200, dev);
diff --git a/drivers/staging/ipack/bridges/tpci200.h b/drivers/staging/ipack/bridges/tpci200.h
index e3a7e5d..7eb994d 100644
--- a/drivers/staging/ipack/bridges/tpci200.h
+++ b/drivers/staging/ipack/bridges/tpci200.h
@@ -112,10 +112,10 @@
*
*/
struct slot_irq {
- int vector;
- int (*handler)(void *);
- void *arg;
- char name[IPACK_IRQ_NAME_SIZE];
+ int vector;
+ int (*handler)(void *);
+ void *arg;
+ const char *name;
};
/**
diff --git a/drivers/staging/ipack/ipack.h b/drivers/staging/ipack/ipack.h
index 524ea93..96274f4 100644
--- a/drivers/staging/ipack/ipack.h
+++ b/drivers/staging/ipack/ipack.h
@@ -11,8 +11,6 @@
#include <linux/device.h>
-#define IPACK_BOARD_NAME_SIZE 16
-#define IPACK_IRQ_NAME_SIZE 50
#define IPACK_IDPROM_OFFSET_I 0x01
#define IPACK_IDPROM_OFFSET_P 0x03
#define IPACK_IDPROM_OFFSET_A 0x05
@@ -66,8 +64,6 @@ struct ipack_addr_space {
* by the carrier board throught @ops.
*/
struct ipack_device {
- char board_name[IPACK_BOARD_NAME_SIZE];
- char bus_name[IPACK_BOARD_NAME_SIZE];
unsigned int bus_nr;
unsigned int slot;
unsigned int irq;
--
1.7.10
This patch adds a proper device model to the driver. The carrier boards are
managed like other ipack device, the way to recognize them is using the
platform data field from struct device.
The carrier driver should register itself as ipack_driver before it registers a
new bus device. It should implement the match/probe functions to recognize the
device as bus device using the field platform_data that was filled by ipack
driver before.
Signed-off-by: Samuel Iglesias Gonsalvez <[email protected]>
---
drivers/staging/ipack/bridges/tpci200.c | 70 +++++++++++++++++++++++--------
drivers/staging/ipack/bridges/tpci200.h | 2 +-
drivers/staging/ipack/devices/ipoctal.c | 5 +--
drivers/staging/ipack/ipack.c | 68 ++++++++++++++++++++++++------
drivers/staging/ipack/ipack.h | 55 +++++++++++++-----------
5 files changed, 140 insertions(+), 60 deletions(-)
diff --git a/drivers/staging/ipack/bridges/tpci200.c b/drivers/staging/ipack/bridges/tpci200.c
index 4e812a7..c7bfb98 100644
--- a/drivers/staging/ipack/bridges/tpci200.c
+++ b/drivers/staging/ipack/bridges/tpci200.c
@@ -17,6 +17,7 @@
#include "tpci200.h"
static struct ipack_bus_ops tpci200_bus_ops;
+static struct ipack_driver tpci200_ipack_drv;
/* TPCI200 controls registers */
static int control_reg[] = {
@@ -407,7 +408,7 @@ static struct ipack_device *tpci200_slot_register(const char *board_name,
goto err_unlock;
}
- dev = kzalloc(sizeof(struct ipack_device), GFP_KERNEL);
+ dev = ipack_device_register(tpci200->info->ipack_bus, slot_position);
if (dev == NULL) {
pr_info("Slot [%s %d:%d] Unable to allocate memory for new slot !\n",
TPCI200_SHORTNAME,
@@ -426,8 +427,6 @@ static struct ipack_device *tpci200_slot_register(const char *board_name,
strncpy(dev->board_name, board_name, size-1);
dev->board_name[size-1] = '\0';
- dev->bus_nr = tpci200->info->drv.bus_nr;
- dev->slot = slot_position;
/*
* Give the same IRQ number as the slot number.
* The TPCI200 has assigned his own two IRQ by PCI bus driver
@@ -445,15 +444,9 @@ static struct ipack_device *tpci200_slot_register(const char *board_name,
dev->ops = &tpci200_bus_ops;
tpci200->slots[slot_position].dev = dev;
- if (ipack_device_register(dev) < 0)
- goto err_unregister;
-
mutex_unlock(&tpci200->mutex);
return dev;
-err_unregister:
- tpci200_slot_unregister(dev);
- kfree(dev);
err_unlock:
mutex_unlock(&tpci200->mutex);
return NULL;
@@ -1116,20 +1109,19 @@ static int tpci200_pciprobe(struct pci_dev *pdev,
return -ENODEV;
}
- tpci200->info->drv.dev = &pdev->dev;
- tpci200->info->drv.slots = TPCI200_NB_SLOT;
-
- /* Register the bus in the industry pack driver */
- ret = ipack_bus_register(&tpci200->info->drv);
- if (ret < 0) {
+ /* Register the carrier in the industry pack bus driver */
+ tpci200->info->ipack_bus = ipack_bus_register(&tpci200_ipack_drv, 0,
+ TPCI200_NB_SLOT);
+ if (!tpci200->info->ipack_bus) {
pr_err("error registering the carrier on ipack driver\n");
tpci200_uninstall(tpci200);
kfree(tpci200->info);
kfree(tpci200);
return -EFAULT;
}
+
/* save the bus number given by ipack to logging purpose */
- tpci200->number = tpci200->info->drv.bus_nr;
+ tpci200->number = tpci200->info->ipack_bus->bus_nr;
dev_set_drvdata(&pdev->dev, tpci200);
/* add the registered device in an internal linked list */
list_add_tail(&tpci200->list, &tpci200_list);
@@ -1141,7 +1133,8 @@ static void __tpci200_pci_remove(struct tpci200_board *tpci200)
tpci200_uninstall(tpci200);
tpci200_remove_sysfs_files(tpci200);
list_del(&tpci200->list);
- ipack_bus_unregister(&tpci200->info->drv);
+ ipack_bus_unregister(tpci200->info->ipack_bus);
+ kfree(tpci200->info);
kfree(tpci200);
}
@@ -1171,9 +1164,49 @@ static struct pci_driver tpci200_pci_drv = {
.remove = __devexit_p(tpci200_pci_remove),
};
+static int tpci200_ipack_match(struct ipack_device *dev)
+{
+ /*
+ * If this device is a bus device, platform_data
+ * has a pointer to the corresponding ipack_driver
+ */
+ if ((dev->dev.platform_data) &&
+ (dev->dev.platform_data == &tpci200_ipack_drv))
+ return 0;
+
+ return -ENODEV;
+}
+
+static int tpci200_ipack_probe(struct ipack_device *dev)
+{
+ /*
+ * Already matched the device with the driver in
+ * the match function. There is not needed to do more.
+ */
+ return 0;
+}
+
+static struct ipack_driver_ops tpci200_ipack_drv_ops = {
+ .match = tpci200_ipack_match,
+ .probe = tpci200_ipack_probe,
+ .remove = NULL,
+};
+
static int __init tpci200_drvr_init_module(void)
{
- return pci_register_driver(&tpci200_pci_drv);
+ int ret;
+
+ tpci200_ipack_drv.ops = &tpci200_ipack_drv_ops;
+ ret = ipack_driver_register(&tpci200_ipack_drv, THIS_MODULE,
+ KBUILD_MODNAME);
+ if (ret)
+ return ret;
+
+ ret = pci_register_driver(&tpci200_pci_drv);
+ if (ret)
+ ipack_driver_unregister(&tpci200_ipack_drv);
+
+ return ret;
}
static void __exit tpci200_drvr_exit_module(void)
@@ -1184,6 +1217,7 @@ static void __exit tpci200_drvr_exit_module(void)
__tpci200_pci_remove(tpci200);
pci_unregister_driver(&tpci200_pci_drv);
+ ipack_driver_unregister(&tpci200_ipack_drv);
}
MODULE_DESCRIPTION("TEWS TPCI-200 device driver");
diff --git a/drivers/staging/ipack/bridges/tpci200.h b/drivers/staging/ipack/bridges/tpci200.h
index c052107..e3a7e5d 100644
--- a/drivers/staging/ipack/bridges/tpci200.h
+++ b/drivers/staging/ipack/bridges/tpci200.h
@@ -151,7 +151,7 @@ struct tpci200_infos {
void __iomem *ioidint_space;
void __iomem *mem8_space;
spinlock_t access_lock;
- struct ipack_bus_device drv;
+ struct ipack_bus_device *ipack_bus;
};
struct tpci200_board {
struct list_head list;
diff --git a/drivers/staging/ipack/devices/ipoctal.c b/drivers/staging/ipack/devices/ipoctal.c
index c88f391..7e370d3 100644
--- a/drivers/staging/ipack/devices/ipoctal.c
+++ b/drivers/staging/ipack/devices/ipoctal.c
@@ -868,11 +868,8 @@ static struct ipack_driver_ops ipoctal_drv_ops = {
static int __init ipoctal_init(void)
{
- driver.owner = THIS_MODULE;
driver.ops = &ipoctal_drv_ops;
- driver.driver.name = KBUILD_MODNAME;
- ipack_driver_register(&driver);
- return 0;
+ return ipack_driver_register(&driver, THIS_MODULE, KBUILD_MODNAME);
}
static void __exit ipoctal_exit(void)
diff --git a/drivers/staging/ipack/ipack.c b/drivers/staging/ipack/ipack.c
index ad06e06..50b914a 100644
--- a/drivers/staging/ipack/ipack.c
+++ b/drivers/staging/ipack/ipack.c
@@ -13,10 +13,12 @@
#include <linux/module.h>
#include <linux/mutex.h>
+#include <linux/slab.h>
#include "ipack.h"
#define to_ipack_dev(device) container_of(device, struct ipack_device, dev)
#define to_ipack_driver(drv) container_of(drv, struct ipack_driver, driver)
+#define to_ipack_bus(device) container_of(device, struct ipack_bus_device, dev.dev)
/* used when allocating bus numbers */
#define IPACK_MAXBUS 64
@@ -28,13 +30,25 @@ struct ipack_busmap {
};
static struct ipack_busmap busmap;
+static void ipack_device_release(struct device *dev)
+{
+ struct ipack_device *device = to_ipack_dev(dev);
+ kfree(device);
+}
+
+static void ipack_bus_release(struct device *dev)
+{
+ struct ipack_bus_device *bus = to_ipack_bus(dev);
+ kfree(bus);
+}
+
static int ipack_bus_match(struct device *device, struct device_driver *driver)
{
int ret;
struct ipack_device *dev = to_ipack_dev(device);
struct ipack_driver *drv = to_ipack_driver(driver);
- if (!drv->ops->match)
+ if ((!drv->ops) || (!drv->ops->match))
return -EINVAL;
ret = drv->ops->match(dev);
@@ -92,21 +106,43 @@ error_find_busnum:
return busnum;
}
-int ipack_bus_register(struct ipack_bus_device *bus)
+struct ipack_bus_device *ipack_bus_register(struct ipack_driver *drv, int irqv, int slots)
{
int bus_nr;
+ struct ipack_bus_device *bus;
+
+ bus = kzalloc(sizeof(struct ipack_bus_device), GFP_KERNEL);
+ if (!bus)
+ return NULL;
bus_nr = ipack_assign_bus_number();
if (bus_nr < 0)
- return -1;
+ goto err_bus_register;
bus->bus_nr = bus_nr;
- return 0;
+ bus->vector = irqv;
+ bus->slots = slots;
+ bus->dev.dev.platform_data = &drv->driver;
+ bus->dev.dev.bus = &ipack_bus_type;
+ bus->dev.dev.release = ipack_bus_release;
+ dev_set_name(&bus->dev.dev, "ipack-bus%d", bus_nr);
+
+ if (device_register(&bus->dev.dev) < 0) {
+ pr_err(KBUILD_MODNAME ": failed to register bus device %d\n",
+ bus_nr);
+ goto err_bus_register;
+ }
+ return bus;
+
+err_bus_register:
+ kfree(bus);
+ return NULL;
}
EXPORT_SYMBOL_GPL(ipack_bus_register);
int ipack_bus_unregister(struct ipack_bus_device *bus)
{
+ device_unregister(&bus->dev.dev);
mutex_lock(&ipack_mutex);
clear_bit(bus->bus_nr, busmap.busmap);
mutex_unlock(&ipack_mutex);
@@ -114,8 +150,11 @@ int ipack_bus_unregister(struct ipack_bus_device *bus)
}
EXPORT_SYMBOL_GPL(ipack_bus_unregister);
-int ipack_driver_register(struct ipack_driver *edrv)
+int ipack_driver_register(struct ipack_driver *edrv, struct module *owner,
+ char *name)
{
+ edrv->driver.owner = owner;
+ edrv->driver.name = name;
edrv->driver.bus = &ipack_bus_type;
return driver_register(&edrv->driver);
}
@@ -127,26 +166,31 @@ void ipack_driver_unregister(struct ipack_driver *edrv)
}
EXPORT_SYMBOL_GPL(ipack_driver_unregister);
-static void ipack_device_release(struct device *dev)
-{
-}
-
-int ipack_device_register(struct ipack_device *dev)
+struct ipack_device *ipack_device_register(struct ipack_bus_device *bus, int slot)
{
int ret;
+ struct ipack_device *dev;
+
+ dev = kzalloc(sizeof(struct ipack_device), GFP_KERNEL);
+ if (!dev)
+ return NULL;
dev->dev.bus = &ipack_bus_type;
dev->dev.release = ipack_device_release;
+ dev->dev.parent = &bus->dev.dev;
+ dev->slot = slot;
dev_set_name(&dev->dev,
- "%s.%u.%u", dev->board_name, dev->bus_nr, dev->slot);
+ "ipack-dev.%u.%u", dev->bus_nr, dev->slot);
ret = device_register(&dev->dev);
if (ret < 0) {
pr_err("error registering the device.\n");
dev->driver->ops->remove(dev);
+ kfree(dev);
+ return NULL;
}
- return ret;
+ return dev;
}
EXPORT_SYMBOL_GPL(ipack_device_register);
diff --git a/drivers/staging/ipack/ipack.h b/drivers/staging/ipack/ipack.h
index 7f408ad..524ea93 100644
--- a/drivers/staging/ipack/ipack.h
+++ b/drivers/staging/ipack/ipack.h
@@ -94,36 +94,16 @@ struct ipack_driver_ops {
};
/**
- * struct ipack_driver -- Specific data to each mezzanine board driver
+ * struct ipack_driver -- Specific data to each ipack board driver
*
* @driver: Device driver kernel representation
* @ops: Mezzanine driver operations specific for the ipack bus.
*/
struct ipack_driver {
- struct module *owner;
struct device_driver driver;
struct ipack_driver_ops *ops;
};
-/*
- * ipack_driver_register -- Register a new mezzanine driver
- *
- * Called by the mezzanine driver to register itself as a driver
- * that can manage ipack devices.
- */
-
-int ipack_driver_register(struct ipack_driver *edrv);
-void ipack_driver_unregister(struct ipack_driver *edrv);
-
-/*
- * ipack_device_register -- register a new mezzanine device
- *
- * Register a new ipack device (mezzanine device). The call is done by
- * the carrier device driver.
- */
-int ipack_device_register(struct ipack_device *dev);
-void ipack_device_unregister(struct ipack_device *dev);
-
/**
* struct ipack_bus_ops - available operations on a bridge module
*
@@ -162,7 +142,7 @@ struct ipack_bus_ops {
* @vector: IRQ base vector. IRQ vectors are $vector + $slot_number
*/
struct ipack_bus_device {
- struct device *dev;
+ struct ipack_device dev;
int slots;
int bus_nr;
int vector;
@@ -171,12 +151,37 @@ struct ipack_bus_device {
/**
* ipack_bus_register -- register a new ipack bus
*
- * The carrier board device driver should call this function to register itself
- * as available bus in ipack.
+ * @drv: pointer to struct ipack_driver that corresponds to the carrier driver
+ * @irqv: IRQ vector for the bus device.
+ * @slots: numer of slots available in the bus device.
+ *
+ * The carrier board device should call this function to register itself as
+ * available bus device in ipack. It will call the match(), probe() functions of
+ * the driver with the field 'platform_data' in struct device filled with the
+ * pointer to ipack_driver given here.
*/
-int ipack_bus_register(struct ipack_bus_device *bus);
+struct ipack_bus_device *ipack_bus_register(struct ipack_driver *drv, int irqv, int slots);
/**
* ipack_bus_unregister -- unregister an ipack bus
*/
int ipack_bus_unregister(struct ipack_bus_device *bus);
+
+/*
+ * ipack_driver_register -- Register a new driver
+ *
+ * Called by a ipack driver to register itself as a driver
+ * that can manage ipack devices.
+ */
+
+int ipack_driver_register(struct ipack_driver *edrv, struct module *owner, char *name);
+void ipack_driver_unregister(struct ipack_driver *edrv);
+
+/*
+ * ipack_device_register -- register a new mezzanine device
+ *
+ * Register a new ipack device (mezzanine device). The call is done by
+ * the carrier device driver.
+ */
+struct ipack_device *ipack_device_register(struct ipack_bus_device *bus, int slot);
+void ipack_device_unregister(struct ipack_device *dev);
--
1.7.10
Removed TPCI200_SHORTNAME. For the pr_* the name of the module is already
included due to pr_fmt declaration.
In other cases, KBUILD_MODNAME is used instead.
Signed-off-by: Samuel Iglesias Gonsalvez <[email protected]>
---
drivers/staging/ipack/bridges/tpci200.c | 73 ++++++++++++++-----------------
drivers/staging/ipack/bridges/tpci200.h | 2 -
2 files changed, 34 insertions(+), 41 deletions(-)
diff --git a/drivers/staging/ipack/bridges/tpci200.c b/drivers/staging/ipack/bridges/tpci200.c
index e6bc2cc..e92a019 100644
--- a/drivers/staging/ipack/bridges/tpci200.c
+++ b/drivers/staging/ipack/bridges/tpci200.c
@@ -55,16 +55,15 @@ static struct tpci200_board *check_slot(struct ipack_device *dev)
}
if (dev->slot >= TPCI200_NB_SLOT) {
- pr_info("Slot [%s %d:%d] doesn't exist! Last tpci200 slot is %d.\n",
- TPCI200_SHORTNAME, dev->bus_nr, dev->slot,
- TPCI200_NB_SLOT-1);
+ pr_info("Slot [%d:%d] doesn't exist! Last tpci200 slot is %d.\n",
+ dev->bus_nr, dev->slot, TPCI200_NB_SLOT-1);
return NULL;
}
BUG_ON(tpci200->slots == NULL);
if (tpci200->slots[dev->slot].dev == NULL) {
- pr_info("Slot [%s %d:%d] is not registered !\n",
- TPCI200_SHORTNAME, dev->bus_nr, dev->slot);
+ pr_info("Slot [%d:%d] is not registered !\n", dev->bus_nr,
+ dev->slot);
return NULL;
}
@@ -125,8 +124,8 @@ static struct ipack_addr_space *get_slot_address_space(struct ipack_device *dev,
addr = &dev->mem_space;
break;
default:
- pr_err("Slot [%s %d:%d] space number %d doesn't exist !\n",
- TPCI200_SHORTNAME, dev->bus_nr, dev->slot, space);
+ pr_err("Slot [%d:%d] space number %d doesn't exist !\n",
+ dev->bus_nr, dev->slot, space);
return NULL;
break;
}
@@ -351,8 +350,7 @@ static irqreturn_t tpci200_interrupt(int irq, void *dev_id)
if (unhandled_ints) {
for (i = 0; i < TPCI200_NB_SLOT; i++) {
if (unhandled_ints & ((TPCI200_INT0_EN | TPCI200_INT1_EN) << (2*i))) {
- pr_info("No registered ISR for slot [%s %d:%d]!. IRQ will be disabled.\n",
- TPCI200_SHORTNAME,
+ pr_info("No registered ISR for slot [%d:%d]!. IRQ will be disabled.\n",
tpci200->number, i);
reg_value = readw(
(void __iomem *)(tpci200->info->interface_regs +
@@ -392,8 +390,8 @@ static struct ipack_device *tpci200_slot_register(unsigned int tpci200_number,
}
if (slot_position >= TPCI200_NB_SLOT) {
- pr_info("Slot [%s %d:%d] doesn't exist!\n",
- TPCI200_SHORTNAME, tpci200_number, slot_position);
+ pr_info("Slot [%d:%d] doesn't exist!\n", tpci200_number,
+ slot_position);
return NULL;
}
@@ -401,15 +399,14 @@ static struct ipack_device *tpci200_slot_register(unsigned int tpci200_number,
return NULL;
if (tpci200->slots[slot_position].dev != NULL) {
- pr_err("Slot [%s %d:%d] already installed !\n",
- TPCI200_SHORTNAME, tpci200_number, slot_position);
+ pr_err("Slot [%d:%d] already installed !\n", tpci200_number,
+ slot_position);
goto err_unlock;
}
dev = ipack_device_register(tpci200->info->ipack_bus, slot_position);
if (dev == NULL) {
- pr_info("Slot [%s %d:%d] Unable to allocate memory for new slot !\n",
- TPCI200_SHORTNAME,
+ pr_info("Slot [%d:%d] Unable to allocate memory for new slot !\n",
tpci200_number, slot_position);
goto err_unlock;
}
@@ -681,7 +678,7 @@ static int tpci200_register(struct tpci200_board *tpci200)
res = request_irq(tpci200->info->pdev->irq,
tpci200_interrupt, IRQF_SHARED,
- TPCI200_SHORTNAME, (void *) tpci200);
+ KBUILD_MODNAME, (void *) tpci200);
if (res) {
pr_err("(bn 0x%X, sn 0x%X) unable to register IRQ !",
tpci200->info->pdev->bus->number,
@@ -796,31 +793,31 @@ static int tpci200_slot_unmap_space(struct ipack_device *dev, int space)
switch (space) {
case IPACK_IO_SPACE:
if (dev->io_space.address == NULL) {
- pr_info("Slot [%s %d:%d] IO space not mapped !\n",
- TPCI200_SHORTNAME, dev->bus_nr, dev->slot);
+ pr_info("Slot [%d:%d] IO space not mapped !\n",
+ dev->bus_nr, dev->slot);
goto out_unlock;
}
virt_addr_space = &dev->io_space;
break;
case IPACK_ID_SPACE:
if (dev->id_space.address == NULL) {
- pr_info("Slot [%s %d:%d] ID space not mapped !\n",
- TPCI200_SHORTNAME, dev->bus_nr, dev->slot);
+ pr_info("Slot [%d:%d] ID space not mapped !\n",
+ dev->bus_nr, dev->slot);
goto out_unlock;
}
virt_addr_space = &dev->id_space;
break;
case IPACK_MEM_SPACE:
if (dev->mem_space.address == NULL) {
- pr_info("Slot [%s %d:%d] MEM space not mapped !\n",
- TPCI200_SHORTNAME, dev->bus_nr, dev->slot);
+ pr_info("Slot [%d:%d] MEM space not mapped !\n",
+ dev->bus_nr, dev->slot);
goto out_unlock;
}
virt_addr_space = &dev->mem_space;
break;
default:
- pr_err("Slot [%s %d:%d] space number %d doesn't exist !\n",
- TPCI200_SHORTNAME, dev->bus_nr, dev->slot, space);
+ pr_err("Slot [%d:%d] space number %d doesn't exist !\n",
+ dev->bus_nr, dev->slot, space);
res = -EINVAL;
goto out_unlock;
break;
@@ -883,8 +880,8 @@ static int tpci200_slot_map_space(struct ipack_device *dev,
switch (space) {
case IPACK_IO_SPACE:
if (dev->io_space.address != NULL) {
- pr_err("Slot [%s %d:%d] IO space already mapped !\n",
- TPCI200_SHORTNAME, tpci200->number, dev->slot);
+ pr_err("Slot [%d:%d] IO space already mapped !\n",
+ tpci200->number, dev->slot);
res = -EINVAL;
goto out_unlock;
}
@@ -895,8 +892,8 @@ static int tpci200_slot_map_space(struct ipack_device *dev,
break;
case IPACK_ID_SPACE:
if (dev->id_space.address != NULL) {
- pr_err("Slot [%s %d:%d] ID space already mapped !\n",
- TPCI200_SHORTNAME, tpci200->number, dev->slot);
+ pr_err("Slot [%d:%d] ID space already mapped !\n",
+ tpci200->number, dev->slot);
res = -EINVAL;
goto out_unlock;
}
@@ -907,8 +904,7 @@ static int tpci200_slot_map_space(struct ipack_device *dev,
break;
case IPACK_MEM_SPACE:
if (dev->mem_space.address != NULL) {
- pr_err("Slot [%s %d:%d] MEM space already mapped !\n",
- TPCI200_SHORTNAME,
+ pr_err("Slot [%d:%d] MEM space already mapped !\n",
tpci200->number, dev->slot);
res = -EINVAL;
goto out_unlock;
@@ -916,9 +912,9 @@ static int tpci200_slot_map_space(struct ipack_device *dev,
virt_addr_space = &dev->mem_space;
if (memory_size > tpci200->slots[dev->slot].mem_phys.size) {
- pr_err("Slot [%s %d:%d] request is 0x%X memory, only 0x%X available !\n",
- TPCI200_SHORTNAME, dev->bus_nr, dev->slot,
- memory_size, tpci200->slots[dev->slot].mem_phys.size);
+ pr_err("Slot [%d:%d] request is 0x%X memory, only 0x%X available !\n",
+ dev->bus_nr, dev->slot, memory_size,
+ tpci200->slots[dev->slot].mem_phys.size);
res = -EINVAL;
goto out_unlock;
}
@@ -927,8 +923,7 @@ static int tpci200_slot_map_space(struct ipack_device *dev,
size_to_map = memory_size;
break;
default:
- pr_err("Slot [%s %d:%d] space %d doesn't exist !\n",
- TPCI200_SHORTNAME,
+ pr_err("Slot [%d:%d] space %d doesn't exist !\n",
tpci200->number, dev->slot, space);
res = -EINVAL;
goto out_unlock;
@@ -964,16 +959,16 @@ static int tpci200_request_irq(struct ipack_device *dev, int vector,
}
if (tpci200->slots[dev->slot].irq != NULL) {
- pr_err("Slot [%s %d:%d] IRQ already registered !\n",
- TPCI200_SHORTNAME, dev->bus_nr, dev->slot);
+ pr_err("Slot [%d:%d] IRQ already registered !\n", dev->bus_nr,
+ dev->slot);
res = -EINVAL;
goto out_unlock;
}
slot_irq = kzalloc(sizeof(struct slot_irq), GFP_KERNEL);
if (slot_irq == NULL) {
- pr_err("Slot [%s %d:%d] unable to allocate memory for IRQ !\n",
- TPCI200_SHORTNAME, dev->bus_nr, dev->slot);
+ pr_err("Slot [%d:%d] unable to allocate memory for IRQ !\n",
+ dev->bus_nr, dev->slot);
res = -ENOMEM;
goto out_unlock;
}
diff --git a/drivers/staging/ipack/bridges/tpci200.h b/drivers/staging/ipack/bridges/tpci200.h
index 7eb994d..0b547ee 100644
--- a/drivers/staging/ipack/bridges/tpci200.h
+++ b/drivers/staging/ipack/bridges/tpci200.h
@@ -24,8 +24,6 @@
#include "../ipack.h"
-#define TPCI200_SHORTNAME "TPCI200"
-
#define TPCI200_NB_SLOT 0x4
#define TPCI200_NB_BAR 0x6
--
1.7.10
On Mon, May 14, 2012 at 12:41:25PM +0200, Samuel Iglesias Gonsalvez wrote:
> /* Read status register */
> - status_reg = readw((unsigned short *) (tpci200->info->interface_regs +
> + status_reg = readw((void __iomem *) (tpci200->info->interface_regs +
> TPCI200_STATUS_REG));
>
Later in the patch, you do the right thing which is to declare this
as an __iomem pointer in the header. The cast here can be removed
and it all fits on one line.
Remove it in a follow on patch if you want. Same for several of the
other casts in this patch.
regards,
dan carpenter
On Mon, 2012-05-14 at 15:26 +0300, Dan Carpenter wrote:
> On Mon, May 14, 2012 at 12:41:25PM +0200, Samuel Iglesias Gonsalvez wrote:
> > /* Read status register */
> > - status_reg = readw((unsigned short *) (tpci200->info->interface_regs +
> > + status_reg = readw((void __iomem *) (tpci200->info->interface_regs +
> > TPCI200_STATUS_REG));
> >
>
> Later in the patch, you do the right thing which is to declare this
> as an __iomem pointer in the header. The cast here can be removed
> and it all fits on one line.
>
> Remove it in a follow on patch if you want. Same for several of the
> other casts in this patch.
Thanks a lot. I will do it.
Regards,
Sam
On Mon, May 14, 2012 at 12:41:26PM +0200, Samuel Iglesias Gonsalvez wrote:
> This patch adds a proper device model to the driver. The carrier boards are
> managed like other ipack device, the way to recognize them is using the
> platform data field from struct device.
Wait, what? Why would you use the platform data field? Why is that
needed at all? You can specify the "type" of the device, but it seems
that you really want two different things here, busses and devices,
right? So use two different devices and manage them differently, don't
make them the "same but different" by looking at the platform data
field. That's not what the platform data field is for at all, sorry.
Please rework this patch, I can't take it as-is, sorry.
greg k-h
On Mon, 2012-05-14 at 13:46 -0700, Greg Kroah-Hartman wrote:
> On Mon, May 14, 2012 at 12:41:26PM +0200, Samuel Iglesias Gonsalvez wrote:
> > This patch adds a proper device model to the driver. The carrier boards are
> > managed like other ipack device, the way to recognize them is using the
> > platform data field from struct device.
>
> Wait, what? Why would you use the platform data field? Why is that
> needed at all? You can specify the "type" of the device, but it seems
> that you really want two different things here, busses and devices,
> right? So use two different devices and manage them differently, don't
> make them the "same but different" by looking at the platform data
> field. That's not what the platform data field is for at all, sorry.
I don't use platform_data to tell apart the devices from buses.
Although they don't have auto-discovery, the buses drivers do the
matching. I am not avoiding this step.
The "real" bus devices are already registered in PCI, USB, VME, etc, as
their interface with the rest of the system is through one of these
buses. The problem is to make a relation between a ipack bus device and
its driver, if we want it to be a registered device in ipack, as this
patch does.
Platform_data field is filled with the driver that the device belongs
to, facilitating the task.
There is a similar example in the VME bus with the devices that don't
support auto-discovery, as shown in drivers/staging/vme/vme.c in the
vme_bus_match() function.
Another option is what you say: use two different devices and manage
them differently. It will be needed to add new match/probe functions and
do similar stuff due to the lack of auto-discovery at this case.
A third option is use bus devices like VME bridges in the vme bus
driver, i.e, they are not devices, just an abstraction that provides
some functionality to the mezzanine devices.
I prefer the first option because it reuses the code of the probe/match
functions inside the ipack bus driver and it shows the hierarchy through
sysfs as everything is a registered device.
What do you think?
Sam
On Tue, 2012-05-15 at 16:11 +0200, Samuel Iglesias Gonsálvez wrote:
[snip]
>
> A third option is use bus devices like VME bridges in the vme bus
> driver, i.e, they are not devices, just an abstraction that provides
> some functionality to the mezzanine devices.
>
> I prefer the first option because it reuses the code of the probe/match
> functions inside the ipack bus driver and it shows the hierarchy through
> sysfs as everything is a registered device.
>
> What do you think?
Thinking again about it, I think I don't win anything interesting
registering a new device in case of an ipack bus device.
It is just needed to establish the parent of the ipack device to the
"real" device of the bus (i.e, the PCI, USB device...), like the
aforementioned VME bridge in the drivers/staging/vme driver.
So, I will rewrite the patch keeping it simple: maintaining the change
in ipack_device_release function and ipack_device_register with some
little modifications. This is actually the important stuff.
And delaying the change in ipack_bus_register to whenever it is really
necessary, if it is.
Sorry for the inconveniences,
Sam