All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] dmaengine: qcom_hidma: cleanup sysfs entries during remove
@ 2016-10-06 23:23 ` Sinan Kaya
  0 siblings, 0 replies; 7+ messages in thread
From: Sinan Kaya @ 2016-10-06 23:23 UTC (permalink / raw)
  To: dmaengine, timur, cov
  Cc: linux-arm-msm, linux-arm-kernel, Sinan Kaya, linux-kernel

The 4.8-rc8 kernel is printing duplicate file entry warnings while removing
the HIDMA object. This is caused by stale sysfs entries remaining from the
previous execution.

_sysfs_warn_dup+0x5c/0x78
 sysfs_add_file_mode_ns+0x13c/0x1c0
 sysfs_create_file_ns+0x2c/0x40
 device_create_file+0x54/0xa0
 hidma_probe+0x7c8/0x808

Create hidma_sysfs_init and hidma_sysfs_uninit functions and call them from
the probe and remove path. To do proper clean up, adding the attrs object
to the device data structure to keep it around until remove call is made.

Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
---
 drivers/dma/qcom/hidma.c | 31 +++++++++++++++++++++++++------
 drivers/dma/qcom/hidma.h |  3 +++
 2 files changed, 28 insertions(+), 6 deletions(-)

diff --git a/drivers/dma/qcom/hidma.c b/drivers/dma/qcom/hidma.c
index f4fe4ee..414ea12 100644
--- a/drivers/dma/qcom/hidma.c
+++ b/drivers/dma/qcom/hidma.c
@@ -578,8 +578,17 @@ static ssize_t hidma_show_values(struct device *dev,
 	return strlen(buf);
 }
 
-static int hidma_create_sysfs_entry(struct hidma_dev *dev, char *name,
-				    int mode)
+static int hidma_sysfs_uninit(struct hidma_dev *dev)
+{
+	if (!dev->chid_attrs)
+		return -ENOMEM;
+
+	device_remove_file(dev->ddev.dev, dev->chid_attrs);
+	return 0;
+}
+
+static struct device_attribute*
+hidma_create_sysfs_entry(struct hidma_dev *dev, char *name, int mode)
 {
 	struct device_attribute *attrs;
 	char *name_copy;
@@ -587,20 +596,29 @@ static int hidma_create_sysfs_entry(struct hidma_dev *dev, char *name,
 	attrs = devm_kmalloc(dev->ddev.dev, sizeof(struct device_attribute),
 			     GFP_KERNEL);
 	if (!attrs)
-		return -ENOMEM;
+		return NULL;
 
 	name_copy = devm_kstrdup(dev->ddev.dev, name, GFP_KERNEL);
 	if (!name_copy)
-		return -ENOMEM;
+		return NULL;
 
 	attrs->attr.name = name_copy;
 	attrs->attr.mode = mode;
 	attrs->show = hidma_show_values;
 	sysfs_attr_init(&attrs->attr);
+	return attrs;
+}
 
-	return device_create_file(dev->ddev.dev, attrs);
+static int hidma_sysfs_init(struct hidma_dev *dev)
+{
+	dev->chid_attrs = hidma_create_sysfs_entry(dev, "chid", S_IRUGO);
+	if (!dev->chid_attrs)
+		return -ENOMEM;
+
+	return device_create_file(dev->ddev.dev, dev->chid_attrs);
 }
 
+
 #ifdef CONFIG_GENERIC_MSI_IRQ_DOMAIN
 static void hidma_write_msi_msg(struct msi_desc *desc, struct msi_msg *msg)
 {
@@ -823,7 +841,7 @@ static int hidma_probe(struct platform_device *pdev)
 	dmadev->irq = chirq;
 	tasklet_init(&dmadev->task, hidma_issue_task, (unsigned long)dmadev);
 	hidma_debug_init(dmadev);
-	hidma_create_sysfs_entry(dmadev, "chid", S_IRUGO);
+	hidma_sysfs_init(dmadev);
 	dev_info(&pdev->dev, "HI-DMA engine driver registration complete\n");
 	pm_runtime_mark_last_busy(dmadev->ddev.dev);
 	pm_runtime_put_autosuspend(dmadev->ddev.dev);
@@ -849,6 +867,7 @@ static int hidma_remove(struct platform_device *pdev)
 	dma_async_device_unregister(&dmadev->ddev);
 	devm_free_irq(dmadev->ddev.dev, dmadev->irq, dmadev->lldev);
 	tasklet_kill(&dmadev->task);
+	hidma_sysfs_uninit(dmadev);
 	hidma_debug_uninit(dmadev);
 	hidma_ll_uninit(dmadev->lldev);
 	hidma_free(dmadev);
diff --git a/drivers/dma/qcom/hidma.h b/drivers/dma/qcom/hidma.h
index 05f8ba4..c7d0142 100644
--- a/drivers/dma/qcom/hidma.h
+++ b/drivers/dma/qcom/hidma.h
@@ -130,6 +130,9 @@ struct hidma_dev {
 	struct dentry			*debugfs;
 	struct dentry			*stats;
 
+	/* sysfs entry for the channel id */
+	struct device_attribute		*chid_attrs;
+
 	/* Task delivering issue_pending */
 	struct tasklet_struct		task;
 };
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH] dmaengine: qcom_hidma: cleanup sysfs entries during remove
@ 2016-10-06 23:23 ` Sinan Kaya
  0 siblings, 0 replies; 7+ messages in thread
