Re: [PATCH 12/13] ASoC: amd: acp: Add pci legacy driver support for acp7.0 platform

[syed saba kareem]

On 10/23/23 13:20, Amadeusz Sławiński wrote:
> On 10/21/2023 4:50 PM, Syed Saba Kareem wrote:
> > Add pci legacy driver support and create platform driver for
> > acp7.0 platform.
> >
> > Signed-off-by: Syed Saba Kareem <Syed.SabaKareem@xxxxxxx>
> > ---
>
> ...
>
> > +
> > +static struct snd_soc_dai_driver acp70_dai[] = {
> > +{
> > +    .name = "acp-i2s-sp",
> > +    .id = I2S_SP_INSTANCE,
> > +    .playback = {
> > +        .stream_name = "I2S SP Playback",
> > +        .rates = SNDRV_PCM_RATE_8000_96000,
> > +        .formats = SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S8 |
> > +               SNDRV_PCM_FMTBIT_U8 | SNDRV_PCM_FMTBIT_S32_LE,
>
> Any reason to not go from lowest bit width to higher? Similarly in 
> further definitions.
>  This has to be corrected, will push the changes as an incremental patch.
> > +        .channels_min = 2,
> > +        .channels_max = 8,
> > +        .rate_min = 8000,
> > +        .rate_max = 96000,
> > +    },
>
> ...
>
> > +
> > +static int __maybe_unused acp70_pcm_resume(struct device *dev)
> > +{
> > +    struct acp_dev_data *adata = dev_get_drvdata(dev);
> > +    struct acp_stream *stream;
> > +    struct snd_pcm_substream *substream;
> > +    snd_pcm_uframes_t buf_in_frames;
> > +    u64 buf_size;
> > +
> > +    spin_lock(&adata->acp_lock);
> > +    list_for_each_entry(stream, &adata->stream_list, list) {
> > +        if (stream) {
> > +            substream = stream->substream;
> > +            if (substream && substream->runtime) {
> > +                buf_in_frames = (substream->runtime->buffer_size);
> > +                buf_size = frames_to_bytes(substream->runtime, 
> > buf_in_frames);
> > +                config_pte_for_stream(adata, stream);
> > +                config_acp_dma(adata, stream, buf_size);
> > +                if (stream->dai_id)
> > +                    restore_acp_i2s_params(substream, adata, stream);
> > +                else
> > +                    restore_acp_pdm_params(substream, adata);
> > +            }
> > +        }
> > +    }
> > +        spin_unlock(&adata->acp_lock);
> > +        return 0;
>
> Indentation is wrong in above two lines.
>   This has to be corrected, will push the changes as an incremental 
> patch.
> > +}
> > +
> > +static const struct dev_pm_ops acp70_dma_pm_ops = {
> > +    SET_SYSTEM_SLEEP_PM_OPS(NULL, acp70_pcm_resume)
> > +};
> > +