linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/4] Staging: ipack: improve the register of a bus and a device in the bus.
@ 2012-05-18  9:10 Samuel Iglesias Gonsalvez
  2012-05-18  9:10 ` [PATCH 2/4] Staging: ipack: remove board_name and bus_name fields from struct ipack_device Samuel Iglesias Gonsalvez
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Samuel Iglesias Gonsalvez @ 2012-05-18  9:10 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Samuel Iglesias Gonsalvez, devel, linux-kernel

It adds and removes some fields in the struct ipack_device and
ipack_bus_device to make it cleaner.

The API has change to group all the operations on these structures inside
of the ipack driver.

Signed-off-by: Samuel Iglesias Gonsalvez <siglesias@igalia.com>
---
 drivers/staging/ipack/bridges/tpci200.c |   63 +++++++--------------------
 drivers/staging/ipack/bridges/tpci200.h |    2 +-
 drivers/staging/ipack/devices/ipoctal.c |   55 ++++++++++++++----------
 drivers/staging/ipack/ipack.c           |   57 +++++++++++++++++++------
 drivers/staging/ipack/ipack.h           |   71 +++++++++++++++++--------------
 5 files changed, 131 insertions(+), 117 deletions(-)

diff --git a/drivers/staging/ipack/bridges/tpci200.c b/drivers/staging/ipack/bridges/tpci200.c
index 4e812a7..392aec0 100644
--- a/drivers/staging/ipack/bridges/tpci200.c
+++ b/drivers/staging/ipack/bridges/tpci200.c
@@ -407,53 +407,22 @@ static struct ipack_device *tpci200_slot_register(const char *board_name,
 		goto err_unlock;
 	}
 
-	dev = kzalloc(sizeof(struct ipack_device), GFP_KERNEL);
-	if (dev == NULL) {
-		pr_info("Slot [%s %d:%d] Unable to allocate memory for new slot !\n",
-			TPCI200_SHORTNAME,
-			tpci200_number, slot_position);
-		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';
-	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
 	 */
-	dev->irq = slot_position;
-
-	dev->id_space.address = NULL;
-	dev->id_space.size = 0;
-	dev->io_space.address = NULL;
-	dev->io_space.size = 0;
-	dev->mem_space.address = NULL;
-	dev->mem_space.size = 0;
+	dev = ipack_device_register(tpci200->info->ipack_bus,
+				    slot_position, slot_position);
+	if (dev == NULL) {
+		pr_info("Slot [%d:%d] Unable to register an ipack device\n",
+			tpci200_number, slot_position);
+		goto err_unlock;
+	}
 
-	/* Give the operations structure */
-	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;
@@ -874,7 +843,6 @@ static int tpci200_slot_unregister(struct ipack_device *dev)
 
 	ipack_device_unregister(dev);
 	tpci200->slots[dev->slot].dev = NULL;
-	kfree(dev);
 	mutex_unlock(&tpci200->mutex);
 
 	return 0;
@@ -1116,20 +1084,20 @@ 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(&pdev->dev,
+						      TPCI200_NB_SLOT,
+						      &tpci200_bus_ops);
+	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 +1109,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);
 }
 
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..29f6fa8 100644
--- a/drivers/staging/ipack/devices/ipoctal.c
+++ b/drivers/staging/ipack/devices/ipoctal.c
@@ -73,7 +73,8 @@ static inline void ipoctal_write_io_reg(struct ipoctal *ipoctal,
 	unsigned long offset;
 
 	offset = ((void __iomem *) dest) - ipoctal->dev->io_space.address;
-	ipoctal->dev->ops->write8(ipoctal->dev, IPACK_IO_SPACE, offset, value);
+	ipoctal->dev->bus->ops->write8(ipoctal->dev, IPACK_IO_SPACE, offset,
+				       value);
 }
 
 static inline void ipoctal_write_cr_cmd(struct ipoctal *ipoctal,
@@ -90,7 +91,8 @@ static inline unsigned char ipoctal_read_io_reg(struct ipoctal *ipoctal,
 	unsigned char value;
 
 	offset = ((void __iomem *) src) - ipoctal->dev->io_space.address;
-	ipoctal->dev->ops->read8(ipoctal->dev, IPACK_IO_SPACE, offset, &value);
+	ipoctal->dev->bus->ops->read8(ipoctal->dev, IPACK_IO_SPACE, offset,
+				      &value);
 	return value;
 }
 
@@ -341,12 +343,12 @@ static int ipoctal_check_model(struct ipack_device *dev, unsigned char *id)
 	unsigned char manufacturerID;
 	unsigned char board_id;
 
-	dev->ops->read8(dev, IPACK_ID_SPACE,
+	dev->bus->ops->read8(dev, IPACK_ID_SPACE,
 			IPACK_IDPROM_OFFSET_MANUFACTURER_ID, &manufacturerID);
 	if (manufacturerID != IP_OCTAL_MANUFACTURER_ID)
 		return -ENODEV;
 
-	dev->ops->read8(dev, IPACK_ID_SPACE,
+	dev->bus->ops->read8(dev, IPACK_ID_SPACE,
 			IPACK_IDPROM_OFFSET_MODEL, (unsigned char *)&board_id);
 
 	switch (board_id) {
@@ -376,7 +378,8 @@ static int ipoctal_inst_slot(struct ipoctal *ipoctal, unsigned int bus_nr,
 	char name[20];
 	unsigned char board_id;
 
-	res = ipoctal->dev->ops->map_space(ipoctal->dev, 0, IPACK_ID_SPACE);
+	res = ipoctal->dev->bus->ops->map_space(ipoctal->dev, 0,
+						IPACK_ID_SPACE);
 	if (res) {
 		pr_err("Unable to map slot [%d:%d] ID space!\n", bus_nr, slot);
 		return res;
@@ -384,18 +387,20 @@ static int ipoctal_inst_slot(struct ipoctal *ipoctal, unsigned int bus_nr,
 
 	res = ipoctal_check_model(ipoctal->dev, &board_id);
 	if (res) {
-		ipoctal->dev->ops->unmap_space(ipoctal->dev, IPACK_ID_SPACE);
+		ipoctal->dev->bus->ops->unmap_space(ipoctal->dev,
+						    IPACK_ID_SPACE);
 		goto out_unregister_id_space;
 	}
 	ipoctal->board_id = board_id;
 
-	res = ipoctal->dev->ops->map_space(ipoctal->dev, 0, IPACK_IO_SPACE);
+	res = ipoctal->dev->bus->ops->map_space(ipoctal->dev, 0,
+						IPACK_IO_SPACE);
 	if (res) {
 		pr_err("Unable to map slot [%d:%d] IO space!\n", bus_nr, slot);
 		goto out_unregister_id_space;
 	}
 
-	res = ipoctal->dev->ops->map_space(ipoctal->dev,
+	res = ipoctal->dev->bus->ops->map_space(ipoctal->dev,
 					   0x8000, IPACK_MEM_SPACE);
 	if (res) {
 		pr_err("Unable to map slot [%d:%d] MEM space!\n", bus_nr, slot);
@@ -434,9 +439,9 @@ static int ipoctal_inst_slot(struct ipoctal *ipoctal, unsigned int bus_nr,
 	 * Depending of the carrier these addresses are accesible or not.
 	 * More info in the datasheet.
 	 */
-	ipoctal->dev->ops->request_irq(ipoctal->dev, vector,
+	ipoctal->dev->bus->ops->request_irq(ipoctal->dev, vector,
 				       ipoctal_irq_handler, ipoctal);
-	ipoctal->dev->ops->write8(ipoctal->dev, IPACK_ID_SPACE, 0, vector);
+	ipoctal->dev->bus->ops->write8(ipoctal->dev, IPACK_ID_SPACE, 0, vector);
 
 	/* Register the TTY device */
 
@@ -502,11 +507,11 @@ static int ipoctal_inst_slot(struct ipoctal *ipoctal, unsigned int bus_nr,
 	return 0;
 
 out_unregister_slot_unmap:
-	ipoctal->dev->ops->unmap_space(ipoctal->dev, IPACK_ID_SPACE);
+	ipoctal->dev->bus->ops->unmap_space(ipoctal->dev, IPACK_ID_SPACE);
 out_unregister_io_space:
-	ipoctal->dev->ops->unmap_space(ipoctal->dev, IPACK_IO_SPACE);
+	ipoctal->dev->bus->ops->unmap_space(ipoctal->dev, IPACK_IO_SPACE);
 out_unregister_id_space:
-	ipoctal->dev->ops->unmap_space(ipoctal->dev, IPACK_MEM_SPACE);
+	ipoctal->dev->bus->ops->unmap_space(ipoctal->dev, IPACK_MEM_SPACE);
 	return res;
 }
 
@@ -799,13 +804,20 @@ static int ipoctal_match(struct ipack_device *dev)
 	int res;
 	unsigned char board_id;
 
-	res = dev->ops->map_space(dev, 0, IPACK_ID_SPACE);
+	if ((!dev->bus->ops) || (!dev->bus->ops->map_space) ||
+	    (!dev->bus->ops->unmap_space))
+		return 0;
+
+	res = dev->bus->ops->map_space(dev, 0, IPACK_ID_SPACE);
 	if (res)
-		return res;
+		return 0;
 
 	res = ipoctal_check_model(dev, &board_id);
-	dev->ops->unmap_space(dev, IPACK_ID_SPACE);
-	return res;
+	dev->bus->ops->unmap_space(dev, IPACK_ID_SPACE);
+	if (!res)
+		return 1;
+
+	return 0;
 }
 
 static int ipoctal_probe(struct ipack_device *dev)
@@ -843,8 +855,8 @@ static void __ipoctal_remove(struct ipoctal *ipoctal)
 	put_tty_driver(ipoctal->tty_drv);
 
 	/* Tell the carrier board to free all the resources for this device */
-	if (ipoctal->dev->ops->remove_device != NULL)
-		ipoctal->dev->ops->remove_device(ipoctal->dev);
+	if (ipoctal->dev->bus->ops->remove_device != NULL)
+		ipoctal->dev->bus->ops->remove_device(ipoctal->dev);
 
 	list_del(&ipoctal->list);
 	kfree(ipoctal);
@@ -868,11 +880,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..2b4fa51 100644
--- a/drivers/staging/ipack/ipack.c
+++ b/drivers/staging/ipack/ipack.c
@@ -13,6 +13,7 @@
 
 #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)
@@ -28,13 +29,19 @@ 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 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,16 +99,27 @@ error_find_busnum:
 	return busnum;
 }
 
-int ipack_bus_register(struct ipack_bus_device *bus)
+struct ipack_bus_device *ipack_bus_register(struct device *parent, int slots,
+					    struct ipack_bus_ops *ops)
 {
 	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;
+	if (bus_nr < 0) {
+		kfree(bus);
+		return NULL;
+	}
 
 	bus->bus_nr = bus_nr;
-	return 0;
+	bus->parent = parent;
+	bus->slots = slots;
+	bus->ops = ops;
+	return bus;
 }
 EXPORT_SYMBOL_GPL(ipack_bus_register);
 
@@ -110,12 +128,16 @@ int ipack_bus_unregister(struct ipack_bus_device *bus)
 	mutex_lock(&ipack_mutex);
 	clear_bit(bus->bus_nr, busmap.busmap);
 	mutex_unlock(&ipack_mutex);
+	kfree(bus);
 	return 0;
 }
 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 +149,35 @@ 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 irqv)
 {
 	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->parent;
+	dev->slot = slot;
+	dev->bus_nr = bus->bus_nr;
+	dev->irq = irqv;
+	dev->bus = bus;
 	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..d50af71 100644
--- a/drivers/staging/ipack/ipack.h
+++ b/drivers/staging/ipack/ipack.h
@@ -49,13 +49,11 @@ struct ipack_addr_space {
 /**
  *	struct ipack_device
  *
- *	@board_name: IP mezzanine board name
- *	@bus_name: IP carrier board name
  *	@bus_nr: IP bus number where the device is plugged
  *	@slot: Slot where the device is plugged in the carrier board
  *	@irq: IRQ vector
  *	@driver: Pointer to the ipack_driver that manages the device
- *	@ops: Carrier board operations to access the device
+ *	@bus: ipack_bus_device where the device is plugged to.
  *	@id_space: Virtual address to ID space.
  *	@io_space: Virtual address to IO space.
  *	@mem_space: Virtual address to MEM space.
@@ -63,7 +61,7 @@ struct ipack_addr_space {
  *
  * Warning: Direct access to mapped memory is possible but the endianness
  * is not the same with PCI carrier or VME carrier. The endianness is managed
- * by the carrier board throught @ops.
+ * by the carrier board throught bus->ops.
  */
 struct ipack_device {
 	char board_name[IPACK_BOARD_NAME_SIZE];
@@ -72,14 +70,14 @@ struct ipack_device {
 	unsigned int slot;
 	unsigned int irq;
 	struct ipack_driver *driver;
-	struct ipack_bus_ops *ops;
+	struct ipack_bus_device *bus;
 	struct ipack_addr_space id_space;
 	struct ipack_addr_space io_space;
 	struct ipack_addr_space mem_space;
 	struct device dev;
 };
 
-/*
+/**
  *	struct ipack_driver_ops -- callbacks to mezzanine driver for installing/removing one device
  *
  *	@match: Match function
@@ -94,36 +92,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
  *
@@ -159,24 +137,51 @@ struct ipack_bus_ops {
  *	@dev: pointer to carrier device
  *	@slots: number of slots available
  *	@bus_nr: ipack bus number
- *	@vector: IRQ base vector. IRQ vectors are $vector + $slot_number
+ *	@ops: bus operations for the mezzanine drivers
  */
 struct ipack_bus_device {
-	struct device *dev;
+	struct device *parent;
 	int slots;
 	int bus_nr;
-	int vector;
+	struct ipack_bus_ops *ops;
 };
 
 /**
  *	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.
+ * @parent: pointer to the parent device, if any.
+ * @slots: number of slots available in the bus device.
+ * @ops: bus operations for the mezzanine drivers.
+ *
+ * The carrier board device should call this function to register itself as
+ * available bus device in ipack.
  */
-int ipack_bus_register(struct ipack_bus_device *bus);
+struct ipack_bus_device *ipack_bus_register(struct device *parent, int slots,
+					    struct ipack_bus_ops *ops);
 
 /**
  *	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
+ *
+ * @bus: ipack bus device it is plugged to.
+ * @slot: slot position in the bus device.
+ * @irqv: IRQ vector for the mezzanine.
+ *
+ * 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, int irqv);
+void ipack_device_unregister(struct ipack_device *dev);
-- 
1.7.10


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/4] Staging: ipack: remove board_name and bus_name fields from struct ipack_device
  2012-05-18  9:10 [PATCH 1/4] Staging: ipack: improve the register of a bus and a device in the bus Samuel Iglesias Gonsalvez
@ 2012-05-18  9:10 ` Samuel Iglesias Gonsalvez
  2012-05-19  0:35   ` Greg Kroah-Hartman
  2012-05-18  9:10 ` [PATCH 3/4] Staging ipack/bridges/tpci200: remove TPCI200_SHORTNAME constant Samuel Iglesias Gonsalvez
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 9+ messages in thread
From: Samuel Iglesias Gonsalvez @ 2012-05-18  9:10 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Samuel Iglesias Gonsalvez, devel, linux-kernel

