On 2/18/2022 1:11 AM, Stephen Boyd wrote: Thanks for Your time Stephen!!!
Quoting Srinivasa Rao Mandadapu (2022-02-15 21:11:29)
On 2/15/2022 6:40 AM, Stephen Boyd wrote: Thanks for your time Stephen!!!
Quoting Srinivasa Rao Mandadapu (2022-02-14 06:58:22)
Add support function to get dma control and lpaif handle to avoid repeated code in platform driver
Signed-off-by: Srinivasa Rao Mandadapu quic_srivasam@quicinc.com Co-developed-by: Venkata Prasad Potturu quic_potturu@quicinc.com Signed-off-by: Venkata Prasad Potturu quic_potturu@quicinc.com Reviewed-by: Srinivas Kandagatla srinivas.kandagatla@linaro.org
sound/soc/qcom/lpass-platform.c | 113 +++++++++++++++++++++++----------------- 1 file changed, 66 insertions(+), 47 deletions(-)
diff --git a/sound/soc/qcom/lpass-platform.c b/sound/soc/qcom/lpass-platform.c index a44162c..5d77240 100644 --- a/sound/soc/qcom/lpass-platform.c +++ b/sound/soc/qcom/lpass-platform.c @@ -177,6 +177,49 @@ static int lpass_platform_pcmops_close(struct snd_soc_component *component, return 0; }
+static void __lpass_get_lpaif_handle(struct snd_pcm_substream *substream,
const?
Okay. will add const to substream pointer.
struct snd_soc_component *component,const?
Here const is giving compilation errors in below code.
Ok
l_id = pcm_data->dma_ch;l_dmactl = drvdata->rd_dmactl;} else {l_dmactl = drvdata->wr_dmactl;l_id = pcm_data->dma_ch - v->wrdma_channel_start;}l_map = drvdata->lpaif_map;break;case LPASS_DP_RX:l_id = pcm_data->dma_ch;l_dmactl = drvdata->hdmi_rd_dmactl;l_map = drvdata->hdmiif_map;break;default:break;}if (dmactl)*dmactl = l_dmactl;if (id)*id = l_id;if (map)*map = l_map;Why not 'return 0' here and return -EINVAL in the default case above? Then we can skip the checks for !map or !dmactl below and simply bail out if it failed with an error value.
Here the check is for input params. some users call for only damctl or only map.
Yes the check is to make sure there's a pointer to store the value into. I get that. The users are all internal to this driver though because the function is static. If the function returned an error because it couldn't find something then we wouldn't have to test the resulting pointers for success, instead we could check for a return value. Similarly, it may be clearer to have a single get for each item and then return a pointer from the function vs. passing it in to extract something. It may duplicate some code but otherwise the code would be clearer because we're getting one thing and can pass an error value through the pointer with PTR_ERR().
Okay. Agreed. But in initial review comments, asked to make common function for code readability,
and to avoid duplicate code.
Anyway will change accordingly and re post it.