Re: [PATCH] uio/pdrv_genirq: Add OF support

[Wolfram Sang <w.sang@pengutronix.de>]

[-- Attachment #1: Type: text/plain, Size: 4741 bytes --]

On Thu, Mar 31, 2011 at 02:30:00PM +0200, Michal Simek wrote:
> Support OF support. "generic-uio" compatible property is used.

And exactly this was the issue last time (when I tried). This is a
generic property, which is linux-specific and not describing HW. The
agreement back then was to we probably need to add compatible-entries at
runtime (something like new_id for USB). So the uio-of-driver could be
matched against any device. Otherwise, we would collect a lot of
potential entries like "vendor,special-card1". Although I wonder
meanwhile if it is really going to be that bad; we don't have so much
UIO-driver in tree as well. Maybe worth a try?

> Signed-off-by: Michal Simek <monstr@monstr.eu>
> ---
>  drivers/uio/uio_pdrv_genirq.c |   60 ++++++++++++++++++++++++++++++++++++++--
>  1 files changed, 57 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/uio/uio_pdrv_genirq.c b/drivers/uio/uio_pdrv_genirq.c
> index 7174d51..9e89806 100644
> --- a/drivers/uio/uio_pdrv_genirq.c
> +++ b/drivers/uio/uio_pdrv_genirq.c
> @@ -23,6 +23,10 @@
>  #include <linux/pm_runtime.h>
>  #include <linux/slab.h>
>  
> +#include <linux/of.h>
> +#include <linux/of_platform.h>
> +#include <linux/of_address.h>
> +
>  #define DRIVER_NAME "uio_pdrv_genirq"
>  
>  struct uio_pdrv_genirq_platdata {
> @@ -92,11 +96,44 @@ static int uio_pdrv_genirq_irqcontrol(struct uio_info *dev_info, s32 irq_on)
>  static int uio_pdrv_genirq_probe(struct platform_device *pdev)
>  {
>  	struct uio_info *uioinfo = pdev->dev.platform_data;
> -	struct uio_pdrv_genirq_platdata *priv;
> +	struct uio_pdrv_genirq_platdata *priv = NULL;

unrelated?

>  	struct uio_mem *uiomem;
>  	int ret = -EINVAL;
>  	int i;
>  
> +	if (!uioinfo) {
> +		struct resource r_irq; /* Interrupt resources */
> +		int rc = 0;
> +
> +		rc = of_address_to_resource(pdev->dev.of_node, 0,
> +							&pdev->resource[0]);
> +		if (rc) {
> +			dev_err(&pdev->dev, "invalid address\n");
> +			goto bad2;
> +		}
> +		pdev->num_resources = 1;
> +
> +		/* alloc uioinfo for one device */
> +		uioinfo = kzalloc(sizeof(*uioinfo), GFP_KERNEL);
> +		if (!uioinfo) {
> +			ret = -ENOMEM;
> +			dev_err(&pdev->dev, "unable to kmalloc\n");
> +			goto bad2;
> +		}
> +		uioinfo->name = pdev->dev.of_node->name;
> +		/* Use version for storing full IP name for identification */
> +		uioinfo->version = pdev->dev.of_node->full_name;

I don't think this is apropriate, but will leave that to Hans.

> +		/* Get IRQ for the device */
> +		rc = of_irq_to_resource(pdev->dev.of_node, 0, &r_irq);
> +		if (rc == NO_IRQ)
> +			dev_err(&pdev->dev, "no IRQ found\n");

No error, I think. Sometimes just mmaping the registers is enough.

> +		else {
> +			uioinfo->irq = r_irq.start;
> +			dev_info(&pdev->dev, "irq %d\n", (u32)uioinfo->irq);
> +		}
> +	}
> +
>  	if (!uioinfo || !uioinfo->name || !uioinfo->version) {
>  		dev_err(&pdev->dev, "missing platform_data\n");
>  		goto bad0;
> @@ -176,10 +213,15 @@ static int uio_pdrv_genirq_probe(struct platform_device *pdev)
>  
>  	platform_set_drvdata(pdev, priv);
>  	return 0;
> - bad1:
> +
> +bad1:

The spaces before labels are intentional, better keep them.

>  	kfree(priv);
>  	pm_runtime_disable(&pdev->dev);
> - bad0:
> +bad0:
> +	/* kfree uioinfo for CONFIG_OF */
> +	if (!pdev->dev.platform_data)
> +		kfree(uioinfo);
> +bad2:
>  	return ret;
>  }
>  
> @@ -215,6 +257,17 @@ static const struct dev_pm_ops uio_pdrv_genirq_dev_pm_ops = {
>  	.runtime_resume = uio_pdrv_genirq_runtime_nop,
>  };
>  
> +#ifdef CONFIG_OF
> +/* Match table for of_platform binding */
> +static const struct of_device_id __devinitconst uio_of_genirq_match[] = {
> +	{ .compatible = "generic-uio", },
> +	{ /* end of list */ },
> +};
> +MODULE_DEVICE_TABLE(of, uio_of_genirq_match);
> +#else
> +# define uio_of_genirq_match NULL
> +#endif
> +
>  static struct platform_driver uio_pdrv_genirq = {
>  	.probe = uio_pdrv_genirq_probe,
>  	.remove = uio_pdrv_genirq_remove,
> @@ -222,6 +275,7 @@ static struct platform_driver uio_pdrv_genirq = {
>  		.name = DRIVER_NAME,
>  		.owner = THIS_MODULE,
>  		.pm = &uio_pdrv_genirq_dev_pm_ops,
> +		.of_match_table = uio_of_genirq_match,
>  	},
>  };
>  
> -- 
> 1.5.5.6
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]