[PATCH 2/2] Sysfs interface for accessing non-volatile configuration.

[PATCH 2/2] Sysfs interface for accessing non-volatile configuration.

[Javier Cardona]

This will create the following sysfs directories:
/sys/class/net/ethX
		...
		|-- boot_options
		|   |-- bootflag
		|   `-- boottime
		...
		|-- mesh_ie
		|   |-- capability
		|   |-- mesh_id
		|   |-- metric_id
		|   `-- protocol_id

Signed-off-by: Javier Cardona <javier@cozybit.com>
---
 drivers/net/wireless/libertas/Makefile     |    1 +
 drivers/net/wireless/libertas/defs.h       |    4 +-
 drivers/net/wireless/libertas/main.c       |    3 +
 drivers/net/wireless/libertas/persistcfg.c |  422 ++++++++++++++++++++++++++++
 drivers/net/wireless/libertas/persistcfg.h |    5 +
 5 files changed, 434 insertions(+), 1 deletions(-)
 create mode 100644 drivers/net/wireless/libertas/persistcfg.c
 create mode 100644 drivers/net/wireless/libertas/persistcfg.h

diff --git a/drivers/net/wireless/libertas/Makefile b/drivers/net/wireless/libertas/Makefile
index f0724e3..09b6353 100644
--- a/drivers/net/wireless/libertas/Makefile
+++ b/drivers/net/wireless/libertas/Makefile
@@ -3,6 +3,7 @@ libertas-objs := main.o wext.o \
 		cmdresp.o scan.o	  \
 		11d.o 		  \
 		debugfs.o	  \
+		persistcfg.o	\
 		ethtool.o assoc.o
 
 usb8xxx-objs += if_usb.o
diff --git a/drivers/net/wireless/libertas/defs.h b/drivers/net/wireless/libertas/defs.h
index 0120d6c..4a7f936 100644
--- a/drivers/net/wireless/libertas/defs.h
+++ b/drivers/net/wireless/libertas/defs.h
@@ -40,6 +40,7 @@
 #define LBS_DEB_THREAD	0x00100000
 #define LBS_DEB_HEX	0x00200000
 #define LBS_DEB_SDIO	0x00400000
+#define LBS_DEB_SYSFS	0x00800000
 
 extern unsigned int lbs_debug;
 
