[PATCH v2 2/9] staging: kpc2000: add separate show functions for kpc_uio_class device attributes and defined them as read-only.

Greg KH gregkh at linuxfoundation.org
Fri May 17 07:15:19 UTC 2019


On Thu, May 16, 2019 at 10:38:07PM +0100, Jeremy Sowden wrote:
> Define separate simple show functions for each attribute instead of
> having a one big one containing a chain of conditionals.
> 
> Replaced scnprintf calls with sprintf since all the outputs are short
> bounded strings or single integers.
> 
> All of the device attributes are read-only, so use DEVICE_ATTR_RO to
> define them.
> 
> Signed-off-by: Jeremy Sowden <jeremy at azazel.net>
> ---
>  drivers/staging/kpc2000/kpc2000/cell_probe.c | 136 ++++++++++++-------
>  1 file changed, 90 insertions(+), 46 deletions(-)
> 
> diff --git a/drivers/staging/kpc2000/kpc2000/cell_probe.c b/drivers/staging/kpc2000/kpc2000/cell_probe.c
> index 6a2ebdf20113..3798f8e2e165 100644
> --- a/drivers/staging/kpc2000/kpc2000/cell_probe.c
> +++ b/drivers/staging/kpc2000/kpc2000/cell_probe.c
> @@ -145,55 +145,100 @@ struct kpc_uio_device {
>      u16 core_num;
>  };
>  
> -static ssize_t  show_attr(struct device *dev, struct device_attribute *attr, char *buf)
> +static ssize_t offset_show(struct device *dev, struct device_attribute *attr,
> +			   char *buf)
>  {
> -    struct kpc_uio_device *kudev = dev_get_drvdata(dev);
> -
> -    #define ATTR_NAME_CMP(v)  (strcmp(v, attr->attr.name) == 0)
> -    if ATTR_NAME_CMP("offset"){
> -        return scnprintf(buf, PAGE_SIZE, "%u\n", kudev->cte.offset);
> -    } else if ATTR_NAME_CMP("size"){
> -        return scnprintf(buf, PAGE_SIZE, "%u\n", kudev->cte.length);
> -    } else if ATTR_NAME_CMP("type"){
> -        return scnprintf(buf, PAGE_SIZE, "%u\n", kudev->cte.type);
> -    }
> -    else if ATTR_NAME_CMP("s2c_dma"){
> -        if (kudev->cte.s2c_dma_present){
> -            return scnprintf(buf, PAGE_SIZE, "%u\n", kudev->cte.s2c_dma_channel_num);
> -        } else {
> -            return scnprintf(buf, PAGE_SIZE, "not present\n");
> -        }
> -    } else if ATTR_NAME_CMP("c2s_dma"){
> -        if (kudev->cte.c2s_dma_present){
> -            return scnprintf(buf, PAGE_SIZE, "%u\n", kudev->cte.c2s_dma_channel_num);
> -        } else {
> -            return scnprintf(buf, PAGE_SIZE, "not present\n");
> -        }
> -    }
> -    else if ATTR_NAME_CMP("irq_count"){
> -        return scnprintf(buf, PAGE_SIZE, "%u\n", kudev->cte.irq_count);
> -    } else if ATTR_NAME_CMP("irq_base_num"){
> -        return scnprintf(buf, PAGE_SIZE, "%u\n", kudev->cte.irq_base_num);
> -    } else if ATTR_NAME_CMP("core_num"){
> -        return scnprintf(buf, PAGE_SIZE, "%u\n", kudev->core_num);
> -    } else {
> -        return 0;
> -    }
> -    #undef ATTR_NAME_CMP
> +	struct kpc_uio_device *kudev = dev_get_drvdata(dev);
> +
> +	return sprintf(buf, "%u\n", kudev->cte.offset);
> +}
> +
> +static ssize_t size_show(struct device *dev, struct device_attribute *attr,
> +			 char *buf)
> +{
> +	struct kpc_uio_device *kudev = dev_get_drvdata(dev);
> +
> +	return sprintf(buf, "%u\n", kudev->cte.length);
>  }
>  
> +static ssize_t type_show(struct device *dev, struct device_attribute *attr,
> +			 char *buf)
> +{
> +	struct kpc_uio_device *kudev = dev_get_drvdata(dev);
> +
> +	return sprintf(buf, "%u\n", kudev->cte.type);
> +}
> +
> +static ssize_t s2c_dma_ch_show(struct device *dev,
> +			       struct device_attribute *attr, char *buf)
> +{
> +	return 0;
> +}
> +
> +static ssize_t c2s_dma_ch_show(struct device *dev,
> +			       struct device_attribute *attr, char *buf)
> +{
> +	return 0;
> +}
> +
> +static ssize_t s2c_dma_show(struct device *dev, struct device_attribute *attr,
> +			    char *buf)
> +{
> +	struct kpc_uio_device *kudev = dev_get_drvdata(dev);
>  
> -DEVICE_ATTR(offset,  0444, show_attr, NULL);
> -DEVICE_ATTR(size,    0444, show_attr, NULL);
> -DEVICE_ATTR(type,    0444, show_attr, NULL);
> -DEVICE_ATTR(s2c_dma_ch, 0444, show_attr, NULL);
> -DEVICE_ATTR(c2s_dma_ch, 0444, show_attr, NULL);
> -DEVICE_ATTR(s2c_dma, 0444, show_attr, NULL);
> -DEVICE_ATTR(c2s_dma, 0444, show_attr, NULL);
> -DEVICE_ATTR(irq_count, 0444, show_attr, NULL);
> -DEVICE_ATTR(irq_base_num, 0444, show_attr, NULL);
> -DEVICE_ATTR(core_num, 0444, show_attr, NULL);
> -struct attribute * kpc_uio_class_attrs[] = {
> +	if (!kudev->cte.s2c_dma_present)
> +		return sprintf(buf, "%s", "not present\n");
> +
> +	return sprintf(buf, "%u\n", kudev->cte.s2c_dma_channel_num);
> +}
> +
> +static ssize_t c2s_dma_show(struct device *dev, struct device_attribute *attr,
> +			    char *buf)
> +{
> +	struct kpc_uio_device *kudev = dev_get_drvdata(dev);
> +
> +	if (!kudev->cte.c2s_dma_present)
> +		return sprintf(buf, "%s", "not present\n");
> +
> +	return sprintf(buf, "%u\n", kudev->cte.c2s_dma_channel_num);
> +}
> +
> +static ssize_t irq_count_show(struct device *dev, struct device_attribute *attr,
> +			      char *buf)
> +{
> +	struct kpc_uio_device *kudev = dev_get_drvdata(dev);
> +
> +	return sprintf(buf, "%u\n", kudev->cte.irq_count);
> +}
> +
> +static ssize_t irq_base_num_show(struct device *dev,
> +				 struct device_attribute *attr, char *buf)
> +{
> +	struct kpc_uio_device *kudev = dev_get_drvdata(dev);
> +
> +	return sprintf(buf, "%u\n", kudev->cte.irq_base_num);
> +}
> +
> +static ssize_t core_num_show(struct device *dev, struct device_attribute *attr,
> +			     char *buf)
> +{
> +	struct kpc_uio_device *kudev = dev_get_drvdata(dev);
> +
> +	return sprintf(buf, "%u\n", kudev->core_num);
> +}
> +
> +DEVICE_ATTR_RO(offset);

Please put these DEVICE_ATTR_RO() lines right below the show function,
to make it a bit more obvious.  That's the normal "style" of these.

And make it static.

thanks,

greg k-h


More information about the devel mailing list