Removed board_name and bus_name fields from struct ipack_device that are
completely useless.

Signed-off-by: Samuel Iglesias Gonsalvez <siglesias@igalia.com>
---
 drivers/staging/ipack/bridges/tpci200.c |   20 ++++----------------
 drivers/staging/ipack/bridges/tpci200.h |    8 ++++----
 drivers/staging/ipack/ipack.h           |    4 ----
 3 files changed, 8 insertions(+), 24 deletions(-)

diff --git a/drivers/staging/ipack/bridges/tpci200.c b/drivers/staging/ipack/bridges/tpci200.c
index 392aec0..b460587 100644
--- a/drivers/staging/ipack/bridges/tpci200.c
+++ b/drivers/staging/ipack/bridges/tpci200.c
@@ -371,9 +371,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;
@@ -437,7 +435,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;
 
@@ -450,7 +448,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");
 }
@@ -975,17 +973,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 d50af71..8bc001e 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
@@ -64,8 +62,6 @@ struct ipack_addr_space {
  * by the carrier board throught bus->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


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 3/4] Staging ipack/bridges/tpci200: remove TPCI200_SHORTNAME constant
  2012-05-18  9:10 [PATCH 1/4] Staging: ipack: improve the register of a bus and a device in the bus Samuel Iglesias Gonsalvez
  2012-05-18  9:10 ` [PATCH 2/4] Staging: ipack: remove board_name and bus_name fields from struct ipack_device Samuel Iglesias Gonsalvez
@ 2012-05-18  9:10 ` Samuel Iglesias Gonsalvez
  2012-05-19  0:37   ` [PATCH 3/4] Staging: " Greg Kroah-Hartman
  2012-05-18  9:10 ` [PATCH 4/4] Staging: ipack/bridges/tpci200: remove unneeded casts Samuel Iglesias Gonsalvez
  2012-05-19  0:35 ` [PATCH 1/4] Staging: ipack: improve the register of a bus and a device in the bus Greg Kroah-Hartman
  3 siblings, 1 reply; 9+ messages in thread
From: Samuel Iglesias Gonsalvez @ 2012-05-18  9:10 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Samuel Iglesias Gonsalvez, devel, linux-kernel

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 <siglesias@igalia.com>
---
 drivers/staging/ipack/bridges/tpci200.c |   70 +++++++++++++++----------------
 drivers/staging/ipack/bridges/tpci200.h |    2 -
 2 files changed, 33 insertions(+), 39 deletions(-)

diff --git a/drivers/staging/ipack/bridges/tpci200.c b/drivers/staging/ipack/bridges/tpci200.c
index b460587..c043345 100644
--- a/drivers/staging/ipack/bridges/tpci200.c
+++ b/drivers/staging/ipack/bridges/tpci200.c
@@ -54,16 +54,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;
 	}
 
@@ -124,8 +123,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;
 	}
@@ -350,8 +349,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 +
@@ -391,8 +389,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;
 	}
 
@@ -400,8 +398,8 @@ 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;
 	}
 
@@ -668,7 +666,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,
@@ -783,31 +781,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;
@@ -869,8 +867,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;
 		}
@@ -881,8 +879,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;
 		}
@@ -893,8 +891,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;
@@ -902,9 +899,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;
 		}
@@ -913,8 +910,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;
@@ -950,16 +946,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


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 4/4] Staging: ipack/bridges/tpci200: remove unneeded casts
  2012-05-18  9:10 [PATCH 1/4] Staging: ipack: improve the register of a bus and a device in the bus Samuel Iglesias Gonsalvez
  2012-05-18  9:10 ` [PATCH 2/4] Staging: ipack: remove board_name and bus_name fields from struct ipack_device Samuel Iglesias Gonsalvez
  2012-05-18  9:10 ` [PATCH 3/4] Staging ipack/bridges/tpci200: remove TPCI200_SHORTNAME constant Samuel Iglesias Gonsalvez
@ 2012-05-18  9:10 ` Samuel Iglesias Gonsalvez
  2012-05-19  0:35 ` [PATCH 1/4] Staging: ipack: improve the register of a bus and a device in the bus Greg Kroah-Hartman
  3 siblings, 0 replies; 9+ messages in thread
From: Samuel Iglesias Gonsalvez @ 2012-05-18  9:10 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Samuel Iglesias Gonsalvez, devel, linux-kernel

Signed-off-by: Samuel Iglesias Gonsalvez <siglesias@igalia.com>
---
 drivers/staging/ipack/bridges/tpci200.c |   36 ++++++++++++++-----------------
 1 file changed, 16 insertions(+), 20 deletions(-)

diff --git a/drivers/staging/ipack/bridges/tpci200.c b/drivers/staging/ipack/bridges/tpci200.c
index c043345..ad28707 100644
--- a/drivers/staging/ipack/bridges/tpci200.c
+++ b/drivers/staging/ipack/bridges/tpci200.c
@@ -321,8 +321,8 @@ static irqreturn_t tpci200_interrupt(int irq, void *dev_id)
 	spin_lock_irqsave(&tpci200->info->access_lock, flags);
 
 	/* Read status register */
-	status_reg = readw((void __iomem *) (tpci200->info->interface_regs +
-					       TPCI200_STATUS_REG));
+	status_reg = readw(tpci200->info->interface_regs +
+			   TPCI200_STATUS_REG);
 
 	if (status_reg & TPCI200_SLOT_INT_MASK) {
 		unhandled_ints = status_reg & TPCI200_SLOT_INT_MASK;
@@ -334,12 +334,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((void __iomem *)
-				      (tpci200->slots[i].dev->io_space.address +
-				       0xC0));
-				readw((void __iomem *)
-				      (tpci200->slots[i].dev->io_space.address +
-				       0xC2));
+				readw(tpci200->slots[i].dev->io_space.address +
+				      0xC0);
+				readw(tpci200->slots[i].dev->io_space.address +
+				      0xC2);
 
 				unhandled_ints &= ~(((TPCI200_A_INT0 | TPCI200_A_INT1) << (2*i)));
 			}
@@ -352,13 +350,13 @@ static irqreturn_t tpci200_interrupt(int irq, void *dev_id)
 				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 +
-								     control_reg[i]));
+					tpci200->info->interface_regs +
+					control_reg[i]);
 				reg_value &=
 					~(TPCI200_INT0_EN | TPCI200_INT1_EN);
 				writew(reg_value,
-				       (void __iomem *)(tpci200->info->interface_regs +
-							control_reg[i]));
+				       (tpci200->info->interface_regs +
+					control_reg[i]));
 			}
 		}
 	}
@@ -659,9 +657,8 @@ static int tpci200_register(struct tpci200_board *tpci200)
 			(void __iomem *)mem_base + TPCI200_MEM8_GAP*i;
 		tpci200->slots[i].mem_phys.size = TPCI200_MEM8_SIZE;
 
-		writew(slot_ctrl,
-		       (void __iomem *)(tpci200->info->interface_regs +
-					  control_reg[i]));
+		writew(slot_ctrl, (tpci200->info->interface_regs +
+				   control_reg[i]));
 	}
 
 	res = request_irq(tpci200->info->pdev->irq,
@@ -703,8 +700,8 @@ static int __tpci200_request_irq(struct tpci200_board *tpci200,
 	 * clock rate 8 MHz
 	 */
 	slot_ctrl = TPCI200_INT0_EN | TPCI200_INT1_EN;
-	writew(slot_ctrl, (void __iomem *)(tpci200->info->interface_regs +
-					     control_reg[dev->slot]));
+	writew(slot_ctrl, (tpci200->info->interface_regs +
+			   control_reg[dev->slot]));
 
 	return 0;
 }
@@ -723,9 +720,8 @@ static void __tpci200_free_irq(struct tpci200_board *tpci200,
 	 * clock rate 8 MHz
 	 */
 	slot_ctrl = 0;
-	writew(slot_ctrl,
-	       (void __iomem *)(tpci200->info->interface_regs +
-				control_reg[dev->slot]));
+	writew(slot_ctrl, (tpci200->info->interface_regs +
+			   control_reg[dev->slot]));
 }
 
 static int tpci200_free_irq(struct ipack_device *dev)
-- 
1.7.10


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/4] Staging: ipack: remove board_name and bus_name fields from struct ipack_device
  2012-05-18  9:10 ` [PATCH 2/4] Staging: ipack: remove board_name and bus_name fields from struct ipack_device Samuel Iglesias Gonsalvez