@@ -81,7 +82,8 @@ do { if ((lbs_debug & (grp)) == (grp)) \
 #define lbs_deb_usbd(dev, fmt, args...) LBS_DEB_LL(LBS_DEB_USB, " usbd", "%s:" fmt, (dev)->bus_id, ##args)
 #define lbs_deb_cs(fmt, args...)        LBS_DEB_LL(LBS_DEB_CS, " cs", fmt, ##args)
 #define lbs_deb_thread(fmt, args...)    LBS_DEB_LL(LBS_DEB_THREAD, " thread", fmt, ##args)
-#define lbs_deb_sdio(fmt, args...)      LBS_DEB_LL(LBS_DEB_SDIO, " thread", fmt, ##args)
+#define lbs_deb_sdio(fmt, args...)      LBS_DEB_LL(LBS_DEB_SDIO, " sdio", fmt, ##args)
+#define lbs_deb_sysfs(fmt, args...)     LBS_DEB_LL(LBS_DEB_SYSFS, " sysfs", fmt, ##args)
 
 #define lbs_pr_info(format, args...) \
 	printk(KERN_INFO DRV_NAME": " format, ## args)
diff --git a/drivers/net/wireless/libertas/main.c b/drivers/net/wireless/libertas/main.c
index 5001888..27769cb 100644
--- a/drivers/net/wireless/libertas/main.c
+++ b/drivers/net/wireless/libertas/main.c
@@ -23,6 +23,7 @@
 #include "scan.h"
 #include "assoc.h"
 #include "cmd.h"
+#include "persistcfg.h"
 
 #define DRIVER_RELEASE_VERSION "323.p0"
 const char lbs_driver_version[] = "COMM-USB8388-" DRIVER_RELEASE_VERSION
@@ -1218,6 +1219,7 @@ int lbs_start_card(struct lbs_private *priv)
 	if (device_create_file(&dev->dev, &dev_attr_lbs_rtap))
 		lbs_pr_err("cannot register lbs_rtap attribute\n");
 
+	lbs_persist_config_init(dev);
 	lbs_update_channel(priv);
 
 	/* 5.0.16p0 is known to NOT support any mesh */
@@ -1278,6 +1280,7 @@ int lbs_stop_card(struct lbs_private *priv)
 
 	lbs_debugfs_remove_one(priv);
 	device_remove_file(&dev->dev, &dev_attr_lbs_rtap);
+	lbs_persist_config_remove(dev);
 	if (priv->mesh_tlv)
 		device_remove_file(&dev->dev, &dev_attr_lbs_mesh);
 
diff --git a/drivers/net/wireless/libertas/persistcfg.c b/drivers/net/wireless/libertas/persistcfg.c
new file mode 100644
index 0000000..bd7a47a
--- /dev/null
+++ b/drivers/net/wireless/libertas/persistcfg.c
@@ -0,0 +1,422 @@
+#include <linux/moduleparam.h>
+#include <linux/delay.h>
+#include <linux/etherdevice.h>
+#include <linux/netdevice.h>
+#include <linux/if_arp.h>
+#include <linux/kthread.h>
+#include <linux/kfifo.h>
+
+#include "host.h"
+#include "decl.h"
+#include "dev.h"
+#include "wext.h"
+#include "debugfs.h"
+#include "scan.h"
+#include "assoc.h"
+#include "cmd.h"
+#include "persistcfg.h"
+
+static int mesh_get_default_parameters(struct device *dev,
+		struct mrvl_mesh_defaults *defs)
+{
+	struct lbs_private *priv = to_net_dev(dev)->priv;
+	struct cmd_ds_mesh_config cmd;
+	int ret;
+
+	memset(&cmd, 0, sizeof(struct cmd_ds_mesh_config));
+	ret = lbs_mesh_config_send(priv, &cmd, CMD_ACT_MESH_CONFIG_GET,
+			CMD_TYPE_MESH_GET_DEFAULTS);
+
+	if (ret)
+		return -EOPNOTSUPP;
+
+	memcpy(defs, &cmd.data[0], sizeof(struct mrvl_mesh_defaults));
+
+	return 0;
+}
+
+/**
+ * @brief Get function for sysfs attribute bootflag
+ */
+static ssize_t bootflag_get(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	struct mrvl_mesh_defaults defs;
+	int ret;
+
+	ret = mesh_get_default_parameters(dev, &defs);
+
+	if (ret)
+		return ret;
+
+	return snprintf(buf, 12, "0x%X\n",
+		le32_to_cpu(cpu_to_le32(defs.bootflag)));
+}
+
+/**
+ * @brief Set function for sysfs attribute bootflag
+ */
+static ssize_t bootflag_set(struct device *dev,
+		struct device_attribute *attr, const char *buf, size_t count)
+{
+	struct lbs_private *priv = to_net_dev(dev)->priv;
+	struct cmd_ds_mesh_config cmd;
+	uint32_t datum;
+	int ret;
+
+	memset(&cmd, 0, sizeof(cmd));
+	ret = sscanf(buf, "%x", &datum);
+	if (ret != 1)
+		return -EINVAL;
+
+	*((uint32_t *)&cmd.data[0]) = cpu_to_le32(!!datum);
+	cmd.length = cpu_to_le16(sizeof(uint32_t));
+	ret = lbs_mesh_config_send(priv, &cmd, CMD_ACT_MESH_CONFIG_SET,
+					CMD_TYPE_MESH_SET_BOOTFLAG);
+	if (ret)
+		return ret;
+
+	return strlen(buf);
+}
+
+/**
+ * @brief Get function for sysfs attribute boottime
+ */
+static ssize_t boottime_get(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	struct lbs_private *priv = to_net_dev(dev)->priv;
+	struct cmd_ds_mesh_config cmd;
+	struct mrvl_mesh_defaults *defs;
+	int ret;
+
+	memset(&cmd, 0, sizeof(struct cmd_ds_mesh_config));
+	ret = lbs_mesh_config_send(priv, &cmd, CMD_ACT_MESH_CONFIG_GET,
+			CMD_TYPE_MESH_GET_DEFAULTS);
+
+	if (ret)
+		return -EOPNOTSUPP;
+
+	defs = (struct mrvl_mesh_defaults *)&cmd.data[0];
+
+	return snprintf(buf, 12, "0x%X\n", le32_to_cpu(defs->boottime));
+}
+
+/**
+ * @brief Set function for sysfs attribute boottime
+ */
+static ssize_t boottime_set(struct device *dev,
+		struct device_attribute *attr, const char *buf, size_t count)
+{
+	struct lbs_private *priv = to_net_dev(dev)->priv;
+	struct cmd_ds_mesh_config cmd;
+	uint32_t datum;
+	int ret;
+
+	memset(&cmd, 0, sizeof(cmd));
+	ret = sscanf(buf, "%x", &datum);
+	if (ret != 1)
+		return -EINVAL;
+
+	/* A too small boot time will result in the device booting into
+	 * standalone (no-host) mode before the host can take control of it,
+	 * so the change will be hard to revert.  This may be a desired
+	 * feature (e.g to configure a very fast boot time for devices that
+	 * will not be attached to a host), but dangerous.  So I'm enforcing a
+	 * lower limit of 20 seconds:  remove and recompile the driver if this
+	 * does not work for you.
+	 */
+	datum = (datum < 20) ? 20 : datum;
+	cmd.data[0] = (uint8_t) datum;
+	cmd.length = cpu_to_le16(sizeof(uint8_t));
+	ret = lbs_mesh_config_send(priv, &cmd, CMD_ACT_MESH_CONFIG_SET,
+					CMD_TYPE_MESH_SET_BOOTTIME);
+	if (ret)
+		return ret;
+
+	return strlen(buf);
+}
+
+/**
+ * @brief Get function for sysfs attribute mesh_id
+ */
+static ssize_t mesh_id_get(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	struct mrvl_mesh_defaults defs;
+	int maxlen;
+	int ret;
+
+	ret = mesh_get_default_parameters(dev, &defs);
+
+	if (ret)
+		return ret;
+
+	if (defs.meshie.val.mesh_id_len > IW_ESSID_MAX_SIZE) {
+		printk(KERN_ERR "Inconsistent mesh ID length");
+		defs.meshie.val.mesh_id_len = IW_ESSID_MAX_SIZE;
+	}
+
+	/* SSID not null terminated: reserve room for \0 + \n */
+	maxlen = defs.meshie.val.mesh_id_len + 2;
+	maxlen = (PAGE_SIZE > maxlen) ? maxlen : PAGE_SIZE;
+
+	defs.meshie.val.mesh_id[defs.meshie.val.mesh_id_len] = '\0';
+
+
+	return snprintf(buf, maxlen, "%s\n", defs.meshie.val.mesh_id);
+}
+
+/**
+ * @brief Set function for sysfs attribute mesh_id
+ */
+static ssize_t mesh_id_set(struct device *dev,
+		struct device_attribute *attr, const char *buf, size_t count)
+{
+	struct cmd_ds_mesh_config cmd;
+	struct mrvl_mesh_defaults defs;
+	struct mrvl_meshie *ie;
+	struct lbs_private *priv = to_net_dev(dev)->priv;
+	int len;
+	int ret;
+
+	if (count < 2 || count > IW_ESSID_MAX_SIZE + 1)
+		return -EINVAL;
+
+	memset(&cmd, 0, sizeof(struct cmd_ds_mesh_config));
+	ie = (struct mrvl_meshie *) &cmd.data[0];
+
+	/* fetch all other Information Element parameters */
+	ret = mesh_get_default_parameters(dev, &defs);
+
+	cmd.length = cpu_to_le16(sizeof(struct mrvl_meshie));
+
+	/* transfer IE elements */
+	memcpy((uint8_t *) ie, (uint8_t *) &defs.meshie,
+						sizeof(struct mrvl_meshie));
+
+	len = count - 1;
+	memcpy(ie->val.mesh_id, buf, len);
+	/* SSID len */
+	ie->val.mesh_id_len = len;
+	/* IE len */
+	ie->hdr.len = sizeof(struct mrvl_meshie_val) - IW_ESSID_MAX_SIZE +
+		len;
+
+	ret = lbs_mesh_config_send(priv, &cmd, CMD_ACT_MESH_CONFIG_SET,
+					CMD_TYPE_MESH_SET_MESH_IE);
+	if (ret)
+		return ret;
+
+	return strlen(buf);
+}
+
+/**
+ * @brief Get function for sysfs attribute protocol_id
+ */
+static ssize_t protocol_id_get(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	struct mrvl_mesh_defaults defs;
+	int ret;
+
+	ret = mesh_get_default_parameters(dev, &defs);
+
+	if (ret)
+		return ret;
+
+	return snprintf(buf, 5, "%d\n", defs.meshie.val.active_protocol_id);
+}
+
+/**
+ * @brief Set function for sysfs attribute protocol_id
+ */
+static ssize_t protocol_id_set(struct device *dev,
+		struct device_attribute *attr, const char *buf, size_t count)
+{
+	struct cmd_ds_mesh_config cmd;
+	struct mrvl_mesh_defaults defs;
+	struct mrvl_meshie *ie;
+	struct lbs_private *priv = to_net_dev(dev)->priv;
+	uint32_t datum;
+	int ret;
+
+	memset(&cmd, 0, sizeof(cmd));
+	ret = sscanf(buf, "%x", &datum);
+	if (ret != 1)
+		return -EINVAL;
+
+	/* fetch all other Information Element parameters */
+	ret = mesh_get_default_parameters(dev, &defs);
+
+	cmd.length = cpu_to_le16(sizeof(struct mrvl_meshie));
+
+	/* transfer IE elements */
+	ie = (struct mrvl_meshie *) &cmd.data[0];
+	memcpy((uint8_t *) ie, (uint8_t *) &defs.meshie,
+						sizeof(struct mrvl_meshie));
+	/* update protocol id */
+	ie->val.active_protocol_id = (uint8_t) datum;
+
+	ret = lbs_mesh_config_send(priv, &cmd, CMD_ACT_MESH_CONFIG_SET,
+					CMD_TYPE_MESH_SET_MESH_IE);
+	if (ret)
+		return ret;
+
+	return strlen(buf);
+}
+
+/**
+ * @brief Get function for sysfs attribute metric_id
+ */
+static ssize_t metric_id_get(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	struct mrvl_mesh_defaults defs;
+	int ret;
+
+	ret = mesh_get_default_parameters(dev, &defs);
+
+	if (ret)
+		return ret;
+
+	return snprintf(buf, 5, "%d\n", defs.meshie.val.active_metric_id);
+}
+
+/**
+ * @brief Set function for sysfs attribute metric_id
+ */
+static ssize_t metric_id_set(struct device *dev,
+		struct device_attribute *attr, const char *buf, size_t count)
+{
+	struct cmd_ds_mesh_config cmd;
+	struct mrvl_mesh_defaults defs;
+	struct mrvl_meshie *ie;
+	struct lbs_private *priv = to_net_dev(dev)->priv;
+	uint32_t datum;
+	int ret;
+
+	memset(&cmd, 0, sizeof(cmd));
+	ret = sscanf(buf, "%x", &datum);
+	if (ret != 1)
+		return -EINVAL;
+
+	/* fetch all other Information Element parameters */
+	ret = mesh_get_default_parameters(dev, &defs);
+
+	cmd.length = cpu_to_le16(sizeof(struct mrvl_meshie));
+
+	/* transfer IE elements */
+	ie = (struct mrvl_meshie *) &cmd.data[0];
+	memcpy((uint8_t *) ie, (uint8_t *) &defs.meshie,
+						sizeof(struct mrvl_meshie));
+	/* update protocol id */
+	ie->val.active_metric_id = (uint8_t) datum;
+
+	ret = lbs_mesh_config_send(priv, &cmd, CMD_ACT_MESH_CONFIG_SET,
+					CMD_TYPE_MESH_SET_MESH_IE);
+	if (ret)
+		return ret;
+
+	return strlen(buf);
+}
+
+/**
+ * @brief Get function for sysfs attribute capability
+ */
+static ssize_t capability_get(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	struct mrvl_mesh_defaults defs;
+	int ret;
+
+	ret = mesh_get_default_parameters(dev, &defs);
+
+	if (ret)
+		return ret;
+
+	return snprintf(buf, 5, "%d\n", defs.meshie.val.mesh_capability);
+}
+
+/**
+ * @brief Set function for sysfs attribute capability
+ */
+static ssize_t capability_set(struct device *dev,
+		struct device_attribute *attr, const char *buf, size_t count)
+{
+	struct cmd_ds_mesh_config cmd;
+	struct mrvl_mesh_defaults defs;
+	struct mrvl_meshie *ie;
+	struct lbs_private *priv = to_net_dev(dev)->priv;
+	uint32_t datum;
+	int ret;
+
+	memset(&cmd, 0, sizeof(cmd));
+	ret = sscanf(buf, "%x", &datum);
+	if (ret != 1)
+		return -EINVAL;
+
+	/* fetch all other Information Element parameters */
+	ret = mesh_get_default_parameters(dev, &defs);
+
+	cmd.length = cpu_to_le16(sizeof(struct mrvl_meshie));
+
+	/* transfer IE elements */
+	ie = (struct mrvl_meshie *) &cmd.data[0];
+	memcpy((uint8_t *) ie, (uint8_t *) &defs.meshie,
+						sizeof(struct mrvl_meshie));
+	/* update value */
+	ie->val.mesh_capability = (uint8_t) datum;
+
+	ret = lbs_mesh_config_send(priv, &cmd, CMD_ACT_MESH_CONFIG_SET,
+					CMD_TYPE_MESH_SET_MESH_IE);
+	if (ret)
+		return ret;
+
+	return strlen(buf);
+}
+
+
+static DEVICE_ATTR(bootflag, 0644, bootflag_get, bootflag_set);
+static DEVICE_ATTR(boottime, 0644, boottime_get, boottime_set);
+static DEVICE_ATTR(mesh_id, 0644, mesh_id_get, mesh_id_set);
+static DEVICE_ATTR(protocol_id, 0644, protocol_id_get, protocol_id_set);
+static DEVICE_ATTR(metric_id, 0644, metric_id_get, metric_id_set);
+static DEVICE_ATTR(capability, 0644, capability_get, capability_set);
+
+static struct attribute *boot_opts_attrs[] = {
+	&dev_attr_bootflag.attr,
+	&dev_attr_boottime.attr,
+	NULL
+};
+
+static struct attribute_group boot_opts_group = {
+	.name = "boot_options",
+	.attrs = boot_opts_attrs,
+};
+
+static struct attribute *mesh_ie_attrs[] = {
+	&dev_attr_mesh_id.attr,
+	&dev_attr_protocol_id.attr,
+	&dev_attr_metric_id.attr,
+	&dev_attr_capability.attr,
+	NULL
+};
+
+static struct attribute_group mesh_ie_group = {
+	.name = "mesh_ie",
+	.attrs = mesh_ie_attrs,
+};
+
+void lbs_persist_config_init(struct net_device *dev)
+{
+	int ret;
+	ret = sysfs_create_group(&(dev->dev.kobj), &boot_opts_group);
+	ret = sysfs_create_group(&(dev->dev.kobj), &mesh_ie_group);
+}
+
+void lbs_persist_config_remove(struct net_device *dev)
+{
+	sysfs_remove_group(&(dev->dev.kobj), &boot_opts_group);
+	sysfs_remove_group(&(dev->dev.kobj), &mesh_ie_group);
+}
diff --git a/drivers/net/wireless/libertas/persistcfg.h b/drivers/net/wireless/libertas/persistcfg.h
new file mode 100644
index 0000000..6ab9ff6
--- /dev/null
+++ b/drivers/net/wireless/libertas/persistcfg.h
@@ -0,0 +1,5 @@
+#ifndef _LBS_PERSISTCFG_H_
+#define _LBS_PERSISTCFG_H_
+void lbs_persist_config_init(struct net_device *net);
+void lbs_persist_config_remove(struct net_device *net);
+#endif
-- 
1.5.2.4

Re: [PATCH 2/2] Sysfs interface for accessing non-volatile configuration.

[Holger Schurig]

Javier,

here are some notes from the CF/embedded point of view.


My understanding is that currently ONLY the USB dongle / OLPC 
firmware does support mesh. The other firmware's don't support 
mesh at all:

# /sys/class/net/eth1/boot_options
# grep . *
grep: bootflag: Operation not supported
grep: boottime: Operation not supported

or, from the log:

cmd: DNLD_CMD: command 0x00a3, seq 20, size 144
DNLD_CMD: a3 00 90 00 14 00 00 00 03 00 00 00 05 00 00 00
DNLD_CMD: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
DNLD_CMD: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
cmd: CMD_RESP: response 0x80a3, seq 20, size 144
CMD_RESP: a3 80 90 00 14 00 02 00 03 00 00 00 05 00 00 00
CMD_RESP: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
PREP_CMD: command 0x00a3 failed: 2


So I would like to have one (or both) of the following things:

*) a runtime check (e.g. by comparing firmware version) if
   the firmware supports mesh at all, and then doing this only
   in that case. Probably as lbs_has_mesh() call, we're going
   to need this in several places
*) a kernel-config option, because many people use CF/SDIO
   cards in embedded environmnents and like to have things
   small. No need for #ifdef/#endif, because the compiler can
   savely optimize things out: "#define lbs_has_mesh() (0)"
   if mesh is turned off via Kconfig.

