2013-05-29 11:28:10

by Michal Simek

[permalink] [raw]
Subject: Re: [RFC PATCH 2/2] uio: Add two platform uio drivers to one

Hi Hans,

any comment on this?

Thanks,
Michal

On 05/23/2013 04:01 PM, Michal Simek wrote:
> - Remove Userspace I/O platform driver without IRQ support
> but add this functionality to genirq driver
> - Remove code duplication from OF binding
>
> Signed-off-by: Michal Simek <[email protected]>
> ---
> The main reason for this change is to have one
> compatibility string for UIO with and without IRQ.
>
> ---
> drivers/uio/Kconfig | 7 ---
> drivers/uio/Makefile | 1 -
> drivers/uio/uio_pdrv.c | 113 ------------------------------------------
> drivers/uio/uio_pdrv_genirq.c | 30 ++++-------
> 4 files changed, 9 insertions(+), 142 deletions(-)
> delete mode 100644 drivers/uio/uio_pdrv.c
>
> diff --git a/drivers/uio/Kconfig b/drivers/uio/Kconfig
> index e92eeaf..2ff4c90 100644
> --- a/drivers/uio/Kconfig
> +++ b/drivers/uio/Kconfig
> @@ -23,13 +23,6 @@ config UIO_CIF
> To compile this driver as a module, choose M here: the module
> will be called uio_cif.
>
> -config UIO_PDRV
> - tristate "Userspace I/O platform driver"
> - help
> - Generic platform driver for Userspace I/O devices.
> -
> - If you don't know what to do here, say N.
> -
> config UIO_PDRV_GENIRQ
> tristate "Userspace I/O platform driver with generic IRQ handling"
> help
> diff --git a/drivers/uio/Makefile b/drivers/uio/Makefile
> index b354c53..ea015a2 100644
> --- a/drivers/uio/Makefile
> +++ b/drivers/uio/Makefile
> @@ -1,6 +1,5 @@
> obj-$(CONFIG_UIO) += uio.o
> obj-$(CONFIG_UIO_CIF) += uio_cif.o
> -obj-$(CONFIG_UIO_PDRV) += uio_pdrv.o
> obj-$(CONFIG_UIO_PDRV_GENIRQ) += uio_pdrv_genirq.o
> obj-$(CONFIG_UIO_DMEM_GENIRQ) += uio_dmem_genirq.o
> obj-$(CONFIG_UIO_AEC) += uio_aec.o
> diff --git a/drivers/uio/uio_pdrv.c b/drivers/uio/uio_pdrv.c
> deleted file mode 100644
> index 39be9e0..0000000
> --- a/drivers/uio/uio_pdrv.c
> +++ /dev/null
> @@ -1,113 +0,0 @@
> -/*
> - * drivers/uio/uio_pdrv.c
> - *
> - * Copyright (C) 2008 by Digi International Inc.
> - * All rights reserved.
> - *
> - * This program is free software; you can redistribute it and/or modify it
> - * under the terms of the GNU General Public License version 2 as published by
> - * the Free Software Foundation.
> - */
> -#include <linux/platform_device.h>
> -#include <linux/uio_driver.h>
> -#include <linux/stringify.h>
> -#include <linux/module.h>
> -#include <linux/slab.h>
> -
> -#define DRIVER_NAME "uio_pdrv"
> -
> -struct uio_platdata {
> - struct uio_info *uioinfo;
> -};
> -
> -static int uio_pdrv_probe(struct platform_device *pdev)
> -{
> - struct uio_info *uioinfo = pdev->dev.platform_data;
> - struct uio_platdata *pdata;
> - struct uio_mem *uiomem;
> - int ret = -ENODEV;
> - int i;
> -
> - if (!uioinfo || !uioinfo->name || !uioinfo->version) {
> - dev_dbg(&pdev->dev, "%s: err_uioinfo\n", __func__);
> - goto err_uioinfo;
> - }
> -
> - pdata = kzalloc(sizeof(*pdata), GFP_KERNEL);
> - if (!pdata) {
> - ret = -ENOMEM;
> - dev_dbg(&pdev->dev, "%s: err_alloc_pdata\n", __func__);
> - goto err_alloc_pdata;
> - }
> -
> - pdata->uioinfo = uioinfo;
> -
> - uiomem = &uioinfo->mem[0];
> -
> - for (i = 0; i < pdev->num_resources; ++i) {
> - struct resource *r = &pdev->resource[i];
> -
> - if (r->flags != IORESOURCE_MEM)
> - continue;
> -
> - if (uiomem >= &uioinfo->mem[MAX_UIO_MAPS]) {
> - dev_warn(&pdev->dev, "device has more than "
> - __stringify(MAX_UIO_MAPS)
> - " I/O memory resources.\n");
> - break;
> - }
> -
> - uiomem->memtype = UIO_MEM_PHYS;
> - uiomem->addr = r->start;
> - uiomem->size = resource_size(r);
> - uiomem->name = r->name;
> - ++uiomem;
> - }
> -
> - while (uiomem < &uioinfo->mem[MAX_UIO_MAPS]) {
> - uiomem->size = 0;
> - ++uiomem;
> - }
> -
> - pdata->uioinfo->priv = pdata;
> -
> - ret = uio_register_device(&pdev->dev, pdata->uioinfo);
> -
> - if (ret) {
> - kfree(pdata);
> -err_alloc_pdata:
> -err_uioinfo:
> - return ret;
> - }
> -
> - platform_set_drvdata(pdev, pdata);
> -
> - return 0;
> -}
> -
> -static int uio_pdrv_remove(struct platform_device *pdev)
> -{
> - struct uio_platdata *pdata = platform_get_drvdata(pdev);
> -
> - uio_unregister_device(pdata->uioinfo);
> -
> - kfree(pdata);
> -
> - return 0;
> -}
> -
> -static struct platform_driver uio_pdrv = {
> - .probe = uio_pdrv_probe,
> - .remove = uio_pdrv_remove,
> - .driver = {
> - .name = DRIVER_NAME,
> - .owner = THIS_MODULE,
> - },
> -};
> -
> -module_platform_driver(uio_pdrv);
> -
> -MODULE_AUTHOR("Uwe Kleine-Koenig");
> -MODULE_DESCRIPTION("Userspace I/O platform driver");
> -MODULE_LICENSE("GPL v2");
> -MODULE_ALIAS("platform:" DRIVER_NAME);
> diff --git a/drivers/uio/uio_pdrv_genirq.c b/drivers/uio/uio_pdrv_genirq.c
> index d7ba355..ccd2750 100644
> --- a/drivers/uio/uio_pdrv_genirq.c
> +++ b/drivers/uio/uio_pdrv_genirq.c
> @@ -103,8 +103,6 @@ static int uio_pdrv_genirq_probe(struct platform_device *pdev)
> int i;
>
> if (pdev->dev.of_node) {
> - int irq;
> -
> /* alloc uioinfo for one device */
> uioinfo = kzalloc(sizeof(*uioinfo), GFP_KERNEL);
> if (!uioinfo) {
> @@ -114,13 +112,6 @@ static int uio_pdrv_genirq_probe(struct platform_device *pdev)
> }
> uioinfo->name = pdev->dev.of_node->name;
> uioinfo->version = "devicetree";
> -
> - /* Multiple IRQs are not supported */
> - irq = platform_get_irq(pdev, 0);
> - if (irq == -ENXIO)
> - uioinfo->irq = UIO_IRQ_NONE;
> - else
> - uioinfo->irq = irq;
> }
>
> if (!uioinfo || !uioinfo->name || !uioinfo->version) {
> @@ -146,14 +137,6 @@ static int uio_pdrv_genirq_probe(struct platform_device *pdev)
> priv->flags = 0; /* interrupt is enabled to begin with */
> priv->pdev = pdev;
>
> - if (!uioinfo->irq) {
> - ret = platform_get_irq(pdev, 0);
> - if (ret < 0) {
> - dev_err(&pdev->dev, "failed to get IRQ\n");
> - goto bad0;
> - }
> - uioinfo->irq = ret;
> - }
> uiomem = &uioinfo->mem[0];
>
> for (i = 0; i < pdev->num_resources; ++i) {
> @@ -190,10 +173,15 @@ static int uio_pdrv_genirq_probe(struct platform_device *pdev)
> * Interrupt sharing is not supported.
> */
>
> - uioinfo->handler = uio_pdrv_genirq_handler;
> - uioinfo->irqcontrol = uio_pdrv_genirq_irqcontrol;
> - uioinfo->open = uio_pdrv_genirq_open;
> - uioinfo->release = uio_pdrv_genirq_release;
> + ret = platform_get_irq(pdev, 0);
> + if (ret >= 0) {
> + uioinfo->irq = ret;
> + uioinfo->handler = uio_pdrv_genirq_handler;
> + uioinfo->irqcontrol = uio_pdrv_genirq_irqcontrol;
> + uioinfo->open = uio_pdrv_genirq_open;
> + uioinfo->release = uio_pdrv_genirq_release;
> + }
> +
> uioinfo->priv = priv;
>
> /* Enable Runtime PM for this device:
> --
> 1.8.2.3
>


--
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: http://www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform



Attachments:
signature.asc (263.00 B)
OpenPGP digital signature

2013-06-03 12:34:17

by Michal Simek

[permalink] [raw]
Subject: Re: [RFC PATCH 2/2] uio: Add two platform uio drivers to one

Hi Hans and Greg,

can you please comment this?

Thanks,
Michal


On 05/29/2013 01:28 PM, Michal Simek wrote:
> Hi Hans,
>
> any comment on this?
>
> Thanks,
> Michal
>
> On 05/23/2013 04:01 PM, Michal Simek wrote:
>> - Remove Userspace I/O platform driver without IRQ support
>> but add this functionality to genirq driver
>> - Remove code duplication from OF binding
>>
>> Signed-off-by: Michal Simek <[email protected]>
>> ---
>> The main reason for this change is to have one
>> compatibility string for UIO with and without IRQ.
>>
>> ---
>> drivers/uio/Kconfig | 7 ---
>> drivers/uio/Makefile | 1 -
>> drivers/uio/uio_pdrv.c | 113 ------------------------------------------
>> drivers/uio/uio_pdrv_genirq.c | 30 ++++-------
>> 4 files changed, 9 insertions(+), 142 deletions(-)
>> delete mode 100644 drivers/uio/uio_pdrv.c
>>
>> diff --git a/drivers/uio/Kconfig b/drivers/uio/Kconfig
>> index e92eeaf..2ff4c90 100644
>> --- a/drivers/uio/Kconfig
>> +++ b/drivers/uio/Kconfig
>> @@ -23,13 +23,6 @@ config UIO_CIF
>> To compile this driver as a module, choose M here: the module
>> will be called uio_cif.
>>
>> -config UIO_PDRV
>> - tristate "Userspace I/O platform driver"
>> - help
>> - Generic platform driver for Userspace I/O devices.
>> -
>> - If you don't know what to do here, say N.
>> -
>> config UIO_PDRV_GENIRQ
>> tristate "Userspace I/O platform driver with generic IRQ handling"
>> help
>> diff --git a/drivers/uio/Makefile b/drivers/uio/Makefile
>> index b354c53..ea015a2 100644
>> --- a/drivers/uio/Makefile
>> +++ b/drivers/uio/Makefile
>> @@ -1,6 +1,5 @@
>> obj-$(CONFIG_UIO) += uio.o
>> obj-$(CONFIG_UIO_CIF) += uio_cif.o
>> -obj-$(CONFIG_UIO_PDRV) += uio_pdrv.o
>> obj-$(CONFIG_UIO_PDRV_GENIRQ) += uio_pdrv_genirq.o
>> obj-$(CONFIG_UIO_DMEM_GENIRQ) += uio_dmem_genirq.o
>> obj-$(CONFIG_UIO_AEC) += uio_aec.o
>> diff --git a/drivers/uio/uio_pdrv.c b/drivers/uio/uio_pdrv.c
>> deleted file mode 100644
>> index 39be9e0..0000000
>> --- a/drivers/uio/uio_pdrv.c
>> +++ /dev/null
>> @@ -1,113 +0,0 @@
>> -/*
>> - * drivers/uio/uio_pdrv.c
>> - *
>> - * Copyright (C) 2008 by Digi International Inc.
>> - * All rights reserved.
>> - *
>> - * This program is free software; you can redistribute it and/or modify it
>> - * under the terms of the GNU General Public License version 2 as published by
>> - * the Free Software Foundation.
>> - */
>> -#include <linux/platform_device.h>
>> -#include <linux/uio_driver.h>
>> -#include <linux/stringify.h>
>> -#include <linux/module.h>
>> -#include <linux/slab.h>
>> -
>> -#define DRIVER_NAME "uio_pdrv"
>> -
>> -struct uio_platdata {
>> - struct uio_info *uioinfo;
>> -};
>> -
>> -static int uio_pdrv_probe(struct platform_device *pdev)
>> -{
>> - struct uio_info *uioinfo = pdev->dev.platform_data;
>> - struct uio_platdata *pdata;
>> - struct uio_mem *uiomem;
>> - int ret = -ENODEV;
>> - int i;
>> -
>> - if (!uioinfo || !uioinfo->name || !uioinfo->version) {
>> - dev_dbg(&pdev->dev, "%s: err_uioinfo\n", __func__);
>> - goto err_uioinfo;
>> - }
>> -
>> - pdata = kzalloc(sizeof(*pdata), GFP_KERNEL);
>> - if (!pdata) {
>> - ret = -ENOMEM;
>> - dev_dbg(&pdev->dev, "%s: err_alloc_pdata\n", __func__);
>> - goto err_alloc_pdata;
>> - }
>> -
>> - pdata->uioinfo = uioinfo;
>> -
>> - uiomem = &uioinfo->mem[0];
>> -
>> - for (i = 0; i < pdev->num_resources; ++i) {
>> - struct resource *r = &pdev->resource[i];
>> -
>> - if (r->flags != IORESOURCE_MEM)
>> - continue;
>> -
>> - if (uiomem >= &uioinfo->mem[MAX_UIO_MAPS]) {
>> - dev_warn(&pdev->dev, "device has more than "
>> - __stringify(MAX_UIO_MAPS)
>> - " I/O memory resources.\n");
>> - break;
>> - }
>> -
>> - uiomem->memtype = UIO_MEM_PHYS;
>> - uiomem->addr = r->start;
>> - uiomem->size = resource_size(r);
>> - uiomem->name = r->name;
>> - ++uiomem;
>> - }
>> -
>> - while (uiomem < &uioinfo->mem[MAX_UIO_MAPS]) {
>> - uiomem->size = 0;
>> - ++uiomem;
>> - }
>> -
>> - pdata->uioinfo->priv = pdata;
>> -
>> - ret = uio_register_device(&pdev->dev, pdata->uioinfo);
>> -
>> - if (ret) {
>> - kfree(pdata);
>> -err_alloc_pdata:
>> -err_uioinfo:
>> - return ret;
>> - }
>> -
>> - platform_set_drvdata(pdev, pdata);
>> -
>> - return 0;
>> -}
>> -
>> -static int uio_pdrv_remove(struct platform_device *pdev)
>> -{
>> - struct uio_platdata *pdata = platform_get_drvdata(pdev);
>> -
>> - uio_unregister_device(pdata->uioinfo);
>> -
>> - kfree(pdata);
>> -
>> - return 0;
>> -}
>> -
>> -static struct platform_driver uio_pdrv = {
>> - .probe = uio_pdrv_probe,
>> - .remove = uio_pdrv_remove,
>> - .driver = {
>> - .name = DRIVER_NAME,
>> - .owner = THIS_MODULE,
>> - },
>> -};
>> -
>> -module_platform_driver(uio_pdrv);
>> -
>> -MODULE_AUTHOR("Uwe Kleine-Koenig");
>> -MODULE_DESCRIPTION("Userspace I/O platform driver");
>> -MODULE_LICENSE("GPL v2");
>> -MODULE_ALIAS("platform:" DRIVER_NAME);
>> diff --git a/drivers/uio/uio_pdrv_genirq.c b/drivers/uio/uio_pdrv_genirq.c
>> index d7ba355..ccd2750 100644
>> --- a/drivers/uio/uio_pdrv_genirq.c
>> +++ b/drivers/uio/uio_pdrv_genirq.c
>> @@ -103,8 +103,6 @@ static int uio_pdrv_genirq_probe(struct platform_device *pdev)
>> int i;
>>
>> if (pdev->dev.of_node) {
>> - int irq;
>> -
>> /* alloc uioinfo for one device */
>> uioinfo = kzalloc(sizeof(*uioinfo), GFP_KERNEL);
>> if (!uioinfo) {
>> @@ -114,13 +112,6 @@ static int uio_pdrv_genirq_probe(struct platform_device *pdev)
>> }
>> uioinfo->name = pdev->dev.of_node->name;
>> uioinfo->version = "devicetree";
>> -
>> - /* Multiple IRQs are not supported */
>> - irq = platform_get_irq(pdev, 0);
>> - if (irq == -ENXIO)
>> - uioinfo->irq = UIO_IRQ_NONE;
>> - else
>> - uioinfo->irq = irq;
>> }
>>
>> if (!uioinfo || !uioinfo->name || !uioinfo->version) {
>> @@ -146,14 +137,6 @@ static int uio_pdrv_genirq_probe(struct platform_device *pdev)
>> priv->flags = 0; /* interrupt is enabled to begin with */
>> priv->pdev = pdev;
>>
>> - if (!uioinfo->irq) {
>> - ret = platform_get_irq(pdev, 0);
>> - if (ret < 0) {
>> - dev_err(&pdev->dev, "failed to get IRQ\n");
>> - goto bad0;
>> - }
>> - uioinfo->irq = ret;
>> - }
>> uiomem = &uioinfo->mem[0];
>>
>> for (i = 0; i < pdev->num_resources; ++i) {
>> @@ -190,10 +173,15 @@ static int uio_pdrv_genirq_probe(struct platform_device *pdev)
>> * Interrupt sharing is not supported.
>> */
>>
>> - uioinfo->handler = uio_pdrv_genirq_handler;
>> - uioinfo->irqcontrol = uio_pdrv_genirq_irqcontrol;
>> - uioinfo->open = uio_pdrv_genirq_open;
>> - uioinfo->release = uio_pdrv_genirq_release;
>> + ret = platform_get_irq(pdev, 0);
>> + if (ret >= 0) {
>> + uioinfo->irq = ret;
>> + uioinfo->handler = uio_pdrv_genirq_handler;
>> + uioinfo->irqcontrol = uio_pdrv_genirq_irqcontrol;
>> + uioinfo->open = uio_pdrv_genirq_open;
>> + uioinfo->release = uio_pdrv_genirq_release;
>> + }
>> +
>> uioinfo->priv = priv;
>>
>> /* Enable Runtime PM for this device:
>> --
>> 1.8.2.3
>>
>
>


--
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: http://www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform



Attachments:
signature.asc (263.00 B)
OpenPGP digital signature

2013-06-03 21:05:17

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [RFC PATCH 2/2] uio: Add two platform uio drivers to one

On Mon, Jun 03, 2013 at 02:34:11PM +0200, Michal Simek wrote:
> Hi Hans and Greg,
>
> can you please comment this?

I'll let Hans handle this, as he's the UIO maintainer now.

thanks,

greg -h

2013-06-10 08:59:59

by Michal Simek

[permalink] [raw]
Subject: Re: [RFC PATCH 2/2] uio: Add two platform uio drivers to one

Hi Hans,

can you please look at this patch?

Thanks,
Michal


On 05/23/2013 04:01 PM, Michal Simek wrote:
> - Remove Userspace I/O platform driver without IRQ support
> but add this functionality to genirq driver
> - Remove code duplication from OF binding
>
> Signed-off-by: Michal Simek <[email protected]>
> ---
> The main reason for this change is to have one
> compatibility string for UIO with and without IRQ.
>
> ---
> drivers/uio/Kconfig | 7 ---
> drivers/uio/Makefile | 1 -
> drivers/uio/uio_pdrv.c | 113 ------------------------------------------
> drivers/uio/uio_pdrv_genirq.c | 30 ++++-------
> 4 files changed, 9 insertions(+), 142 deletions(-)
> delete mode 100644 drivers/uio/uio_pdrv.c
>
> diff --git a/drivers/uio/Kconfig b/drivers/uio/Kconfig
> index e92eeaf..2ff4c90 100644
> --- a/drivers/uio/Kconfig
> +++ b/drivers/uio/Kconfig
> @@ -23,13 +23,6 @@ config UIO_CIF
> To compile this driver as a module, choose M here: the module
> will be called uio_cif.
>
> -config UIO_PDRV
> - tristate "Userspace I/O platform driver"
> - help
> - Generic platform driver for Userspace I/O devices.
> -
> - If you don't know what to do here, say N.
> -
> config UIO_PDRV_GENIRQ
> tristate "Userspace I/O platform driver with generic IRQ handling"
> help
> diff --git a/drivers/uio/Makefile b/drivers/uio/Makefile
> index b354c53..ea015a2 100644
> --- a/drivers/uio/Makefile
> +++ b/drivers/uio/Makefile
> @@ -1,6 +1,5 @@
> obj-$(CONFIG_UIO) += uio.o
> obj-$(CONFIG_UIO_CIF) += uio_cif.o
> -obj-$(CONFIG_UIO_PDRV) += uio_pdrv.o
> obj-$(CONFIG_UIO_PDRV_GENIRQ) += uio_pdrv_genirq.o
> obj-$(CONFIG_UIO_DMEM_GENIRQ) += uio_dmem_genirq.o
> obj-$(CONFIG_UIO_AEC) += uio_aec.o
> diff --git a/drivers/uio/uio_pdrv.c b/drivers/uio/uio_pdrv.c
> deleted file mode 100644
> index 39be9e0..0000000
> --- a/drivers/uio/uio_pdrv.c
> +++ /dev/null
> @@ -1,113 +0,0 @@
> -/*
> - * drivers/uio/uio_pdrv.c
> - *
> - * Copyright (C) 2008 by Digi International Inc.
> - * All rights reserved.
> - *
> - * This program is free software; you can redistribute it and/or modify it
> - * under the terms of the GNU General Public License version 2 as published by
> - * the Free Software Foundation.
> - */
> -#include <linux/platform_device.h>
> -#include <linux/uio_driver.h>
> -#include <linux/stringify.h>
> -#include <linux/module.h>
> -#include <linux/slab.h>
> -
> -#define DRIVER_NAME "uio_pdrv"
> -
> -struct uio_platdata {
> - struct uio_info *uioinfo;
> -};
> -
> -static int uio_pdrv_probe(struct platform_device *pdev)
> -{
> - struct uio_info *uioinfo = pdev->dev.platform_data;
> - struct uio_platdata *pdata;
> - struct uio_mem *uiomem;
> - int ret = -ENODEV;
> - int i;
> -
> - if (!uioinfo || !uioinfo->name || !uioinfo->version) {
> - dev_dbg(&pdev->dev, "%s: err_uioinfo\n", __func__);
> - goto err_uioinfo;
> - }
> -
> - pdata = kzalloc(sizeof(*pdata), GFP_KERNEL);
> - if (!pdata) {
> - ret = -ENOMEM;
> - dev_dbg(&pdev->dev, "%s: err_alloc_pdata\n", __func__);
> - goto err_alloc_pdata;
> - }
> -
> - pdata->uioinfo = uioinfo;
> -
> - uiomem = &uioinfo->mem[0];
> -
> - for (i = 0; i < pdev->num_resources; ++i) {
> - struct resource *r = &pdev->resource[i];
> -
> - if (r->flags != IORESOURCE_MEM)
> - continue;
> -
> - if (uiomem >= &uioinfo->mem[MAX_UIO_MAPS]) {
> - dev_warn(&pdev->dev, "device has more than "
> - __stringify(MAX_UIO_MAPS)
> - " I/O memory resources.\n");
> - break;
> - }
> -
> - uiomem->memtype = UIO_MEM_PHYS;
> - uiomem->addr = r->start;
> - uiomem->size = resource_size(r);
> - uiomem->name = r->name;
> - ++uiomem;
> - }
> -
> - while (uiomem < &uioinfo->mem[MAX_UIO_MAPS]) {
> - uiomem->size = 0;
> - ++uiomem;
> - }
> -
> - pdata->uioinfo->priv = pdata;
> -
> - ret = uio_register_device(&pdev->dev, pdata->uioinfo);
> -
> - if (ret) {
> - kfree(pdata);
> -err_alloc_pdata:
> -err_uioinfo:
> - return ret;
> - }
> -
> - platform_set_drvdata(pdev, pdata);
> -
> - return 0;
> -}
> -
> -static int uio_pdrv_remove(struct platform_device *pdev)
> -{
> - struct uio_platdata *pdata = platform_get_drvdata(pdev);
> -
> - uio_unregister_device(pdata->uioinfo);
> -
> - kfree(pdata);
> -
> - return 0;
> -}
> -
> -static struct platform_driver uio_pdrv = {
> - .probe = uio_pdrv_probe,
> - .remove = uio_pdrv_remove,
> - .driver = {
> - .name = DRIVER_NAME,
> - .owner = THIS_MODULE,
> - },
> -};
> -
> -module_platform_driver(uio_pdrv);
> -
> -MODULE_AUTHOR("Uwe Kleine-Koenig");
> -MODULE_DESCRIPTION("Userspace I/O platform driver");
> -MODULE_LICENSE("GPL v2");
> -MODULE_ALIAS("platform:" DRIVER_NAME);
> diff --git a/drivers/uio/uio_pdrv_genirq.c b/drivers/uio/uio_pdrv_genirq.c
> index d7ba355..ccd2750 100644
> --- a/drivers/uio/uio_pdrv_genirq.c
> +++ b/drivers/uio/uio_pdrv_genirq.c
> @@ -103,8 +103,6 @@ static int uio_pdrv_genirq_probe(struct platform_device *pdev)
> int i;
>
> if (pdev->dev.of_node) {
> - int irq;
> -
> /* alloc uioinfo for one device */
> uioinfo = kzalloc(sizeof(*uioinfo), GFP_KERNEL);
> if (!uioinfo) {
> @@ -114,13 +112,6 @@ static int uio_pdrv_genirq_probe(struct platform_device *pdev)
> }
> uioinfo->name = pdev->dev.of_node->name;
> uioinfo->version = "devicetree";
> -
> - /* Multiple IRQs are not supported */
> - irq = platform_get_irq(pdev, 0);
> - if (irq == -ENXIO)
> - uioinfo->irq = UIO_IRQ_NONE;
> - else
> - uioinfo->irq = irq;
> }
>
> if (!uioinfo || !uioinfo->name || !uioinfo->version) {
> @@ -146,14 +137,6 @@ static int uio_pdrv_genirq_probe(struct platform_device *pdev)
> priv->flags = 0; /* interrupt is enabled to begin with */
> priv->pdev = pdev;
>
> - if (!uioinfo->irq) {
> - ret = platform_get_irq(pdev, 0);
> - if (ret < 0) {
> - dev_err(&pdev->dev, "failed to get IRQ\n");
> - goto bad0;
> - }
> - uioinfo->irq = ret;
> - }
> uiomem = &uioinfo->mem[0];
>
> for (i = 0; i < pdev->num_resources; ++i) {
> @@ -190,10 +173,15 @@ static int uio_pdrv_genirq_probe(struct platform_device *pdev)
> * Interrupt sharing is not supported.
> */
>
> - uioinfo->handler = uio_pdrv_genirq_handler;
> - uioinfo->irqcontrol = uio_pdrv_genirq_irqcontrol;
> - uioinfo->open = uio_pdrv_genirq_open;
> - uioinfo->release = uio_pdrv_genirq_release;
> + ret = platform_get_irq(pdev, 0);
> + if (ret >= 0) {
> + uioinfo->irq = ret;
> + uioinfo->handler = uio_pdrv_genirq_handler;
> + uioinfo->irqcontrol = uio_pdrv_genirq_irqcontrol;
> + uioinfo->open = uio_pdrv_genirq_open;
> + uioinfo->release = uio_pdrv_genirq_release;
> + }
> +
> uioinfo->priv = priv;
>
> /* Enable Runtime PM for this device:
> --
> 1.8.2.3
>


--
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: http://www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform



Attachments:
signature.asc (263.00 B)
OpenPGP digital signature

2013-06-12 05:43:34

by Michal Simek

[permalink] [raw]
Subject: Re: [RFC PATCH 2/2] uio: Add two platform uio drivers to one

On 06/03/2013 11:05 PM, Greg Kroah-Hartman wrote:
> On Mon, Jun 03, 2013 at 02:34:11PM +0200, Michal Simek wrote:
>> Hi Hans and Greg,
>>
>> can you please comment this?
>
> I'll let Hans handle this, as he's the UIO maintainer now.
>

Hans: Any update on this?

Thanks,
Michal


--
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: http://www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform



Attachments:
signature.asc (263.00 B)
OpenPGP digital signature

2013-06-20 17:12:03

by Michal Simek

[permalink] [raw]
Subject: Re: [RFC PATCH 2/2] uio: Add two platform uio drivers to one

Hi,

On 06/20/2013 06:01 PM, Pavel Machek wrote:
> Hi!
>
>>> can you please comment this?
>>
>> I'll let Hans handle this, as he's the UIO maintainer now.
>
> Looks good to me.
>
> Anyway, Hans disappeared, so Greg takes patches, again. Take a look,
> it seems I already did some changes you wanted, they are in char-misc
> tree.

Can you send me link to that repo?

Thanks,
Michal

--
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: http://www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform



Attachments:
signature.asc (263.00 B)
OpenPGP digital signature

2013-06-20 16:01:29

by Pavel Machek

[permalink] [raw]
Subject: Re: [RFC PATCH 2/2] uio: Add two platform uio drivers to one

Hi!

> > can you please comment this?
>
> I'll let Hans handle this, as he's the UIO maintainer now.

Looks good to me.

Anyway, Hans disappeared, so Greg takes patches, again. Take a look,
it seems I already did some changes you wanted, they are in char-misc
tree.

Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

2013-06-20 16:30:46

by Guenter Roeck

[permalink] [raw]
Subject: Re: [lm-sensors] [PATCH] MAINTAINERS: Remove Hans J. Koch entries

On Thu, Jun 20, 2013 at 09:20:27AM -0700, Joe Perches wrote:
> On Thu, 2013-06-20 at 18:01 +0200, Pavel Machek wrote:
> > Anyway, Hans disappeared, so Greg takes patches, again.
>
> Maybe this is appropriate then.
>
> Signed-off-by: Joe Perches <[email protected]>
> ---
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 69fea4f..dc9d04a 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -5253,9 +5253,8 @@ F: Documentation/hwmon/max16065
> F: drivers/hwmon/max16065.c
>
> MAX6650 HARDWARE MONITOR AND FAN CONTROLLER DRIVER
> -M: "Hans J. Koch" <[email protected]>
> L: [email protected]
> -S: Maintained
> +S: Orphan
> F: Documentation/hwmon/max6650
> F: drivers/hwmon/max6650.c
>
Please just drop that entry entirely, causing it to default to hwmon.

Thanks,
Guenter

> @@ -8795,7 +8794,6 @@ F: fs/hostfs/
> F: fs/hppfs/
>
> USERSPACE I/O (UIO)
> -M: "Hans J. Koch" <[email protected]>
> M: Greg Kroah-Hartman <[email protected]>
> S: Maintained
> F: Documentation/DocBook/uio-howto.tmpl
>
>
>
> _______________________________________________
> lm-sensors mailing list
> [email protected]
> http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
>

2013-06-20 16:20:48

by Joe Perches

[permalink] [raw]
Subject: [PATCH] MAINTAINERS: Remove Hans J. Koch entries

On Thu, 2013-06-20 at 18:01 +0200, Pavel Machek wrote:
> Anyway, Hans disappeared, so Greg takes patches, again.

Maybe this is appropriate then.

Signed-off-by: Joe Perches <[email protected]>
---
diff --git a/MAINTAINERS b/MAINTAINERS
index 69fea4f..dc9d04a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5253,9 +5253,8 @@ F: Documentation/hwmon/max16065
F: drivers/hwmon/max16065.c

MAX6650 HARDWARE MONITOR AND FAN CONTROLLER DRIVER
-M: "Hans J. Koch" <[email protected]>
L: [email protected]
-S: Maintained
+S: Orphan
F: Documentation/hwmon/max6650
F: drivers/hwmon/max6650.c

@@ -8795,7 +8794,6 @@ F: fs/hostfs/
F: fs/hppfs/

USERSPACE I/O (UIO)
-M: "Hans J. Koch" <[email protected]>
M: Greg Kroah-Hartman <[email protected]>
S: Maintained
F: Documentation/DocBook/uio-howto.tmpl

2013-06-20 17:09:58

by Joe Perches

[permalink] [raw]
Subject: Re: [lm-sensors] [PATCH] MAINTAINERS: Remove Hans J. Koch entries

On Thu, 2013-06-20 at 09:30 -0700, Guenter Roeck wrote:
> On Thu, Jun 20, 2013 at 09:20:27AM -0700, Joe Perches wrote:
> > On Thu, 2013-06-20 at 18:01 +0200, Pavel Machek wrote:
> > > Anyway, Hans disappeared, so Greg takes patches, again.
[]
> > MAX6650 HARDWARE MONITOR AND FAN CONTROLLER DRIVER
> > -M: "Hans J. Koch" <[email protected]>
> > L: [email protected]
> > -S: Maintained
> > +S: Orphan
> > F: Documentation/hwmon/max6650
> > F: drivers/hwmon/max6650.c
> >
> Please just drop that entry entirely, causing it to default to hwmon.

It was mostly a nudge for Hans.

Maybe in a few weeks if Hans doesn't reply that'd
make sense.

I don't have any issue with marking specific files
as orphan though. Orphan can help others identify
files that might need care.

I just looked for the first time at max6650.c too.
It's got a spirentcom copyright which amuses me.

2013-06-20 23:13:49

by Pavel Machek

[permalink] [raw]
Subject: Re: [RFC PATCH 2/2] uio: Add two platform uio drivers to one

Hi!

> >>> can you please comment this?
> >>
> >> I'll let Hans handle this, as he's the UIO maintainer now.
> >
> > Looks good to me.
> >
> > Anyway, Hans disappeared, so Greg takes patches, again. Take a look,
> > it seems I already did some changes you wanted, they are in char-misc
> > tree.
>
> Can you send me link to that repo?

I googled a mirror...

https://kernel.googlesource.com/pub/scm/linux/kernel/git/gregkh/char-misc/+/char-misc-next/drivers/uio/uio_pdrv_genirq.c
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

2013-06-21 02:50:45

by Hans J. Koch

[permalink] [raw]
Subject: Re: [PATCH] MAINTAINERS: Remove Hans J. Koch entries

On Thu, Jun 20, 2013 at 09:20:27AM -0700, Joe Perches wrote:
> On Thu, 2013-06-20 at 18:01 +0200, Pavel Machek wrote:
> > Anyway, Hans disappeared, so Greg takes patches, again.

I wasn't able to take part in kernel development because I was heavily
involved in hardware development project. I always thought it would be just
a few more days, but then it became half a year. I know I should have given
an explanation and didn't. Sorry for that.

>
> Maybe this is appropriate then.
>
> Signed-off-by: Joe Perches <[email protected]>
> ---
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 69fea4f..dc9d04a 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -5253,9 +5253,8 @@ F: Documentation/hwmon/max16065
> F: drivers/hwmon/max16065.c
>
> MAX6650 HARDWARE MONITOR AND FAN CONTROLLER DRIVER
> -M: "Hans J. Koch" <[email protected]>
> L: [email protected]
> -S: Maintained
> +S: Orphan
> F: Documentation/hwmon/max6650
> F: drivers/hwmon/max6650.c

ACK to that one. It was never my idea to have a MAINTAINERS entry for that.
Jean Delvare suggested it, so it came in. The MAX6650 was in a project I was
working on 6 years ago, and I wrote the driver at that time. Meanwhile, I
don't even have hardware with a MAX6650 anymore.

Does each little driver really need a MAINTAINERS entry? In my opinion, it
should only be there if it is clear that it's not just a short project work.

>
> @@ -8795,7 +8794,6 @@ F: fs/hostfs/
> F: fs/hppfs/
>
> USERSPACE I/O (UIO)
> -M: "Hans J. Koch" <[email protected]>
> M: Greg Kroah-Hartman <[email protected]>
> S: Maintained
> F: Documentation/DocBook/uio-howto.tmpl

Well, you can do that if you want. But I still feel associated with UIO
and sincerely hope I'm back again to fulfill my duties as a maintainer.

Thanks,
Hans

2013-06-21 03:08:45

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH] MAINTAINERS: Remove Hans J. Koch entries

On Fri, 2013-06-21 at 04:50 +0200, Hans J. Koch wrote:
> Does each little driver really need a MAINTAINERS entry?

Hi Hans.

I think a lot of those driver MAINTAINERS entries are
maybe a little useful when the driver is just written
but are progressively less useful after a few months.

I once wrote a little script to look at whether or not
each listed maintainer had signed-off on a patch to
a listed file block for whatever time frame.

Most had not.

I'd rather people put their names in CREDITS than
MAINTAINERS if they want posterity entries.

> In my opinion, it should only be there if it is clear
> that it's not just a short project work.

I mostly agree with that, but sometimes it's useful
when the implementation is a bit complex and the
original author may still have some memory of the
complexity.

Anyway, I'm not resubmitting that patch. If you want
to remove yourself from whatever entry, please do.

2013-06-21 07:47:56

by Jean Delvare

[permalink] [raw]
Subject: Re: [lm-sensors] [PATCH] MAINTAINERS: Remove Hans J. Koch entries

Hi Hans and all,

On Fri, 21 Jun 2013 04:50:31 +0200, Hans J. Koch wrote:
> On Thu, Jun 20, 2013 at 09:20:27AM -0700, Joe Perches wrote:
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 69fea4f..dc9d04a 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -5253,9 +5253,8 @@ F: Documentation/hwmon/max16065
> > F: drivers/hwmon/max16065.c
> >
> > MAX6650 HARDWARE MONITOR AND FAN CONTROLLER DRIVER
> > -M: "Hans J. Koch" <[email protected]>
> > L: [email protected]
> > -S: Maintained
> > +S: Orphan
> > F: Documentation/hwmon/max6650
> > F: drivers/hwmon/max6650.c
>
> ACK to that one. It was never my idea to have a MAINTAINERS entry for that.
> Jean Delvare suggested it, so it came in. The MAX6650 was in a project I was
> working on 6 years ago, and I wrote the driver at that time. Meanwhile, I
> don't even have hardware with a MAX6650 anymore.

Back then I was maintaining the hwmon subsystem all by myself and had
way more than I could handle on my plate. Contributors kept complaining
that I was too slow reviewing new drivers and having them add
themselves to MAINTAINERS was my way to help them understand that Linux
wasn't only about adding new driver, and that every piece of code added
to the kernel had to be maintained by someone for the years to come.
And that someone was rather them than me.

That being said, I agree it only makes sense for as long as the
original contributor (or anyone else wanting to step in) has the
hardware, interest and knowledge for the driver in question. After
several years it makes sense to drop these individual driver entries if
these conditions are no longer met.

> Does each little driver really need a MAINTAINERS entry? In my opinion, it
> should only be there if it is clear that it's not just a short project work.

The number of drivers in all kernel subsystems keeps growing, and hwmon
is no exception. The number of subsystem maintainers, OTOH, is not
growing, it's typically one or two persons doing all the work. This is
why I value individual driver maintainers, as they help make it all
scale. If existing drivers each have their own maintainer, subsystem
maintainers can focus on subsystem-wide cleanups and reviewing new
drivers.

But of course MAINTAINERS must reflect the reality and not my own
fantasy world where every single driver would have a dedicated
maintainer. If anyone listed as a driver maintainer in MAINTAINERS is
not actually going to do the job for whatever reason, then the entry
should be removed.

--
Jean Delvare

2013-06-21 09:06:33

by Michal Simek

[permalink] [raw]
Subject: Re: [RFC PATCH 2/2] uio: Add two platform uio drivers to one

On 06/21/2013 01:13 AM, Pavel Machek wrote:
> Hi!
>
>>>>> can you please comment this?
>>>>
>>>> I'll let Hans handle this, as he's the UIO maintainer now.
>>>
>>> Looks good to me.
>>>
>>> Anyway, Hans disappeared, so Greg takes patches, again. Take a look,
>>> it seems I already did some changes you wanted, they are in char-misc
>>> tree.
>>
>> Can you send me link to that repo?
>
> I googled a mirror...
>
> https://kernel.googlesource.com/pub/scm/linux/kernel/git/gregkh/char-misc/+/char-misc-next/drivers/uio/uio_pdrv_genirq.c

Ok. I see.
Yeah it can be done in that way too.

I can rebase my patches on the top of that because they are still valid.
1/2 is not there and it is nice way how to simplify the code.

The purpose of 2/2 was to remove that uio_pdrv.c which is not needed
when uio_pdrv_genirq.c also support no IRQ case.

Will be good if you can look at my patches and test them.

Also we should also simplify the driver by using devres groups,
devm_kzalloc, etc.

Thanks,
Michal

--
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: http://www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform



Attachments:
signature.asc (263.00 B)
OpenPGP digital signature

2013-06-21 09:15:26

by Michal Simek

[permalink] [raw]
Subject: Re: [PATCH] MAINTAINERS: Remove Hans J. Koch entries

On 06/21/2013 04:50 AM, Hans J. Koch wrote:
> On Thu, Jun 20, 2013 at 09:20:27AM -0700, Joe Perches wrote:
>> On Thu, 2013-06-20 at 18:01 +0200, Pavel Machek wrote:
>>> Anyway, Hans disappeared, so Greg takes patches, again.
>
> I wasn't able to take part in kernel development because I was heavily
> involved in hardware development project. I always thought it would be just
> a few more days, but then it became half a year. I know I should have given
> an explanation and didn't. Sorry for that.
>
>>
>> Maybe this is appropriate then.
>>
>> Signed-off-by: Joe Perches <[email protected]>
>> ---
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 69fea4f..dc9d04a 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -5253,9 +5253,8 @@ F: Documentation/hwmon/max16065
>> F: drivers/hwmon/max16065.c
>>
>> MAX6650 HARDWARE MONITOR AND FAN CONTROLLER DRIVER
>> -M: "Hans J. Koch" <[email protected]>
>> L: [email protected]
>> -S: Maintained
>> +S: Orphan
>> F: Documentation/hwmon/max6650
>> F: drivers/hwmon/max6650.c
>
> ACK to that one. It was never my idea to have a MAINTAINERS entry for that.
> Jean Delvare suggested it, so it came in. The MAX6650 was in a project I was
> working on 6 years ago, and I wrote the driver at that time. Meanwhile, I
> don't even have hardware with a MAX6650 anymore.
>
> Does each little driver really need a MAINTAINERS entry? In my opinion, it
> should only be there if it is clear that it's not just a short project work.
>
>>
>> @@ -8795,7 +8794,6 @@ F: fs/hostfs/
>> F: fs/hppfs/
>>
>> USERSPACE I/O (UIO)
>> -M: "Hans J. Koch" <[email protected]>
>> M: Greg Kroah-Hartman <[email protected]>
>> S: Maintained
>> F: Documentation/DocBook/uio-howto.tmpl
>
> Well, you can do that if you want. But I still feel associated with UIO
> and sincerely hope I'm back again to fulfill my duties as a maintainer.

ok then can you please comment my patches?
I think it is not necessary to have one driver which support UIO without IRQ
and then second which support both cases.

Thanks,
Michal

--
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: http://www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform



Attachments:
signature.asc (263.00 B)
OpenPGP digital signature