RE: [PATCH v2 3/3] ASoC: codecs: fix port_ready[] dynamic allocation in ASoC codecs

From: Liao, Bard
Date: Fri Aug 28 2020 - 04:25:58 EST


Hi Mark,

> -----Original Message-----
> From: Bard Liao <yung-chuan.liao@xxxxxxxxxxxxxxx>
> Sent: Friday, August 21, 2020 2:27 AM
> To: alsa-devel@xxxxxxxxxxxxxxxx; vkoul@xxxxxxxxxx
> Cc: vinod.koul@xxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; tiwai@xxxxxxx;
> broonie@xxxxxxxxxx; gregkh@xxxxxxxxxxxxxxxxxxx; jank@xxxxxxxxxxx;
> srinivas.kandagatla@xxxxxxxxxx; rander.wang@xxxxxxxxxxxxxxx;
> ranjani.sridharan@xxxxxxxxxxxxxxx; hui.wang@xxxxxxxxxxxxx; pierre-
> louis.bossart@xxxxxxxxxxxxxxx; Kale, Sanyog R <sanyog.r.kale@xxxxxxxxx>; Lin,
> Mengdong <mengdong.lin@xxxxxxxxx>; Liao, Bard <bard.liao@xxxxxxxxx>
> Subject: [PATCH v2 3/3] ASoC: codecs: fix port_ready[] dynamic allocation in
> ASoC codecs
>
> From: Pierre-Louis Bossart <pierre-louis.bossart@xxxxxxxxxxxxxxx>
>
> As port_ready is now changed to a fixed array in sdw.h, we can't dynamic
> allocate it in codec drivers.
>
> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@xxxxxxxxxxxxxxx>
> Reviewed-by: Rander Wang <rander.wang@xxxxxxxxxxxxxxx>
> Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski@xxxxxxxxxxxxxxx>
> Signed-off-by: Bard Liao <yung-chuan.liao@xxxxxxxxxxxxxxx>
> ---
> sound/soc/codecs/max98373-sdw.c | 15 +--------------
> sound/soc/codecs/rt1308-sdw.c | 14 +-------------
> sound/soc/codecs/rt5682-sdw.c | 15 +--------------
> sound/soc/codecs/rt700-sdw.c | 15 +--------------
> sound/soc/codecs/rt711-sdw.c | 15 +--------------
> sound/soc/codecs/rt715-sdw.c | 33 +--------------------------------
> 6 files changed, 6 insertions(+), 101 deletions(-)

Sorry to ping you, but the patch is really easy to be ignored since I
forget to add ASoC prefix in the cover letter.
Could you Ack it if it looks good to you, please?