Also, you create /sys/class/net/ethX/mesh_ie. This doesn't make
sense to me. /sys/class/net/mshX/boot_options seems to be more 
logical. However, I don't know much about mesh ...

Re: [PATCH 2/2] Sysfs interface for accessing non-volatile configuration.

[David Woodhouse]

On Sat, 2008-05-17 at 21:01 -0700, Javier Cardona wrote:
> @@ -1218,6 +1219,7 @@ int lbs_start_card(struct lbs_private *priv)
>         if (device_create_file(&dev->dev, &dev_attr_lbs_rtap))
>                 lbs_pr_err("cannot register lbs_rtap attribute\n");
>  
> +       lbs_persist_config_init(dev);
>         lbs_update_channel(priv);
>  
>         /* 5.0.16p0 is known to NOT support any mesh */
> @@ -1278,6 +1280,7 @@ int lbs_stop_card(struct lbs_private *priv)
>  
>         lbs_debugfs_remove_one(priv);
>         device_remove_file(&dev->dev, &dev_attr_lbs_rtap);
> +       lbs_persist_config_remove(dev);
>         if (priv->mesh_tlv)
>                 device_remove_file(&dev->dev, &dev_attr_lbs_mesh);
>  

As Holger points out, we should call those only if (priv->mesh_tlv).

> +	return snprintf(buf, 12, "0x%X\n",
> +		le32_to_cpu(cpu_to_le32(defs.bootflag)));

Que? Sparse loves that one :)

> +	*((uint32_t *)&cmd.data[0]) = cpu_to_le32(!!datum);

Sparse doesn't much like that either. It should be (__le32 *)

> +static ssize_t boottime_get(struct device *dev,
> +		struct device_attribute *attr, char *buf)
> +{
> +	struct lbs_private *priv = to_net_dev(dev)->priv;
> +	struct cmd_ds_mesh_config cmd;
> +	struct mrvl_mesh_defaults *defs;
> +	int ret;
> +
> +	memset(&cmd, 0, sizeof(struct cmd_ds_mesh_config));
> +	ret = lbs_mesh_config_send(priv, &cmd, CMD_ACT_MESH_CONFIG_GET,
> +			CMD_TYPE_MESH_GET_DEFAULTS);
> +
> +	if (ret)
> +		return -EOPNOTSUPP;

Why not using mesh_get_default_parameters()? 

> +	/* A too small boot time will result in the device booting into
> +	 * standalone (no-host) mode before the host can take control of it,
> +	 * so the change will be hard to revert.  This may be a desired
> +	 * feature (e.g to configure a very fast boot time for devices that
> +	 * will not be attached to a host), but dangerous.  So I'm enforcing a
> +	 * lower limit of 20 seconds:  remove and recompile the driver if this
> +	 * does not work for you.
> +	 */
> +	datum = (datum < 20) ? 20 : datum;

Hm. The driver should be able to cope with a device which is already
booted -- it sends it a reset command and then initialises it. I don't
think we have to be so paranoid, do we? If there's a lower limit on the
boottime, it would be the time it takes for an already-active driver to
react when the device resets. Which is much less than 20 seconds.

-- 
dwmw2

Re: [PATCH 2/2] Sysfs interface for accessing non-volatile configuration.

[Javier Cardona]

Holger,

Thanks for your comments.

On Tue, May 20, 2008 at 1:07 AM, Holger Schurig
<hs4233@mail.mn-solutions.de> wrote:
> *) a runtime check (e.g. by comparing firmware version) if
>   the firmware supports mesh at all, and then doing this only
>   in that case. Probably as lbs_has_mesh() call, we're going
>   to need this in several places
>
> *) a kernel-config option, because many people use CF/SDIO
>   cards in embedded environmnents and like to have things
>   small. No need for #ifdef/#endif, because the compiler can
>   savely optimize things out: "#define lbs_has_mesh() (0)"
>   if mesh is turned off via Kconfig.