@ 2012-05-19  0:35   ` Greg Kroah-Hartman
  0 siblings, 0 replies; 9+ messages in thread
From: Greg Kroah-Hartman @ 2012-05-19  0:35 UTC (permalink / raw)
  To: Samuel Iglesias Gonsalvez; +Cc: devel, linux-kernel

On Fri, May 18, 2012 at 11:10:06AM +0200, Samuel Iglesias Gonsalvez wrote:
> Removed board_name and bus_name fields from struct ipack_device that are
> completely useless.
> 
> Signed-off-by: Samuel Iglesias Gonsalvez <siglesias@igalia.com>
> ---
>  drivers/staging/ipack/bridges/tpci200.c |   20 ++++----------------
>  drivers/staging/ipack/bridges/tpci200.h |    8 ++++----
>  drivers/staging/ipack/ipack.h           |    4 ----
>  3 files changed, 8 insertions(+), 24 deletions(-)

Much nicer, and I'll take this, but why are you doing this:

> @@ -450,7 +448,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));

?  That implies that the name isn't already in the path, which it should
be (or at the least, a symlink should be there, so this sysfs file
should not be needed at all, right?)

Also:
> +	slot_irq->name = dev_name(&dev->dev);

Why do that?  Who needs this name now that we already have it in the
device?  Can't that field be removed entirely now also?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/4] Staging: ipack: improve the register of a bus and a device in the bus.
  2012-05-18  9:10 [PATCH 1/4] Staging: ipack: improve the register of a bus and a device in the bus Samuel Iglesias Gonsalvez
                   ` (2 preceding siblings ...)
  2012-05-18  9:10 ` [PATCH 4/4] Staging: ipack/bridges/tpci200: remove unneeded casts Samuel Iglesias Gonsalvez
@ 2012-05-19  0:35 ` Greg Kroah-Hartman
  3 siblings, 0 replies; 9+ messages in thread
