linux-kernel - Re: [PATCH v6 2/2] 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]

Message-ID: <20180129082533.6edmqgbauo6q5dgz@flea.lan>
Date:   Mon, 29 Jan 2018 09:25:33 +0100
From:   Maxime Ripard <maxime.ripard@...e-electrons.com>
To:     Arnd Bergmann <arnd@...db.de>,
        Linus Walleij <linus.walleij@...aro.org>
Cc:     Yong Deng <yong.deng@...ewell.com>,
        Mauro Carvalho Chehab <mchehab@...nel.org>,
        Rob Herring <robh+dt@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Chen-Yu Tsai <wens@...e.org>,
        "David S. Miller" <davem@...emloft.net>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Hans Verkuil <hans.verkuil@...co.com>,
        Randy Dunlap <rdunlap@...radead.org>,
        Stanimir Varbanov <stanimir.varbanov@...aro.org>,
        Hugues Fruchet <hugues.fruchet@...com>,
        Yannick Fertre <yannick.fertre@...com>,
        Philipp Zabel <p.zabel@...gutronix.de>,
        Benjamin Gaignard <benjamin.gaignard@...aro.org>,
        Ramesh Shanmugasundaram <ramesh.shanmugasundaram@...renesas.com>,
        Sakari Ailus <sakari.ailus@...ux.intel.com>,
        Rick Chang <rick.chang@...iatek.com>,
        linux-media@...r.kernel.org,
        "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
        <devicetree@...r.kernel.org>,
        Linux ARM <linux-arm-kernel@...ts.infradead.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        linux-sunxi <linux-sunxi@...glegroups.com>, megous@...ous.com,
        Thomas Petazzoni <thomas.petazzoni@...e-electrons.com>
Subject: Re: [PATCH v6 2/2] media: V3s: Add support for Allwinner CSI.

Hi Linus,

On Sat, Jan 27, 2018 at 05:14:26PM +0100, Linus Walleij wrote:
> > +void sun6i_csi_update_buf_addr(struct sun6i_csi *csi, dma_addr_t addr)
> > +{
> > +       struct sun6i_csi_dev *sdev = sun6i_csi_to_dev(csi);
> > +       /* transform physical address to bus address */
> > +       dma_addr_t bus_addr = addr - PHYS_OFFSET;
> 
> I am sorry if this is an unjustified drive-by comment. Maybe you
> have already investigate other ways to do this.

It's definitely not unjustified :)

> Accessing PHYS_OFFSET directly seems unintuitive and not good
> practice.
> 
> But normally an dma_addr_t only comes from some function inside
> <linux/dma-mapping.h> such as: dma_alloc_coherent() for a contigous
> buffer which is coherent in physical memory, or from some buffer <=
> 64KB that is switching ownership between device and CPU explicitly
> with dma_map* or so. Did you check with Documentation/DMA-API.txt?

So, I've discussed this with Arnd a month ago or so, because I'm not
really fond of the current approach but we haven't found better way to
do it yet.

The issue is that all the DMA accesses are done not through the main
AXI bus, but through a separate bus dedicated for memory accesses,
where the RAM is mapped at the address 0. So the CPU and DMA devices
have a different mapping for the RAM.

I guess we could address this by using the field dma_pfn_offset that
seems to be used in similar situations. However, in DT systems, that
field is filled only with the parent's node dma-ranges property. In
our case, and since the DT parenthood is based on the "control" bus,
and not the "data" bus, our parent node would be the AXI bus, and not
the memory bus that enforce those constraints.

And other devices doing DMA through regular DMA accesses won't have
that mapping, so we definitely shouldn't enforce it for all the
devices there, but only the one connected to the separate memory bus.

tl; dr: the DT is not really an option to store that info.

I suggested setting dma_pfn_offset at probe, but Arnd didn't seem too
fond of that approach either at the time.

So, well, I guess we could do better. We just have no idea how :)

Maxime

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

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