linux-kernel - RE: [PATCH 1/2] ASoC: fsl_xcvr: Add XCVR ASoC CPU DAI driver

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 18 Sep 2020 15:33:56 +0000
From:   "Viorel Suman (OSS)" <viorel.suman@....nxp.com>
To:     Mark Brown <broonie@...nel.org>,
        "Viorel Suman (OSS)" <viorel.suman@....nxp.com>
CC:     Liam Girdwood <lgirdwood@...il.com>,
        Rob Herring <robh+dt@...nel.org>,
        Jaroslav Kysela <perex@...ex.cz>,
        Takashi Iwai <tiwai@...e.com>, Timur Tabi <timur@...nel.org>,
        Nicolin Chen <nicoleotsuka@...il.com>,
        Xiubo Li <Xiubo.Lee@...il.com>,
        Fabio Estevam <festevam@...il.com>,
        Shengjiu Wang <shengjiu.wang@...il.com>,
        Philipp Zabel <p.zabel@...gutronix.de>,
        Matthias Schiffer <matthias.schiffer@...tq-group.com>,
        Cosmin-Gabriel Samoila <cosmin.samoila@....com>,
        "alsa-devel@...a-project.org" <alsa-devel@...a-project.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linuxppc-dev@...ts.ozlabs.org" <linuxppc-dev@...ts.ozlabs.org>,
        dl-linux-imx <linux-imx@....com>,
        Viorel Suman <viorel.suman@...il.com>
Subject: RE: [PATCH 1/2] ASoC: fsl_xcvr: Add XCVR ASoC CPU DAI driver

> On Fri, Sep 18, 2020 at 03:02:39PM +0000, Viorel Suman (OSS) wrote:
> 
> Please fix your mail client to word wrap within paragraphs at something
> substantially less than 80 columns.  Doing this makes your messages much easier
> to read and reply to.
> 

My bad, will do.

> > > > +	regmap_read(regmap, FSL_XCVR_EXT_ISR, &isr);
> > > > +	regmap_write(regmap, FSL_XCVR_EXT_ISR_CLR, isr);
> 
> > > This will unconditionally clear any interrupts, even those we don't
> > > understand - it might be better to only clear bits that are
> > > supported so the IRQ core can complain if there's something unexpected
> showing up.
> 
> > The ARM core registers itself in "fsl_xcvr_prepare" (the code below) just for a
> subset of all supported interrupts:
> > =====
> > 	ret = regmap_update_bits(xcvr->regmap, FSL_XCVR_EXT_IER0,
> > 				 FSL_XCVR_IRQ_EARC_ALL,
> FSL_XCVR_IRQ_EARC_ALL); =====
> > FSL_XCVR_IRQ_EARC_ALL - this mask represents all the interrupts we are
> > interested in and we handle in interrupt handler, But this is just a subset of all
> interrupts the M0+ core is able to assert. Not very intuitive, I think I need to
> reword it somehow.
> 
> That's not the issue, the issue is that if we get into the ISR we just ack all the bits
> that are flagged by the hardware regardless of if we actually handled them.  This
> won't work if there are ever systems that share the interrupt and it works
> against safety/debugging features that the interrupt has in case something goes
> wrong and we get spurious interrupts.
> 

Thank you for explanation, will fix it in next version.

> > > > +	if (isr & FSL_XCVR_IRQ_FIFO_UOFL_ERR)
> > > > +		dev_dbg(dev, "RX/TX FIFO full/empty\n");
> 
> > > Should this be dev_err()?
> 
> > The interrupt may be asserted right before DMA starts to fill the TX FIFO if I
> recall correctly.
> > I've added it just to debug the IP behavior, will check and change it to err it in
> next version if it is the case.
> 
> If it does come up normally then a comment or something to explain why this
> happens normally would probably be good.

Sure, ok.