lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Thu, 11 Jul 2019 06:09:53 +0000 From: "S.j. Wang" <shengjiu.wang@....com> To: Nicolin Chen <nicoleotsuka@...il.com> CC: "timur@...nel.org" <timur@...nel.org>, "Xiubo.Lee@...il.com" <Xiubo.Lee@...il.com>, "festevam@...il.com" <festevam@...il.com>, "broonie@...nel.org" <broonie@...nel.org>, "alsa-devel@...a-project.org" <alsa-devel@...a-project.org>, "linuxppc-dev@...ts.ozlabs.org" <linuxppc-dev@...ts.ozlabs.org>, "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org> Subject: Re: [PATCH V3 2/2] ASoC: fsl_esai: recover the channel swap after xrun > > Hi Shengjiu, > > Mostly looks good to me, just some small comments. > > On Mon, Jul 08, 2019 at 02:38:52PM +0800, shengjiu.wang@....com wrote: > > > +static void fsl_esai_hw_reset(unsigned long arg) { > > + struct fsl_esai *esai_priv = (struct fsl_esai *)arg; > > + u32 saisr, tfcr, rfcr; > > + bool tx = true, rx = false, enabled[2]; > > Could we swap the lines of u32 and bool? It'd look better. > > > + regmap_update_bits(esai_priv->regmap, REG_ESAI_TCR, > > + ESAI_xCR_xPR_MASK, ESAI_xCR_xPR); > > + regmap_update_bits(esai_priv->regmap, REG_ESAI_RCR, > > + ESAI_xCR_xPR_MASK, ESAI_xCR_xPR); > > Let's add a line of comments for these two: > /* Enforce ESAI personal resets for both TX and RX */ > > > + /* > > + * Restore registers by regcache_sync, and ignore > > + * return value > > + */ > > Could fit into single-line? > > > + regmap_update_bits(esai_priv->regmap, REG_ESAI_TCR, > > + ESAI_xCR_xPR_MASK, 0); > > + regmap_update_bits(esai_priv->regmap, REG_ESAI_RCR, > > + ESAI_xCR_xPR_MASK, 0); > > + > > + regmap_update_bits(esai_priv->regmap, REG_ESAI_PRRC, > > + ESAI_PRRC_PDC_MASK, ESAI_PRRC_PDC(ESAI_GPIO)); > > + regmap_update_bits(esai_priv->regmap, REG_ESAI_PCRC, > > + ESAI_PCRC_PC_MASK, ESAI_PCRC_PC(ESAI_GPIO)); > > Could remove the blank line and add a line of comments: > /* Remove ESAI personal resets by configuring PCRC and PRRC also */ > > Btw, I still feel this personal reset can be stuffed into one of the wrapper > functions. But let's keep this simple for now. > > > + regmap_read(esai_priv->regmap, REG_ESAI_SAISR, &saisr); > > Why do we read saisr here? All its bits would get cleared by the hardware > reset. If it's a must to clear again, we should add a line of comments to > emphasize it. This line can be removed. Best regards Wang Shengjiu
Powered by blists - more mailing lists