From: Sinan Kaya @ 2016-10-06 23:23 UTC (permalink / raw)
  To: linux-arm-kernel

The 4.8-rc8 kernel is printing duplicate file entry warnings while removing
the HIDMA object. This is caused by stale sysfs entries remaining from the
previous execution.

_sysfs_warn_dup+0x5c/0x78
 sysfs_add_file_mode_ns+0x13c/0x1c0
 sysfs_create_file_ns+0x2c/0x40
 device_create_file+0x54/0xa0
 hidma_probe+0x7c8/0x808

Create hidma_sysfs_init and hidma_sysfs_uninit functions and call them from
the probe and remove path. To do proper clean up, adding the attrs object
to the device data structure to keep it around until remove call is made.

Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
---
 drivers/dma/qcom/hidma.c | 31 +++++++++++++++++++++++++------
 drivers/dma/qcom/hidma.h |  3 +++
 2 files changed, 28 insertions(+), 6 deletions(-)

diff --git a/drivers/dma/qcom/hidma.c b/drivers/dma/qcom/hidma.c
index f4fe4ee..414ea12 100644
--- a/drivers/dma/qcom/hidma.c
+++ b/drivers/dma/qcom/hidma.c
@@ -578,8 +578,17 @@ static ssize_t hidma_show_values(struct device *dev,
 	return strlen(buf);
 }
 
