* [RFC PATCHv3 1/7] HSI: Introducing HSI framework
2010-10-11 9:06 [RFC PATCHv3 0/7] HSI framework and drivers Carlos Chinea
@ 2010-10-11 9:06 ` Carlos Chinea
0 siblings, 0 replies; 5+ messages in thread
From: Carlos Chinea @ 2010-10-11 9:06 UTC (permalink / raw)
To: linux-kernel; +Cc: linux-omap
Adds HSI framework in to the linux kernel.
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.
HSI provides multiplexing for up to 16 logical channels,
low-latency and full duplex communication.
Signed-off-by: Carlos Chinea <carlos.chinea@nokia.com>
---
drivers/Kconfig | 2 +
drivers/Makefile | 1 +
drivers/hsi/Kconfig | 13 ++
drivers/hsi/Makefile | 4 +
drivers/hsi/hsi.c | 516 +++++++++++++++++++++++++++++++++++++++++++++++
include/linux/hsi/hsi.h | 376 ++++++++++++++++++++++++++++++++++
6 files changed, 912 insertions(+), 0 deletions(-)
create mode 100644 drivers/hsi/Kconfig
create mode 100644 drivers/hsi/Makefile
create mode 100644 drivers/hsi/hsi.c
create mode 100644 include/linux/hsi/hsi.h
diff --git a/drivers/Kconfig b/drivers/Kconfig
index a2b902f..4fe39f9 100644
--- a/drivers/Kconfig
+++ b/drivers/Kconfig
@@ -50,6 +50,8 @@ source "drivers/i2c/Kconfig"
source "drivers/spi/Kconfig"
+source "drivers/hsi/Kconfig"
+
source "drivers/pps/Kconfig"
source "drivers/gpio/Kconfig"
diff --git a/drivers/Makefile b/drivers/Makefile
index 63e3aa9..6c6922a 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -47,6 +47,7 @@ obj-$(CONFIG_SCSI) += scsi/
obj-$(CONFIG_ATA) += ata/
obj-$(CONFIG_MTD) += mtd/
obj-$(CONFIG_SPI) += spi/
+obj-$(CONFIG_HSI) += hsi/
obj-y += net/
obj-$(CONFIG_ATM) += atm/
obj-$(CONFIG_FUSION) += message/
diff --git a/drivers/hsi/Kconfig b/drivers/hsi/Kconfig
new file mode 100644
index 0000000..5af62ce
--- /dev/null
+++ b/drivers/hsi/Kconfig
@@ -0,0 +1,13 @@
+#
+# HSI driver configuration
+#
+menuconfig HSI
+ bool "HSI support"
+ ---help---
+ The "High speed synchronous Serial Interface" is
+ synchronous serial interface used mainly to connect
+ application engines and cellular modems.
+
+if HSI
+
+endif # HSI
diff --git a/drivers/hsi/Makefile b/drivers/hsi/Makefile
new file mode 100644
index 0000000..b42b6cf
--- /dev/null
+++ b/drivers/hsi/Makefile
@@ -0,0 +1,4 @@
+#
+# Makefile for HSI
+#
+obj-$(CONFIG_HSI) += hsi.o
diff --git a/drivers/hsi/hsi.c b/drivers/hsi/hsi.c
new file mode 100644
index 0000000..78f1df4
--- /dev/null
+++ b/drivers/hsi/hsi.c
@@ -0,0 +1,516 @@
+/*
+ * hsi.c
+ *
+ * HSI core.
+ *
+ * Copyright (C) 2010 Nokia Corporation. All rights reserved.
+ *
+ * Contact: Carlos Chinea <carlos.chinea@nokia.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
+ * 02110-1301 USA
+ */
+#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>
+
+struct hsi_cl_info {
+ struct list_head list;
+ struct hsi_board_info info;
+};
+
+static LIST_HEAD(hsi_board_list);
+
+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)
+{
+ return sprintf(buf, "hsi:%s\n", dev_name(dev));
+}
+
+static struct device_attribute hsi_bus_dev_attrs[] = {
+ __ATTR_RO(modalias),
+ __ATTR_NULL,
+};
+
+static int hsi_bus_uevent(struct device *dev, struct kobj_uevent_env *env)
+{
+ add_uevent_var(env, "MODALIAS=hsi:%s", dev_name(dev));
+
+ return 0;
+}
+
+static int hsi_bus_match(struct device *dev, struct device_driver *driver)
+{
+ return strcmp(dev_name(dev), driver->name) == 0;
+}
+
+static struct bus_type hsi_bus_type = {
+ .name = "hsi",
+ .dev_attrs = hsi_bus_dev_attrs,
+ .match = hsi_bus_match,
+ .uevent = hsi_bus_uevent,
+};
+
+static void hsi_client_release(struct device *dev)
+{
+ kfree(to_hsi_client(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)
+ return;
+ cl->device.type = &hsi_cl;
+ cl->tx_cfg = info->tx_cfg;
+ cl->rx_cfg = info->rx_cfg;
+ cl->device.bus = &hsi_bus_type;
+ cl->device.parent = &port->device;
+ 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) {
+ pr_err("hsi: failed to register client: %s\n", info->name);
+ kfree(cl);
+ }
+}
+
+/**
+ * hsi_register_board_info - Register HSI clients information
+ * @info: Array of HSI clients on the board
+ * @len: Length of the array
+ *
+ * HSI clients are statically declared and registered on board files.
+ *
+ * HSI clients will be automatically registered to the HSI bus once the
+ * controller and the port where the clients wishes to attach are registered
+ * to it.
+ *
+ * Return -errno on failure, 0 on success.
+ */
+int __init hsi_register_board_info(struct hsi_board_info const *info,
+ unsigned int len)
+{
+ struct hsi_cl_info *cl_info;
+
+ cl_info = kzalloc(sizeof(*cl_info) * len, GFP_KERNEL);
+ if (!cl_info)
+ return -ENOMEM;
+
+ for (; len; len--, info++, cl_info++) {
+ cl_info->info = *info;
+ list_add_tail(&cl_info->list, &hsi_board_list);
+ }
+
+ return 0;
+}
+
+static void hsi_scan_board_info(struct hsi_controller *hsi)
+{
+ struct hsi_cl_info *cl_info;
+ struct hsi_port *p;
+
+ list_for_each_entry(cl_info, &hsi_board_list, list)
+ if (cl_info->info.hsi_id == hsi->id) {
+ p = hsi_find_port_num(hsi, cl_info->info.port);
+ if (!p)
+ continue;
+ hsi_new_client(p, &cl_info->info);
+ }
+}
+
+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;
+}
+
+static int hsi_remove_port(struct device *dev, void *data __maybe_unused)
+{
+ device_for_each_child(dev, NULL, hsi_remove_client);
+ device_unregister(dev);
+
+ return 0;
+}
+
+static void hsi_controller_release(struct device *dev __maybe_unused)
+{
+}
+
+static void hsi_port_release(struct device *dev __maybe_unused)
+{
+}
+
+/**
+ * hsi_unregister_controller - Unregister an HSI controller
+ * @hsi: The HSI controller to register
+ */
+void hsi_unregister_controller(struct hsi_controller *hsi)
+{
+ device_for_each_child(&hsi->device, NULL, hsi_remove_port);
+ device_unregister(&hsi->device);
+}
+EXPORT_SYMBOL_GPL(hsi_unregister_controller);
+
+/**
+ * hsi_register_controller - Register an HSI controller and its ports
+ * @hsi: The HSI controller to register
+ *
+ * Returns -errno on failure, 0 on success.
+ */
+int hsi_register_controller(struct hsi_controller *hsi)
+{
+ unsigned int i;
+ int err;
+
+ hsi->device.type = &hsi_ctrl;
+ hsi->device.bus = &hsi_bus_type;
+ hsi->device.release = hsi_controller_release;
+ err = device_register(&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);
+ if (err < 0)
+ goto out;
+ }
+ /* Populate HSI bus with HSI clients */
+ hsi_scan_board_info(hsi);
+
+ return 0;
+out:
+ hsi_unregister_controller(hsi);
+
+ return err;
+}
+EXPORT_SYMBOL_GPL(hsi_register_controller);
+
+/**
+ * hsi_register_client_driver - Register an HSI client to the HSI bus
+ * @drv: HSI client driver to register
+ *
+ * Returns -errno on failure, 0 on success.
+ */
+int hsi_register_client_driver(struct hsi_client_driver *drv)
+{
+ drv->driver.bus = &hsi_bus_type;
+
+ return driver_register(&drv->driver);
+}
+EXPORT_SYMBOL_GPL(hsi_register_client_driver);
+
+static inline int hsi_dummy_msg(struct hsi_msg *msg __maybe_unused)
+{
+ return 0;
+}
+
+static inline int hsi_dummy_cl(struct hsi_client *cl __maybe_unused)
+{
+ return 0;
+}
+
+/**
+ * hsi_alloc_controller - Allocate an HSI controller and its ports
+ * @n_ports: Number of ports on the HSI controller
+ * @flags: Kernel allocation flags
+ *
+ * Return NULL on failure or a pointer to an hsi_controller on success.
+ */
+struct hsi_controller *hsi_alloc_controller(unsigned int n_ports, gfp_t flags)
+{
+ struct hsi_controller *hsi;
+ 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);
+ }
+ hsi->num_ports = n_ports;
+ hsi->port = port;
+
+ return hsi;
+out:
+ kfree(port);
+
+ 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
+ *
+ * Client is responsible to free the buffers pointed by the scatterlists.
+ */
+void hsi_free_msg(struct hsi_msg *msg)
+{
+ if (!msg)
+ return;
+ sg_free_table(&msg->sgt);
+ kfree(msg);
+}
+EXPORT_SYMBOL_GPL(hsi_free_msg);
+
+/**
+ * hsi_alloc_msg - Allocate an HSI message
+ * @nents: Number of memory entries
+ * @flags: Kernel allocation flags
+ *
+ * nents can be 0. This mainly makes sense for read transfer.
+ * In that case, HSI drivers will call the complete callback when
+ * there is data to be read without consuming it.
+ *
+ * Return NULL on failure or a pointer to an hsi_msg on success.
+ */
+struct hsi_msg *hsi_alloc_msg(unsigned int nents, gfp_t flags)
+{
+ struct hsi_msg *msg;
+ int err;
+
+ msg = kzalloc(sizeof(*msg), flags);
+ if (!msg)
+ return NULL;
+
+ if (!nents)
+ return msg;
+
+ err = sg_alloc_table(&msg->sgt, nents, flags);
+ if (unlikely(err)) {
+ kfree(msg);
+ msg = NULL;
+ }
+
+ return msg;
+}
+EXPORT_SYMBOL_GPL(hsi_alloc_msg);
+
+/**
+ * hsi_async - Submit an HSI transfer to the controller
+ * @cl: HSI client sending the transfer
+ * @msg: The HSI transfer passed to controller
+ *
+ * The HSI message must have the channel, ttype, complete and destructor
+ * fields set beforehand. If nents > 0 then the client has to initialize
+ * also the scatterlists to point to the buffers to write to or read from.
+ *
+ * HSI controllers relay on pre-allocated buffers from their clients and they
+ * do not allocate buffers on their own.
+ *
+ * Once the HSI message transfer finishes, the HSI controller calls the
+ * complete callback with the status and actual_len fields of the HSI message
+ * updated. The complete callback can be called before returning from
+ * hsi_async.
+ *
+ * Returns -errno on failure or 0 on success
+ */
+int hsi_async(struct hsi_client *cl, struct hsi_msg *msg)
+{
+ struct hsi_port *port = hsi_get_port(cl);
+
+ if (!hsi_port_claimed(cl))
+ return -EACCES;
+
+ WARN_ON_ONCE(!msg->destructor || !msg->complete);
+ msg->cl = cl;
+
+ return port->async(msg);
+}
+EXPORT_SYMBOL_GPL(hsi_async);
+
+/**
+ * hsi_claim_port - Claim the HSI client's port
+ * @cl: HSI client that wants to claim its port
+ * @share: Flag to indicate if the client wants to share the port or not.
+ *
+ * Returns -errno on failure, 0 on success.
+ */
+int hsi_claim_port(struct hsi_client *cl, unsigned int share)
+{
+ struct hsi_port *port = hsi_get_port(cl);
+ int err = 0;
+
+ mutex_lock(&port->lock);
+ if ((port->claimed) && (!port->shared || !share)) {
+ err = -EBUSY;
+ goto out;
+ }
+ port->claimed++;
+ port->shared = !!share;
+ cl->pclaimed = 1;
+out:
+ mutex_unlock(&port->lock);
+
+ return err;
+}
+EXPORT_SYMBOL_GPL(hsi_claim_port);
+
+/**
+ * hsi_release_port - Release the HSI client's port
+ * @cl: HSI client which previously claimed its port
+ */
+void hsi_release_port(struct hsi_client *cl)
+{
+ struct hsi_port *port = hsi_get_port(cl);
+
+ mutex_lock(&port->lock);
+ /* Allow HW driver to do some cleanup */
+ port->release(cl);
+ if (cl->pclaimed)
+ port->claimed--;
+ BUG_ON(port->claimed < 0);
+ cl->pclaimed = 0;
+ if (!port->claimed)
+ port->shared = 0;
+ mutex_unlock(&port->lock);
+}
+EXPORT_SYMBOL_GPL(hsi_release_port);
+
+static int hsi_start_rx(struct hsi_client *cl, void *data __maybe_unused)
+{
+ if (cl->hsi_start_rx)
+ (*cl->hsi_start_rx)(cl);
+
+ return 0;
+}
+
+static int hsi_stop_rx(struct hsi_client *cl, void *data __maybe_unused)
+{
+ if (cl->hsi_stop_rx)
+ (*cl->hsi_stop_rx)(cl);
+
+ return 0;
+}
+
+static int hsi_port_for_each_client(struct hsi_port *port, void *data,
+ int (*fn)(struct hsi_client *cl, void *data))
+{
+ struct hsi_client *cl;
+
+ 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);
+
+ return 0;
+}
+
+/**
+ * hsi_event -Notifies clients about port events
+ * @port: Port where the event occurred
+ * @event: The event type
+ *
+ * Clients should not be concerned about wake line behavior. However, due
+ * to a race condition in HSI HW protocol, clients need to be notified
+ * about wake line changes, so they can implement a workaround for it.
+ *
+ * Events:
+ * HSI_EVENT_START_RX - Incoming wake line high
+ * HSI_EVENT_STOP_RX - Incoming wake line down
+ */
+void hsi_event(struct hsi_port *port, unsigned int 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);
+}
+EXPORT_SYMBOL_GPL(hsi_event);
+
+static int __init hsi_init(void)
+{
+ return bus_register(&hsi_bus_type);
+}
+postcore_initcall(hsi_init);
diff --git a/include/linux/hsi/hsi.h b/include/linux/hsi/hsi.h
new file mode 100644
index 0000000..54ae22a
--- /dev/null
+++ b/include/linux/hsi/hsi.h
@@ -0,0 +1,376 @@
+/*
+ * hsi.h
+ *
+ * HSI core header file.
+ *
+ * Copyright (C) 2010 Nokia Corporation. All rights reserved.
+ *
+ * Contact: Carlos Chinea <carlos.chinea@nokia.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
+ * 02110-1301 USA
+ */
+
+#ifndef __LINUX_HSI_H__
+#define __LINUX_HSI_H__
+
+#include <linux/device.h>
+#include <linux/mutex.h>
+#include <linux/scatterlist.h>
+#include <linux/spinlock.h>
+#include <linux/list.h>
+
+/* HSI message ttype */
+#define HSI_MSG_READ 0
+#define HSI_MSG_WRITE 1
+
+/* HSI configuration values */
+#define HSI_MODE_STREAM 1
+#define HSI_MODE_FRAME 2
+#define HSI_FLOW_SYNC 0 /* Synchronized flow */
+#define HSI_FLOW_PIPE 1 /* Pipelined flow */
+#define HSI_ARB_RR 0 /* Round-robin arbitration */
+#define HSI_ARB_PRIO 1 /* Channel priority arbitration */
+
+#define HSI_MAX_CHANNELS 16
+
+/* HSI message status codes */
+enum {
+ HSI_STATUS_COMPLETED, /* Message transfer is completed */
+ HSI_STATUS_PENDING, /* Message pending to be read/write (POLL) */
+ HSI_STATUS_PROCEEDING, /* Message transfer is ongoing */
+ HSI_STATUS_QUEUED, /* Message waiting to be served */
+ HSI_STATUS_ERROR, /* Error when message transfer was ongoing */
+};
+
+/* HSI port event codes */
+enum {
+ HSI_EVENT_START_RX,
+ HSI_EVENT_STOP_RX,
+};
+
+/**
+ * struct hsi_config - Configuration for RX/TX HSI modules
+ * @mode: Bit transmission mode (STREAM or FRAME)
+ * @flow: Flow type (SYNCHRONIZED or PIPELINE)
+ * @channels: Number of channels to use [1..16]
+ * @speed: Max bit transmission speed (Kbit/s)
+ * @arb_mode: Arbitration mode for TX frame (Round robin, priority)
+ */
+struct hsi_config {
+ unsigned int mode;
+ unsigned int flow;
+ unsigned int channels;
+ unsigned int speed;
+ unsigned int arb_mode; /* TX only */
+};
+
+/**
+ * struct hsi_board_info - HSI client board info
+ * @name: Name for the HSI device
+ * @hsi_id: HSI controller id where the client sits
+ * @port: Port number in the controller where the client sits
+ * @tx_cfg: HSI TX configuration
+ * @rx_cfg: HSI RX configuration
+ * @platform_data: Platform related data
+ * @archdata: Architecture-dependent device data
+ */
+struct hsi_board_info {
+ const char *name;
+ int hsi_id;
+ unsigned int port;
+ struct hsi_config tx_cfg;
+ struct hsi_config rx_cfg;
+ void *platform_data;
+ struct dev_archdata *archdata;
+};
+
+#ifdef CONFIG_HSI
+extern int hsi_register_board_info(struct hsi_board_info const *info,
+ unsigned int len);
+#else
+static inline int hsi_register_board_info(struct hsi_board_info const *info,
+ unsigned int len)
+{
+ return 0;
+}
+#endif
+
+/**
+ * struct hsi_client - HSI client attached to an HSI port
+ * @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
+ */
+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: */
+ unsigned int pclaimed:1;
+ struct list_head link;
+};
+
+#define to_hsi_client(dev) container_of(dev, struct hsi_client, device)
+
+static inline void hsi_client_set_drvdata(struct hsi_client *cl, void *data)
+{
+ dev_set_drvdata(&cl->device, data);
+}
+
+static inline void *hsi_client_drvdata(struct hsi_client *cl)
+{
+ return dev_get_drvdata(&cl->device);
+}
+
+/**
+ * struct hsi_client_driver - Driver associated to an HSI client
+ * @driver: Driver model representation of the driver
+ */
+struct hsi_client_driver {
+ struct device_driver driver;
+};
+
+#define to_hsi_client_driver(drv) container_of(drv, struct hsi_client_driver,\
+ driver)
+
+int hsi_register_client_driver(struct hsi_client_driver *drv);
+
+static inline void hsi_unregister_client_driver(struct hsi_client_driver *drv)
+{
+ driver_unregister(&drv->driver);
+}
+
+/**
+ * struct hsi_msg - HSI message descriptor
+ * @link: Free to use by the current descriptor owner
+ * @cl: HSI device client that issues the transfer
+ * @sgt: Head of the scatterlist array
+ * @context: Client context data associated to the transfer
+ * @complete: Transfer completion callback
+ * @destructor: Destructor to free resources when flushing
+ * @status: Status of the transfer when completed
+ * @actual_len: Actual length of data transfered on completion
+ * @channel: Channel were to TX/RX the message
+ * @ttype: Transfer type (TX if set, RX otherwise)
+ * @break_frame: if true HSI will send/receive a break frame (FRAME MODE)
+ */
+struct hsi_msg {
+ struct list_head link;
+ struct hsi_client *cl;
+ struct sg_table sgt;
+ void *context;
+
+ void (*complete)(struct hsi_msg *msg);
+ void (*destructor)(struct hsi_msg *msg);
+
+ int status;
+ unsigned int actual_len;
+ unsigned int channel;
+ unsigned int ttype:1;
+ unsigned int break_frame:1;
+};
+
+struct hsi_msg *hsi_alloc_msg(unsigned int n_frag, gfp_t flags);
+void hsi_free_msg(struct hsi_msg *msg);
+
+/**
+ * struct hsi_port - HSI port device
+ * @device: Driver model representation of the device
+ * @tx_cfg: Current TX path configuration
+ * @rx_cfg: Current RX path configuration
+ * @num: Port number
+ * @shared: Set when port can be shared by different clients
+ * @claimed: Reference count of clients which claimed the port
+ * @lock: Serialize port claim
+ * @async: Asynchronous transfer callback
+ * @setup: Callback to set the HSI client configuration
+ * @flush: Callback to clean the HW state and destroy all pending transfers
+ * @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.
+ */
+struct hsi_port {
+ struct device device;
+ struct hsi_config tx_cfg;
+ struct hsi_config rx_cfg;
+ unsigned int num;
+ unsigned int shared:1;
+ int claimed;
+ struct mutex lock;
+ int (*async)(struct hsi_msg *msg);
+ int (*setup)(struct hsi_client *cl);
+ int (*flush)(struct hsi_client *cl);
+ 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;
+};
+
+#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_claim_port(struct hsi_client *cl, unsigned int share);
+void hsi_release_port(struct hsi_client *cl);
+
+static inline int hsi_port_claimed(struct hsi_client *cl)
+{
+ return cl->pclaimed;
+}
+
+static inline void hsi_port_set_drvdata(struct hsi_port *port, void *data)
+{
+ dev_set_drvdata(&port->device, data);
+}
+
+static inline void *hsi_port_drvdata(struct hsi_port *port)
+{
+ return dev_get_drvdata(&port->device);
+}
+
+/**
+ * struct hsi_controller - HSI controller device
+ * @device: Driver model representation of the device
+ * @id: HSI controller ID
+ * @num_ports: Number of ports in the HSI controller
+ * @port: Array of HSI ports
+ */
+struct hsi_controller {
+ struct device device;
+ int id;
+ unsigned int num_ports;
+ 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);
+int hsi_register_controller(struct hsi_controller *hsi);
+void hsi_unregister_controller(struct hsi_controller *hsi);
+
+static inline void hsi_controller_set_drvdata(struct hsi_controller *hsi,
+ void *data)
+{
+ dev_set_drvdata(&hsi->device, data);
+}
+
+static inline void *hsi_controller_drvdata(struct hsi_controller *hsi)
+{
+ return dev_get_drvdata(&hsi->device);
+}
+
+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;
+}
+
+/*
+ * API for HSI clients
+ */
+int hsi_async(struct hsi_client *cl, struct hsi_msg *msg);
+
+/**
+ * hsi_setup - Configure the client's port
+ * @cl: Pointer to the HSI client
+ *
+ * When sharing ports, clients should either relay on a single
+ * client setup or have the same setup for all of them.
+ *
+ * Return -errno on failure, 0 on success
+ */
+static inline int hsi_setup(struct hsi_client *cl)
+{
+ if (!hsi_port_claimed(cl))
+ return -EACCES;
+ return hsi_get_port(cl)->setup(cl);
+}
+
+/**
+ * hsi_flush - Flush all pending transactions on the client's port
+ * @cl: Pointer to the HSI client
+ *
+ * This function will destroy all pending hsi_msg in the port and reset
+ * the HW port so it is ready to receive and transmit from a clean state.
+ *
+ * Return -errno on failure, 0 on success
+ */
+static inline int hsi_flush(struct hsi_client *cl)
+{
+ if (!hsi_port_claimed(cl))
+ return -EACCES;
+ return hsi_get_port(cl)->flush(cl);
+}
+
+/**
+ * hsi_async_read - Submit a read transfer
+ * @cl: Pointer to the HSI client
+ * @msg: HSI message descriptor of the transfer
+ *
+ * Return -errno on failure, 0 on success
+ */
+static inline int hsi_async_read(struct hsi_client *cl, struct hsi_msg *msg)
+{
+ msg->ttype = HSI_MSG_READ;
+ return hsi_async(cl, msg);
+}
+
+/**
+ * hsi_async_write - Submit a write transfer
+ * @cl: Pointer to the HSI client
+ * @msg: HSI message descriptor of the transfer
+ *
+ * Return -errno on failure, 0 on success
+ */
+static inline int hsi_async_write(struct hsi_client *cl, struct hsi_msg *msg)
+{
+ msg->ttype = HSI_MSG_WRITE;
+ return hsi_async(cl, msg);
+}
+
+/**
+ * hsi_start_tx - Signal the port that the client wants to start a TX
+ * @cl: Pointer to the HSI client
+ *
+ * Return -errno on failure, 0 on success
+ */
+static inline int hsi_start_tx(struct hsi_client *cl)
+{
+ if (!hsi_port_claimed(cl))
+ return -EACCES;
+ return hsi_get_port(cl)->start_tx(cl);
+}
+
+/**
+ * hsi_stop_tx - Signal the port that the client no longer wants to transmit
+ * @cl: Pointer to the HSI client
+ *
+ * Return -errno on failure, 0 on success
+ */
+static inline int hsi_stop_tx(struct hsi_client *cl)
+{
+ if (!hsi_port_claimed(cl))
+ return -EACCES;
+ return hsi_get_port(cl)->stop_tx(cl);
+}
+#endif /* __LINUX_HSI_H__ */
--
1.5.6.5
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [RFC PATCHv3 1/7] HSI: Introducing HSI framework
[not found] <5BC1940B997A254A9C54B26A6B7E50C807A1810400@mucse403.eu.infineon.com>
@ 2010-10-17 15:58 ` Peter Henn
2010-10-18 10:53 ` Carlos Chinea
0 siblings, 1 reply; 5+ messages in thread
From: Peter Henn @ 2010-10-17 15:58 UTC (permalink / raw)
To: Carlos Chinea; +Cc: Linux Kernel Development, linux-omap
Hello Carlos,
* Carlos Chinea <carlos.chinea@nokia.com> [101011 01:58]:
> Adds HSI framework in to the linux kernel.
>
> 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.
>
...
> diff --git a/include/linux/hsi/hsi.h b/include/linux/hsi/hsi.h
> new file mode 100644
> index 0000000..54ae22a
> --- /dev/null
> +++ b/include/linux/hsi/hsi.h
> @@ -0,0 +1,376 @@
> +/*
> + * hsi.h
> + *
> + * HSI core header file.
> + *
> + * Copyright (C) 2010 Nokia Corporation. All rights reserved.
> + *
> + * Contact: Carlos Chinea <carlos.chinea@nokia.com>
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * version 2 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
> + * 02110-1301 USA
> + */
> +
> +#ifndef __LINUX_HSI_H__
> +#define __LINUX_HSI_H__
> +
> +#include <linux/device.h>
> +#include <linux/mutex.h>
> +#include <linux/scatterlist.h>
> +#include <linux/spinlock.h>
> +#include <linux/list.h>
> +
> +/* HSI message ttype */
> +#define HSI_MSG_READ 0
> +#define HSI_MSG_WRITE 1
> +
> +/* HSI configuration values */
> +#define HSI_MODE_STREAM 1
> +#define HSI_MODE_FRAME 2
> +#define HSI_FLOW_SYNC 0 /* Synchronized flow */
> +#define HSI_FLOW_PIPE 1 /* Pipelined flow */
> +#define HSI_ARB_RR 0 /* Round-robin arbitration */
> +#define HSI_ARB_PRIO 1 /* Channel priority arbitration */
> +
> +#define HSI_MAX_CHANNELS 16
> +
> +/* HSI message status codes */
> +enum {
> + HSI_STATUS_COMPLETED, /* Message transfer is completed */
> + HSI_STATUS_PENDING, /* Message pending to be read/write (POLL) */
> + HSI_STATUS_PROCEEDING, /* Message transfer is ongoing */
> + HSI_STATUS_QUEUED, /* Message waiting to be served */
> + HSI_STATUS_ERROR, /* Error when message transfer was ongoing */
> +};
> +
> +/* HSI port event codes */
> +enum {
> + HSI_EVENT_START_RX,
> + HSI_EVENT_STOP_RX,
> +};
> +
> +/**
> + * struct hsi_config - Configuration for RX/TX HSI modules
> + * @mode: Bit transmission mode (STREAM or FRAME)
> + * @flow: Flow type (SYNCHRONIZED or PIPELINE)
> + * @channels: Number of channels to use [1..16]
> + * @speed: Max bit transmission speed (Kbit/s)
> + * @arb_mode: Arbitration mode for TX frame (Round robin, priority)
> + */
> +struct hsi_config {
> + unsigned int mode;
> + unsigned int flow;
> + unsigned int channels;
> + unsigned int speed;
> + unsigned int arb_mode; /* TX only */
> +};
> +
Enums for "mode" and "flow"!
Does the flow control be really a TX link property?
In RX direction a speed range helps changing the baudrate on the fly.
A configuration flag for wakeup is missing, which defines the wakeup
either GPIO or Ready line controlled.
A maximum RX frame burst counter value for each channel is missing.
A TX send priority for each channel is possibly missing, too.
TX and RX initial port properties AKA config could be combined e.g.:
struct hsi_port_properties {
__u16 tx_ch_count; /* required number of TX channels */
__u16 rx_ch_count; /* required number of RX channels */
__u32 tx_speed; /* TX speed in HZ */
__u32 rx_min_speed; /* RX min. speed in HZ */
__u32 rx_max_speed; /* RX max. speed in HZ */
enum hsi_transfer_mode tx_transfer_mode;
enum hsi_transfer_mode rx_transfer_mode;
enum hsi_flow_ctrl_mode flow_ctrl_mode; /* only need for RX */
enum hsi_arb_mode arb_mode; /* only need for TX */
bool tx_wakeup_mode; /* true: wakeup line ... */
bool rx_wakeup_mode; /* ... false: ready line */
};
Example for channel:
struct hsi_channel_properties {
...
unsigned char prio; /* only need for TX */
unsigned short frame_burst; /* only need for RX */
}
> +/**
> + * struct hsi_board_info - HSI client board info
> + * @name: Name for the HSI device
> + * @hsi_id: HSI controller id where the client sits
> + * @port: Port number in the controller where the client sits
> + * @tx_cfg: HSI TX configuration
> + * @rx_cfg: HSI RX configuration
> + * @platform_data: Platform related data
> + * @archdata: Architecture-dependent device data
> + */
> +struct hsi_board_info {
> + const char *name;
> + int hsi_id;
> + unsigned int port;
> + struct hsi_config tx_cfg;
> + struct hsi_config rx_cfg;
> + void *platform_data;
> + struct dev_archdata *archdata;
> +};
> +
> +#ifdef CONFIG_HSI
> +extern int hsi_register_board_info(struct hsi_board_info const *info,
> + unsigned int len);
> +#else
> +static inline int hsi_register_board_info(struct hsi_board_info const *info,
> + unsigned int len)
> +{
> + return 0;
> +}
> +#endif
> +
> +/**
> + * struct hsi_client - HSI client attached to an HSI port
> + * @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
> + */
> +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: */
> + unsigned int pclaimed:1;
> + struct list_head link;
> +};
> +
> +#define to_hsi_client(dev) container_of(dev, struct hsi_client, device)
> +
> +static inline void hsi_client_set_drvdata(struct hsi_client *cl, void *data)
> +{
> + dev_set_drvdata(&cl->device, data);
> +}
> +
> +static inline void *hsi_client_drvdata(struct hsi_client *cl)
> +{
> + return dev_get_drvdata(&cl->device);
> +}
> +
> +/**
> + * struct hsi_client_driver - Driver associated to an HSI client
> + * @driver: Driver model representation of the driver
> + */
> +struct hsi_client_driver {
> + struct device_driver driver;
> +};
Either using here direct "struct device_driver" or supporting an own
hsi_probe, hsi_remove called from the framework, example:
struct hsi_client_driver {
const char *name;
int (*probe) (struct hsi_client *);
void (*disconnect) (struct hsi_client *);
int (*suspend) (struct hsi_client *, pm_message_t);
int (*resume) (struct hsi_client *);
int (*event) (struct hsi_client *, u32 hsi_event);
struct device_driver driver;
};
#define to_hsi_client_driver(d) \
container_of(d, struct hsi_client_driver, driver)
> +
> +#define to_hsi_client_driver(drv) container_of(drv, struct hsi_client_driver,\
> + driver)
> +
> +int hsi_register_client_driver(struct hsi_client_driver *drv);
> +
> +static inline void hsi_unregister_client_driver(struct hsi_client_driver *drv)
> +{
> + driver_unregister(&drv->driver);
> +}
> +
> +/**
> + * struct hsi_msg - HSI message descriptor
> + * @link: Free to use by the current descriptor owner
> + * @cl: HSI device client that issues the transfer
> + * @sgt: Head of the scatterlist array
> + * @context: Client context data associated to the transfer
> + * @complete: Transfer completion callback
> + * @destructor: Destructor to free resources when flushing
> + * @status: Status of the transfer when completed
> + * @actual_len: Actual length of data transfered on completion
> + * @channel: Channel were to TX/RX the message
> + * @ttype: Transfer type (TX if set, RX otherwise)
> + * @break_frame: if true HSI will send/receive a break frame (FRAME MODE)
> + */
> +struct hsi_msg {
> + struct list_head link;
> + struct hsi_client *cl;
> + struct sg_table sgt;
> + void *context;
> +
> + void (*complete)(struct hsi_msg *msg);
> + void (*destructor)(struct hsi_msg *msg);
> +
> + int status;
> + unsigned int actual_len;
> + unsigned int channel;
> + unsigned int ttype:1;
> + unsigned int break_frame:1;
> +};
Either HSI transfer data or that break "1 bit message".
Normally a break affects all HSI channels of a HSI link and is planned
to be used as an error indication of the HSI link. It could be a
special HSI port initialization/reset/flush or setup property.
> +
> +struct hsi_msg *hsi_alloc_msg(unsigned int n_frag, gfp_t flags);
> +void hsi_free_msg(struct hsi_msg *msg);
> +
> +/**
> + * struct hsi_port - HSI port device
> + * @device: Driver model representation of the device
> + * @tx_cfg: Current TX path configuration
> + * @rx_cfg: Current RX path configuration
> + * @num: Port number
> + * @shared: Set when port can be shared by different clients
> + * @claimed: Reference count of clients which claimed the port
> + * @lock: Serialize port claim
> + * @async: Asynchronous transfer callback
> + * @setup: Callback to set the HSI client configuration
> + * @flush: Callback to clean the HW state and destroy all pending transfers
> + * @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.
> + */
> +struct hsi_port {
> + struct device device;
> + struct hsi_config tx_cfg;
> + struct hsi_config rx_cfg;
> + unsigned int num;
> + unsigned int shared:1;
> + int claimed;
> + struct mutex lock;
> + int (*async)(struct hsi_msg *msg);
> + int (*setup)(struct hsi_client *cl);
A synchronous call makes problems using a HSI-USB dongle like the
Asamalab with its delayed transaction over USB. Please add an
completion callback here.
> + int (*flush)(struct hsi_client *cl);
Name "flush" could be unclear, if it is indeed a "port reset".
> + 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;
> +};
The number of available DMA engines helps a link layer,
if (DMA_engines < parallel_channel_endpoint_conections_via_DMA)
Something like that filled up from the HCD and given by the registration
call:
struct hsi_hcd_limits {
unsigned int tx_max_ch_bits;
unsigned int rx_max_ch_bits;
unsigned int tx_min_speed;
unsigned int tx_max_speed;
unsigned int rx_min_speed;
unsigned int rx_max_speed;
unsigned int max_burst_jobs;
unsigned int tx_wakeup_support:1;
unsigned int rx_wakeup_support:1;
unsigned int tx_loopback_support:1;
};
> +
> +#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_claim_port(struct hsi_client *cl, unsigned int share);
> +void hsi_release_port(struct hsi_client *cl);
> +
> +static inline int hsi_port_claimed(struct hsi_client *cl)
> +{
> + return cl->pclaimed;
> +}
> +
> +static inline void hsi_port_set_drvdata(struct hsi_port *port, void *data)
> +{
> + dev_set_drvdata(&port->device, data);
> +}
> +
> +static inline void *hsi_port_drvdata(struct hsi_port *port)
> +{
> + return dev_get_drvdata(&port->device);
> +}
> +
> +/**
> + * struct hsi_controller - HSI controller device
> + * @device: Driver model representation of the device
> + * @id: HSI controller ID
> + * @num_ports: Number of ports in the HSI controller
> + * @port: Array of HSI ports
> + */
> +struct hsi_controller {
> + struct device device;
Device is only needed for platform device. PCIe or USB related
controller would have their own device, like in USB-core in the hcd.c
> + int id;
> + unsigned int num_ports;
> + 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);
> +int hsi_register_controller(struct hsi_controller *hsi);
> +void hsi_unregister_controller(struct hsi_controller *hsi);
> +
> +static inline void hsi_controller_set_drvdata(struct hsi_controller *hsi,
> + void *data)
> +{
> + dev_set_drvdata(&hsi->device, data);
> +}
> +
> +static inline void *hsi_controller_drvdata(struct hsi_controller *hsi)
> +{
> + return dev_get_drvdata(&hsi->device);
> +}
> +
> +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;
> +}
> +
> +/*
> + * API for HSI clients
> + */
> +int hsi_async(struct hsi_client *cl, struct hsi_msg *msg);
> +
> +/**
> + * hsi_setup - Configure the client's port
> + * @cl: Pointer to the HSI client
> + *
> + * When sharing ports, clients should either relay on a single
> + * client setup or have the same setup for all of them.
> + *
> + * Return -errno on failure, 0 on success
> + */
> +static inline int hsi_setup(struct hsi_client *cl)
> +{
> + if (!hsi_port_claimed(cl))
> + return -EACCES;
> + return hsi_get_port(cl)->setup(cl);
> +}
> +
> +/**
> + * hsi_flush - Flush all pending transactions on the client's port
> + * @cl: Pointer to the HSI client
> + *
> + * This function will destroy all pending hsi_msg in the port and reset
> + * the HW port so it is ready to receive and transmit from a clean state.
> + *
> + * Return -errno on failure, 0 on success
> + */
> +static inline int hsi_flush(struct hsi_client *cl)
> +{
> + if (!hsi_port_claimed(cl))
> + return -EACCES;
> + return hsi_get_port(cl)->flush(cl);
> +}
Also here "hsi_port_reset" would be more self-explanatory than hsi_flush.
Regards,
Peter
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RFC PATCHv3 1/7] HSI: Introducing HSI framework
2010-10-17 15:58 ` [RFC PATCHv3 1/7] HSI: Introducing HSI framework Peter Henn
@ 2010-10-18 10:53 ` Carlos Chinea
2010-10-23 12:41 ` Peter Henn
0 siblings, 1 reply; 5+ messages in thread
From: Carlos Chinea @ 2010-10-18 10:53 UTC (permalink / raw)
To: ext Peter Henn; +Cc: Linux Kernel Development, linux-omap@vger.kernel.org
Hi Peter,
First, thanks for your comments :)
Mine follow...
On Sun, 2010-10-17 at 17:58 +0200, ext Peter Henn wrote:
> Hello Carlos,
>
> * Carlos Chinea <carlos.chinea@nokia.com> [101011 01:58]:
> > Adds HSI framework in to the linux kernel.
> >
> > 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.
> >
>
> ...
>
> > diff --git a/include/linux/hsi/hsi.h b/include/linux/hsi/hsi.h
> > new file mode 100644
> > index 0000000..54ae22a
> > --- /dev/null
> > +++ b/include/linux/hsi/hsi.h
> > @@ -0,0 +1,376 @@
> > +/*
> > + * hsi.h
> > + *
> > + * HSI core header file.
> > + *
> > + * Copyright (C) 2010 Nokia Corporation. All rights reserved.
> > + *
> > + * Contact: Carlos Chinea <carlos.chinea@nokia.com>
> > + *
> > + * This program is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU General Public License
> > + * version 2 as published by the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope that it will be useful, but
> > + * WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> > + * General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU General Public License
> > + * along with this program; if not, write to the Free Software
> > + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
> > + * 02110-1301 USA
> > + */
> > +
> > +#ifndef __LINUX_HSI_H__
> > +#define __LINUX_HSI_H__
> > +
> > +#include <linux/device.h>
> > +#include <linux/mutex.h>
> > +#include <linux/scatterlist.h>
> > +#include <linux/spinlock.h>
> > +#include <linux/list.h>
> > +
> > +/* HSI message ttype */
> > +#define HSI_MSG_READ 0
> > +#define HSI_MSG_WRITE 1
> > +
> > +/* HSI configuration values */
> > +#define HSI_MODE_STREAM 1
> > +#define HSI_MODE_FRAME 2
> > +#define HSI_FLOW_SYNC 0 /* Synchronized flow */
> > +#define HSI_FLOW_PIPE 1 /* Pipelined flow */
> > +#define HSI_ARB_RR 0 /* Round-robin arbitration */
> > +#define HSI_ARB_PRIO 1 /* Channel priority arbitration */
> > +
> > +#define HSI_MAX_CHANNELS 16
> > +
> > +/* HSI message status codes */
> > +enum {
> > + HSI_STATUS_COMPLETED, /* Message transfer is completed */
> > + HSI_STATUS_PENDING, /* Message pending to be read/write (POLL) */
> > + HSI_STATUS_PROCEEDING, /* Message transfer is ongoing */
> > + HSI_STATUS_QUEUED, /* Message waiting to be served */
> > + HSI_STATUS_ERROR, /* Error when message transfer was ongoing */
> > +};
> > +
> > +/* HSI port event codes */
> > +enum {
> > + HSI_EVENT_START_RX,
> > + HSI_EVENT_STOP_RX,
> > +};
> > +
> > +/**
> > + * struct hsi_config - Configuration for RX/TX HSI modules
> > + * @mode: Bit transmission mode (STREAM or FRAME)
> > + * @flow: Flow type (SYNCHRONIZED or PIPELINE)
> > + * @channels: Number of channels to use [1..16]
> > + * @speed: Max bit transmission speed (Kbit/s)
> > + * @arb_mode: Arbitration mode for TX frame (Round robin, priority)
> > + */
> > +struct hsi_config {
> > + unsigned int mode;
> > + unsigned int flow;
> > + unsigned int channels;
> > + unsigned int speed;
> > + unsigned int arb_mode; /* TX only */
> > +};
> > +
>
> Enums for "mode" and "flow"!
> Does the flow control be really a TX link property?
Hmm, yep it does not. I'll fix that.
> In RX direction a speed range helps changing the baudrate on the fly.
Clients just tell which is the TX max bit rate allowed.
In the case of TX the HW driver is allowed to change the speed from the maximum to the HW minimum.
In the case of RX the HW driver has to be able to handle that TX speed from the transmitter.
> A configuration flag for wakeup is missing, which defines the wakeup
> either GPIO or Ready line controlled.
This is just tied to the HW controller, it is internal data that can be
passed to HW driver through other ways.
Clients can only set HW configuration values that affects directly their
protocol behavior.
> A maximum RX frame burst counter value for each channel is missing.
Again tied to the HW driver.
> A TX send priority for each channel is possibly missing, too.
>
Do you have a HW implementing that kind of feature ? And also does
any protocol make direct use of that feature, meaning it changes the
behavior of the protocol ?
If the answers are yes, I am willing to add support for that, otherwise
I will wait until someone really needs it. Please note the current
ARB_PRIO value means that channel 0 has the highest priority and
MAX_CHANNEL the lowest.
> TX and RX initial port properties AKA config could be combined e.g.:
It is a matter of taste, and with the tx flow change the memory
footprint will be the same.
> struct hsi_port_properties {
> __u16 tx_ch_count; /* required number of TX channels */
> __u16 rx_ch_count; /* required number of RX channels */
> __u32 tx_speed; /* TX speed in HZ */
> __u32 rx_min_speed; /* RX min. speed in HZ */
> __u32 rx_max_speed; /* RX max. speed in HZ */
> enum hsi_transfer_mode tx_transfer_mode;
> enum hsi_transfer_mode rx_transfer_mode;
> enum hsi_flow_ctrl_mode flow_ctrl_mode; /* only need for RX */
> enum hsi_arb_mode arb_mode; /* only need for TX */
> bool tx_wakeup_mode; /* true: wakeup line ... */
> bool rx_wakeup_mode; /* ... false: ready line */
> };
>
> Example for channel:
> struct hsi_channel_properties {
> ...
> unsigned char prio; /* only need for TX */
> unsigned short frame_burst; /* only need for RX */
> }
>
> > +/**
> > + * struct hsi_board_info - HSI client board info
> > + * @name: Name for the HSI device
> > + * @hsi_id: HSI controller id where the client sits
> > + * @port: Port number in the controller where the client sits
> > + * @tx_cfg: HSI TX configuration
> > + * @rx_cfg: HSI RX configuration
> > + * @platform_data: Platform related data
> > + * @archdata: Architecture-dependent device data
> > + */
> > +struct hsi_board_info {
> > + const char *name;
> > + int hsi_id;
> > + unsigned int port;
> > + struct hsi_config tx_cfg;
> > + struct hsi_config rx_cfg;
> > + void *platform_data;
> > + struct dev_archdata *archdata;
> > +};
> > +
> > +#ifdef CONFIG_HSI
> > +extern int hsi_register_board_info(struct hsi_board_info const *info,
> > + unsigned int len);
> > +#else
> > +static inline int hsi_register_board_info(struct hsi_board_info const *info,
> > + unsigned int len)
> > +{
> > + return 0;
> > +}
> > +#endif
> > +
> > +/**
> > + * struct hsi_client - HSI client attached to an HSI port
> > + * @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
> > + */
> > +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: */
> > + unsigned int pclaimed:1;
> > + struct list_head link;
> > +};
> > +
> > +#define to_hsi_client(dev) container_of(dev, struct hsi_client, device)
> > +
> > +static inline void hsi_client_set_drvdata(struct hsi_client *cl, void *data)
> > +{
> > + dev_set_drvdata(&cl->device, data);
> > +}
> > +
> > +static inline void *hsi_client_drvdata(struct hsi_client *cl)
> > +{
> > + return dev_get_drvdata(&cl->device);
> > +}
> > +
> > +/**
> > + * struct hsi_client_driver - Driver associated to an HSI client
> > + * @driver: Driver model representation of the driver
> > + */
> > +struct hsi_client_driver {
> > + struct device_driver driver;
> > +};
>
> Either using here direct "struct device_driver" or supporting an own
> hsi_probe, hsi_remove called from the framework, example:
>
> struct hsi_client_driver {
> const char *name;
> int (*probe) (struct hsi_client *);
> void (*disconnect) (struct hsi_client *);
> int (*suspend) (struct hsi_client *, pm_message_t);
> int (*resume) (struct hsi_client *);
> int (*event) (struct hsi_client *, u32 hsi_event);
>
> struct device_driver driver;
> };
Hmm, I will keep it as it is. No need to have currently custom probe()/
remove(). They will just be a wrap up. Having the device_driver wrap
like this forces the clients to register using
hsi_register_client_driver instead of allowing them to use also the
device_register() directly which will be wrong as they will not be tied
to the HSI bus.
> #define to_hsi_client_driver(d) \
> container_of(d, struct hsi_client_driver, driver)
>
Ok for the cosmetic change.
>
> > +
> > +#define to_hsi_client_driver(drv) container_of(drv, struct hsi_client_driver,\
> > + driver)
> > +
> > +int hsi_register_client_driver(struct hsi_client_driver *drv);
> > +
> > +static inline void hsi_unregister_client_driver(struct hsi_client_driver *drv)
> > +{
> > + driver_unregister(&drv->driver);
> > +}
> > +
> > +/**
> > + * struct hsi_msg - HSI message descriptor
> > + * @link: Free to use by the current descriptor owner
> > + * @cl: HSI device client that issues the transfer
> > + * @sgt: Head of the scatterlist array
> > + * @context: Client context data associated to the transfer
> > + * @complete: Transfer completion callback
> > + * @destructor: Destructor to free resources when flushing
> > + * @status: Status of the transfer when completed
> > + * @actual_len: Actual length of data transfered on completion
> > + * @channel: Channel were to TX/RX the message
> > + * @ttype: Transfer type (TX if set, RX otherwise)
> > + * @break_frame: if true HSI will send/receive a break frame (FRAME MODE)
> > + */
> > +struct hsi_msg {
> > + struct list_head link;
> > + struct hsi_client *cl;
> > + struct sg_table sgt;
> > + void *context;
> > +
> > + void (*complete)(struct hsi_msg *msg);
> > + void (*destructor)(struct hsi_msg *msg);
> > +
> > + int status;
> > + unsigned int actual_len;
> > + unsigned int channel;
> > + unsigned int ttype:1;
> > + unsigned int break_frame:1;
> > +};
>
> Either HSI transfer data or that break "1 bit message".
>
And that's how it works. The omap_ssi driver works exactly like that.
Maybe some extra documentation explaining this is needed.
> Normally a break affects all HSI channels of a HSI link and is planned
> to be used as an error indication of the HSI link. It could be a
> special HSI port initialization/reset/flush or setup property.
>
Yes but yet again the client wants to send/receive a frame. It is just
that turns to be a special one that only the HW can generate.
So I prefer to keep that on the HSI sending/receiving API.
>
> > +
> > +struct hsi_msg *hsi_alloc_msg(unsigned int n_frag, gfp_t flags);
> > +void hsi_free_msg(struct hsi_msg *msg);
> > +
> > +/**
> > + * struct hsi_port - HSI port device
> > + * @device: Driver model representation of the device
> > + * @tx_cfg: Current TX path configuration
> > + * @rx_cfg: Current RX path configuration
> > + * @num: Port number
> > + * @shared: Set when port can be shared by different clients
> > + * @claimed: Reference count of clients which claimed the port
> > + * @lock: Serialize port claim
> > + * @async: Asynchronous transfer callback
> > + * @setup: Callback to set the HSI client configuration
> > + * @flush: Callback to clean the HW state and destroy all pending transfers
> > + * @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.
> > + */
> > +struct hsi_port {
> > + struct device device;
> > + struct hsi_config tx_cfg;
> > + struct hsi_config rx_cfg;
> > + unsigned int num;
> > + unsigned int shared:1;
> > + int claimed;
> > + struct mutex lock;
> > + int (*async)(struct hsi_msg *msg);
> > + int (*setup)(struct hsi_client *cl);
>
> A synchronous call makes problems using a HSI-USB dongle like the
> Asamalab with its delayed transaction over USB.
I expect hsi_setup to be called in process context, therefore being able
to block. So in that case, the driver can wait for the transaction to
succeed before returning. Maybe some extra comments, about which
interfaces must be called in process context, are needed.
> Please add an
> completion callback here.
>
> > + int (*flush)(struct hsi_client *cl);
>
> Name "flush" could be unclear, if it is indeed a "port reset".
>
I'll give a thought on that.
> > + 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;
> > +};
>
> The number of available DMA engines helps a link layer,
> if (DMA_engines < parallel_channel_endpoint_conections_via_DMA)
>
Hmm, the whole idea here is that clients are not bother about DMAs. They
just sent a request and it is the HW driver which decides what to use
for the transfer. In the case of not having enough DMA channels
available, the HW driver can just fall back to PIO transfers (omap_ssi
does that) or just hold the request in its internal queue/s until DMA
resources are available.
> Something like that filled up from the HCD and given by the registration
> call:
> struct hsi_hcd_limits {
> unsigned int tx_max_ch_bits;
> unsigned int rx_max_ch_bits;
> unsigned int tx_min_speed;
> unsigned int tx_max_speed;
> unsigned int rx_min_speed;
> unsigned int rx_max_speed;
> unsigned int max_burst_jobs;
> unsigned int tx_wakeup_support:1;
> unsigned int rx_wakeup_support:1;
> unsigned int tx_loopback_support:1;
> };
>
>
> > +
> > +#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_claim_port(struct hsi_client *cl, unsigned int share);
> > +void hsi_release_port(struct hsi_client *cl);
> > +
> > +static inline int hsi_port_claimed(struct hsi_client *cl)
> > +{
> > + return cl->pclaimed;
> > +}
> > +
> > +static inline void hsi_port_set_drvdata(struct hsi_port *port, void *data)
> > +{
> > + dev_set_drvdata(&port->device, data);
> > +}
> > +
> > +static inline void *hsi_port_drvdata(struct hsi_port *port)
> > +{
> > + return dev_get_drvdata(&port->device);
> > +}
> > +
> > +/**
> > + * struct hsi_controller - HSI controller device
> > + * @device: Driver model representation of the device
> > + * @id: HSI controller ID
> > + * @num_ports: Number of ports in the HSI controller
> > + * @port: Array of HSI ports
> > + */
> > +struct hsi_controller {
> > + struct device device;
>
> Device is only needed for platform device. PCIe or USB related
> controller would have their own device, like in USB-core in the hcd.c
>
Hmmm no. It is needed. The device is not tied to the platform bus is
tied to the HSI bus. In the case of USB , it is expected that
registration of the HSI controller device is done in the probe()
function of the usb_driver. This works pretty much the same also for the
platform bus (see omap_ssi) and others.
> > + int id;
> > + unsigned int num_ports;
> > + 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);
> > +int hsi_register_controller(struct hsi_controller *hsi);
> > +void hsi_unregister_controller(struct hsi_controller *hsi);
> > +
> > +static inline void hsi_controller_set_drvdata(struct hsi_controller *hsi,
> > + void *data)
> > +{
> > + dev_set_drvdata(&hsi->device, data);
> > +}
> > +
> > +static inline void *hsi_controller_drvdata(struct hsi_controller *hsi)
> > +{
> > + return dev_get_drvdata(&hsi->device);
> > +}
> > +
> > +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;
> > +}
> > +
> > +/*
> > + * API for HSI clients
> > + */
> > +int hsi_async(struct hsi_client *cl, struct hsi_msg *msg);
> > +
> > +/**
> > + * hsi_setup - Configure the client's port
> > + * @cl: Pointer to the HSI client
> > + *
> > + * When sharing ports, clients should either relay on a single
> > + * client setup or have the same setup for all of them.
> > + *
> > + * Return -errno on failure, 0 on success
> > + */
> > +static inline int hsi_setup(struct hsi_client *cl)
> > +{
> > + if (!hsi_port_claimed(cl))
> > + return -EACCES;
> > + return hsi_get_port(cl)->setup(cl);
> > +}
> > +
> > +/**
> > + * hsi_flush - Flush all pending transactions on the client's port
> > + * @cl: Pointer to the HSI client
> > + *
> > + * This function will destroy all pending hsi_msg in the port and reset
> > + * the HW port so it is ready to receive and transmit from a clean state.
> > + *
> > + * Return -errno on failure, 0 on success
> > + */
> > +static inline int hsi_flush(struct hsi_client *cl)
> > +{
> > + if (!hsi_port_claimed(cl))
> > + return -EACCES;
> > + return hsi_get_port(cl)->flush(cl);
> > +}
>
> Also here "hsi_port_reset" would be more self-explanatory than hsi_flush.
Well, as I said I will have a look to this, but reset can also be
understood that you reset the whole port (=> reseting also the
configuration to the default HW values) and does not what this function
does. It is just destroying all pending request and flush all the
internal buffers, leaving the port in a clean state ready to
send/receive more frames with the same configuration.
> Regards,
>
> Peter
>
Br,
Carlos
--
Carlos Chinea <carlos.chinea@nokia.com>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RFC PATCHv3 1/7] HSI: Introducing HSI framework
2010-10-18 10:53 ` Carlos Chinea
@ 2010-10-23 12:41 ` Peter Henn
2010-11-03 10:05 ` Carlos Chinea
0 siblings, 1 reply; 5+ messages in thread
From: Peter Henn @ 2010-10-23 12:41 UTC (permalink / raw)
To: Carlos Chinea; +Cc: Linux Kernel Development, linux-omap@vger.kernel.org
Hello Carlos,
please find my comments inline
Am 18.10.2010 12:53, schrieb Carlos Chinea:
> Hi Peter,
>
>>> +/**
>>> + * struct hsi_config - Configuration for RX/TX HSI modules
>>> + * @mode: Bit transmission mode (STREAM or FRAME)
>>> + * @flow: Flow type (SYNCHRONIZED or PIPELINE)
>>> + * @channels: Number of channels to use [1..16]
>>> + * @speed: Max bit transmission speed (Kbit/s)
>>> + * @arb_mode: Arbitration mode for TX frame (Round robin, priority)
>>> + */
>>> +struct hsi_config {
>>> + unsigned int mode;
>>> + unsigned int flow;
>>> + unsigned int channels;
>>> + unsigned int speed;
>>> + unsigned int arb_mode; /* TX only */
>>> +};
>>> +
>>
>> Enums for "mode" and "flow"!
>> Does the flow control be really a TX link property?
>
> Hmm, yep it does not. I'll fix that.
>
>> In RX direction a speed range helps changing the baudrate on the fly.
>
> Clients just tell which is the TX max bit rate allowed.
> In the case of TX the HW driver is allowed to change the speed from the maximum to the HW minimum.
yes
> In the case of RX the HW driver has to be able to handle that TX speed from the transmitter.
The RX speed shall setup the following things:
- the maximum system clock need to ensure proper data receiving. That
effects not even the receiver, which mostly reconstruct the RX clock
from data and flag, but all the other data management hardware behind.
Too high values would prevent power saving here.
- the minimum value shall be used to setup the frame timeout and
message timeout counter inside the HSI controller hardware
>
>> A configuration flag for wakeup is missing, which defines the wakeup
>> either GPIO or Ready line controlled.
>
> This is just tied to the HW controller, it is internal data that can be
> passed to HW driver through other ways.
Yes, it is a host controller setting. Could you add this to the
framework, which acts here as an API?
> Clients can only set HW configuration values that affects directly their
> protocol behavior.
>
>> A maximum RX frame burst counter value for each channel is missing.
>
> Again tied to the HW driver.
Yepp, the same as above.
>> A TX send priority for each channel is possibly missing, too.
>>
>
> Do you have a HW implementing that kind of feature ? And also does
> any protocol make direct use of that feature, meaning it changes the
> behavior of the protocol ?
>
> If the answers are yes, I am willing to add support for that, otherwise
> I will wait until someone really needs it. Please note the current
> ARB_PRIO value means that channel 0 has the highest priority and
> MAX_CHANNEL the lowest.
I know at least two HSI chip vendors, which has a more complex
arbitration and priority mechanism than only preferring channel 0 or
round robin. You may drop me a separate email, if you are interested to
known details. Just reserving a byte for each channel would work here to
represent such a special priority behavior.
>
>> TX and RX initial port properties AKA config could be combined e.g.:
>
> It is a matter of taste, and with the tx flow change the memory
> footprint will be the same.
Yepp, and I guess normally both hsi_config structures for tx and rx
setup will be handled mostly close together or not?
>
>> struct hsi_port_properties {
>> __u16 tx_ch_count; /* required number of TX channels */
>> __u16 rx_ch_count; /* required number of RX channels */
>> __u32 tx_speed; /* TX speed in HZ */
>> __u32 rx_min_speed; /* RX min. speed in HZ */
>> __u32 rx_max_speed; /* RX max. speed in HZ */
>> enum hsi_transfer_mode tx_transfer_mode;
>> enum hsi_transfer_mode rx_transfer_mode;
>> enum hsi_flow_ctrl_mode flow_ctrl_mode; /* only need for RX */
>> enum hsi_arb_mode arb_mode; /* only need for TX */
>> bool tx_wakeup_mode; /* true: wakeup line ... */
>> bool rx_wakeup_mode; /* ... false: ready line */
>> };
>>
>> Example for channel:
>> struct hsi_channel_properties {
>> ...
>> unsigned char prio; /* only need for TX */
>> unsigned short frame_burst; /* only need for RX */
>> }
>>
>>> +/**
>>> + * struct hsi_board_info - HSI client board info
>>> + * @name: Name for the HSI device
>>> + * @hsi_id: HSI controller id where the client sits
>>> + * @port: Port number in the controller where the client sits
>>> + * @tx_cfg: HSI TX configuration
>>> + * @rx_cfg: HSI RX configuration
>>> + * @platform_data: Platform related data
>>> + * @archdata: Architecture-dependent device data
>>> + */
>>> +struct hsi_board_info {
>>> + const char *name;
>>> + int hsi_id;
>>> + unsigned int port;
>>> + struct hsi_config tx_cfg;
>>> + struct hsi_config rx_cfg;
>>> + void *platform_data;
>>> + struct dev_archdata *archdata;
>>> +};
>>> +
>>> +#ifdef CONFIG_HSI
>>> +extern int hsi_register_board_info(struct hsi_board_info const *info,
>>> + unsigned int len);
>>> +#else
>>> +static inline int hsi_register_board_info(struct hsi_board_info const *info,
>>> + unsigned int len)
>>> +{
>>> + return 0;
>>> +}
>>> +#endif
>>> +
>>> +/**
>>> + * struct hsi_client - HSI client attached to an HSI port
>>> + * @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
>>> + */
>>> +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: */
>>> + unsigned int pclaimed:1;
>>> + struct list_head link;
>>> +};
>>> +
>>> +#define to_hsi_client(dev) container_of(dev, struct hsi_client, device)
>>> +
>>> +static inline void hsi_client_set_drvdata(struct hsi_client *cl, void *data)
>>> +{
>>> + dev_set_drvdata(&cl->device, data);
>>> +}
>>> +
>>> +static inline void *hsi_client_drvdata(struct hsi_client *cl)
>>> +{
>>> + return dev_get_drvdata(&cl->device);
>>> +}
>>> +
>>> +/**
>>> + * struct hsi_client_driver - Driver associated to an HSI client
>>> + * @driver: Driver model representation of the driver
>>> + */
>>> +struct hsi_client_driver {
>>> + struct device_driver driver;
>>> +};
>>
>> Either using here direct "struct device_driver" or supporting an own
>> hsi_probe, hsi_remove called from the framework, example:
>>
>> struct hsi_client_driver {
>> const char *name;
>> int (*probe) (struct hsi_client *);
>> void (*disconnect) (struct hsi_client *);
>> int (*suspend) (struct hsi_client *, pm_message_t);
>> int (*resume) (struct hsi_client *);
>> int (*event) (struct hsi_client *, u32 hsi_event);
>>
>> struct device_driver driver;
>> };
>
> Hmm, I will keep it as it is. No need to have currently custom probe()/
> remove(). They will just be a wrap up. Having the device_driver wrap
> like this forces the clients to register using
Yes, SPI uses these simple wrappers.
> hsi_register_client_driver instead of allowing them to use also the
> device_register() directly which will be wrong as they will not be tied
> to the HSI bus.
So the hsi_client will also in future plans direct related to the Linux
Device Core. And there is no plan for some client specific
configuration support by that framework as a general code part?
However, that becomes anyway difficult using the HSI-port sharing
mechanism for different vendor specific HSI client drivers.
>
>> #define to_hsi_client_driver(d) \
>> container_of(d, struct hsi_client_driver, driver)
>>
> Ok for the cosmetic change.
>>
>>> +
>>> +#define to_hsi_client_driver(drv) container_of(drv, struct hsi_client_driver,\
>>> + driver)
>>> +
>>> +int hsi_register_client_driver(struct hsi_client_driver *drv);
>>> +
>>> +static inline void hsi_unregister_client_driver(struct hsi_client_driver *drv)
>>> +{
>>> + driver_unregister(&drv->driver);
>>> +}
>>> +
>>> +/**
>>> + * struct hsi_msg - HSI message descriptor
>>> + * @link: Free to use by the current descriptor owner
>>> + * @cl: HSI device client that issues the transfer
>>> + * @sgt: Head of the scatterlist array
>>> + * @context: Client context data associated to the transfer
>>> + * @complete: Transfer completion callback
>>> + * @destructor: Destructor to free resources when flushing
>>> + * @status: Status of the transfer when completed
>>> + * @actual_len: Actual length of data transfered on completion
>>> + * @channel: Channel were to TX/RX the message
>>> + * @ttype: Transfer type (TX if set, RX otherwise)
>>> + * @break_frame: if true HSI will send/receive a break frame (FRAME MODE)
>>> + */
>>> +struct hsi_msg {
>>> + struct list_head link;
>>> + struct hsi_client *cl;
>>> + struct sg_table sgt;
>>> + void *context;
>>> +
>>> + void (*complete)(struct hsi_msg *msg);
>>> + void (*destructor)(struct hsi_msg *msg);
>>> +
>>> + int status;
>>> + unsigned int actual_len;
>>> + unsigned int channel;
>>> + unsigned int ttype:1;
>>> + unsigned int break_frame:1;
>>> +};
>>
>> Either HSI transfer data or that break "1 bit message".
>>
>
> And that's how it works. The omap_ssi driver works exactly like that.
> Maybe some extra documentation explaining this is needed.
>
Yes, if sgt _and_ break_frame bit is defined, what shall the HSI
controller do:
- send/have received 1st the message, than the break frame
- send/have received 1st the break frame, then the message
- send/have received only the break frame
>> Normally a break affects all HSI channels of a HSI link and is planned
>> to be used as an error indication of the HSI link. It could be a
>> special HSI port initialization/reset/flush or setup property.
>>
>
> Yes but yet again the client wants to send/receive a frame. It is just
> that turns to be a special one that only the HW can generate.
> So I prefer to keep that on the HSI sending/receiving API.
>
>>
>>> +
>>> +struct hsi_msg *hsi_alloc_msg(unsigned int n_frag, gfp_t flags);
>>> +void hsi_free_msg(struct hsi_msg *msg);
>>> +
>>> +/**
>>> + * struct hsi_port - HSI port device
>>> + * @device: Driver model representation of the device
>>> + * @tx_cfg: Current TX path configuration
>>> + * @rx_cfg: Current RX path configuration
>>> + * @num: Port number
>>> + * @shared: Set when port can be shared by different clients
>>> + * @claimed: Reference count of clients which claimed the port
>>> + * @lock: Serialize port claim
>>> + * @async: Asynchronous transfer callback
>>> + * @setup: Callback to set the HSI client configuration
>>> + * @flush: Callback to clean the HW state and destroy all pending transfers
>>> + * @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.
>>> + */
>>> +struct hsi_port {
>>> + struct device device;
>>> + struct hsi_config tx_cfg;
>>> + struct hsi_config rx_cfg;
>>> + unsigned int num;
>>> + unsigned int shared:1;
>>> + int claimed;
>>> + struct mutex lock;
>>> + int (*async)(struct hsi_msg *msg);
>>> + int (*setup)(struct hsi_client *cl);
>>
>> A synchronous call makes problems using a HSI-USB dongle like the
>> Asamalab with its delayed transaction over USB.
>
> I expect hsi_setup to be called in process context, therefore being able
> to block. So in that case, the driver can wait for the transaction to
> succeed before returning. Maybe some extra comments, about which
> interfaces must be called in process context, are needed.
That would make at least baudrate changes controlled by a link layer
driver code difficult. Please keep in mind that we might have something
like a selective suspend known form USB in the future here, which
requires more often changing the baudrate to reduce power consumption.
>
>> Please add an
>> completion callback here.
>>
>>> + int (*flush)(struct hsi_client *cl);
>>
>> Name "flush" could be unclear, if it is indeed a "port reset".
>>
>
> I'll give a thought on that.
>
>>> + 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;
>>> +};
>>
>> The number of available DMA engines helps a link layer,
>> if (DMA_engines < parallel_channel_endpoint_conections_via_DMA)
>>
>
> Hmm, the whole idea here is that clients are not bother about DMAs. They
> just sent a request and it is the HW driver which decides what to use
> for the transfer. In the case of not having enough DMA channels
> available, the HW driver can just fall back to PIO transfers (omap_ssi
> does that) or just hold the request in its internal queue/s until DMA
> resources are available.
PIO mode for single frames or short frames would be ok and possibly more
efficient than using DMA. But how shall both link layer drivers - host
and mode side - prevent one of the HSI Host controller drivers falling
back into PIO mode for long HSI frame bursts, if that controller has
limited resources? That becomes especially important for the poor HSI
controller, which has to spend all its DMA resources for incoming HSI
frame burst, because the sender does take care here.
>
>> Something like that filled up from the HCD and given by the registration
>> call:
>> struct hsi_hcd_limits {
>> unsigned int tx_max_ch_bits;
>> unsigned int rx_max_ch_bits;
>> unsigned int tx_min_speed;
>> unsigned int tx_max_speed;
>> unsigned int rx_min_speed;
>> unsigned int rx_max_speed;
>> unsigned int max_burst_jobs;
>> unsigned int tx_wakeup_support:1;
>> unsigned int rx_wakeup_support:1;
>> unsigned int tx_loopback_support:1;
>> };
>>
>>
>>> +
>>> +#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_claim_port(struct hsi_client *cl, unsigned int share);
>>> +void hsi_release_port(struct hsi_client *cl);
>>> +
>>> +static inline int hsi_port_claimed(struct hsi_client *cl)
>>> +{
>>> + return cl->pclaimed;
>>> +}
Would you please explain, which use cases you have in mind for that type
of "HSI port sharing" between HSI client drivers in the documentation?
I would expect you are thinking about some kind of HSI port/link/channel
high-jacking between a vendor specific hsi_client driver and the
hsi_char_driver.
>>> +
>>> +static inline void hsi_port_set_drvdata(struct hsi_port *port, void *data)
>>> +{
>>> + dev_set_drvdata(&port->device, data);
>>> +}
>>> +
>>> +static inline void *hsi_port_drvdata(struct hsi_port *port)
>>> +{
>>> + return dev_get_drvdata(&port->device);
>>> +}
>>> +
>>> +/**
>>> + * struct hsi_controller - HSI controller device
>>> + * @device: Driver model representation of the device
>>> + * @id: HSI controller ID
>>> + * @num_ports: Number of ports in the HSI controller
>>> + * @port: Array of HSI ports
>>> + */
>>> +struct hsi_controller {
>>> + struct device device;
>>
>> Device is only needed for platform device. PCIe or USB related
>> controller would have their own device, like in USB-core in the hcd.c
>>
>
> Hmmm no. It is needed. The device is not tied to the platform bus is
> tied to the HSI bus. In the case of USB , it is expected that
> registration of the HSI controller device is done in the probe()
> function of the usb_driver. This works pretty much the same also for the
> platform bus (see omap_ssi) and others.
Yes, you are right! I had another approach in mind.
>
>>> + int id;
>>> + unsigned int num_ports;
>>> + 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);
>>> +int hsi_register_controller(struct hsi_controller *hsi);
>>> +void hsi_unregister_controller(struct hsi_controller *hsi);
>>> +
>>> +static inline void hsi_controller_set_drvdata(struct hsi_controller *hsi,
>>> + void *data)
>>> +{
>>> + dev_set_drvdata(&hsi->device, data);
>>> +}
>>> +
>>> +static inline void *hsi_controller_drvdata(struct hsi_controller *hsi)
>>> +{
>>> + return dev_get_drvdata(&hsi->device);
>>> +}
>>> +
>>> +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;
>>> +}
>>> +
>>> +/*
>>> + * API for HSI clients
>>> + */
>>> +int hsi_async(struct hsi_client *cl, struct hsi_msg *msg);
>>> +
>>> +/**
>>> + * hsi_setup - Configure the client's port
>>> + * @cl: Pointer to the HSI client
>>> + *
>>> + * When sharing ports, clients should either relay on a single
>>> + * client setup or have the same setup for all of them.
>>> + *
>>> + * Return -errno on failure, 0 on success
>>> + */
>>> +static inline int hsi_setup(struct hsi_client *cl)
>>> +{
>>> + if (!hsi_port_claimed(cl))
>>> + return -EACCES;
>>> + return hsi_get_port(cl)->setup(cl);
>>> +}
>>> +
>>> +/**
>>> + * hsi_flush - Flush all pending transactions on the client's port
>>> + * @cl: Pointer to the HSI client
>>> + *
>>> + * This function will destroy all pending hsi_msg in the port and reset
>>> + * the HW port so it is ready to receive and transmit from a clean state.
>>> + *
>>> + * Return -errno on failure, 0 on success
>>> + */
>>> +static inline int hsi_flush(struct hsi_client *cl)
>>> +{
>>> + if (!hsi_port_claimed(cl))
>>> + return -EACCES;
>>> + return hsi_get_port(cl)->flush(cl);
>>> +}
>>
>> Also here "hsi_port_reset" would be more self-explanatory than hsi_flush.
>
> Well, as I said I will have a look to this, but reset can also be
> understood that you reset the whole port (=> reseting also the
> configuration to the default HW values) and does not what this function
> does. It is just destroying all pending request and flush all the
> internal buffers, leaving the port in a clean state ready to
> send/receive more frames with the same configuration.
>
>> Regards,
>>
>> Peter
>>
>
> Br,
> Carlos
Kind Regards,
Peter
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RFC PATCHv3 1/7] HSI: Introducing HSI framework
2010-10-23 12:41 ` Peter Henn
@ 2010-11-03 10:05 ` Carlos Chinea
0 siblings, 0 replies; 5+ messages in thread
From: Carlos Chinea @ 2010-11-03 10:05 UTC (permalink / raw)
To: ext Peter Henn; +Cc: Linux Kernel Development, linux-omap@vger.kernel.org
Hi,
my reply under
On Sat, 2010-10-23 at 14:41 +0200, ext Peter Henn wrote:
> Hello Carlos,
> please find my comments inline
>
> Am 18.10.2010 12:53, schrieb Carlos Chinea:
> > Hi Peter,
> >
> >>> +/**
> >>> + * struct hsi_config - Configuration for RX/TX HSI modules
> >>> + * @mode: Bit transmission mode (STREAM or FRAME)
> >>> + * @flow: Flow type (SYNCHRONIZED or PIPELINE)
> >>> + * @channels: Number of channels to use [1..16]
> >>> + * @speed: Max bit transmission speed (Kbit/s)
> >>> + * @arb_mode: Arbitration mode for TX frame (Round robin, priority)
> >>> + */
> >>> +struct hsi_config {
> >>> + unsigned int mode;
> >>> + unsigned int flow;
> >>> + unsigned int channels;
> >>> + unsigned int speed;
> >>> + unsigned int arb_mode; /* TX only */
> >>> +};
> >>> +
> >>
> >> Enums for "mode" and "flow"!
> >> Does the flow control be really a TX link property?
> >
> > Hmm, yep it does not. I'll fix that.
> >
> >> In RX direction a speed range helps changing the baudrate on the fly.
> >
> > Clients just tell which is the TX max bit rate allowed.
> > In the case of TX the HW driver is allowed to change the speed from the maximum to the HW minimum.
> yes
>
> > In the case of RX the HW driver has to be able to handle that TX speed from the transmitter.
> The RX speed shall setup the following things:
> - the maximum system clock need to ensure proper data receiving. That
> effects not even the receiver, which mostly reconstruct the RX clock
> from data and flag, but all the other data management hardware behind.
You got me confuse here. It is my understanding that an RX clock is
indeed needed to poll the state of the FLAG/DATA lines for changes in
their levels. Maybe your HW is event driven, but the one I have access
to is not.
> Too high values would prevent power saving here.
Well and that's actually the final point of this. With this value we can
prevent DVFS to set a too slow RX clock freq that will break HSI/SSI
communication.
And one of the reasons why speed is in the client configuration is that
I am expecting that DVFS support will be handled in the clients, as some
kind of handshaking will be needed with the other end.
> - the minimum value shall be used to setup the frame timeout and
> message timeout counter inside the HSI controller hardware
>
My current position on the RX timeout is that hsi clients do not need to
be bother about it. It is something that can be for example enabled by
default or configure through other means in the HW driver.
Even if the timeout is disabled by default, I will expect clients to
catch those kind of problems with their own protocol timers.
> >
> >> A configuration flag for wakeup is missing, which defines the wakeup
> >> either GPIO or Ready line controlled.
> >
> > This is just tied to the HW controller, it is internal data that can be
> > passed to HW driver through other ways.
> Yes, it is a host controller setting. Could you add this to the
> framework, which acts here as an API?
>
No the clients do not know anything about the specifics of the
underneath HW, like the use of GPIO wake lines. You get that info
through for example the platform resource data, along with irq numbers,
I/O memory areas, etc.
> > Clients can only set HW configuration values that affects directly their
> > protocol behavior.
> >
> >> A maximum RX frame burst counter value for each channel is missing.
> >
> > Again tied to the HW driver.
> Yepp, the same as above.
>
>
> >> A TX send priority for each channel is possibly missing, too.
> >>
> >
> > Do you have a HW implementing that kind of feature ? And also does
> > any protocol make direct use of that feature, meaning it changes the
> > behavior of the protocol ?
> >
> > If the answers are yes, I am willing to add support for that, otherwise
> > I will wait until someone really needs it. Please note the current
> > ARB_PRIO value means that channel 0 has the highest priority and
> > MAX_CHANNEL the lowest.
> I know at least two HSI chip vendors, which has a more complex
> arbitration and priority mechanism than only preferring channel 0 or
> round robin. You may drop me a separate email, if you are interested to
> known details. Just reserving a byte for each channel would work here to
> represent such a special priority behavior.
>
Ok, but still I want to know: does any protocol make use of that
feature ? Otherwise, this can be added later on when needed.
> >
> >> TX and RX initial port properties AKA config could be combined e.g.:
> >
> > It is a matter of taste, and with the tx flow change the memory
> > footprint will be the same.
> Yepp, and I guess normally both hsi_config structures for tx and rx
> setup will be handled mostly close together or not?
So... Sometimes you may just want to change the configuration of one of
the modules. But this is beside the point. What you are proposing is a
cosmetic change for the sake of it, unless that this is enforce by the
kernel coding style, or there is any technical impact I will not change
this.
Of course I will still fix the flow field issue.
>
> >
> >> struct hsi_port_properties {
> >> __u16 tx_ch_count; /* required number of TX channels */
> >> __u16 rx_ch_count; /* required number of RX channels */
> >> __u32 tx_speed; /* TX speed in HZ */
> >> __u32 rx_min_speed; /* RX min. speed in HZ */
> >> __u32 rx_max_speed; /* RX max. speed in HZ */
> >> enum hsi_transfer_mode tx_transfer_mode;
> >> enum hsi_transfer_mode rx_transfer_mode;
> >> enum hsi_flow_ctrl_mode flow_ctrl_mode; /* only need for RX */
> >> enum hsi_arb_mode arb_mode; /* only need for TX */
> >> bool tx_wakeup_mode; /* true: wakeup line ... */
> >> bool rx_wakeup_mode; /* ... false: ready line */
> >> };
> >>
> >> Example for channel:
> >> struct hsi_channel_properties {
> >> ...
> >> unsigned char prio; /* only need for TX */
> >> unsigned short frame_burst; /* only need for RX */
> >> }
> >>
> >>> +/**
> >>> + * struct hsi_board_info - HSI client board info
> >>> + * @name: Name for the HSI device
> >>> + * @hsi_id: HSI controller id where the client sits
> >>> + * @port: Port number in the controller where the client sits
> >>> + * @tx_cfg: HSI TX configuration
> >>> + * @rx_cfg: HSI RX configuration
> >>> + * @platform_data: Platform related data
> >>> + * @archdata: Architecture-dependent device data
> >>> + */
> >>> +struct hsi_board_info {
> >>> + const char *name;
> >>> + int hsi_id;
> >>> + unsigned int port;
> >>> + struct hsi_config tx_cfg;
> >>> + struct hsi_config rx_cfg;
> >>> + void *platform_data;
> >>> + struct dev_archdata *archdata;
> >>> +};
> >>> +
> >>> +#ifdef CONFIG_HSI
> >>> +extern int hsi_register_board_info(struct hsi_board_info const *info,
> >>> + unsigned int len);
> >>> +#else
> >>> +static inline int hsi_register_board_info(struct hsi_board_info const *info,
> >>> + unsigned int len)
> >>> +{
> >>> + return 0;
> >>> +}
> >>> +#endif
> >>> +
> >>> +/**
> >>> + * struct hsi_client - HSI client attached to an HSI port
> >>> + * @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
> >>> + */
> >>> +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: */
> >>> + unsigned int pclaimed:1;
> >>> + struct list_head link;
> >>> +};
> >>> +
> >>> +#define to_hsi_client(dev) container_of(dev, struct hsi_client, device)
> >>> +
> >>> +static inline void hsi_client_set_drvdata(struct hsi_client *cl, void *data)
> >>> +{
> >>> + dev_set_drvdata(&cl->device, data);
> >>> +}
> >>> +
> >>> +static inline void *hsi_client_drvdata(struct hsi_client *cl)
> >>> +{
> >>> + return dev_get_drvdata(&cl->device);
> >>> +}
> >>> +
> >>> +/**
> >>> + * struct hsi_client_driver - Driver associated to an HSI client
> >>> + * @driver: Driver model representation of the driver
> >>> + */
> >>> +struct hsi_client_driver {
> >>> + struct device_driver driver;
> >>> +};
> >>
> >> Either using here direct "struct device_driver" or supporting an own
> >> hsi_probe, hsi_remove called from the framework, example:
> >>
> >> struct hsi_client_driver {
> >> const char *name;
> >> int (*probe) (struct hsi_client *);
> >> void (*disconnect) (struct hsi_client *);
> >> int (*suspend) (struct hsi_client *, pm_message_t);
> >> int (*resume) (struct hsi_client *);
> >> int (*event) (struct hsi_client *, u32 hsi_event);
> >>
> >> struct device_driver driver;
> >> };
> >
> > Hmm, I will keep it as it is. No need to have currently custom probe()/
> > remove(). They will just be a wrap up. Having the device_driver wrap
> > like this forces the clients to register using
> Yes, SPI uses these simple wrappers.
And ? IMO they should get rid off them.
>
> > hsi_register_client_driver instead of allowing them to use also the
> > device_register() directly which will be wrong as they will not be tied
> > to the HSI bus.
> So the hsi_client will also in future plans direct related to the Linux
> Device Core. And there is no plan for some client specific
> configuration support by that framework as a general code part?
>
> However, that becomes anyway difficult using the HSI-port sharing
> mechanism for different vendor specific HSI client drivers.
No I think you got this wrong. hsi_clients in general implement
protocols no HW drivers. hsi_controllers implement the HW drivers.
Port sharing is just there to handle the situation where there are two
or more protocols using the same port at the same time, which is the
case of N900 where APE communicates through SSI with the CMT DSP and the
CMT MPU at the same time.
-- snip --
> >>
> >> Either HSI transfer data or that break "1 bit message".
> >>
> >
> > And that's how it works. The omap_ssi driver works exactly like that.
> > Maybe some extra documentation explaining this is needed.
> >
> Yes, if sgt _and_ break_frame bit is defined, what shall the HSI
> controller do:
> - send/have received 1st the message, than the break frame
> - send/have received 1st the break frame, then the message
> - send/have received only the break frame
>
Option 3: send/receive only the break frame.
> >> Normally a break affects all HSI channels of a HSI link and is planned
> >> to be used as an error indication of the HSI link. It could be a
> >> special HSI port initialization/reset/flush or setup property.
> >>
> >
> > Yes but yet again the client wants to send/receive a frame. It is just
> > that turns to be a special one that only the HW can generate.
> > So I prefer to keep that on the HSI sending/receiving API.
> >
> >>
> >>> +
> >>> +struct hsi_msg *hsi_alloc_msg(unsigned int n_frag, gfp_t flags);
> >>> +void hsi_free_msg(struct hsi_msg *msg);
> >>> +
> >>> +/**
> >>> + * struct hsi_port - HSI port device
> >>> + * @device: Driver model representation of the device
> >>> + * @tx_cfg: Current TX path configuration
> >>> + * @rx_cfg: Current RX path configuration
> >>> + * @num: Port number
> >>> + * @shared: Set when port can be shared by different clients
> >>> + * @claimed: Reference count of clients which claimed the port
> >>> + * @lock: Serialize port claim
> >>> + * @async: Asynchronous transfer callback
> >>> + * @setup: Callback to set the HSI client configuration
> >>> + * @flush: Callback to clean the HW state and destroy all pending transfers
> >>> + * @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.
> >>> + */
> >>> +struct hsi_port {
> >>> + struct device device;
> >>> + struct hsi_config tx_cfg;
> >>> + struct hsi_config rx_cfg;
> >>> + unsigned int num;
> >>> + unsigned int shared:1;
> >>> + int claimed;
> >>> + struct mutex lock;
> >>> + int (*async)(struct hsi_msg *msg);
> >>> + int (*setup)(struct hsi_client *cl);
> >>
> >> A synchronous call makes problems using a HSI-USB dongle like the
> >> Asamalab with its delayed transaction over USB.
> >
> > I expect hsi_setup to be called in process context, therefore being able
> > to block. So in that case, the driver can wait for the transaction to
> > succeed before returning. Maybe some extra comments, about which
> > interfaces must be called in process context, are needed.
> That would make at least baudrate changes controlled by a link layer
> driver code difficult. Please keep in mind that we might have something
> like a selective suspend known form USB in the future here, which
> requires more often changing the baudrate to reduce power consumption.
>
Did you mean that HSI block will be switch off if not in use ?
It is the duty of HW driver to save the state before the block is switch
off. In fact, that is what the omap_ssi driver does. HSI clients do not
care about if the HW supports off-mode or not.
If you wanted to say that there will need for DVFS support then you have
two options:
- This can be handled completed by the HW and its driver and therefore
there is no need of hsi_setup. Something quite hard to achive with the
current specs IMO.
- This needs the help of a protocol (ex: telling the other end to stop
sending and switch to a different TX speed, change the clock freq, and
then tell him to restart communication) which will imply that you will
need to block anyway to wait for acks and responses.
> >
> >> Please add an
> >> completion callback here.
> >>
> >>> + int (*flush)(struct hsi_client *cl);
> >>
> >> Name "flush" could be unclear, if it is indeed a "port reset".
> >>
> >
> > I'll give a thought on that.
> >
> >>> + 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;
> >>> +};
> >>
> >> The number of available DMA engines helps a link layer,
> >> if (DMA_engines < parallel_channel_endpoint_conections_via_DMA)
> >>
> >
> > Hmm, the whole idea here is that clients are not bother about DMAs. They
> > just sent a request and it is the HW driver which decides what to use
> > for the transfer. In the case of not having enough DMA channels
> > available, the HW driver can just fall back to PIO transfers (omap_ssi
> > does that) or just hold the request in its internal queue/s until DMA
> > resources are available.
> PIO mode for single frames or short frames would be ok and possibly more
> efficient than using DMA. But how shall both link layer drivers - host
> and mode side - prevent one of the HSI Host controller drivers falling
> back into PIO mode for long HSI frame bursts, if that controller has
> limited resources?
Well, it is the controllers decision to fallback or to queue. It is the
same for any resources managed by the driver, it is implementation
specific for that controllers driver what to do in case it runs out of
resources. Clients should not care if a DMA is used or not for
transmission/reception.
> That becomes especially important for the poor HSI
> controller, which has to spend all its DMA resources for incoming HSI
> frame burst, because the sender does take care here.
>
Why does the sender care if the receiver uses a DMA or not ?
> >
> >> Something like that filled up from the HCD and given by the registration
> >> call:
> >> struct hsi_hcd_limits {
> >> unsigned int tx_max_ch_bits;
> >> unsigned int rx_max_ch_bits;
> >> unsigned int tx_min_speed;
> >> unsigned int tx_max_speed;
> >> unsigned int rx_min_speed;
> >> unsigned int rx_max_speed;
> >> unsigned int max_burst_jobs;
> >> unsigned int tx_wakeup_support:1;
> >> unsigned int rx_wakeup_support:1;
> >> unsigned int tx_loopback_support:1;
> >> };
> >>
> >>
> >>> +
> >>> +#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_claim_port(struct hsi_client *cl, unsigned int share);
> >>> +void hsi_release_port(struct hsi_client *cl);
> >>> +
> >>> +static inline int hsi_port_claimed(struct hsi_client *cl)
> >>> +{
> >>> + return cl->pclaimed;
> >>> +}
> Would you please explain, which use cases you have in mind for that type
> of "HSI port sharing" between HSI client drivers in the documentation?
>
> I would expect you are thinking about some kind of HSI port/link/channel
> high-jacking between a vendor specific hsi_client driver and the
> hsi_char_driver.
See above my comment about port sharing in N900 case.
Br,
Carlos
--
Carlos Chinea <carlos.chinea@nokia.com>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2010-11-03 10:04 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <5BC1940B997A254A9C54B26A6B7E50C807A1810400@mucse403.eu.infineon.com>
2010-10-17 15:58 ` [RFC PATCHv3 1/7] HSI: Introducing HSI framework Peter Henn
2010-10-18 10:53 ` Carlos Chinea
2010-10-23 12:41 ` Peter Henn
2010-11-03 10:05 ` Carlos Chinea
2010-10-11 9:06 [RFC PATCHv3 0/7] HSI framework and drivers Carlos Chinea
2010-10-11 9:06 ` [RFC PATCHv3 1/7] HSI: Introducing HSI framework Carlos Chinea
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).