[PATCH v3] ASoC: ti: omap-mcbsp: duplicate sysfs error

[PATCH v3] ASoC: ti: omap-mcbsp: duplicate sysfs error

[David Owens]

Convert to managed versions of sysfs and clk allocation to simplify
unbinding and error handling in probe.  Managed sysfs node
creation specifically addresses the following error seen the second time
probe is attempted after sdma_pcm_platform_register() previously requested
probe deferral:

sysfs: cannot create duplicate filename '/devices/platform/68000000.ocp/49022000.mcbsp/max_tx_thres'

Signed-off-by: David Owens <dowens@precisionplanting.com>
---

Changes in v3:
 * Whitespace changes only to allow clean applly

Changes in v2:
 * Improved error handling

---
 sound/soc/ti/omap-mcbsp-priv.h |  2 --
 sound/soc/ti/omap-mcbsp-st.c   | 21 ++++-----------------
 sound/soc/ti/omap-mcbsp.c      | 26 ++++----------------------
 3 files changed, 8 insertions(+), 41 deletions(-)

diff --git a/sound/soc/ti/omap-mcbsp-priv.h b/sound/soc/ti/omap-mcbsp-priv.h
index 7865cda4bf0a..da519ea1f303 100644
--- a/sound/soc/ti/omap-mcbsp-priv.h
+++ b/sound/soc/ti/omap-mcbsp-priv.h
@@ -316,8 +316,6 @@ static inline int omap_mcbsp_read(struct omap_mcbsp *mcbsp, u16 reg,
 
 /* Sidetone specific API */
 int omap_mcbsp_st_init(struct platform_device *pdev);
-void omap_mcbsp_st_cleanup(struct platform_device *pdev);
-
 int omap_mcbsp_st_start(struct omap_mcbsp *mcbsp);
 int omap_mcbsp_st_stop(struct omap_mcbsp *mcbsp);
 
diff --git a/sound/soc/ti/omap-mcbsp-st.c b/sound/soc/ti/omap-mcbsp-st.c
index 0bc7d26c660a..402a57a502e6 100644
--- a/sound/soc/ti/omap-mcbsp-st.c
+++ b/sound/soc/ti/omap-mcbsp-st.c
@@ -292,14 +292,11 @@ static ssize_t st_taps_store(struct device *dev,
 
 static DEVICE_ATTR_RW(st_taps);
 
-static const struct attribute *sidetone_attrs[] = {
+static struct attribute *sidetone_attrs[] = {
 	&dev_attr_st_taps.attr,
 	NULL,
 };
-
-static const struct attribute_group sidetone_attr_group = {
-	.attrs = (struct attribute **)sidetone_attrs,
-};
+ATTRIBUTE_GROUPS(sidetone);
 
 int omap_mcbsp_st_start(struct omap_mcbsp *mcbsp)
 {
@@ -347,7 +344,7 @@ int omap_mcbsp_st_init(struct platform_device *pdev)
 	if (!st_data)
 		return -ENOMEM;
 
-	st_data->mcbsp_iclk = clk_get(mcbsp->dev, "ick");
+	st_data->mcbsp_iclk = devm_clk_get(mcbsp->dev, "ick");
 	if (IS_ERR(st_data->mcbsp_iclk)) {
 		dev_warn(mcbsp->dev,
 			 "Failed to get ick, sidetone might be broken\n");
@@ -359,7 +356,7 @@ int omap_mcbsp_st_init(struct platform_device *pdev)
 	if (!st_data->io_base_st)
 		return -ENOMEM;
 
-	ret = sysfs_create_group(&mcbsp->dev->kobj, &sidetone_attr_group);
+	ret = devm_device_add_group(mcbsp->dev, &sidetone_group);
 	if (ret)
 		return ret;
 
@@ -368,16 +365,6 @@ int omap_mcbsp_st_init(struct platform_device *pdev)
 	return 0;
 }
 
-void omap_mcbsp_st_cleanup(struct platform_device *pdev)
-{
-	struct omap_mcbsp *mcbsp = platform_get_drvdata(pdev);
-
-	if (mcbsp->st_data) {
-		sysfs_remove_group(&mcbsp->dev->kobj, &sidetone_attr_group);
-		clk_put(mcbsp->st_data->mcbsp_iclk);
-	}
-}
-
 static int omap_mcbsp_st_info_volsw(struct snd_kcontrol *kcontrol,
 				    struct snd_ctl_elem_info *uinfo)
 {
diff --git a/sound/soc/ti/omap-mcbsp.c b/sound/soc/ti/omap-mcbsp.c
index 4479d74f0a45..395493a2d965 100644
--- a/sound/soc/ti/omap-mcbsp.c
+++ b/sound/soc/ti/omap-mcbsp.c
@@ -595,16 +595,13 @@ static ssize_t dma_op_mode_store(struct device *dev,
 
 static DEVICE_ATTR_RW(dma_op_mode);
 
-static const struct attribute *additional_attrs[] = {
+static struct attribute *additional_attrs[] = {
 	&dev_attr_max_tx_thres.attr,
 	&dev_attr_max_rx_thres.attr,
 	&dev_attr_dma_op_mode.attr,
 	NULL,
 };
-
-static const struct attribute_group additional_attr_group = {
-	.attrs = (struct attribute **)additional_attrs,
-};
+ATTRIBUTE_GROUPS(additional);
 
 /*
  * McBSP1 and McBSP3 are directly mapped on 1610 and 1510.
@@ -702,8 +699,7 @@ static int omap_mcbsp_init(struct platform_device *pdev)
 		mcbsp->max_tx_thres = max_thres(mcbsp) - 0x10;
 		mcbsp->max_rx_thres = max_thres(mcbsp) - 0x10;
 
-		ret = sysfs_create_group(&mcbsp->dev->kobj,
-					 &additional_attr_group);
+		ret = devm_device_add_group(mcbsp->dev, &additional_group);
 		if (ret) {
 			dev_err(mcbsp->dev,
 				"Unable to create additional controls\n");
@@ -711,16 +707,7 @@ static int omap_mcbsp_init(struct platform_device *pdev)
 		}
 	}
 
-	ret = omap_mcbsp_st_init(pdev);
-	if (ret)
-		goto err_st;
-
-	return 0;
-
-err_st:
-	if (mcbsp->pdata->buffer_size)
-		sysfs_remove_group(&mcbsp->dev->kobj, &additional_attr_group);
-	return ret;
+	return omap_mcbsp_st_init(pdev);
 }
 
 /*
@@ -1431,11 +1418,6 @@ static int asoc_mcbsp_remove(struct platform_device *pdev)
 	if (cpu_latency_qos_request_active(&mcbsp->pm_qos_req))
 		cpu_latency_qos_remove_request(&mcbsp->pm_qos_req);
 
-	if (mcbsp->pdata->buffer_size)
-		sysfs_remove_group(&mcbsp->dev->kobj, &additional_attr_group);
-
-	omap_mcbsp_st_cleanup(pdev);
-
 	return 0;
 }
 
-- 
2.34.1

Re: [PATCH v3] ASoC: ti: omap-mcbsp: duplicate sysfs error

[Mark Brown]

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

On Thu, Jun 16, 2022 at 03:26:45PM -0500, David Owens wrote:

> sysfs: cannot create duplicate filename '/devices/platform/68000000.ocp/49022000.mcbsp/max_tx_thres'
> 
> Signed-off-by: David Owens <dowens@precisionplanting.com>
> ---

Please carry forward tags like acks unless you make changes to the patch
which would invalidate them.  No need to resend for this alone.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v3] ASoC: ti: omap-mcbsp: duplicate sysfs error

[Mark Brown]

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

On Thu, Jun 16, 2022 at 03:26:45PM -0500, David Owens wrote:
> Convert to managed versions of sysfs and clk allocation to simplify
> unbinding and error handling in probe.  Managed sysfs node
> creation specifically addresses the following error seen the second time
> probe is attempted after sdma_pcm_platform_register() previously requested
> probe deferral:

This breaks an x86 allmodconfig build:

In file included from /build/stage/linux/include/linux/kobject.h:20,
                 from /build/stage/linux/include/linux/module.h:21,
                 from /build/stage/linux/sound/soc/ti/omap-mcbsp.c:12:
/build/stage/linux/sound/soc/ti/omap-mcbsp.c:604:18: error: ‘additional_groups’ defined but not used [-Werror=unused-variable]
  604 | ATTRIBUTE_GROUPS(additional);
      |                  ^~~~~~~~~~
/build/stage/linux/include/linux/sysfs.h:154:38: note: in definition of macro ‘__ATTRIBUTE_GROUPS’
  154 | static const struct attribute_group *_name##_groups[] = {       \
      |                                      ^~~~~
/build/stage/linux/sound/soc/ti/omap-mcbsp.c:604:1: note: in expansion of macro ‘ATTRIBUTE_GROUPS’
  604 | ATTRIBUTE_GROUPS(additional);
      | ^~~~~~~~~~~~~~~~
In file included from /build/stage/linux/include/linux/kobject.h:20,
                 from /build/stage/linux/include/linux/module.h:21,
                 from /build/stage/linux/sound/soc/ti/omap-mcbsp-st.c:12:
/build/stage/linux/sound/soc/ti/omap-mcbsp-st.c:299:18: error: ‘sidetone_groups’ defined but not used [-Werror=unused-variable]
  299 | ATTRIBUTE_GROUPS(sidetone);
      |                  ^~~~~~~~
/build/stage/linux/include/linux/sysfs.h:154:38: note: in definition of macro ‘__ATTRIBUTE_GROUPS’
  154 | static const struct attribute_group *_name##_groups[] = {       \
      |                                      ^~~~~
/build/stage/linux/sound/soc/ti/omap-mcbsp-st.c:299:1: note: in expansion of macro ‘ATTRIBUTE_GROUPS’
  299 | ATTRIBUTE_GROUPS(sidetone);
      | ^~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v3] ASoC: ti: omap-mcbsp: duplicate sysfs error

[Péter Ujfalusi]

On 6/16/22 23:26, David Owens wrote:
> Convert to managed versions of sysfs and clk allocation to simplify
> unbinding and error handling in probe.  Managed sysfs node
> creation specifically addresses the following error seen the second time
> probe is attempted after sdma_pcm_platform_register() previously requested
> probe deferral> 
> sysfs: cannot create duplicate filename '/devices/platform/68000000.ocp/49022000.mcbsp/max_tx_thres'

This does not compile (on linux-next)?

In file included from /home/stens/work/kernel/linux/include/linux/kobject.h:20,
                 from /home/stens/work/kernel/linux/include/linux/module.h:21,
                 from /home/stens/work/kernel/linux/sound/soc/ti/omap-mcbsp-st.c:12:
/home/stens/work/kernel/linux/sound/soc/ti/omap-mcbsp-st.c:299:18: error: 'sidetone_groups' defined but not used [-Werror=unused-variable]
  299 | ATTRIBUTE_GROUPS(sidetone);
      |                  ^~~~~~~~
/home/stens/work/kernel/linux/include/linux/sysfs.h:154:38: note: in definition of macro '__ATTRIBUTE_GROUPS'
  154 | static const struct attribute_group *_name##_groups[] = {       \
      |                                      ^~~~~
/home/stens/work/kernel/linux/sound/soc/ti/omap-mcbsp-st.c:299:1: note: in expansion of macro 'ATTRIBUTE_GROUPS'
  299 | ATTRIBUTE_GROUPS(sidetone);
      | ^~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
make[4]: *** [/home/stens/work/kernel/linux/scripts/Makefile.build:250: sound/soc/ti/omap-mcbsp-st.o] Error 1
make[4]: *** Waiting for unfinished jobs....
In file included from /home/stens/work/kernel/linux/include/linux/kobject.h:20,
                 from /home/stens/work/kernel/linux/include/linux/module.h:21,
                 from /home/stens/work/kernel/linux/sound/soc/ti/omap-mcbsp.c:12:
/home/stens/work/kernel/linux/sound/soc/ti/omap-mcbsp.c:604:18: error: 'additional_groups' defined but not used [-Werror=unused-variable]
  604 | ATTRIBUTE_GROUPS(additional);
      |                  ^~~~~~~~~~
/home/stens/work/kernel/linux/include/linux/sysfs.h:154:38: note: in definition of macro '__ATTRIBUTE_GROUPS'
  154 | static const struct attribute_group *_name##_groups[] = {       \
      |                                      ^~~~~
/home/stens/work/kernel/linux/sound/soc/ti/omap-mcbsp.c:604:1: note: in expansion of macro 'ATTRIBUTE_GROUPS'
  604 | ATTRIBUTE_GROUPS(additional);
      | ^~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors


> 
> Signed-off-by: David Owens <dowens@precisionplanting.com>
> ---
> 
> Changes in v3:
>  * Whitespace changes only to allow clean applly
> 
> Changes in v2:
>  * Improved error handling
> 
> ---
>  sound/soc/ti/omap-mcbsp-priv.h |  2 --
>  sound/soc/ti/omap-mcbsp-st.c   | 21 ++++-----------------
>  sound/soc/ti/omap-mcbsp.c      | 26 ++++----------------------
>  3 files changed, 8 insertions(+), 41 deletions(-)
> 
> diff --git a/sound/soc/ti/omap-mcbsp-priv.h b/sound/soc/ti/omap-mcbsp-priv.h
> index 7865cda4bf0a..da519ea1f303 100644
> --- a/sound/soc/ti/omap-mcbsp-priv.h
> +++ b/sound/soc/ti/omap-mcbsp-priv.h
> @@ -316,8 +316,6 @@ static inline int omap_mcbsp_read(struct omap_mcbsp *mcbsp, u16 reg,
>  
>  /* Sidetone specific API */
>  int omap_mcbsp_st_init(struct platform_device *pdev);
> -void omap_mcbsp_st_cleanup(struct platform_device *pdev);
> -
>  int omap_mcbsp_st_start(struct omap_mcbsp *mcbsp);
>  int omap_mcbsp_st_stop(struct omap_mcbsp *mcbsp);
>  
> diff --git a/sound/soc/ti/omap-mcbsp-st.c b/sound/soc/ti/omap-mcbsp-st.c
> index 0bc7d26c660a..402a57a502e6 100644
> --- a/sound/soc/ti/omap-mcbsp-st.c
> +++ b/sound/soc/ti/omap-mcbsp-st.c
> @@ -292,14 +292,11 @@ static ssize_t st_taps_store(struct device *dev,
>  
>  static DEVICE_ATTR_RW(st_taps);
>  
> -static const struct attribute *sidetone_attrs[] = {
> +static struct attribute *sidetone_attrs[] = {
>  	&dev_attr_st_taps.attr,
>  	NULL,
>  };
> -
> -static const struct attribute_group sidetone_attr_group = {
> -	.attrs = (struct attribute **)sidetone_attrs,
> -};
> +ATTRIBUTE_GROUPS(sidetone);
>  
>  int omap_mcbsp_st_start(struct omap_mcbsp *mcbsp)
>  {
> @@ -347,7 +344,7 @@ int omap_mcbsp_st_init(struct platform_device *pdev)
>  	if (!st_data)
>  		return -ENOMEM;
>  
> -	st_data->mcbsp_iclk = clk_get(mcbsp->dev, "ick");
> +	st_data->mcbsp_iclk = devm_clk_get(mcbsp->dev, "ick");
>  	if (IS_ERR(st_data->mcbsp_iclk)) {
>  		dev_warn(mcbsp->dev,
>  			 "Failed to get ick, sidetone might be broken\n");
> @@ -359,7 +356,7 @@ int omap_mcbsp_st_init(struct platform_device *pdev)
>  	if (!st_data->io_base_st)
>  		return -ENOMEM;
>  
> -	ret = sysfs_create_group(&mcbsp->dev->kobj, &sidetone_attr_group);
> +	ret = devm_device_add_group(mcbsp->dev, &sidetone_group);
>  	if (ret)
>  		return ret;
>  
> @@ -368,16 +365,6 @@ int omap_mcbsp_st_init(struct platform_device *pdev)
>  	return 0;
>  }
>  
> -void omap_mcbsp_st_cleanup(struct platform_device *pdev)
> -{
> -	struct omap_mcbsp *mcbsp = platform_get_drvdata(pdev);
> -
> -	if (mcbsp->st_data) {
> -		sysfs_remove_group(&mcbsp->dev->kobj, &sidetone_attr_group);
> -		clk_put(mcbsp->st_data->mcbsp_iclk);
> -	}
> -}
> -
>  static int omap_mcbsp_st_info_volsw(struct snd_kcontrol *kcontrol,
>  				    struct snd_ctl_elem_info *uinfo)
>  {
> diff --git a/sound/soc/ti/omap-mcbsp.c b/sound/soc/ti/omap-mcbsp.c
> index 4479d74f0a45..395493a2d965 100644
> --- a/sound/soc/ti/omap-mcbsp.c
> +++ b/sound/soc/ti/omap-mcbsp.c
> @@ -595,16 +595,13 @@ static ssize_t dma_op_mode_store(struct device *dev,
>  
>  static DEVICE_ATTR_RW(dma_op_mode);
>  
> -static const struct attribute *additional_attrs[] = {
> +static struct attribute *additional_attrs[] = {
>  	&dev_attr_max_tx_thres.attr,
>  	&dev_attr_max_rx_thres.attr,
>  	&dev_attr_dma_op_mode.attr,
>  	NULL,
>  };
> -
> -static const struct attribute_group additional_attr_group = {
> -	.attrs = (struct attribute **)additional_attrs,
> -};
> +ATTRIBUTE_GROUPS(additional);
>  
>  /*
>   * McBSP1 and McBSP3 are directly mapped on 1610 and 1510.
> @@ -702,8 +699,7 @@ static int omap_mcbsp_init(struct platform_device *pdev)
>  		mcbsp->max_tx_thres = max_thres(mcbsp) - 0x10;
>  		mcbsp->max_rx_thres = max_thres(mcbsp) - 0x10;
>  
> -		ret = sysfs_create_group(&mcbsp->dev->kobj,
> -					 &additional_attr_group);
> +		ret = devm_device_add_group(mcbsp->dev, &additional_group);
>  		if (ret) {
>  			dev_err(mcbsp->dev,
>  				"Unable to create additional controls\n");
> @@ -711,16 +707,7 @@ static int omap_mcbsp_init(struct platform_device *pdev)
>  		}
>  	}
>  
> -	ret = omap_mcbsp_st_init(pdev);
> -	if (ret)
> -		goto err_st;
> -
> -	return 0;
> -
> -err_st:
> -	if (mcbsp->pdata->buffer_size)
> -		sysfs_remove_group(&mcbsp->dev->kobj, &additional_attr_group);
> -	return ret;
> +	return omap_mcbsp_st_init(pdev);
>  }
>  
>  /*
> @@ -1431,11 +1418,6 @@ static int asoc_mcbsp_remove(struct platform_device *pdev)
>  	if (cpu_latency_qos_request_active(&mcbsp->pm_qos_req))
>  		cpu_latency_qos_remove_request(&mcbsp->pm_qos_req);
>  
> -	if (mcbsp->pdata->buffer_size)
> -		sysfs_remove_group(&mcbsp->dev->kobj, &additional_attr_group);
> -
> -	omap_mcbsp_st_cleanup(pdev);
> -
>  	return 0;
>  }
>  

-- 
Péter

Re: [PATCH v3] ASoC: ti: omap-mcbsp: duplicate sysfs error

[Péter Ujfalusi]

On 6/18/22 09:26, Péter Ujfalusi wrote:
> 
> 
> On 6/16/22 23:26, David Owens wrote:
>> Convert to managed versions of sysfs and clk allocation to simplify
>> unbinding and error handling in probe.  Managed sysfs node
>> creation specifically addresses the following error seen the second time
>> probe is attempted after sdma_pcm_platform_register() previously requested
>> probe deferral> 
>> sysfs: cannot create duplicate filename '/devices/platform/68000000.ocp/49022000.mcbsp/max_tx_thres'
> 
> This does not compile (on linux-next)?

The foillowing diff is needed:

diff --git a/sound/soc/ti/omap-mcbsp-st.c b/sound/soc/ti/omap-mcbsp-st.c
index 402a57a502e6..bae4f67165c1 100644
--- a/sound/soc/ti/omap-mcbsp-st.c
+++ b/sound/soc/ti/omap-mcbsp-st.c
@@ -296,7 +296,10 @@ static struct attribute *sidetone_attrs[] = {
 	&dev_attr_st_taps.attr,
 	NULL,
 };
-ATTRIBUTE_GROUPS(sidetone);
+
+static const struct attribute_group sidetone_group = {
+	.attrs = sidetone_attrs,
+};

 int omap_mcbsp_st_start(struct omap_mcbsp *mcbsp)
 {
diff --git a/sound/soc/ti/omap-mcbsp.c b/sound/soc/ti/omap-mcbsp.c
index 176abe4271e9..6fcd860778e9 100644
--- a/sound/soc/ti/omap-mcbsp.c
+++ b/sound/soc/ti/omap-mcbsp.c
@@ -601,7 +601,10 @@ static struct attribute *additional_attrs[] = {
 	&dev_attr_dma_op_mode.attr,
 	NULL,
 };
-ATTRIBUTE_GROUPS(additional);
+
+static const struct attribute_group additional_group = {
+	.attrs = additional_attrs,
+};

 /*
  * McBSP1 and McBSP3 are directly mapped on 1610 and 1510.




> 
> In file included from /home/stens/work/kernel/linux/include/linux/kobject.h:20,
>                  from /home/stens/work/kernel/linux/include/linux/module.h:21,
>                  from /home/stens/work/kernel/linux/sound/soc/ti/omap-mcbsp-st.c:12:
> /home/stens/work/kernel/linux/sound/soc/ti/omap-mcbsp-st.c:299:18: error: 'sidetone_groups' defined but not used [-Werror=unused-variable]
>   299 | ATTRIBUTE_GROUPS(sidetone);
>       |                  ^~~~~~~~
> /home/stens/work/kernel/linux/include/linux/sysfs.h:154:38: note: in definition of macro '__ATTRIBUTE_GROUPS'
>   154 | static const struct attribute_group *_name##_groups[] = {       \
>       |                                      ^~~~~
> /home/stens/work/kernel/linux/sound/soc/ti/omap-mcbsp-st.c:299:1: note: in expansion of macro 'ATTRIBUTE_GROUPS'
>   299 | ATTRIBUTE_GROUPS(sidetone);
>       | ^~~~~~~~~~~~~~~~
> cc1: all warnings being treated as errors
> make[4]: *** [/home/stens/work/kernel/linux/scripts/Makefile.build:250: sound/soc/ti/omap-mcbsp-st.o] Error 1
> make[4]: *** Waiting for unfinished jobs....
> In file included from /home/stens/work/kernel/linux/include/linux/kobject.h:20,
>                  from /home/stens/work/kernel/linux/include/linux/module.h:21,
>                  from /home/stens/work/kernel/linux/sound/soc/ti/omap-mcbsp.c:12:
> /home/stens/work/kernel/linux/sound/soc/ti/omap-mcbsp.c:604:18: error: 'additional_groups' defined but not used [-Werror=unused-variable]
>   604 | ATTRIBUTE_GROUPS(additional);
>       |                  ^~~~~~~~~~
> /home/stens/work/kernel/linux/include/linux/sysfs.h:154:38: note: in definition of macro '__ATTRIBUTE_GROUPS'
>   154 | static const struct attribute_group *_name##_groups[] = {       \
>       |                                      ^~~~~
> /home/stens/work/kernel/linux/sound/soc/ti/omap-mcbsp.c:604:1: note: in expansion of macro 'ATTRIBUTE_GROUPS'
>   604 | ATTRIBUTE_GROUPS(additional);
>       | ^~~~~~~~~~~~~~~~
> cc1: all warnings being treated as errors
> 
> 
>>
>> Signed-off-by: David Owens <dowens@precisionplanting.com>
>> ---
>>
>> Changes in v3:
>>  * Whitespace changes only to allow clean applly
>>
>> Changes in v2:
>>  * Improved error handling
>>
>> ---
>>  sound/soc/ti/omap-mcbsp-priv.h |  2 --
>>  sound/soc/ti/omap-mcbsp-st.c   | 21 ++++-----------------
>>  sound/soc/ti/omap-mcbsp.c      | 26 ++++----------------------
>>  3 files changed, 8 insertions(+), 41 deletions(-)
>>
>> diff --git a/sound/soc/ti/omap-mcbsp-priv.h b/sound/soc/ti/omap-mcbsp-priv.h
>> index 7865cda4bf0a..da519ea1f303 100644
>> --- a/sound/soc/ti/omap-mcbsp-priv.h
>> +++ b/sound/soc/ti/omap-mcbsp-priv.h
>> @@ -316,8 +316,6 @@ static inline int omap_mcbsp_read(struct omap_mcbsp *mcbsp, u16 reg,
>>  
>>  /* Sidetone specific API */
>>  int omap_mcbsp_st_init(struct platform_device *pdev);
>> -void omap_mcbsp_st_cleanup(struct platform_device *pdev);
>> -
>>  int omap_mcbsp_st_start(struct omap_mcbsp *mcbsp);
>>  int omap_mcbsp_st_stop(struct omap_mcbsp *mcbsp);
>>  
>> diff --git a/sound/soc/ti/omap-mcbsp-st.c b/sound/soc/ti/omap-mcbsp-st.c
>> index 0bc7d26c660a..402a57a502e6 100644
>> --- a/sound/soc/ti/omap-mcbsp-st.c
>> +++ b/sound/soc/ti/omap-mcbsp-st.c
>> @@ -292,14 +292,11 @@ static ssize_t st_taps_store(struct device *dev,
>>  
>>  static DEVICE_ATTR_RW(st_taps);
>>  
>> -static const struct attribute *sidetone_attrs[] = {
>> +static struct attribute *sidetone_attrs[] = {
>>  	&dev_attr_st_taps.attr,
>>  	NULL,
>>  };
>> -
>> -static const struct attribute_group sidetone_attr_group = {
>> -	.attrs = (struct attribute **)sidetone_attrs,
>> -};
>> +ATTRIBUTE_GROUPS(sidetone);
>>  
>>  int omap_mcbsp_st_start(struct omap_mcbsp *mcbsp)
>>  {
>> @@ -347,7 +344,7 @@ int omap_mcbsp_st_init(struct platform_device *pdev)
>>  	if (!st_data)
>>  		return -ENOMEM;
>>  
>> -	st_data->mcbsp_iclk = clk_get(mcbsp->dev, "ick");
>> +	st_data->mcbsp_iclk = devm_clk_get(mcbsp->dev, "ick");
>>  	if (IS_ERR(st_data->mcbsp_iclk)) {
>>  		dev_warn(mcbsp->dev,
>>  			 "Failed to get ick, sidetone might be broken\n");
>> @@ -359,7 +356,7 @@ int omap_mcbsp_st_init(struct platform_device *pdev)
>>  	if (!st_data->io_base_st)
>>  		return -ENOMEM;
>>  
>> -	ret = sysfs_create_group(&mcbsp->dev->kobj, &sidetone_attr_group);
>> +	ret = devm_device_add_group(mcbsp->dev, &sidetone_group);
>>  	if (ret)
>>  		return ret;
>>  
>> @@ -368,16 +365,6 @@ int omap_mcbsp_st_init(struct platform_device *pdev)
>>  	return 0;
>>  }
>>  
>> -void omap_mcbsp_st_cleanup(struct platform_device *pdev)
>> -{
>> -	struct omap_mcbsp *mcbsp = platform_get_drvdata(pdev);
>> -
>> -	if (mcbsp->st_data) {
>> -		sysfs_remove_group(&mcbsp->dev->kobj, &sidetone_attr_group);
>> -		clk_put(mcbsp->st_data->mcbsp_iclk);
>> -	}
>> -}
>> -
>>  static int omap_mcbsp_st_info_volsw(struct snd_kcontrol *kcontrol,
>>  				    struct snd_ctl_elem_info *uinfo)
>>  {
>> diff --git a/sound/soc/ti/omap-mcbsp.c b/sound/soc/ti/omap-mcbsp.c
>> index 4479d74f0a45..395493a2d965 100644
>> --- a/sound/soc/ti/omap-mcbsp.c
>> +++ b/sound/soc/ti/omap-mcbsp.c
>> @@ -595,16 +595,13 @@ static ssize_t dma_op_mode_store(struct device *dev,
>>  
>>  static DEVICE_ATTR_RW(dma_op_mode);
>>  
>> -static const struct attribute *additional_attrs[] = {
>> +static struct attribute *additional_attrs[] = {
>>  	&dev_attr_max_tx_thres.attr,
>>  	&dev_attr_max_rx_thres.attr,
>>  	&dev_attr_dma_op_mode.attr,
>>  	NULL,
>>  };
>> -
>> -static const struct attribute_group additional_attr_group = {
>> -	.attrs = (struct attribute **)additional_attrs,
>> -};
>> +ATTRIBUTE_GROUPS(additional);
>>  
>>  /*
>>   * McBSP1 and McBSP3 are directly mapped on 1610 and 1510.
>> @@ -702,8 +699,7 @@ static int omap_mcbsp_init(struct platform_device *pdev)
>>  		mcbsp->max_tx_thres = max_thres(mcbsp) - 0x10;
>>  		mcbsp->max_rx_thres = max_thres(mcbsp) - 0x10;
>>  
>> -		ret = sysfs_create_group(&mcbsp->dev->kobj,
>> -					 &additional_attr_group);
>> +		ret = devm_device_add_group(mcbsp->dev, &additional_group);
>>  		if (ret) {
>>  			dev_err(mcbsp->dev,
>>  				"Unable to create additional controls\n");
>> @@ -711,16 +707,7 @@ static int omap_mcbsp_init(struct platform_device *pdev)
>>  		}
>>  	}
>>  
>> -	ret = omap_mcbsp_st_init(pdev);
>> -	if (ret)
>> -		goto err_st;
>> -
>> -	return 0;
>> -
>> -err_st:
>> -	if (mcbsp->pdata->buffer_size)
>> -		sysfs_remove_group(&mcbsp->dev->kobj, &additional_attr_group);
>> -	return ret;
>> +	return omap_mcbsp_st_init(pdev);
>>  }
>>  
>>  /*
>> @@ -1431,11 +1418,6 @@ static int asoc_mcbsp_remove(struct platform_device *pdev)
>>  	if (cpu_latency_qos_request_active(&mcbsp->pm_qos_req))
>>  		cpu_latency_qos_remove_request(&mcbsp->pm_qos_req);
>>  
>> -	if (mcbsp->pdata->buffer_size)
>> -		sysfs_remove_group(&mcbsp->dev->kobj, &additional_attr_group);
>> -
>> -	omap_mcbsp_st_cleanup(pdev);
>> -
>>  	return 0;
>>  }
>>  
> 

-- 
Péter

Re: [PATCH v3] ASoC: ti: omap-mcbsp: duplicate sysfs error

[David Owens]

On 6/18/22 01:36, Péter Ujfalusi wrote:

>
> On 6/18/22 09:26, Péter Ujfalusi wrote:
>>
>> On 6/16/22 23:26, David Owens wrote:
>>> Convert to managed versions of sysfs and clk allocation to simplify
>>> unbinding and error handling in probe.  Managed sysfs node
>>> creation specifically addresses the following error seen the second time
>>> probe is attempted after sdma_pcm_platform_register() previously requested
>>> probe deferral>
>>> sysfs: cannot create duplicate filename '/devices/platform/68000000.ocp/49022000.mcbsp/max_tx_thres'
>> This does not compile (on linux-next)?
> The foillowing diff is needed:
>
> diff --git a/sound/soc/ti/omap-mcbsp-st.c b/sound/soc/ti/omap-mcbsp-st.c
> index 402a57a502e6..bae4f67165c1 100644
> --- a/sound/soc/ti/omap-mcbsp-st.c
> +++ b/sound/soc/ti/omap-mcbsp-st.c
> @@ -296,7 +296,10 @@ static struct attribute *sidetone_attrs[] = {
>       &dev_attr_st_taps.attr,
>       NULL,
>  };
> -ATTRIBUTE_GROUPS(sidetone);
> +
> +static const struct attribute_group sidetone_group = {
> +     .attrs = sidetone_attrs,
> +};
>
>  int omap_mcbsp_st_start(struct omap_mcbsp *mcbsp)
>  {
> diff --git a/sound/soc/ti/omap-mcbsp.c b/sound/soc/ti/omap-mcbsp.c
> index 176abe4271e9..6fcd860778e9 100644
> --- a/sound/soc/ti/omap-mcbsp.c
> +++ b/sound/soc/ti/omap-mcbsp.c
> @@ -601,7 +601,10 @@ static struct attribute *additional_attrs[] = {
>       &dev_attr_dma_op_mode.attr,
>       NULL,
>  };
> -ATTRIBUTE_GROUPS(additional);
> +
> +static const struct attribute_group additional_group = {
> +     .attrs = additional_attrs,
> +};
>
>  /*
>   * McBSP1 and McBSP3 are directly mapped on 1610 and 1510.

Thanks for the recommendation, Péter.  I was under the impression ATTRIBUTE_GROUPS() resolved to the same attribute_group struct as the change above but perhaps that's not the case.  I'll update the patch and resubmit.

-Dave

>
>
>
>> In file included from /home/stens/work/kernel/linux/include/linux/kobject.h:20,
>>                  from /home/stens/work/kernel/linux/include/linux/module.h:21,
>>                  from /home/stens/work/kernel/linux/sound/soc/ti/omap-mcbsp-st.c:12:
>> /home/stens/work/kernel/linux/sound/soc/ti/omap-mcbsp-st.c:299:18: error: 'sidetone_groups' defined but not used [-Werror=unused-variable]
>>   299 | ATTRIBUTE_GROUPS(sidetone);
>>       |                  ^~~~~~~~
>> /home/stens/work/kernel/linux/include/linux/sysfs.h:154:38: note: in definition of macro '__ATTRIBUTE_GROUPS'
>>   154 | static const struct attribute_group *_name##_groups[] = {       \
>>       |                                      ^~~~~
>> /home/stens/work/kernel/linux/sound/soc/ti/omap-mcbsp-st.c:299:1: note: in expansion of macro 'ATTRIBUTE_GROUPS'
>>   299 | ATTRIBUTE_GROUPS(sidetone);
>>       | ^~~~~~~~~~~~~~~~
>> cc1: all warnings being treated as errors
>> make[4]: *** [/home/stens/work/kernel/linux/scripts/Makefile.build:250: sound/soc/ti/omap-mcbsp-st.o] Error 1
>> make[4]: *** Waiting for unfinished jobs....
>> In file included from /home/stens/work/kernel/linux/include/linux/kobject.h:20,
>>                  from /home/stens/work/kernel/linux/include/linux/module.h:21,
>>                  from /home/stens/work/kernel/linux/sound/soc/ti/omap-mcbsp.c:12:
>> /home/stens/work/kernel/linux/sound/soc/ti/omap-mcbsp.c:604:18: error: 'additional_groups' defined but not used [-Werror=unused-variable]
>>   604 | ATTRIBUTE_GROUPS(additional);
>>       |                  ^~~~~~~~~~
>> /home/stens/work/kernel/linux/include/linux/sysfs.h:154:38: note: in definition of macro '__ATTRIBUTE_GROUPS'
>>   154 | static const struct attribute_group *_name##_groups[] = {       \
>>       |                                      ^~~~~
>> /home/stens/work/kernel/linux/sound/soc/ti/omap-mcbsp.c:604:1: note: in expansion of macro 'ATTRIBUTE_GROUPS'
>>   604 | ATTRIBUTE_GROUPS(additional);
>>       | ^~~~~~~~~~~~~~~~
>> cc1: all warnings being treated as errors
>>
>>
>>> Signed-off-by: David Owens <dowens@precisionplanting.com>
>>> ---
>>>
>>> Changes in v3:
>>>  * Whitespace changes only to allow clean applly
>>>
>>> Changes in v2:
>>>  * Improved error handling
>>>
>>> ---
>>>  sound/soc/ti/omap-mcbsp-priv.h |  2 --
>>>  sound/soc/ti/omap-mcbsp-st.c   | 21 ++++-----------------
>>>  sound/soc/ti/omap-mcbsp.c      | 26 ++++----------------------
>>>  3 files changed, 8 insertions(+), 41 deletions(-)
>>>
>>> diff --git a/sound/soc/ti/omap-mcbsp-priv.h b/sound/soc/ti/omap-mcbsp-priv.h
>>> index 7865cda4bf0a..da519ea1f303 100644
>>> --- a/sound/soc/ti/omap-mcbsp-priv.h
>>> +++ b/sound/soc/ti/omap-mcbsp-priv.h
>>> @@ -316,8 +316,6 @@ static inline int omap_mcbsp_read(struct omap_mcbsp *mcbsp, u16 reg,
>>>
>>>  /* Sidetone specific API */
>>>  int omap_mcbsp_st_init(struct platform_device *pdev);
>>> -void omap_mcbsp_st_cleanup(struct platform_device *pdev);
>>> -
>>>  int omap_mcbsp_st_start(struct omap_mcbsp *mcbsp);
>>>  int omap_mcbsp_st_stop(struct omap_mcbsp *mcbsp);
>>>
>>> diff --git a/sound/soc/ti/omap-mcbsp-st.c b/sound/soc/ti/omap-mcbsp-st.c
>>> index 0bc7d26c660a..402a57a502e6 100644
>>> --- a/sound/soc/ti/omap-mcbsp-st.c
>>> +++ b/sound/soc/ti/omap-mcbsp-st.c
>>> @@ -292,14 +292,11 @@ static ssize_t st_taps_store(struct device *dev,
>>>
>>>  static DEVICE_ATTR_RW(st_taps);
>>>
>>> -static const struct attribute *sidetone_attrs[] = {
>>> +static struct attribute *sidetone_attrs[] = {
>>>     &dev_attr_st_taps.attr,
>>>     NULL,
>>>  };
>>> -
>>> -static const struct attribute_group sidetone_attr_group = {
>>> -   .attrs = (struct attribute **)sidetone_attrs,
>>> -};
>>> +ATTRIBUTE_GROUPS(sidetone);
>>>
>>>  int omap_mcbsp_st_start(struct omap_mcbsp *mcbsp)
>>>  {
>>> @@ -347,7 +344,7 @@ int omap_mcbsp_st_init(struct platform_device *pdev)
>>>     if (!st_data)
>>>             return -ENOMEM;
>>>
>>> -   st_data->mcbsp_iclk = clk_get(mcbsp->dev, "ick");
>>> +   st_data->mcbsp_iclk = devm_clk_get(mcbsp->dev, "ick");
>>>     if (IS_ERR(st_data->mcbsp_iclk)) {
>>>             dev_warn(mcbsp->dev,
>>>                      "Failed to get ick, sidetone might be broken\n");
>>> @@ -359,7 +356,7 @@ int omap_mcbsp_st_init(struct platform_device *pdev)
>>>     if (!st_data->io_base_st)
>>>             return -ENOMEM;
>>>
>>> -   ret = sysfs_create_group(&mcbsp->dev->kobj, &sidetone_attr_group);
>>> +   ret = devm_device_add_group(mcbsp->dev, &sidetone_group);
>>>     if (ret)
>>>             return ret;
>>>
>>> @@ -368,16 +365,6 @@ int omap_mcbsp_st_init(struct platform_device *pdev)
>>>     return 0;
>>>  }
>>>
>>> -void omap_mcbsp_st_cleanup(struct platform_device *pdev)
>>> -{
>>> -   struct omap_mcbsp *mcbsp = platform_get_drvdata(pdev);
>>> -
>>> -   if (mcbsp->st_data) {
>>> -           sysfs_remove_group(&mcbsp->dev->kobj, &sidetone_attr_group);
>>> -           clk_put(mcbsp->st_data->mcbsp_iclk);
>>> -   }
>>> -}
>>> -
>>>  static int omap_mcbsp_st_info_volsw(struct snd_kcontrol *kcontrol,
>>>                                 struct snd_ctl_elem_info *uinfo)
>>>  {
>>> diff --git a/sound/soc/ti/omap-mcbsp.c b/sound/soc/ti/omap-mcbsp.c
>>> index 4479d74f0a45..395493a2d965 100644
>>> --- a/sound/soc/ti/omap-mcbsp.c
>>> +++ b/sound/soc/ti/omap-mcbsp.c
>>> @@ -595,16 +595,13 @@ static ssize_t dma_op_mode_store(struct device *dev,
>>>
>>>  static DEVICE_ATTR_RW(dma_op_mode);
>>>
>>> -static const struct attribute *additional_attrs[] = {
>>> +static struct attribute *additional_attrs[] = {
>>>     &dev_attr_max_tx_thres.attr,
>>>     &dev_attr_max_rx_thres.attr,
>>>     &dev_attr_dma_op_mode.attr,
>>>     NULL,
>>>  };
>>> -
>>> -static const struct attribute_group additional_attr_group = {
>>> -   .attrs = (struct attribute **)additional_attrs,
>>> -};
>>> +ATTRIBUTE_GROUPS(additional);
>>>
>>>  /*
>>>   * McBSP1 and McBSP3 are directly mapped on 1610 and 1510.
>>> @@ -702,8 +699,7 @@ static int omap_mcbsp_init(struct platform_device *pdev)
>>>             mcbsp->max_tx_thres = max_thres(mcbsp) - 0x10;
>>>             mcbsp->max_rx_thres = max_thres(mcbsp) - 0x10;
>>>
>>> -           ret = sysfs_create_group(&mcbsp->dev->kobj,
>>> -                                    &additional_attr_group);
>>> +           ret = devm_device_add_group(mcbsp->dev, &additional_group);
>>>             if (ret) {
>>>                     dev_err(mcbsp->dev,
>>>                             "Unable to create additional controls\n");
>>> @@ -711,16 +707,7 @@ static int omap_mcbsp_init(struct platform_device *pdev)
>>>             }
>>>     }
>>>
>>> -   ret = omap_mcbsp_st_init(pdev);
>>> -   if (ret)
>>> -           goto err_st;
>>> -
>>> -   return 0;
>>> -
>>> -err_st:
>>> -   if (mcbsp->pdata->buffer_size)
>>> -           sysfs_remove_group(&mcbsp->dev->kobj, &additional_attr_group);
>>> -   return ret;
>>> +   return omap_mcbsp_st_init(pdev);
>>>  }
>>>
>>>  /*
>>> @@ -1431,11 +1418,6 @@ static int asoc_mcbsp_remove(struct platform_device *pdev)
>>>     if (cpu_latency_qos_request_active(&mcbsp->pm_qos_req))
>>>             cpu_latency_qos_remove_request(&mcbsp->pm_qos_req);
>>>
>>> -   if (mcbsp->pdata->buffer_size)
>>> -           sysfs_remove_group(&mcbsp->dev->kobj, &additional_attr_group);
>>> -
>>> -   omap_mcbsp_st_cleanup(pdev);
>>> -
>>>     return 0;
>>>  }
>>>
This email is intended solely for the use of the individual to whom it is addressed and may contain confidential and/or privileged material. Any views or opinions presented are solely those of the author and do not necessarily represent those of Precision Planting. If you are not the intended recipient, be advised that you have received this email in error and that any use, dissemination, forwarding, printing, or copying of this email is strictly prohibited. Neither AGCO nor the sender accepts any responsibility for viruses, and it is your responsibility to scan, and virus check the e-mail and its attachment(s) (if any).

Re: [PATCH v3] ASoC: ti: omap-mcbsp: duplicate sysfs error

[Mark Brown]

On Thu, 16 Jun 2022 15:26:45 -0500, David Owens wrote:
> Convert to managed versions of sysfs and clk allocation to simplify
> unbinding and error handling in probe.  Managed sysfs node
> creation specifically addresses the following error seen the second time
> probe is attempted after sdma_pcm_platform_register() previously requested
> probe deferral:
> 
> sysfs: cannot create duplicate filename '/devices/platform/68000000.ocp/49022000.mcbsp/max_tx_thres'
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[1/1] ASoC: ti: omap-mcbsp: duplicate sysfs error
      commit: 6d31e225742a1955db8a0c6f6f52beb748a2895e

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark