Re: [PATCH 02/11] mfd: flexcard: add flexcard core device

[Lee Jones <lee.jones@linaro.org>]

On Wed, 25 Mar 2015, Holger Dengler wrote:

> From: Benedikt Spranger <b.spranger@linutronix.de>
> 
> The Flexcard PCI BAR0 contain registers for configuration but also
> for informational purpose like error counter, statistical information
> and some timestamps. The read-only mmap of the misc device offers the
> userspace a fast access to these registers.

This code looks like it belongs in drivers/misc.

</review>

> Signed-off-by: Holger Dengler <dengler@linutronix.de>
> Signed-off-by: Benedikt Spranger <b.spranger@linutronix.de>
> cc: Samuel Ortiz <sameo@linux.intel.com>
> cc: Lee Jones <lee.jones@linaro.org>
> ---
>  drivers/mfd/flexcard/core.c  | 107 ++++++++++++++++++++++++++++++++++++++++++-
>  include/linux/mfd/flexcard.h |   6 +++
>  include/uapi/linux/Kbuild    |   1 +
>  3 files changed, 112 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mfd/flexcard/core.c b/drivers/mfd/flexcard/core.c
> index 99df3d5..1e7fb0f 100644
> --- a/drivers/mfd/flexcard/core.c
> +++ b/drivers/mfd/flexcard/core.c
> @@ -14,11 +14,16 @@
>  #include <linux/module.h>
>  #include <linux/init.h>
>  #include <linux/flexcard.h>
> +#include <linux/fs.h>
> +#include <linux/kref.h>
> +#include <linux/miscdevice.h>
>  #include <linux/pci.h>
>  #include <linux/platform_device.h>
>  #include <linux/mfd/core.h>
>  #include <linux/mfd/flexcard.h>
>  
> +static DEFINE_IDA(flexcard_ida);
> +
>  static const char drv_name[] = "flexcard";
>  
>  static int flexcard_tiny_can(struct flexcard_device *priv,
> @@ -104,23 +109,106 @@ static int flexcard_tiny_probe(struct flexcard_device *priv)
>  	return mfd_add_devices(&pdev->dev, 0, priv->cells, nr, NULL, 0, NULL);
>  }
>  
> +static int flexcard_misc_open(struct inode *inode, struct file *filp)
> +{
> +	struct flexcard_device *priv =
> +		container_of(filp->private_data, struct flexcard_device, dev);
> +	int ret;
> +
> +	filp->private_data = priv;
> +	ret = nonseekable_open(inode, filp);
> +	if (ret) {
> +		dev_err(&priv->pdev->dev, "unable to open file: %d\n", ret);
> +		return ret;
> +	}
> +
> +	kref_get(&priv->ref);
> +	return 0;
> +}
> +
> +static void flexcard_dev_release(struct kref *ref)
> +{
> +	struct flexcard_device *priv =
> +		container_of(ref, struct flexcard_device, ref);
> +
> +	kfree(priv);
> +	dev_info(&priv->pdev->dev, "misc device release");
> +}
> +
> +static int flexcard_misc_release(struct inode *inode, struct file *filp)
> +{
> +	struct flexcard_device *priv = filp->private_data;
> +
> +	filp->private_data = NULL;
> +	kref_put(&priv->ref, flexcard_dev_release);
> +
> +	return 0;
> +}
> +
> +static int flexcard_misc_mmap(struct file *filp, struct vm_area_struct *vma)
> +{
> +	struct flexcard_device *priv = filp->private_data;
> +	unsigned long offset, vsize, psize, addr;
> +
> +	if (vma->vm_flags & (VM_WRITE | VM_EXEC))
> +		return -EPERM;
> +
> +	offset = vma->vm_pgoff << PAGE_SHIFT;
> +	vsize = vma->vm_end - vma->vm_start;
> +	psize = pci_resource_len(priv->pdev, 0) - offset;
> +	addr = (pci_resource_start(priv->pdev, 0) + offset) >> PAGE_SHIFT;
> +
> +	if (vsize > psize) {
> +		dev_err(&priv->pdev->dev,
> +			"requested mmap mapping too large\n");
> +		return -EINVAL;
> +	}
> +
> +	vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
> +
> +	return io_remap_pfn_range(vma, vma->vm_start, addr, vsize,
> +				  vma->vm_page_prot);
> +}
> +
> +static const struct file_operations flexcard_misc_fops = {
> +	.owner		= THIS_MODULE,
> +	.open		= flexcard_misc_open,
> +	.release	= flexcard_misc_release,
> +	.mmap		= flexcard_misc_mmap,
> +	.llseek		= no_llseek,
> +};
> +
>  static int flexcard_probe(struct pci_dev *pdev,
>  			  const struct pci_device_id *id)
>  {
>  	struct flexcard_device *priv;
>  	int ret;
>  
> -	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> +	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
>  	if (!priv)
>  		return -ENOMEM;
>  
> +	ret = ida_simple_get(&flexcard_ida, 0, 0, GFP_KERNEL);
> +	if (ret < 0) {
> +		dev_err(&pdev->dev, "could not get new Flexcard id:%d\n", ret);
> +		goto out_free;
> +	}
> +
> +	priv->cardnr = ret;
>  	pci_set_drvdata(pdev, priv);
>  	priv->pdev = pdev;
> +	kref_init(&priv->ref);
> +
> +	snprintf(priv->name, sizeof(priv->name), "flexcard%d", priv->cardnr);
> +	priv->dev.minor = MISC_DYNAMIC_MINOR;
> +	priv->dev.name = priv->name;
> +	priv->dev.fops = &flexcard_misc_fops;
> +	priv->dev.parent = &pdev->dev;
>  
>  	ret = pci_enable_device(pdev);
>  	if (ret) {
>  		dev_err(&pdev->dev, "unable to enable device: %d\n", ret);
> -		return ret;
> +		goto out_put;
>  	}
>  
>  	pci_set_master(pdev);
> @@ -143,6 +231,12 @@ static int flexcard_probe(struct pci_dev *pdev,
>  		goto out_unmap;
>  	}
>  
> +	ret = misc_register(&priv->dev);
> +	if (ret) {
> +		dev_err(&pdev->dev, "unable to register miscdevice: %d\n", ret);
> +		goto out_remove;
> +	}
> +
>  	dev_info(&pdev->dev, "HW %02x.%02x.%02x FW %02x.%02x.%02x\n",
>  		 priv->conf->fc_hw_ver.maj, priv->conf->fc_hw_ver.min,
>  		 priv->conf->fc_hw_ver.dev, priv->conf->fc_fw_ver.maj,
> @@ -150,12 +244,18 @@ static int flexcard_probe(struct pci_dev *pdev,
>  
>  	return 0;
>  
> +out_remove:
> +	mfd_remove_devices(&pdev->dev);
>  out_unmap:
>  	iounmap(priv->conf);
>  out_release:
>  	pci_release_regions(pdev);
>  out_disable:
>  	pci_disable_device(pdev);
> +out_put:
> +	ida_simple_remove(&flexcard_ida, priv->cardnr);
> +out_free:
> +	kref_put(&priv->ref, flexcard_dev_release);
>  
>  	return ret;
>  }
> @@ -164,10 +264,13 @@ static void flexcard_remove(struct pci_dev *pdev)
>  {
>  	struct flexcard_device *priv = pci_get_drvdata(pdev);
>  
> +	misc_deregister(&priv->dev);
>  	mfd_remove_devices(&pdev->dev);
>  	iounmap(priv->conf);
>  	pci_release_regions(pdev);
>  	pci_disable_device(pdev);
> +	ida_simple_remove(&flexcard_ida, priv->cardnr);
> +	kref_put(&priv->ref, flexcard_dev_release);
>  }
>  
>  #define PCI_VENDOR_ID_EBEL	0x1974
> diff --git a/include/linux/mfd/flexcard.h b/include/linux/mfd/flexcard.h
> index 20d0f40..84e155c 100644
> --- a/include/linux/mfd/flexcard.h
> +++ b/include/linux/mfd/flexcard.h
> @@ -19,11 +19,17 @@
>  #define FLEXCARD_FR_OFFSET	0x4000
>  #define FLEXCARD_FR_SIZE	0x2000
>  
> +#define FLEXCARD_MAX_NAME	16
> +
>  struct flexcard_device {
>  	struct pci_dev *pdev;
>  	struct fc_conf_bar __iomem *conf;
>  	struct mfd_cell *cells;
>  	struct resource *res;
> +	struct miscdevice dev;
> +	struct kref ref;
> +	int cardnr;
> +	char name[FLEXCARD_MAX_NAME];
>  };
>  
>  enum flexcard_cell_id {
> diff --git a/include/uapi/linux/Kbuild b/include/uapi/linux/Kbuild
> index 68ceb97..d5c5002 100644
> --- a/include/uapi/linux/Kbuild
> +++ b/include/uapi/linux/Kbuild
> @@ -127,6 +127,7 @@ header-y += filter.h
>  header-y += firewire-cdev.h
>  header-y += firewire-constants.h
>  header-y += flat.h
> +header-y += flexcard.h
>  header-y += fou.h
>  header-y += fs.h
>  header-y += fsl_hypervisor.h

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog