On Wed, Nov 4, 2026 at 6:48 PM Valerio Setti <[email protected]> wrote: > > The 'codec_clk' is only used in 'probe' to read its rate, which is then > cached in clk_frequency and used by 'imx_sgtl5000_dai_init()'. Holding the > clk until 'remove()' serves no purpose. > > Release the clock right after reading its rate. This allows turning > 'codec_clk' into a local variable, simplifying the probe error path and > dropping the 'remove()' callback, which has nothing left to do. > > Signed-off-by: Valerio Setti <[email protected]>
Reviewed-by: Shengjiu Wang <[email protected]> Best regards Shengjiu Wang > --- > While reviewing [1] I realized that the driver could be simplified a bit. > 'codec_clk' is only used in the 'probe()' function with 'clk_get_rate()', > so it can be a local variable there. Also it must be put when no more > used, still in the 'probe()' function. > This allows to remove 'imx_sgtl5000_remove()'. > > This patch does not depend on [1] and they are addressing different things. > The two should apply cleanly on 7.4-rc1. > > [1] https://lore.kernel.org/all/[email protected]/ > --- > sound/soc/fsl/imx-sgtl5000.c | 24 +++++++----------------- > 1 file changed, 7 insertions(+), 17 deletions(-) > > diff --git a/sound/soc/fsl/imx-sgtl5000.c b/sound/soc/fsl/imx-sgtl5000.c > index 8bcf54ef709e..032ea3e066bd 100644 > --- a/sound/soc/fsl/imx-sgtl5000.c > +++ b/sound/soc/fsl/imx-sgtl5000.c > @@ -20,7 +20,6 @@ struct imx_sgtl5000_data { > struct snd_soc_card card; > char codec_dai_name[DAI_NAME_SIZE]; > char platform_name[DAI_NAME_SIZE]; > - struct clk *codec_clk; > unsigned int clk_frequency; > }; > > @@ -54,8 +53,9 @@ static int imx_sgtl5000_probe(struct platform_device *pdev) > struct device_node *ssi_np, *codec_np; > struct platform_device *ssi_pdev; > struct i2c_client *codec_dev; > - struct imx_sgtl5000_data *data = NULL; > + struct imx_sgtl5000_data *data; > struct snd_soc_dai_link_component *comp; > + struct clk *codec_clk; > int int_port, ext_port; > int ret; > > @@ -129,13 +129,14 @@ static int imx_sgtl5000_probe(struct platform_device > *pdev) > goto put_device; > } > > - data->codec_clk = clk_get(&codec_dev->dev, NULL); > - if (IS_ERR(data->codec_clk)) { > - ret = PTR_ERR(data->codec_clk); > + codec_clk = clk_get(&codec_dev->dev, NULL); > + if (IS_ERR(codec_clk)) { > + ret = PTR_ERR(codec_clk); > goto put_device; > } > > - data->clk_frequency = clk_get_rate(data->codec_clk); > + data->clk_frequency = clk_get_rate(codec_clk); > + clk_put(codec_clk); > > data->dai.cpus = &comp[0]; > data->dai.codecs = &comp[1]; > @@ -185,22 +186,12 @@ static int imx_sgtl5000_probe(struct platform_device > *pdev) > put_device: > put_device(&codec_dev->dev); > fail: > - if (data && !IS_ERR(data->codec_clk)) > - clk_put(data->codec_clk); > of_node_put(ssi_np); > of_node_put(codec_np); > > return ret; > } > > -static void imx_sgtl5000_remove(struct platform_device *pdev) > -{ > - struct snd_soc_card *card = platform_get_drvdata(pdev); > - struct imx_sgtl5000_data *data = snd_soc_card_get_drvdata(card); > - > - clk_put(data->codec_clk); > -} > - > static const struct of_device_id imx_sgtl5000_dt_ids[] = { > { .compatible = "fsl,imx-audio-sgtl5000", }, > { /* sentinel */ } > @@ -214,7 +205,6 @@ static struct platform_driver imx_sgtl5000_driver = { > .of_match_table = imx_sgtl5000_dt_ids, > }, > .probe = imx_sgtl5000_probe, > - .remove = imx_sgtl5000_remove, > }; > module_platform_driver(imx_sgtl5000_driver); > > > --- > base-commit: 6628e7d4c6b25c8d18f1345e3a20d21829aae7e4 > change-id: 20261104-imx-sgtl5000-cleanup-371e704e5de6 > > Best regards, > -- > Valerio Setti <[email protected]> >
