[PATCH 0/6] HSI fixes for 3.4

[PATCH 0/6] HSI fixes for 3.4

[Carlos Chinea]

Hi,

This patch set contains fixes for the issues found by Greg KH

https://lkml.org/lkml/2012/4/2/511
https://lkml.org/lkml/2012/4/2/512

I would like to get feedback from those patches and specially
from the hsi_event rework patch.

Br,
Carlos Chinea

Carlos Chinea (6):
  HSI: hsi: Rework hsi_controller release
  HSI: hsi: Fix error path cleanup on client registration
  HSI: hsi: Remove controllers and ports from the bus
  HSI: hsi: Rework hsi_event interface
  HSI: hsi_char: Remove max_data_size from sysfs
  HSI: Add HSI ABI documentation

 Documentation/ABI/testing/sysfs-bus-hsi |   19 +++
 drivers/hsi/clients/hsi_char.c          |    2 +-
 drivers/hsi/hsi.c                       |  223 ++++++++++++++++---------------
 include/linux/hsi/hsi.h                 |   31 +++--
 4 files changed, 155 insertions(+), 120 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-hsi

-- 
1.7.5.4

[PATCH 1/6] HSI: hsi: Rework hsi_controller release

[Carlos Chinea]

Use the proper release mechanism for hsi_controller and
hsi_ports structures. Free the structures through their
associated device release callbacks.

Signed-off-by: Carlos Chinea <carlos.chinea@nokia.com>
---
 drivers/hsi/hsi.c       |  108 ++++++++++++++++++++++++++++------------------
 include/linux/hsi/hsi.h |    6 +-
 2 files changed, 69 insertions(+), 45 deletions(-)

diff --git a/drivers/hsi/hsi.c b/drivers/hsi/hsi.c
index 4e2d79b..c17d12c 100644
--- a/drivers/hsi/hsi.c
+++ b/drivers/hsi/hsi.c
@@ -140,12 +140,17 @@ static int hsi_remove_port(struct device *dev, void *data __maybe_unused)
 	return 0;
 }
 
-static void hsi_controller_release(struct device *dev __maybe_unused)
+static void hsi_controller_release(struct device *dev)
 {
+	struct hsi_controller *hsi = to_hsi_controller(dev);
+
+	kfree(hsi->port);
+	kfree(hsi);
 }
 
