Hello Jarkko,
On Sun, Aug 23, 2009 at 11:24:25AM +0200, Jarkko Nikula wrote:
> Use more descriptive than numerical value when showing and storing the
> McBSP DMA operating mode. Show function is using similar syntax than e.g.
> the led triggers so that all possible values for store function are
> printed but with current value surrounded with square brackets.
I liked the idea as I said before. But let's try to improve it a bit.
>
> Signed-off-by: Jarkko Nikula <jhnik...@gmail.com>
> Cc: Peter Ujfalusi <peter.ujfal...@nokia.com>
> Cc: Eduardo Valentin <eduardo.valen...@nokia.com>
> ---
> arch/arm/plat-omap/mcbsp.c | 52 ++++++++++++++++++++++++-------------------
> 1 files changed, 29 insertions(+), 23 deletions(-)
>
> diff --git a/arch/arm/plat-omap/mcbsp.c b/arch/arm/plat-omap/mcbsp.c
> index b63a720..28e29b9 100644
> --- a/arch/arm/plat-omap/mcbsp.c
> +++ b/arch/arm/plat-omap/mcbsp.c
> @@ -1161,25 +1161,31 @@ static DEVICE_ATTR(prop, 0644, prop##_show,
> prop##_store);
> THRESHOLD_PROP_BUILDER(max_tx_thres);
> THRESHOLD_PROP_BUILDER(max_rx_thres);
>
> +static const char *dma_op_modes[] = {
> + "element", "threshold", "frame",
> +};
> +
> static ssize_t dma_op_mode_show(struct device *dev,
> struct device_attribute *attr, char *buf)
> {
> struct omap_mcbsp *mcbsp = dev_get_drvdata(dev);
> - int dma_op_mode;
> + int dma_op_mode, i = 0;
> + ssize_t len = 0;
> + const char * const *s;
>
> spin_lock_irq(&mcbsp->lock);
> dma_op_mode = mcbsp->dma_op_mode;
> spin_unlock_irq(&mcbsp->lock);
>
> - return sprintf(buf, "current mode: %d\n"
> - "possible mode values are:\n"
> - "%d - %s\n"
> - "%d - %s\n"
> - "%d - %s\n",
> - dma_op_mode,
> - MCBSP_DMA_MODE_ELEMENT, "element mode",
> - MCBSP_DMA_MODE_THRESHOLD, "threshold mode",
> - MCBSP_DMA_MODE_FRAME, "frame mode");
> + for (s = &dma_op_modes[i]; i < ARRAY_SIZE(dma_op_modes); s++, i++) {
> + if (dma_op_mode == i)
> + len += sprintf(buf + len, "[%s] ", *s);
> + else
> + len += sprintf(buf + len, "%s ", *s);
> + }
> + len += sprintf(len+buf, "\n");
Just a tiny thing, add spaces before and after + operator:
+ len += sprintf(len + buf, "\n");
> +
> + return len;
> }
>
> static ssize_t dma_op_mode_store(struct device *dev,
> @@ -1187,26 +1193,26 @@ static ssize_t dma_op_mode_store(struct device *dev,
> const char *buf, size_t size)
> {
> struct omap_mcbsp *mcbsp = dev_get_drvdata(dev);
> - unsigned long val;
> - int status;
> + const char * const *s;
> + char *p;
> + int len, i = 0;
>
> - status = strict_strtoul(buf, 0, &val);
> - if (status)
> - return status;
> + p = memchr(buf, '\n', size);
> + len = p ? p - buf : size;
I guess here we have two better options, please use one of these:
* strstrip
* sysfs_streq
>
> - spin_lock_irq(&mcbsp->lock);
> + for (s = &dma_op_modes[i]; i < ARRAY_SIZE(dma_op_modes); s++, i++)
> + if (*s && len == strlen(*s) && !strncmp(buf, *s, len))
how about using strnicmp ??
> + break;
>
> + if (i == ARRAY_SIZE(dma_op_modes))
> + return -EINVAL;
> +
> + spin_lock_irq(&mcbsp->lock);
> if (!mcbsp->free) {
> size = -EBUSY;
> goto unlock;
> }
> -
> - if (val > MCBSP_DMA_MODE_FRAME || val < MCBSP_DMA_MODE_ELEMENT) {
> - size = -EINVAL;
> - goto unlock;
> - }
> -
> - mcbsp->dma_op_mode = val;
> + mcbsp->dma_op_mode = i;
>
> unlock:
> spin_unlock_irq(&mcbsp->lock);
> --
> 1.6.3.3
--
Eduardo Valentin
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
