[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