-static void hsi_port_release(struct device *dev __maybe_unused)
+static void hsi_port_release(struct device *dev)
 {
+	kfree(to_hsi_port(dev));
 }
 
 /**
@@ -172,18 +177,14 @@ int hsi_register_controller(struct hsi_controller *hsi)
 
 	hsi->device.type = &hsi_ctrl;
 	hsi->device.bus = &hsi_bus_type;
-	hsi->device.release = hsi_controller_release;
-	err = device_register(&hsi->device);
+	err = device_add(&hsi->device);
 	if (err < 0)
 		return err;
 	for (i = 0; i < hsi->num_ports; i++) {
-		hsi->port[i].device.parent = &hsi->device;
-		hsi->port[i].device.bus = &hsi_bus_type;
-		hsi->port[i].device.release = hsi_port_release;
-		hsi->port[i].device.type = &hsi_port;
-		INIT_LIST_HEAD(&hsi->port[i].clients);
-		spin_lock_init(&hsi->port[i].clock);
-		err = device_register(&hsi->port[i].device);
+		hsi->port[i]->device.parent = &hsi->device;
+		hsi->port[i]->device.bus = &hsi_bus_type;
+		hsi->port[i]->device.type = &hsi_port;
+		err = device_add(&hsi->port[i]->device);
 		if (err < 0)
 			goto out;
 	}
@@ -192,7 +193,9 @@ int hsi_register_controller(struct hsi_controller *hsi)
 
 	return 0;
 out:
-	hsi_unregister_controller(hsi);
+	while (i-- > 0)
+		device_del(&hsi->port[i]->device);
+	device_del(&hsi->device);
 
 	return err;
 }
@@ -223,6 +226,29 @@ static inline int hsi_dummy_cl(struct hsi_client *cl __maybe_unused)
 }
 
 /**
+ * hsi_put_controller - Free an HSI controller
+ *
+ * @hsi: Pointer to the HSI controller to freed
+ *
+ * HSI controller drivers should only use this function if they need
+ * to free their allocated hsi_controller structures before a successful
+ * call to hsi_register_controller. Other use is not allowed.
+ */
+void hsi_put_controller(struct hsi_controller *hsi)
+{
+	unsigned int i;
+
+	if (!hsi)
+		return;
+
+	for (i = 0; i < hsi->num_ports; i++)
+		if (hsi->port && hsi->port[i])
+			put_device(&hsi->port[i]->device);
+	put_device(&hsi->device);
+}
+EXPORT_SYMBOL_GPL(hsi_put_controller);
+
+/**
  * hsi_alloc_controller - Allocate an HSI controller and its ports
  * @n_ports: Number of ports on the HSI controller
  * @flags: Kernel allocation flags
@@ -232,55 +258,53 @@ static inline int hsi_dummy_cl(struct hsi_client *cl __maybe_unused)
 struct hsi_controller *hsi_alloc_controller(unsigned int n_ports, gfp_t flags)
 {
 	struct hsi_controller	*hsi;
-	struct hsi_port		*port;
+	struct hsi_port		**port;
 	unsigned int		i;
 
 	if (!n_ports)
 		return NULL;
 
-	port = kzalloc(sizeof(*port)*n_ports, flags);
-	if (!port)
-		return NULL;
 	hsi = kzalloc(sizeof(*hsi), flags);
 	if (!hsi)
-		goto out;
-	for (i = 0; i < n_ports; i++) {
-		dev_set_name(&port[i].device, "port%d", i);
-		port[i].num = i;
-		port[i].async = hsi_dummy_msg;
-		port[i].setup = hsi_dummy_cl;
-		port[i].flush = hsi_dummy_cl;
-		port[i].start_tx = hsi_dummy_cl;
-		port[i].stop_tx = hsi_dummy_cl;
-		port[i].release = hsi_dummy_cl;
-		mutex_init(&port[i].lock);
+		return NULL;
+	port = kzalloc(sizeof(*port)*n_ports, flags);
+	if (!port) {
+		kfree(hsi);
+		return NULL;
 	}
 	hsi->num_ports = n_ports;
 	hsi->port = port;
+	hsi->device.release = hsi_controller_release;
+	device_initialize(&hsi->device);
+
+	for (i = 0; i < n_ports; i++) {
+		port[i] = kzalloc(sizeof(**port), flags);
+		if (port[i] == NULL)
+			goto out;
+		port[i]->num = i;
+		port[i]->async = hsi_dummy_msg;
+		port[i]->setup = hsi_dummy_cl;
+		port[i]->flush = hsi_dummy_cl;
+		port[i]->start_tx = hsi_dummy_cl;
+		port[i]->stop_tx = hsi_dummy_cl;
+		port[i]->release = hsi_dummy_cl;
+		mutex_init(&port[i]->lock);
+		INIT_LIST_HEAD(&hsi->port[i]->clients);
+		spin_lock_init(&hsi->port[i]->clock);
+		dev_set_name(&port[i]->device, "port%d", i);
+		hsi->port[i]->device.release = hsi_port_release;
+		device_initialize(&hsi->port[i]->device);
+	}
 
 	return hsi;
 out:
-	kfree(port);
+	hsi_put_controller(hsi);
 
 	return NULL;
 }
 EXPORT_SYMBOL_GPL(hsi_alloc_controller);
 
 /**
- * hsi_free_controller - Free an HSI controller
- * @hsi: Pointer to HSI controller
- */
-void hsi_free_controller(struct hsi_controller *hsi)
-{
-	if (!hsi)
-		return;
-
-	kfree(hsi->port);
-	kfree(hsi);
-}
-EXPORT_SYMBOL_GPL(hsi_free_controller);
-
-/**
  * hsi_free_msg - Free an HSI message
  * @msg: Pointer to the HSI message
  *
diff --git a/include/linux/hsi/hsi.h b/include/linux/hsi/hsi.h
index 4b17806..7f3b726 100644
--- a/include/linux/hsi/hsi.h
+++ b/include/linux/hsi/hsi.h
@@ -270,13 +270,13 @@ struct hsi_controller {
 	struct module		*owner;
 	unsigned int		id;
 	unsigned int		num_ports;
-	struct hsi_port		*port;
+	struct hsi_port		**port;
 };
 
 #define to_hsi_controller(dev) container_of(dev, struct hsi_controller, device)
 
 struct hsi_controller *hsi_alloc_controller(unsigned int n_ports, gfp_t flags);
-void hsi_free_controller(struct hsi_controller *hsi);
+void hsi_put_controller(struct hsi_controller *hsi);
 int hsi_register_controller(struct hsi_controller *hsi);
 void hsi_unregister_controller(struct hsi_controller *hsi);
 
@@ -294,7 +294,7 @@ static inline void *hsi_controller_drvdata(struct hsi_controller *hsi)
 static inline struct hsi_port *hsi_find_port_num(struct hsi_controller *hsi,
 							unsigned int num)
 {
-	return (num < hsi->num_ports) ? &hsi->port[num] : NULL;
+	return (num < hsi->num_ports) ? hsi->port[num] : NULL;
 }
 
 /*
-- 
1.7.5.4

[PATCH 2/6] HSI: hsi: Fix error path cleanup on client registration

[Carlos Chinea]

HSI client structure should be freed on error path after
calling device_registration by dropping a reference to it.

Signed-off-by: Carlos Chinea <carlos.chinea@nokia.com>
---
 drivers/hsi/hsi.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/hsi/hsi.c b/drivers/hsi/hsi.c
index c17d12c..5c76a36 100644
--- a/drivers/hsi/hsi.c
+++ b/drivers/hsi/hsi.c
@@ -100,7 +100,7 @@ static void hsi_new_client(struct hsi_port *port, struct hsi_board_info *info)
 		cl->device.archdata = *info->archdata;
 	if (device_register(&cl->device) < 0) {
 		pr_err("hsi: failed to register client: %s\n", info->name);
-		kfree(cl);
+		put_device(&cl->device);
 	}
 }
 
-- 
1.7.5.4

[PATCH 3/6] HSI: hsi: Remove controllers and ports from the bus

[Carlos Chinea]

HSI controllers and ports do not belong to the HSI bus.
Those devices are not supposed to have a driver attached to them.

Signed-off-by: Carlos Chinea <carlos.chinea@nokia.com>
---
 drivers/hsi/hsi.c |   20 +-------------------
 1 files changed, 1 insertions(+), 19 deletions(-)

diff --git a/drivers/hsi/hsi.c b/drivers/hsi/hsi.c
index 5c76a36..cec1f0c 100644
--- a/drivers/hsi/hsi.c
+++ b/drivers/hsi/hsi.c
@@ -29,18 +29,6 @@
 #include <linux/string.h>
 #include "hsi_core.h"
 
-static struct device_type hsi_ctrl = {
-	.name	= "hsi_controller",
-};
-
-static struct device_type hsi_cl = {
-	.name	= "hsi_client",
-};
-
-static struct device_type hsi_port = {
-	.name	= "hsi_port",
-};
-
 static ssize_t modalias_show(struct device *dev,
 			struct device_attribute *a __maybe_unused, char *buf)
 {
@@ -54,8 +42,7 @@ static struct device_attribute hsi_bus_dev_attrs[] = {
 
 static int hsi_bus_uevent(struct device *dev, struct kobj_uevent_env *env)
 {
-	if (dev->type == &hsi_cl)
-		add_uevent_var(env, "MODALIAS=hsi:%s", dev_name(dev));
+	add_uevent_var(env, "MODALIAS=hsi:%s", dev_name(dev));
 
 	return 0;
 }
@@ -85,7 +72,6 @@ static void hsi_new_client(struct hsi_port *port, struct hsi_board_info *info)
 	cl = kzalloc(sizeof(*cl), GFP_KERNEL);
 	if (!cl)
 		return;
-	cl->device.type = &hsi_cl;
 	cl->tx_cfg = info->tx_cfg;
 	cl->rx_cfg = info->rx_cfg;
 	cl->device.bus = &hsi_bus_type;
@@ -175,15 +161,11 @@ int hsi_register_controller(struct hsi_controller *hsi)
 	unsigned int i;
 	int err;
 
-	hsi->device.type = &hsi_ctrl;
-	hsi->device.bus = &hsi_bus_type;
 	err = device_add(&hsi->device);
 	if (err < 0)
 		return err;
 	for (i = 0; i < hsi->num_ports; i++) {
 		hsi->port[i]->device.parent = &hsi->device;
-		hsi->port[i]->device.bus = &hsi_bus_type;
-		hsi->port[i]->device.type = &hsi_port;
 		err = device_add(&hsi->port[i]->device);
 		if (err < 0)
 			goto out;
-- 
1.7.5.4

[PATCH 4/6] HSI: hsi: Rework hsi_event interface

[Carlos Chinea]

Remove custom hack and make use of the notifier chain interfaces for
delivering events from the ports to their associated clients.
Clients that want to receive port events need to register their callbacks
using hsi_register_port_event(). The callbacks can be called in interrupt
context. Use hsi_unregestier_port_event() to undo the registration.

Signed-off-by: Carlos Chinea <carlos.chinea@nokia.com>
---
 drivers/hsi/hsi.c       |  101 +++++++++++++++++++++++++----------------------
 include/linux/hsi/hsi.h |   25 ++++++-----
 2 files changed, 68 insertions(+), 58 deletions(-)

diff --git a/drivers/hsi/hsi.c b/drivers/hsi/hsi.c
index cec1f0c..2d58f93 100644
--- a/drivers/hsi/hsi.c
+++ b/drivers/hsi/hsi.c
@@ -21,12 +21,11 @@
  */
 #include <linux/hsi/hsi.h>
 #include <linux/compiler.h>
