linux-kernel - Re: [PATCH v2 1/3] media: V3s: Add support for Allwinner CSI.

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:   Thu, 24 Aug 2017 09:43:08 +0800
From:   Yong <yong.deng@...ewell.com>
To:     Maxime Ripard <maxime.ripard@...e-electrons.com>
Cc:     Baruch Siach <baruch@...s.co.il>,
        Mauro Carvalho Chehab <mchehab@...nel.org>,
        Rob Herring <robh+dt@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Chen-Yu Tsai <wens@...e.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        "David S. Miller" <davem@...emloft.net>,
        Hans Verkuil <hverkuil@...all.nl>,
        Arnd Bergmann <arnd@...db.de>,
        Hugues Fruchet <hugues.fruchet@...com>,
        Yannick Fertre <yannick.fertre@...com>,
        Philipp Zabel <p.zabel@...gutronix.de>,
        Benoit Parrot <bparrot@...com>,
        Benjamin Gaignard <benjamin.gaignard@...aro.org>,
        Jean-Christophe Trotin <jean-christophe.trotin@...com>,
        Ramesh Shanmugasundaram <ramesh.shanmugasundaram@...renesas.com>,
        Minghsiu Tsai <minghsiu.tsai@...iatek.com>,
        Krzysztof Kozlowski <krzk@...nel.org>,
        Robert Jarzmik <robert.jarzmik@...e.fr>,
        linux-media@...r.kernel.org, devicetree@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
        linux-sunxi@...glegroups.com
Subject: Re: [PATCH v2 1/3] media: V3s: Add support for Allwinner CSI.

On Wed, 23 Aug 2017 21:24:13 +0200
Maxime Ripard <maxime.ripard@...e-electrons.com> wrote:

> On Wed, Aug 23, 2017 at 10:41:18AM +0800, Yong wrote:
> > > > > > +static irqreturn_t sun6i_csi_isr(int irq, void *dev_id)
> > > > > > +{
> > > > > > +	struct sun6i_csi_dev *sdev = (struct sun6i_csi_dev *)dev_id;
> > > > > > +	struct regmap *regmap = sdev->regmap;
> > > > > > +	u32 status;
> > > > > > +
> > > > > > +	regmap_read(regmap, CSI_CH_INT_STA_REG, &status);
> > > > > > +
> > > > > > +	if ((status & CSI_CH_INT_STA_FIFO0_OF_PD) ||
> > > > > > +	    (status & CSI_CH_INT_STA_FIFO1_OF_PD) ||
> > > > > > +	    (status & CSI_CH_INT_STA_FIFO2_OF_PD) ||
> > > > > > +	    (status & CSI_CH_INT_STA_HB_OF_PD)) {
> > > > > > +		regmap_write(regmap, CSI_CH_INT_STA_REG, status);
> > > > > > +		regmap_update_bits(regmap, CSI_EN_REG, CSI_EN_CSI_EN, 0);
> > > > > > +		regmap_update_bits(regmap, CSI_EN_REG, CSI_EN_CSI_EN,
> > > > > > +				   CSI_EN_CSI_EN);
> > > > > 
> > > > > You need to enable / disable it at every frame? How do you deal with
> > > > > double buffering? (or did you choose to ignore it for now?)
> > > > 
> > > > These *_OF_PD status bits indicate an overflow error condition.
> > > 
> > > Shouldn't we return an error code then? The names of these flags could
> > > be better too.
> > 
> > Then, where and how to deal with the error coce.
> 
> If you want to deal with FIFO overflow, I'm not sure you have anything
> to do. It means, you've been to slow to queue buffers, so I guess
> stopping the pipeline until more buffers are queued would make
> sense. And we should probably increase the sequence number while doing
> so to notify the userspace that some frames were lost.

If there is no queued buffers, the CSI must has been already stoped by
sun6i_video_frame_done. So, the FIFO overflow may only occur on some 
unpredictable conditions or something I don't know.

For sequence number, I can't actually get the number of the lost frames.

Maybe I misunderstood you. Did you mean use IRQ_RETVAL(error) instead
of IRQ_HANDLED?

> 
> Maxime
> 
> -- 
> Maxime Ripard, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com


Thanks,
Yong