On Wed, Dec 23, 2015 at 02:01:14PM -0500, Alex Deucher wrote:
> + stream = adata->play_stream;
> + rtd = stream ? stream->runtime : NULL;
> + if (rtd != NULL) {
> + /* Resume playback stream from a suspended state */
> + sdata = rtd->private_data;
> + config_acp_dma(adata->acp_mmio, sdata);
> + }
Please don't use the ternery operator like this, it's not bad to write
if statements and they're much more legible. I was having to think far
too much about the various variables and if we were safely handling all
of them before I realised the next block overwrites them all anyway.
This is really
stream = adata->play_stream;
if (stream && stream->rtd)
config_acp_dma(adata->acp_mmio, stream->rtd->private_data);
or a couple of nested if statements. Otherwise this patch looks OK.
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