-#include <linux/rwsem.h>
 #include <linux/list.h>
-#include <linux/spinlock.h>
 #include <linux/kobject.h>
 #include <linux/slab.h>
 #include <linux/string.h>
+#include <linux/notifier.h>
 #include "hsi_core.h"
 
 static ssize_t modalias_show(struct device *dev,
@@ -67,7 +66,6 @@ static void hsi_client_release(struct device *dev)
 static void hsi_new_client(struct hsi_port *port, struct hsi_board_info *info)
 {
 	struct hsi_client *cl;
-	unsigned long flags;
 
 	cl = kzalloc(sizeof(*cl), GFP_KERNEL);
 	if (!cl)
@@ -79,9 +77,6 @@ static void hsi_new_client(struct hsi_port *port, struct hsi_board_info *info)
 	cl->device.release = hsi_client_release;
 	dev_set_name(&cl->device, info->name);
 	cl->device.platform_data = info->platform_data;
-	spin_lock_irqsave(&port->clock, flags);
-	list_add_tail(&cl->link, &port->clients);
-	spin_unlock_irqrestore(&port->clock, flags);
 	if (info->archdata)
 		cl->device.archdata = *info->archdata;
 	if (device_register(&cl->device) < 0) {
@@ -106,13 +101,6 @@ static void hsi_scan_board_info(struct hsi_controller *hsi)
 
 static int hsi_remove_client(struct device *dev, void *data __maybe_unused)
 {
-	struct hsi_client *cl = to_hsi_client(dev);
-	struct hsi_port *port = to_hsi_port(dev->parent);
-	unsigned long flags;
-
-	spin_lock_irqsave(&port->clock, flags);
-	list_del(&cl->link);
-	spin_unlock_irqrestore(&port->clock, flags);
 	device_unregister(dev);
 
 	return 0;
@@ -271,8 +259,7 @@ struct hsi_controller *hsi_alloc_controller(unsigned int n_ports, gfp_t flags)
 		port[i]->stop_tx = hsi_dummy_cl;
 		port[i]->release = hsi_dummy_cl;
 		mutex_init(&port[i]->lock);
-		INIT_LIST_HEAD(&hsi->port[i]->clients);
-		spin_lock_init(&hsi->port[i]->clock);
+		ATOMIC_INIT_NOTIFIER_HEAD(&port[i]->n_head);
 		dev_set_name(&port[i]->device, "port%d", i);
 		hsi->port[i]->device.release = hsi_port_release;
 		device_initialize(&hsi->port[i]->device);
@@ -420,37 +407,67 @@ void hsi_release_port(struct hsi_client *cl)
 }
 EXPORT_SYMBOL_GPL(hsi_release_port);
 
-static int hsi_start_rx(struct hsi_client *cl, void *data __maybe_unused)
+static int hsi_event_notifier_call(struct notifier_block *nb,
+				unsigned long event, void *data __maybe_unused)
 {
-	if (cl->hsi_start_rx)
-		(*cl->hsi_start_rx)(cl);
+	struct hsi_client *cl = container_of(nb, struct hsi_client, nb);
+
+	(*cl->ehandler)(cl, event);
 
 	return 0;
 }
 
-static int hsi_stop_rx(struct hsi_client *cl, void *data __maybe_unused)
+/**
+ * hsi_register_port_event - Register a client to receive port events
+ * @cl: HSI client that wants to receive port events
+ * @cb: Event handler callback
+ *
+ * Clients should register a callback to be able to receive
+ * events from the ports. Registration should happen after
+ * claiming the port.
+ * The handler can be called in interrupt context.
+ *
+ * Returns -errno on error, or 0 on success.
+ */
+int hsi_register_port_event(struct hsi_client *cl,
+			void (*handler)(struct hsi_client *, unsigned long))
 {
-	if (cl->hsi_stop_rx)
-		(*cl->hsi_stop_rx)(cl);
+	struct hsi_port *port = hsi_get_port(cl);
 
-	return 0;
+	if (!handler || cl->ehandler)
+		return -EINVAL;
+	if (!hsi_port_claimed(cl))
+		return -EACCES;
+	cl->ehandler = handler;
+	cl->nb.notifier_call = hsi_event_notifier_call;
+
+	return atomic_notifier_chain_register(&port->n_head, &cl->nb);
 }
+EXPORT_SYMBOL_GPL(hsi_register_port_event);
 
-static int hsi_port_for_each_client(struct hsi_port *port, void *data,
-				int (*fn)(struct hsi_client *cl, void *data))
+/**
+ * hsi_unregister_port_event - Stop receiving port events for a client
+ * @cl: HSI client that wants to stop receiving port events
+ *
+ * Clients should call this function before releasing their associated
+ * port.
+ *
+ * Returns -errno on error, or 0 on success.
+ */
+int hsi_unregister_port_event(struct hsi_client *cl)
 {
-	struct hsi_client *cl;
+	struct hsi_port *port = hsi_get_port(cl);
+	int err;
 
-	spin_lock(&port->clock);
-	list_for_each_entry(cl, &port->clients, link) {
-		spin_unlock(&port->clock);
-		(*fn)(cl, data);
-		spin_lock(&port->clock);
-	}
-	spin_unlock(&port->clock);
+	WARN_ON(!hsi_port_claimed(cl));
 
-	return 0;
+	err = atomic_notifier_chain_unregister(&port->n_head, &cl->nb);
+	if (!err)
+		cl->ehandler = NULL;
+
+	return err;
 }
+EXPORT_SYMBOL_GPL(hsi_unregister_port_event);
 
 /**
  * hsi_event -Notifies clients about port events
@@ -464,22 +481,12 @@ static int hsi_port_for_each_client(struct hsi_port *port, void *data,
  * Events:
  * HSI_EVENT_START_RX - Incoming wake line high
  * HSI_EVENT_STOP_RX - Incoming wake line down
+ *
+ * Returns -errno on error, or 0 on success.
  */
-void hsi_event(struct hsi_port *port, unsigned int event)
+int hsi_event(struct hsi_port *port, unsigned long event)
 {
-	int (*fn)(struct hsi_client *cl, void *data);
-
-	switch (event) {
-	case HSI_EVENT_START_RX:
-		fn = hsi_start_rx;
-		break;
-	case HSI_EVENT_STOP_RX:
-		fn = hsi_stop_rx;
-		break;
-	default:
-		return;
-	}
-	hsi_port_for_each_client(port, NULL, fn);
+	return atomic_notifier_call_chain(&port->n_head, event, NULL);
 }
 EXPORT_SYMBOL_GPL(hsi_event);
 
diff --git a/include/linux/hsi/hsi.h b/include/linux/hsi/hsi.h
index 7f3b726..56fae86 100644
--- a/include/linux/hsi/hsi.h
+++ b/include/linux/hsi/hsi.h
@@ -26,9 +26,9 @@
 #include <linux/device.h>
 #include <linux/mutex.h>
 #include <linux/scatterlist.h>
-#include <linux/spinlock.h>
 #include <linux/list.h>
 #include <linux/module.h>
+#include <linux/notifier.h>
 
 /* HSI message ttype */
 #define HSI_MSG_READ	0
@@ -121,18 +121,18 @@ static inline int hsi_register_board_info(struct hsi_board_info const *info,
  * @device: Driver model representation of the device
  * @tx_cfg: HSI TX configuration
  * @rx_cfg: HSI RX configuration
- * @hsi_start_rx: Called after incoming wake line goes high
- * @hsi_stop_rx: Called after incoming wake line goes low
+ * @e_handler: Callback for handling port events (RX Wake High/Low)
+ * @pclaimed: Keeps tracks if the clients claimed its associated HSI port
+ * @nb: Notifier block for port events
  */
 struct hsi_client {
 	struct device		device;
 	struct hsi_config	tx_cfg;
 	struct hsi_config	rx_cfg;
-	void			(*hsi_start_rx)(struct hsi_client *cl);
-	void			(*hsi_stop_rx)(struct hsi_client *cl);
 	/* private: */
+	void			(*ehandler)(struct hsi_client *, unsigned long);
 	unsigned int		pclaimed:1;
-	struct list_head	link;
+	struct notifier_block	nb;
 };
 
 #define to_hsi_client(dev) container_of(dev, struct hsi_client, device)
@@ -147,6 +147,10 @@ static inline void *hsi_client_drvdata(struct hsi_client *cl)
 	return dev_get_drvdata(&cl->device);
 }
 
+int hsi_register_port_event(struct hsi_client *cl,
+			void (*handler)(struct hsi_client *, unsigned long));
+int hsi_unregister_port_event(struct hsi_client *cl);
+
 /**
  * struct hsi_client_driver - Driver associated to an HSI client
  * @driver: Driver model representation of the driver
@@ -214,8 +218,7 @@ void hsi_free_msg(struct hsi_msg *msg);
  * @start_tx: Callback to inform that a client wants to TX data
  * @stop_tx: Callback to inform that a client no longer wishes to TX data
  * @release: Callback to inform that a client no longer uses the port
- * @clients: List of hsi_clients using the port.
- * @clock: Lock to serialize access to the clients list.
+ * @n_head: Notifier chain for signaling port events to the clients.
  */
 struct hsi_port {
 	struct device			device;
@@ -231,14 +234,14 @@ struct hsi_port {
 	int				(*start_tx)(struct hsi_client *cl);
 	int				(*stop_tx)(struct hsi_client *cl);
 	int				(*release)(struct hsi_client *cl);
-	struct list_head		clients;
-	spinlock_t			clock;
+	/* private */
+	struct atomic_notifier_head	n_head;
 };
 
 #define to_hsi_port(dev) container_of(dev, struct hsi_port, device)
 #define hsi_get_port(cl) to_hsi_port((cl)->device.parent)
 
-void hsi_event(struct hsi_port *port, unsigned int event);
+int hsi_event(struct hsi_port *port, unsigned long event);
 int hsi_claim_port(struct hsi_client *cl, unsigned int share);
 void hsi_release_port(struct hsi_client *cl);
 
-- 
1.7.5.4

[PATCH 5/6] HSI: hsi_char: Remove max_data_size from sysfs

[Carlos Chinea]

Remove max_data_size sysfs entry. Otherwise is possible
to have a buffer overrun if its value is increased after
the device is open.

Signed-off-by: Carlos Chinea <carlos.chinea@nokia.com>
---
 drivers/hsi/clients/hsi_char.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/hsi/clients/hsi_char.c b/drivers/hsi/clients/hsi_char.c
index 88a050d..3ad91f6 100644
--- a/drivers/hsi/clients/hsi_char.c
+++ b/drivers/hsi/clients/hsi_char.c
@@ -123,7 +123,7 @@ struct hsc_client_data {
 static unsigned int hsc_major;
 /* Maximum buffer size that hsi_char will accept from userspace */
 static unsigned int max_data_size = 0x1000;
-module_param(max_data_size, uint, S_IRUSR | S_IWUSR);
+module_param(max_data_size, uint, 0);
 MODULE_PARM_DESC(max_data_size, "max read/write data size [4,8..65536] (^2)");
 
 static void hsc_add_tail(struct hsc_channel *channel, struct hsi_msg *msg,
-- 
1.7.5.4

[PATCH 6/6] HSI: Add HSI ABI documentation

[Carlos Chinea]

Adds sysfs HSI framework documentation

Signed-off-by: Carlos Chinea <carlos.chinea@nokia.com>
---
 Documentation/ABI/testing/sysfs-bus-hsi |   19 +++++++++++++++++++
 1 files changed, 19 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-hsi

diff --git a/Documentation/ABI/testing/sysfs-bus-hsi b/Documentation/ABI/testing/sysfs-bus-hsi
new file mode 100644
index 0000000..1b1b282
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-bus-hsi
@@ -0,0 +1,19 @@
+What:		/sys/bus/hsi
+Date:		April 2012
+KernelVersion:	3.4
+Contact:	Carlos Chinea <carlos.chinea@nokia.com>
+Description:
+		High Speed Synchronous Serial Interface (HSI) is a
+		serial interface mainly used for connecting application
+		engines (APE) with cellular modem engines (CMT) in cellular
+		handsets.
+		The bus will be populated with devices (hsi_clients) representing
+		the protocols available in the system. Bus drivers implement
+		those protocols.
+
+What:		/sys/bus/hsi/devices/.../modalias
+Date:		April 2012
+KernelVersion:	3.4
+Contact:	Carlos Chinea <carlos.chinea@nokia.com>
+Description:	Stores the same MODALIAS value emitted by uevent
+		Format: hsi:<hsi_client device name>
-- 
1.7.5.4

Re: [PATCH 5/6] HSI: hsi_char: Remove max_data_size from sysfs

[Greg KH]

On Thu, Apr 19, 2012 at 03:05:26PM +0300, Carlos Chinea wrote:
> Remove max_data_size sysfs entry. Otherwise is possible
> to have a buffer overrun if its value is increased after
> the device is open.
> 
> Signed-off-by: Carlos Chinea <carlos.chinea@nokia.com>
> ---
>  drivers/hsi/clients/hsi_char.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/hsi/clients/hsi_char.c b/drivers/hsi/clients/hsi_char.c
> index 88a050d..3ad91f6 100644
> --- a/drivers/hsi/clients/hsi_char.c
> +++ b/drivers/hsi/clients/hsi_char.c
> @@ -123,7 +123,7 @@ struct hsc_client_data {
>  static unsigned int hsc_major;
>  /* Maximum buffer size that hsi_char will accept from userspace */
>  static unsigned int max_data_size = 0x1000;
> -module_param(max_data_size, uint, S_IRUSR | S_IWUSR);
> +module_param(max_data_size, uint, 0);

You can just change this to be read-only, that way userspace can see
what the value is, and it can't be changed.

But that's up to you, not an issue at all.

greg k-h

Re: [PATCH 6/6] HSI: Add HSI ABI documentation

[Greg KH]

On Thu, Apr 19, 2012 at 03:05:27PM +0300, Carlos Chinea wrote:
> Adds sysfs HSI framework documentation
> 
> Signed-off-by: Carlos Chinea <carlos.chinea@nokia.com>
> ---
>  Documentation/ABI/testing/sysfs-bus-hsi |   19 +++++++++++++++++++
>  1 files changed, 19 insertions(+), 0 deletions(-)
>  create mode 100644 Documentation/ABI/testing/sysfs-bus-hsi
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-hsi b/Documentation/ABI/testing/sysfs-bus-hsi
> new file mode 100644
> index 0000000..1b1b282
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-bus-hsi
> @@ -0,0 +1,19 @@
> +What:		/sys/bus/hsi
> +Date:		April 2012
> +KernelVersion:	3.4
> +Contact:	Carlos Chinea <carlos.chinea@nokia.com>
> +Description:
> +		High Speed Synchronous Serial Interface (HSI) is a
> +		serial interface mainly used for connecting application
> +		engines (APE) with cellular modem engines (CMT) in cellular
> +		handsets.
> +		The bus will be populated with devices (hsi_clients) representing
> +		the protocols available in the system. Bus drivers implement
> +		those protocols.
> +
> +What:		/sys/bus/hsi/devices/.../modalias
> +Date:		April 2012
> +KernelVersion:	3.4
> +Contact:	Carlos Chinea <carlos.chinea@nokia.com>
> +Description:	Stores the same MODALIAS value emitted by uevent
> +		Format: hsi:<hsi_client device name>

You don't have any files in the device directories at all that you
create other than the modalias one?  If so, that's fine, just curious.

greg k-h

Re: [PATCH 0/6] HSI fixes for 3.4

[Greg KH]

On Thu, Apr 19, 2012 at 03:05:21PM +0300, Carlos Chinea wrote:
> Hi,
> 
> This patch set contains fixes for the issues found by Greg KH
> 
> https://lkml.org/lkml/2012/4/2/511
> https://lkml.org/lkml/2012/4/2/512
> 
> I would like to get feedback from those patches and specially
> from the hsi_event rework patch.

All of these look great to me, thanks for doing this rework, I no longer
have any objections to the code at all.  Feel free to add:

	Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

To them if you wish.

thanks,

greg k-h

Re: [PATCH 6/6] HSI: Add HSI ABI documentation

[Carlos Chinea]

On Thu, 2012-04-19 at 08:17 -0700, ext Greg KH wrote:
> On Thu, Apr 19, 2012 at 03:05:27PM +0300, Carlos Chinea wrote:
> > Adds sysfs HSI framework documentation
> > 
> > Signed-off-by: Carlos Chinea <carlos.chinea@nokia.com>
> > ---
> >  Documentation/ABI/testing/sysfs-bus-hsi |   19 +++++++++++++++++++
> >  1 files changed, 19 insertions(+), 0 deletions(-)
> >  create mode 100644 Documentation/ABI/testing/sysfs-bus-hsi
> > 
> > diff --git a/Documentation/ABI/testing/sysfs-bus-hsi b/Documentation/ABI/testing/sysfs-bus-hsi
> > new file mode 100644
> > index 0000000..1b1b282
> > --- /dev/null
> > +++ b/Documentation/ABI/testing/sysfs-bus-hsi
> > @@ -0,0 +1,19 @@
> > +What:		/sys/bus/hsi
> > +Date:		April 2012
> > +KernelVersion:	3.4
> > +Contact:	Carlos Chinea <carlos.chinea@nokia.com>
> > +Description:
> > +		High Speed Synchronous Serial Interface (HSI) is a
> > +		serial interface mainly used for connecting application
> > +		engines (APE) with cellular modem engines (CMT) in cellular
> > +		handsets.
> > +		The bus will be populated with devices (hsi_clients) representing
> > +		the protocols available in the system. Bus drivers implement
> > +		those protocols.
> > +
> > +What:		/sys/bus/hsi/devices/.../modalias
> > +Date:		April 2012
> > +KernelVersion:	3.4
> > +Contact:	Carlos Chinea <carlos.chinea@nokia.com>
> > +Description:	Stores the same MODALIAS value emitted by uevent
> > +		Format: hsi:<hsi_client device name>
> 
> You don't have any files in the device directories at all that you
> create other than the modalias one?  If so, that's fine, just curious.
> 

yes that's case, only the modalias one is created.

Br,
Carlos