linux-kernel - Re: [PATCH 3/3] SPI: Ingenic: Add SFC support for Ingenic SoCs.

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]

Message-ID: <YtxMwyRghFKS/vu5@sirena.org.uk>
Date:   Sat, 23 Jul 2022 20:32:19 +0100
From:   Mark Brown <broonie@...nel.org>
To:     Zhou Yanjie <zhouyanjie@...yeetech.com>
Cc:     tudor.ambarus@...rochip.com, p.yadav@...com, michael@...le.cc,
        miquel.raynal@...tlin.com, richard@....at, vigneshr@...com,
        robh+dt@...nel.org, krzysztof.kozlowski+dt@...aro.org,
        linux-mtd@...ts.infradead.org, linux-spi@...r.kernel.org,
        linux-mips@...r.kernel.org, linux-kernel@...r.kernel.org,
        devicetree@...r.kernel.org, aidanmacdonald.0x0@...il.com,
        tmn505@...il.com, paul@...pouillou.net, dongsheng.qiu@...enic.com,
        aric.pzqi@...enic.com, rick.tyliu@...enic.com,
        jinghui.liu@...enic.com, sernia.zhou@...mail.com,
        reimu@...omaker.com
Subject: Re: [PATCH 3/3] SPI: Ingenic: Add SFC support for Ingenic SoCs.

On Sun, Jul 24, 2022 at 01:06:16AM +0800, Zhou Yanjie wrote:
> On 2022/7/23 上午2:38, Mark Brown wrote:

> > > +++ b/drivers/spi/spi-ingenic-sfc.c
> > > @@ -0,0 +1,662 @@
> > > +// SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > +/*
> > > + * Ingenic SoCs SPI Flash Controller Driver

> > Please make the entire comment a C++ one so things look more
> > intentional.

> I'm sorry, I didn't understand well what you meant :(
> Could you please explain a little more detail?

The above comment block uses both C /* */ and C++ // style comments,
please make it just use the C++ style.

> > > +static irqreturn_t ingenic_sfc_irq_handler(int irq, void *data)
> > > +{
> > > +	struct ingenic_sfc *sfc = data;
> > > +
> > > +	writel(0x1f, sfc->base + SFC_REG_INTC);
> > > +
> > > +	complete(&sfc->completion);
> > > +
> > > +	return IRQ_HANDLED;
> > > +}

> > This doesn't pay any attention to any status registers in the chip so
> > won't work if the interrupt is shared and won't notice any error reports
> > from the device...

> This interrupt is exclusively owned by SFC, do we still
> need to perform the operation you said? I haven't done
> these operations before because I want to minimize the
> overhead and avoid affecting performance.

Even if the device is not shared is there no possibility that the
device can report an unexpected interrupt status?  It's not just
the sharing case, it's also the fact that it looks like there's a
status being reported but we're not checking it so if anything
goes wrong then we're less likely to notice.  I'd worry about
data corruption.

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)