From: Greg Kroah-Hartman @ 2012-05-19  0:35 UTC (permalink / raw)
  To: Samuel Iglesias Gonsalvez; +Cc: devel, linux-kernel

On Fri, May 18, 2012 at 11:10:05AM +0200, Samuel Iglesias Gonsalvez wrote:
> It adds and removes some fields in the struct ipack_device and
> ipack_bus_device to make it cleaner.
> 
> The API has change to group all the operations on these structures inside
> of the ipack driver.
> 
> Signed-off-by: Samuel Iglesias Gonsalvez <siglesias@igalia.com>
> ---
>  drivers/staging/ipack/bridges/tpci200.c |   63 +++++++--------------------
>  drivers/staging/ipack/bridges/tpci200.h |    2 +-
>  drivers/staging/ipack/devices/ipoctal.c |   55 ++++++++++++++----------
>  drivers/staging/ipack/ipack.c           |   57 +++++++++++++++++++------
>  drivers/staging/ipack/ipack.h           |   71 +++++++++++++++++--------------
>  5 files changed, 131 insertions(+), 117 deletions(-)

Much better, thanks for reworking this.  Hopefully it makes more sense
now.

greg k-h

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 3/4] Staging: ipack/bridges/tpci200: remove TPCI200_SHORTNAME constant
  2012-05-18  9:10 ` [PATCH 3/4] Staging ipack/bridges/tpci200: remove TPCI200_SHORTNAME constant Samuel Iglesias Gonsalvez
@ 2012-05-19  0:37   ` Greg Kroah-Hartman
  2012-05-23 10:13     ` Samuel Iglesias Gonsálvez
  0 siblings, 1 reply; 9+ messages in thread
From: Greg Kroah-Hartman @ 2012-05-19  0:37 UTC (permalink / raw)
  To: Samuel Iglesias Gonsalvez; +Cc: devel, linux-kernel

On Fri, May 18, 2012 at 11:10:07AM +0200, Samuel Iglesias Gonsalvez wrote:
> 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 <siglesias@igalia.com>
> ---
>  drivers/staging/ipack/bridges/tpci200.c |   70 +++++++++++++++----------------
>  drivers/staging/ipack/bridges/tpci200.h |    2 -
>  2 files changed, 33 insertions(+), 39 deletions(-)
> 
> diff --git a/drivers/staging/ipack/bridges/tpci200.c b/drivers/staging/ipack/bridges/tpci200.c
> index b460587..c043345 100644
> --- a/drivers/staging/ipack/bridges/tpci200.c
> +++ b/drivers/staging/ipack/bridges/tpci200.c
> @@ -54,16 +54,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);

All of these should really be dev_err() calls, right?

You should have no pr_* calls at all in any driver, and probably none in
the ipack core as well, care to fix them all up?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 3/4] Staging: ipack/bridges/tpci200: remove TPCI200_SHORTNAME constant
  2012-05-19  0:37   ` [PATCH 3/4] Staging: " Greg Kroah-Hartman
@ 2012-05-23 10:13     ` Samuel Iglesias Gonsálvez
  2012-05-23 14:04       ` Greg Kroah-Hartman
  0 siblings, 1 reply; 9+ messages in thread
From: Samuel Iglesias Gonsálvez @ 2012-05-23 10:13 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: devel, linux-kernel

On Fri, 2012-05-18 at 17:37 -0700, Greg Kroah-Hartman wrote:
[...]
> >  	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);
> 
> All of these should really be dev_err() calls, right?
> 
> You should have no pr_* calls at all in any driver, and probably none in
> the ipack core as well, care to fix them all up?

Yes, I will fix it in the next bunch of patches. I don't forget it.

By the way, sometime ago, you suggested me to use the ics interface for
giving the bus number if there is no HW boundary of the number of them.
However, I searched the source code and I didn't find any reference to
the ics interface. Can you tell me where is it declared?

Thanks,

Sam 


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 3/4] Staging: ipack/bridges/tpci200: remove TPCI200_SHORTNAME constant
  2012-05-23 10:13     ` Samuel Iglesias Gonsálvez
@ 2012-05-23 14:04       ` Greg Kroah-Hartman
  0 siblings, 0 replies; 9+ messages in thread
From: Greg Kroah-Hartman @ 2012-05-23 14:04 UTC (permalink / raw)
  To: Samuel Iglesias Gonsálvez; +Cc: devel, linux-kernel