>
> diff --git a/sound/soc/codecs/max98373-sdw.c
> b/sound/soc/codecs/max98373-sdw.c index 5fe724728e84..a3ec92775ea7
> 100644
> --- a/sound/soc/codecs/max98373-sdw.c
> +++ b/sound/soc/codecs/max98373-sdw.c
> @@ -282,7 +282,7 @@ static const struct dev_pm_ops max98373_pm =
> { static int max98373_read_prop(struct sdw_slave *slave) {
> struct sdw_slave_prop *prop = &slave->prop;
> - int nval, i, num_of_ports;
> + int nval, i;
> u32 bit;
> unsigned long addr;
> struct sdw_dpn_prop *dpn;
> @@ -295,7 +295,6 @@ static int max98373_read_prop(struct sdw_slave
> *slave)
> prop->clk_stop_timeout = 20;
>
> nval = hweight32(prop->source_ports);
> - num_of_ports = nval;
> prop->src_dpn_prop = devm_kcalloc(&slave->dev, nval,
> sizeof(*prop->src_dpn_prop),
> GFP_KERNEL);
> @@ -315,7 +314,6 @@ static int max98373_read_prop(struct sdw_slave
> *slave)
>
> /* do this again for sink now */
> nval = hweight32(prop->sink_ports);
> - num_of_ports += nval;
> prop->sink_dpn_prop = devm_kcalloc(&slave->dev, nval,
> sizeof(*prop->sink_dpn_prop),
> GFP_KERNEL);
> @@ -333,17 +331,6 @@ static int max98373_read_prop(struct sdw_slave
> *slave)
> i++;
> }
>
> - /* Allocate port_ready based on num_of_ports */
> - slave->port_ready = devm_kcalloc(&slave->dev, num_of_ports,
> - sizeof(*slave->port_ready),
> - GFP_KERNEL);
> - if (!slave->port_ready)
> - return -ENOMEM;
> -
> - /* Initialize completion */
> - for (i = 0; i < num_of_ports; i++)
> - init_completion(&slave->port_ready[i]);
> -
> /* set the timeout values */
> prop->clk_stop_timeout = 20;
>
> diff --git a/sound/soc/codecs/rt1308-sdw.c b/sound/soc/codecs/rt1308-sdw.c
> index b0ba0d2acbdd..09c69dbab12a 100644
> --- a/sound/soc/codecs/rt1308-sdw.c
> +++ b/sound/soc/codecs/rt1308-sdw.c
> @@ -118,7 +118,7 @@ static int rt1308_clock_config(struct device *dev)
> static int rt1308_read_prop(struct sdw_slave *slave) {
> struct sdw_slave_prop *prop = &slave->prop;
> - int nval, i, num_of_ports = 1;
> + int nval, i;
> u32 bit;
> unsigned long addr;
> struct sdw_dpn_prop *dpn;
> @@ -131,7 +131,6 @@ static int rt1308_read_prop(struct sdw_slave *slave)
>
> /* for sink */
> nval = hweight32(prop->sink_ports);
> - num_of_ports += nval;
> prop->sink_dpn_prop = devm_kcalloc(&slave->dev, nval,
> sizeof(*prop-
> >sink_dpn_prop),
> GFP_KERNEL);
> @@ -149,17 +148,6 @@ static int rt1308_read_prop(struct sdw_slave *slave)
> i++;
> }
>
> - /* Allocate port_ready based on num_of_ports */
> - slave->port_ready = devm_kcalloc(&slave->dev, num_of_ports,
> - sizeof(*slave->port_ready),
> - GFP_KERNEL);
> - if (!slave->port_ready)
> - return -ENOMEM;
> -
> - /* Initialize completion */
> - for (i = 0; i < num_of_ports; i++)
> - init_completion(&slave->port_ready[i]);
> -
> /* set the timeout values */
> prop->clk_stop_timeout = 20;
>
> diff --git a/sound/soc/codecs/rt5682-sdw.c b/sound/soc/codecs/rt5682-sdw.c
> index 94bf6bee78e6..b7c97aba7f17 100644
> --- a/sound/soc/codecs/rt5682-sdw.c
> +++ b/sound/soc/codecs/rt5682-sdw.c
> @@ -537,7 +537,7 @@ static int rt5682_update_status(struct sdw_slave
> *slave, static int rt5682_read_prop(struct sdw_slave *slave) {
> struct sdw_slave_prop *prop = &slave->prop;
> - int nval, i, num_of_ports = 1;
> + int nval, i;
> u32 bit;
> unsigned long addr;
> struct sdw_dpn_prop *dpn;
> @@ -549,7 +549,6 @@ static int rt5682_read_prop(struct sdw_slave *slave)
> prop->sink_ports = 0x2; /* BITMAP: 00000010 */
>
> nval = hweight32(prop->source_ports);
> - num_of_ports += nval;
> prop->src_dpn_prop = devm_kcalloc(&slave->dev, nval,
> sizeof(*prop->src_dpn_prop),
> GFP_KERNEL);
> @@ -569,7 +568,6 @@ static int rt5682_read_prop(struct sdw_slave *slave)
>
> /* do this again for sink now */
> nval = hweight32(prop->sink_ports);
> - num_of_ports += nval;
> prop->sink_dpn_prop = devm_kcalloc(&slave->dev, nval,
> sizeof(*prop->sink_dpn_prop),
> GFP_KERNEL);
> @@ -587,17 +585,6 @@ static int rt5682_read_prop(struct sdw_slave *slave)
> i++;
> }
>
> - /* Allocate port_ready based on num_of_ports */
> - slave->port_ready = devm_kcalloc(&slave->dev, num_of_ports,
> - sizeof(*slave->port_ready),
> - GFP_KERNEL);
> - if (!slave->port_ready)
> - return -ENOMEM;
> -
> - /* Initialize completion */
> - for (i = 0; i < num_of_ports; i++)
> - init_completion(&slave->port_ready[i]);
> -
> /* set the timeout values */
> prop->clk_stop_timeout = 20;
>
> diff --git a/sound/soc/codecs/rt700-sdw.c b/sound/soc/codecs/rt700-sdw.c
> index 4d14048d1197..b19fbcc12c69 100644
> --- a/sound/soc/codecs/rt700-sdw.c
> +++ b/sound/soc/codecs/rt700-sdw.c
> @@ -333,7 +333,7 @@ static int rt700_update_status(struct sdw_slave
> *slave, static int rt700_read_prop(struct sdw_slave *slave) {
> struct sdw_slave_prop *prop = &slave->prop;
> - int nval, i, num_of_ports = 1;
> + int nval, i;
> u32 bit;
> unsigned long addr;
> struct sdw_dpn_prop *dpn;
> @@ -345,7 +345,6 @@ static int rt700_read_prop(struct sdw_slave *slave)
> prop->sink_ports = 0xA; /* BITMAP: 00001010 */
>
> nval = hweight32(prop->source_ports);
> - num_of_ports += nval;
> prop->src_dpn_prop = devm_kcalloc(&slave->dev, nval,
> sizeof(*prop->src_dpn_prop),
> GFP_KERNEL);
> @@ -365,7 +364,6 @@ static int rt700_read_prop(struct sdw_slave *slave)
>
> /* do this again for sink now */
> nval = hweight32(prop->sink_ports);
> - num_of_ports += nval;
> prop->sink_dpn_prop = devm_kcalloc(&slave->dev, nval,
> sizeof(*prop-
> >sink_dpn_prop),
> GFP_KERNEL);
> @@ -383,17 +381,6 @@ static int rt700_read_prop(struct sdw_slave *slave)
> i++;
> }
>
> - /* Allocate port_ready based on num_of_ports */
> - slave->port_ready = devm_kcalloc(&slave->dev, num_of_ports,
> - sizeof(*slave->port_ready),
> - GFP_KERNEL);
> - if (!slave->port_ready)
> - return -ENOMEM;
> -
> - /* Initialize completion */
> - for (i = 0; i < num_of_ports; i++)
> - init_completion(&slave->port_ready[i]);
> -
> /* set the timeout values */
> prop->clk_stop_timeout = 20;
>
> diff --git a/sound/soc/codecs/rt711-sdw.c b/sound/soc/codecs/rt711-sdw.c
> index 45b928954b58..dc4a2b482462 100644
> --- a/sound/soc/codecs/rt711-sdw.c
> +++ b/sound/soc/codecs/rt711-sdw.c
> @@ -337,7 +337,7 @@ static int rt711_update_status(struct sdw_slave
> *slave, static int rt711_read_prop(struct sdw_slave *slave) {
> struct sdw_slave_prop *prop = &slave->prop;
> - int nval, i, num_of_ports = 1;
> + int nval, i;
> u32 bit;
> unsigned long addr;
> struct sdw_dpn_prop *dpn;
> @@ -349,7 +349,6 @@ static int rt711_read_prop(struct sdw_slave *slave)
> prop->sink_ports = 0x8; /* BITMAP: 00001000 */
>
> nval = hweight32(prop->source_ports);
> - num_of_ports += nval;
> prop->src_dpn_prop = devm_kcalloc(&slave->dev, nval,
> sizeof(*prop->src_dpn_prop),
> GFP_KERNEL);
> @@ -369,7 +368,6 @@ static int rt711_read_prop(struct sdw_slave *slave)
>
> /* do this again for sink now */
> nval = hweight32(prop->sink_ports);
> - num_of_ports += nval;
> prop->sink_dpn_prop = devm_kcalloc(&slave->dev, nval,
> sizeof(*prop-
> >sink_dpn_prop),
> GFP_KERNEL);
> @@ -387,17 +385,6 @@ static int rt711_read_prop(struct sdw_slave *slave)
> i++;
> }
>
> - /* Allocate port_ready based on num_of_ports */
> - slave->port_ready = devm_kcalloc(&slave->dev, num_of_ports,
> - sizeof(*slave->port_ready),
> - GFP_KERNEL);
> - if (!slave->port_ready)
> - return -ENOMEM;
> -
> - /* Initialize completion */
> - for (i = 0; i < num_of_ports; i++)
> - init_completion(&slave->port_ready[i]);
> -
> /* set the timeout values */
> prop->clk_stop_timeout = 20;
>
> diff --git a/sound/soc/codecs/rt715-sdw.c b/sound/soc/codecs/rt715-sdw.c
> index d11b23d6b240..d8ed07305ffc 100644
> --- a/sound/soc/codecs/rt715-sdw.c
> +++ b/sound/soc/codecs/rt715-sdw.c
> @@ -431,7 +431,7 @@ static int rt715_update_status(struct sdw_slave
> *slave, static int rt715_read_prop(struct sdw_slave *slave) {
> struct sdw_slave_prop *prop = &slave->prop;
> - int nval, i, num_of_ports = 1;
> + int nval, i;
> u32 bit;
> unsigned long addr;
> struct sdw_dpn_prop *dpn;
> @@ -443,7 +443,6 @@ static int rt715_read_prop(struct sdw_slave *slave)
> prop->sink_ports = 0x0; /* BITMAP: 00000000 */
>
> nval = hweight32(prop->source_ports);
> - num_of_ports += nval;
> prop->src_dpn_prop = devm_kcalloc(&slave->dev, nval,
> sizeof(*prop->src_dpn_prop),
> GFP_KERNEL);
> @@ -460,36 +459,6 @@ static int rt715_read_prop(struct sdw_slave *slave)
> i++;
> }
>
> - /* do this again for sink now */
> - nval = hweight32(prop->sink_ports);
> - num_of_ports += nval;
> - prop->sink_dpn_prop = devm_kcalloc(&slave->dev, nval,
> - sizeof(*prop->sink_dpn_prop),
> - GFP_KERNEL);
> - if (!prop->sink_dpn_prop)
> - return -ENOMEM;
> -
> - dpn = prop->sink_dpn_prop;
> - i = 0;
> - addr = prop->sink_ports;
> - for_each_set_bit(bit, &addr, 32) {
> - dpn[i].num = bit;
> - dpn[i].simple_ch_prep_sm = true;
> - dpn[i].ch_prep_timeout = 10;
> - i++;
> - }
> -
> - /* Allocate port_ready based on num_of_ports */
> - slave->port_ready = devm_kcalloc(&slave->dev, num_of_ports,
> - sizeof(*slave->port_ready),
> - GFP_KERNEL);
> - if (!slave->port_ready)
> - return -ENOMEM;
> -
> - /* Initialize completion */
> - for (i = 0; i < num_of_ports; i++)
> - init_completion(&slave->port_ready[i]);
> -
> /* set the timeout values */
> prop->clk_stop_timeout = 20;
>
> --
> 2.17.1