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@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