On Wed, May 23, 2012 at 12:13:21PM +0200, Samuel Iglesias Gonsálvez wrote:
> On Fri, 2012-05-18 at 17:37 -0700, Greg Kroah-Hartman wrote:
> [...]
> > >  	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);
> > 
> > All of these should really be dev_err() calls, right?
> > 
> > You should have no pr_* calls at all in any driver, and probably none in
> > the ipack core as well, care to fix them all up?
> 
> Yes, I will fix it in the next bunch of patches. I don't forget it.
> 
> By the way, sometime ago, you suggested me to use the ics interface for
> giving the bus number if there is no HW boundary of the number of them.
> However, I searched the source code and I didn't find any reference to
> the ics interface. Can you tell me where is it declared?

My appologies, I should have said "idr" interface, there is a reason why
you couldn't find any "ics" code :)

Sorry about that,

greg k-h

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2012-05-23 14:04 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-05-18  9:10 [PATCH 1/4] Staging: ipack: improve the register of a bus and a device in the bus Samuel Iglesias Gonsalvez
2012-05-18  9:10 ` [PATCH 2/4] Staging: ipack: remove board_name and bus_name fields from struct ipack_device Samuel Iglesias Gonsalvez
2012-05-19  0:35   ` Greg Kroah-Hartman
2012-05-18  9:10 ` [PATCH 3/4] Staging ipack/bridges/tpci200: remove TPCI200_SHORTNAME constant Samuel Iglesias Gonsalvez
2012-05-19  0:37   ` [PATCH 3/4] Staging: " Greg Kroah-Hartman
2012-05-23 10:13     ` Samuel Iglesias Gonsálvez
2012-05-23 14:04       ` Greg Kroah-Hartman
2012-05-18  9:10 ` [PATCH 4/4] Staging: ipack/bridges/tpci200: remove unneeded casts Samuel Iglesias Gonsalvez
2012-05-19  0:35 ` [PATCH 1/4] Staging: ipack: improve the register of a bus and a device in the bus Greg Kroah-Hartman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).