RE: [PATCH v5 01/13] ASoC: Intel: Add catpt device

["Rojewski, Cezary" <cezary.rojewski@intel.com>]

On 2020-09-17 5:15 PM, Rojewski, Cezary wrote:
> On 2020-09-16 5:24 PM, Andy Shevchenko wrote:
>> On Tue, Sep 15, 2020 at 06:29:32PM +0200, Cezary Rojewski wrote:
>>> Declare base structures, registers and device routines for the catpt
>>> solution. Catpt deprecates and is a direct replacement for
>>> sound/soc/intel/haswell. Supports Lynxpoint and Wildcat Point both.

...

>>> +static int __maybe_unused catpt_suspend(struct device *dev)
>>> +{
>>> +	struct catpt_dev *cdev = dev_get_drvdata(dev);
>>> +	struct dma_chan *chan;
>>> +	int ret;
>>> +
>>> +	chan = catpt_dma_request_config_chan(cdev);
>>> +	if (IS_ERR(chan))
>>> +		return PTR_ERR(chan);
>>> +
>>> +	memset(&cdev->dx_ctx, 0, sizeof(cdev->dx_ctx));
>>> +	ret = catpt_ipc_enter_dxstate(cdev, CATPT_DX_STATE_D3, &cdev->dx_ctx);
>>> +	if (ret) {
>>> +		ret = CATPT_IPC_ERROR(ret);
>>> +		goto exit;
>>> +	}
>>> +
>>> +	ret = catpt_dsp_stall(cdev, true);
>>> +	if (ret)
>>> +		goto exit;
>>> +
>>> +	ret = catpt_store_memdumps(cdev, chan);
>>> +	if (ret) {
>>> +		dev_err(cdev->dev, "store memdumps failed: %d\n", ret);
>>> +		goto exit;
>>> +	}
>>> +
>>> +	ret = catpt_store_module_states(cdev, chan);
>>> +	if (ret) {
>>> +		dev_err(cdev->dev, "store module states failed: %d\n", ret);
>>> +		goto exit;
>>> +	}
>>> +
>>> +	ret = catpt_store_streams_context(cdev, chan);
>>> +	if (ret) {
>>> +		dev_err(cdev->dev, "store streams ctx failed: %d\n", ret);
>>> +		goto exit;
>>> +	}
>>
>>> +exit:
>>
>> I would rather name it as 'out_dma_release' or so to explain what's going to be
>> done.
>>
> 
> I find more descriptive labels inviting reader into: "this is an error
> path" thinking and that's why I prefer to stick with simple 'exit'. If
> you think that's not a way to go, can change this.
> 

As I'm sending v7, goto labels have been updated to be move readable as
you requested: 'release_dma_chan'.

Notice that 'if' preceding 'exit' label can be simplified: drop goto,
drop redundant brackets.

I've added that into v7 update too.

Thanks,
Czarek