-static int hidma_create_sysfs_entry(struct hidma_dev *dev, char *name,
-				    int mode)
+static int hidma_sysfs_uninit(struct hidma_dev *dev)
+{
+	if (!dev->chid_attrs)
+		return -ENOMEM;
+
+	device_remove_file(dev->ddev.dev, dev->chid_attrs);
+	return 0;
+}
+
+static struct device_attribute*
+hidma_create_sysfs_entry(struct hidma_dev *dev, char *name, int mode)
 {
 	struct device_attribute *attrs;
 	char *name_copy;
@@ -587,20 +596,29 @@ static int hidma_create_sysfs_entry(struct hidma_dev *dev, char *name,
 	attrs = devm_kmalloc(dev->ddev.dev, sizeof(struct device_attribute),
 			     GFP_KERNEL);
 	if (!attrs)
-		return -ENOMEM;
+		return NULL;
 
 	name_copy = devm_kstrdup(dev->ddev.dev, name, GFP_KERNEL);
 	if (!name_copy)
-		return -ENOMEM;
+		return NULL;
 
 	attrs->attr.name = name_copy;
 	attrs->attr.mode = mode;
 	attrs->show = hidma_show_values;
 	sysfs_attr_init(&attrs->attr);
+	return attrs;
+}
 
-	return device_create_file(dev->ddev.dev, attrs);
+static int hidma_sysfs_init(struct hidma_dev *dev)
+{
+	dev->chid_attrs = hidma_create_sysfs_entry(dev, "chid", S_IRUGO);
+	if (!dev->chid_attrs)
+		return -ENOMEM;
+
+	return device_create_file(dev->ddev.dev, dev->chid_attrs);
 }
 
+
 #ifdef CONFIG_GENERIC_MSI_IRQ_DOMAIN
 static void hidma_write_msi_msg(struct msi_desc *desc, struct msi_msg *msg)
 {
@@ -823,7 +841,7 @@ static int hidma_probe(struct platform_device *pdev)
 	dmadev->irq = chirq;
 	tasklet_init(&dmadev->task, hidma_issue_task, (unsigned long)dmadev);
 	hidma_debug_init(dmadev);
-	hidma_create_sysfs_entry(dmadev, "chid", S_IRUGO);
+	hidma_sysfs_init(dmadev);
 	dev_info(&pdev->dev, "HI-DMA engine driver registration complete\n");
 	pm_runtime_mark_last_busy(dmadev->ddev.dev);
 	pm_runtime_put_autosuspend(dmadev->ddev.dev);
@@ -849,6 +867,7 @@ static int hidma_remove(struct platform_device *pdev)
 	dma_async_device_unregister(&dmadev->ddev);
 	devm_free_irq(dmadev->ddev.dev, dmadev->irq, dmadev->lldev);
 	tasklet_kill(&dmadev->task);
+	hidma_sysfs_uninit(dmadev);
 	hidma_debug_uninit(dmadev);
 	hidma_ll_uninit(dmadev->lldev);
 	hidma_free(dmadev);
diff --git a/drivers/dma/qcom/hidma.h b/drivers/dma/qcom/hidma.h
index 05f8ba4..c7d0142 100644
--- a/drivers/dma/qcom/hidma.h
+++ b/drivers/dma/qcom/hidma.h
@@ -130,6 +130,9 @@ struct hidma_dev {
 	struct dentry			*debugfs;
 	struct dentry			*stats;
 
+	/* sysfs entry for the channel id */
+	struct device_attribute		*chid_attrs;
+
 	/* Task delivering issue_pending */
 	struct tasklet_struct		task;
 };
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] dmaengine: qcom_hidma: cleanup sysfs entries during remove
  2016-10-06 23:23 ` Sinan Kaya
  (?)
@ 2016-10-19 13:26   ` Vinod Koul
  -1 siblings, 0 replies; 7+ messages in thread
From: Vinod Koul @ 2016-10-19 13:26 UTC (permalink / raw)
  To: Sinan Kaya
  Cc: linux-arm-msm, timur, linux-kernel, cov, dmaengine, linux-arm-kernel

On Thu, Oct 06, 2016 at 07:23:09PM -0400, Sinan Kaya wrote:
> The 4.8-rc8 kernel is printing duplicate file entry warnings while removing
> the HIDMA object. This is caused by stale sysfs entries remaining from the
> previous execution.
> 
> _sysfs_warn_dup+0x5c/0x78
>  sysfs_add_file_mode_ns+0x13c/0x1c0
>  sysfs_create_file_ns+0x2c/0x40
>  device_create_file+0x54/0xa0
>  hidma_probe+0x7c8/0x808
> 
> Create hidma_sysfs_init and hidma_sysfs_uninit functions and call them from
> the probe and remove path. To do proper clean up, adding the attrs object
> to the device data structure to keep it around until remove call is made.
> 
> Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
> ---
>  drivers/dma/qcom/hidma.c | 31 +++++++++++++++++++++++++------
>  drivers/dma/qcom/hidma.h |  3 +++
>  2 files changed, 28 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/dma/qcom/hidma.c b/drivers/dma/qcom/hidma.c
> index f4fe4ee..414ea12 100644
> --- a/drivers/dma/qcom/hidma.c
> +++ b/drivers/dma/qcom/hidma.c
> @@ -578,8 +578,17 @@ static ssize_t hidma_show_values(struct device *dev,
>  	return strlen(buf);
>  }
>  
> -static int hidma_create_sysfs_entry(struct hidma_dev *dev, char *name,
> -				    int mode)
> +static int hidma_sysfs_uninit(struct hidma_dev *dev)
> +{
> +	if (!dev->chid_attrs)
> +		return -ENOMEM;

why is this check required? Probe would fail in init case right.
Second returning error doesnt help as you are calling this from remove and
return is not checked so redundant!

-- 
~Vinod

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] dmaengine: qcom_hidma: cleanup sysfs entries during remove
@ 2016-10-19 13:26   ` Vinod Koul
  0 siblings, 0 replies; 7+ messages in thread
From: Vinod Koul @ 2016-10-19 13:26 UTC (permalink / raw)
  To: Sinan Kaya
  Cc: dmaengine, timur, cov, linux-arm-msm, linux-arm-kernel, linux-kernel

On Thu, Oct 06, 2016 at 07:23:09PM -0400, Sinan Kaya wrote:
> The 4.8-rc8 kernel is printing duplicate file entry warnings while removing
> the HIDMA object. This is caused by stale sysfs entries remaining from the
> previous execution.
> 
> _sysfs_warn_dup+0x5c/0x78
>  sysfs_add_file_mode_ns+0x13c/0x1c0
>  sysfs_create_file_ns+0x2c/0x40
>  device_create_file+0x54/0xa0
>  hidma_probe+0x7c8/0x808
> 
> Create hidma_sysfs_init and hidma_sysfs_uninit functions and call them from
> the probe and remove path. To do proper clean up, adding the attrs object
> to the device data structure to keep it around until remove call is made.
> 
> Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
> ---
>  drivers/dma/qcom/hidma.c | 31 +++++++++++++++++++++++++------
>  drivers/dma/qcom/hidma.h |  3 +++
>  2 files changed, 28 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/dma/qcom/hidma.c b/drivers/dma/qcom/hidma.c
> index f4fe4ee..414ea12 100644
> --- a/drivers/dma/qcom/hidma.c
> +++ b/drivers/dma/qcom/hidma.c
> @@ -578,8 +578,17 @@ static ssize_t hidma_show_values(struct device *dev,
>  	return strlen(buf);
>  }
>  
> -static int hidma_create_sysfs_entry(struct hidma_dev *dev, char *name,
> -				    int mode)
> +static int hidma_sysfs_uninit(struct hidma_dev *dev)
> +{
> +	if (!dev->chid_attrs)
> +		return -ENOMEM;

why is this check required? Probe would fail in init case right.
Second returning error doesnt help as you are calling this from remove and
return is not checked so redundant!

-- 
~Vinod

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH] dmaengine: qcom_hidma: cleanup sysfs entries during remove
@ 2016-10-19 13:26   ` Vinod Koul
  0 siblings, 0 replies; 7+ messages in thread
From: Vinod Koul @ 2016-10-19 13:26 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Oct 06, 2016 at 07:23:09PM -0400, Sinan Kaya wrote:
> The 4.8-rc8 kernel is printing duplicate file entry warnings while removing
> the HIDMA object. This is caused by stale sysfs entries remaining from the
> previous execution.
> 
> _sysfs_warn_dup+0x5c/0x78
>  sysfs_add_file_mode_ns+0x13c/0x1c0
>  sysfs_create_file_ns+0x2c/0x40
>  device_create_file+0x54/0xa0
>  hidma_probe+0x7c8/0x808
> 
> Create hidma_sysfs_init and hidma_sysfs_uninit functions and call them from
> the probe and remove path. To do proper clean up, adding the attrs object
> to the device data structure to keep it around until remove call is made.
> 
> Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
> ---
>  drivers/dma/qcom/hidma.c | 31 +++++++++++++++++++++++++------
>  drivers/dma/qcom/hidma.h |  3 +++
>  2 files changed, 28 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/dma/qcom/hidma.c b/drivers/dma/qcom/hidma.c
> index f4fe4ee..414ea12 100644
> --- a/drivers/dma/qcom/hidma.c
> +++ b/drivers/dma/qcom/hidma.c
> @@ -578,8 +578,17 @@ static ssize_t hidma_show_values(struct device *dev,
>  	return strlen(buf);
>  }
>  
> -static int hidma_create_sysfs_entry(struct hidma_dev *dev, char *name,
> -				    int mode)
> +static int hidma_sysfs_uninit(struct hidma_dev *dev)
> +{
> +	if (!dev->chid_attrs)
> +		return -ENOMEM;

why is this check required? Probe would fail in init case right.
Second returning error doesnt help as you are calling this from remove and
return is not checked so redundant!

-- 
~Vinod

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] dmaengine: qcom_hidma: cleanup sysfs entries during remove
  2016-10-19 13:26   ` Vinod Koul
@ 2016-10-19 15:54     ` Sinan Kaya
  -1 siblings, 0 replies; 7+ messages in thread
From: Sinan Kaya @ 2016-10-19 15:54 UTC (permalink / raw)
  To: Vinod Koul
  Cc: dmaengine, timur, cov, linux-arm-msm, linux-arm-kernel, linux-kernel

On 10/19/2016 6:26 AM, Vinod Koul wrote:
>> -static int hidma_create_sysfs_entry(struct hidma_dev *dev, char *name,
>> > -				    int mode)
>> > +static int hidma_sysfs_uninit(struct hidma_dev *dev)
>> > +{
>> > +	if (!dev->chid_attrs)
>> > +		return -ENOMEM;
> why is this check required? Probe would fail in init case right.
> Second returning error doesnt help as you are calling this from remove and
> return is not checked so redundant!

Agreed, I'll get rid of the attrs and also the return value.

-- 
Sinan Kaya
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH] dmaengine: qcom_hidma: cleanup sysfs entries during remove
@ 2016-10-19 15:54     ` Sinan Kaya
  0 siblings, 0 replies; 7+ messages in thread
From: Sinan Kaya @ 2016-10-19 15:54 UTC (permalink / raw)
  To: linux-arm-kernel

On 10/19/2016 6:26 AM, Vinod Koul wrote:
>> -static int hidma_create_sysfs_entry(struct hidma_dev *dev, char *name,
>> > -				    int mode)
>> > +static int hidma_sysfs_uninit(struct hidma_dev *dev)
>> > +{
>> > +	if (!dev->chid_attrs)
>> > +		return -ENOMEM;
> why is this check required? Probe would fail in init case right.
> Second returning error doesnt help as you are calling this from remove and
> return is not checked so redundant!

Agreed, I'll get rid of the attrs and also the return value.

-- 
Sinan Kaya
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2016-10-19 15:54 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-06 23:23 [PATCH] dmaengine: qcom_hidma: cleanup sysfs entries during remove Sinan Kaya
2016-10-06 23:23 ` Sinan Kaya
2016-10-19 13:26 ` Vinod Koul
2016-10-19 13:26   ` Vinod Koul
2016-10-19 13:26   ` Vinod Koul
2016-10-19 15:54   ` Sinan Kaya
2016-10-19 15:54     ` Sinan Kaya

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.