Is David's suggestion acceptable to you?

On Sat, 2008-05-17 at 21:01 -0700, David Woodhouse wrote:
> As Holger points out, we should call those only if (priv->mesh_tlv).

> Also, you create /sys/class/net/ethX/mesh_ie. This doesn't make
> sense to me. /sys/class/net/mshX/boot_options seems to be more
> logical. However, I don't know much about mesh ...

The entries in the mesh_ie directory groups the non-volatile
parameters of the mesh Information Element that is included in all
mesh beacons.

Javier

-- 
Javier Cardona
cozybit Inc.

Re: [PATCH 2/2] Sysfs interface for accessing non-volatile configuration.

[Holger Schurig]

> Is David's suggestion acceptable to you?

Yes, it is. For the long time, I'd like to have a kernel option. 
I'll probably make this by myself, then I can show numbers (e.g. 
if the saving is considerable). For now, go with checking 
mesh_XXX.

> The entries in the mesh_ie directory groups the non-volatile
> parameters of the mesh Information Element that is included in
> all mesh beacons.

Ahh, ok. And since ethX can't produce mesh beacons, then mesh_ie 
entry belongs then to mshX's sysfs-entry, right?

Re: [PATCH 2/2] Sysfs interface for accessing non-volatile configuration.

[David Woodhouse]

