Re: [PATCH v3] ASoC: ti: omap-mcbsp: duplicate sysfs error
From: Péter Ujfalusi
Date: Sat Jun 18 2022 - 02:29:26 EST
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@xxxxxxxxxxxxxxxxxxxxx>
>> ---
>>
>> 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