On Wed, 2008-05-21 at 09:07 +0200, Holger Schurig wrote:
> Ahh, ok. And since ethX can't produce mesh beacons, then mesh_ie 
> entry belongs then to mshX's sysfs-entry, right?

I moved it there already.

-- 
dwmw2

Re: [PATCH 2/2] Sysfs interface for accessing non-volatile configuration.

[Javier Cardona]

David,

On Tue, May 20, 2008 at 7:11 AM, David Woodhouse <dwmw2@infradead.org> wrote:
> On Sat, 2008-05-17 at 21:01 -0700, Javier Cardona wrote:
>> +     /* A too small boot time will result in the device booting into
>> +      * standalone (no-host) mode before the host can take control of it,
>> +      * so the change will be hard to revert.  This may be a desired
>> +      * feature (e.g to configure a very fast boot time for devices that
>> +      * will not be attached to a host), but dangerous.  So I'm enforcing a
>> +      * lower limit of 20 seconds:  remove and recompile the driver if this
>> +      * does not work for you.
>> +      */
>> +     datum = (datum < 20) ? 20 : datum;
>
> Hm. The driver should be able to cope with a device which is already
> booted -- it sends it a reset command and then initialises it. I don't
> think we have to be so paranoid, do we? If there's a lower limit on the
> boottime, it would be the time it takes for an already-active driver to
> react when the device resets. Which is much less than 20 seconds.

If the boottime is set smaller than the host boot time, the device
will transition into standalone mode before it is detected by the
host.  The only recovery when that happens is to either reset the
device via an external reset line or to re-insert it.  That lower
boundary was just intended to avoid support issues of users seeing
their devices disappear after a reboot.

Javier


-- 
Javier Cardona
cozybit Inc.

Re: [PATCH 2/2] Sysfs interface for accessing non-volatile configuration.

[David Woodhouse]

On Wed, 2008-05-21 at 11:59 -0700, Javier Cardona wrote:
> 
> If the boottime is set smaller than the host boot time, the device
> will transition into standalone mode before it is detected by the
> host.  The only recovery when that happens is to either reset the
> device via an external reset line or to re-insert it. 

Nah, the driver will send it a CMD_802_11_RESET when it finds it not
responding properly to bootcmds, and it should be fine.

>  That lower boundary was just intended to avoid support issues of
> users seeing their devices disappear after a reboot.

I remember that well :)

-- 
dwmw2

Re: [PATCH 2/2] Sysfs interface for accessing non-volatile configuration.

[Javier Cardona]

David,

On Wed, May 21, 2008 at 12:06 PM, David Woodhouse <dwmw2@infradead.org> wrote:
> On Wed, 2008-05-21 at 11:59 -0700, Javier Cardona wrote:
>>
>> If the boottime is set smaller than the host boot time, the device
>> will transition into standalone mode before it is detected by the
>> host.  The only recovery when that happens is to either reset the
>> device via an external reset line or to re-insert it.
>
> Nah, the driver will send it a CMD_802_11_RESET when it finds it not
> responding properly to bootcmds, and it should be fine.

You need a live endpoint to send a CMD_802_11_RESET, which you won't
have if the device did not respond to USB enumeration because it was
in standalone mode.

Javier

-- 
Javier Cardona
cozybit Inc.

Re: [PATCH 2/2] Sysfs interface for accessing non-volatile configuration.

[David Woodhouse]

On Wed, 2008-05-21 at 12:18 -0700, Javier Cardona wrote:
> You need a live endpoint to send a CMD_802_11_RESET, which you won't
> have if the device did not respond to USB enumeration because it was
> in standalone mode.

Hm, did that behaviour change recently? I'm sure this recovery was
working before.

-- 
dwmw2

Re: [PATCH 2/2] Sysfs interface for accessing non-volatile configuration.

[Javier Cardona]

David,

On Wed, May 21, 2008 at 12:28 PM, David Woodhouse <dwmw2@infradead.org> wrote:
> On Wed, 2008-05-21 at 12:18 -0700, Javier Cardona wrote:
>> You need a live endpoint to send a CMD_802_11_RESET, which you won't
>> have if the device did not respond to USB enumeration because it was
>> in standalone mode.
>
> Hm, did that behaviour change recently? I'm sure this recovery was
> working before.

I don't think we've ever been able to communicate over USB with a
dongle that is in standalone mode.

Javier


-- 
Javier Cardona
cozybit Inc.

Re: [PATCH 2/2] Sysfs interface for accessing non-volatile configuration.

[David Woodhouse]

On Wed, 2008-05-21 at 12:51 -0700, Javier Cardona wrote:
> David,
> 
> On Wed, May 21, 2008 at 12:28 PM, David Woodhouse <dwmw2@infradead.org> wrote:
> > On Wed, 2008-05-21 at 12:18 -0700, Javier Cardona wrote:
> >> You need a live endpoint to send a CMD_802_11_RESET, which you won't
> >> have if the device did not respond to USB enumeration because it was
> >> in standalone mode.
> >
> > Hm, did that behaviour change recently? I'm sure this recovery was
> > working before.
> 
> I don't think we've ever been able to communicate over USB with a
> dongle that is in standalone mode.

Hm, I thought we had, but maybe I'm confused. At one point were we
resetting the device when we unloaded the module, and we could only
reinitialise the driver if we did so within 5 seconds (the 31.09 boot2
timeout). I could have sworn I'd fixed that.

It would be useful to be able to reset the device from software when
it's in standalone mode -- how hard would it be to fix that?

-- 
dwmw2

Re: [PATCH 2/2] Sysfs interface for accessing non-volatile configuration.

[Dan Williams]

On Wed, 2008-05-21 at 12:51 -0700, Javier Cardona wrote:
> David,
> 
> On Wed, May 21, 2008 at 12:28 PM, David Woodhouse <dwmw2@infradead.org> wrote:
> > On Wed, 2008-05-21 at 12:18 -0700, Javier Cardona wrote:
> >> You need a live endpoint to send a CMD_802_11_RESET, which you won't
> >> have if the device did not respond to USB enumeration because it was
> >> in standalone mode.
> >
> > Hm, did that behaviour change recently? I'm sure this recovery was
> > working before.
> 
> I don't think we've ever been able to communicate over USB with a
> dongle that is in standalone mode.

And they probably don't respond to USB port resets either, do they?  Our
recovery mode was always:

1) CMD_802_11_RESET
2) if that didn't work, do a USB port reset

Dan

Re: [PATCH 2/2] Sysfs interface for accessing non-volatile configuration.

[Javier Cardona]

Dan,

On Wed, May 21, 2008 at 1:05 PM, Dan Williams <dcbw@redhat.com> wrote:
> And they probably don't respond to USB port resets either, do they?

Don't think so.

> Our recovery mode was always:
>
> 1) CMD_802_11_RESET
> 2) if that didn't work, do a USB port reset

I don't think this ever worked on an active antenna in standalone
mode.  This may have been the recovery mode for firmware/usb problems.

Javier


-- 
Javier Cardona
cozybit Inc.