linux-kernel - Re: [PATCH v2 2/6] staging: fbtft: do not override DMA coherent mask

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:   Tue, 03 Jan 2017 15:58:54 +0200
From:   Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
To:     Noralf Trønnes <noralf@...nnes.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        devel@...verdev.osuosl.org,
        Thomas Petazzoni <thomas.petazzoni@...e-electrons.com>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 2/6] staging: fbtft: do not override DMA coherent mask

On Tue, 2017-01-03 at 12:51 +0200, Andy Shevchenko wrote:
> On Mon, 2017-01-02 at 19:14 +0100, Noralf Trønnes wrote:
> > Den 02.01.2017 12:35, skrev Andy Shevchenko:
> > > Usually it's not consumer's business to override resources passed
> > > from
> > > provider, in particularly DMA coherent mask.
> > > --- a/drivers/staging/fbtft/fbtft-core.c
> > > +++ b/drivers/staging/fbtft/fbtft-core.c
> > > @@ -841,7 +841,6 @@ struct fb_info *fbtft_framebuffer_alloc(struct
> > > fbtft_display *display,
> > >   	if (txbuflen > 0) {
> > >   #ifdef CONFIG_HAS_DMA
> > >   		if (dma) {
> > > -			dev->coherent_dma_mask = ~0;
> > 
> > Can we make this conditional like in of_dma_configure():
> > 
> >           if (!dev->coherent_dma_mask)
> >                  dev->coherent_dma_mask = DMA_BIT_MASK(32);
> > 
> > If not, I guess the mask has to be set before adding the spi device
> > in
> > fbtft_device.c to keep it from breaking.
> 
> Good point. I will check this.

So, I was too fast.

Clearly an SPI slave device does not and *should not* know about DMA
capabilities of SPI *host* controller. It's completely out of slave's
business.

The masks and other DMA properties only makes sense for the device which
*actually does DMA*, and apparently SPI slaves do not suit that category
(there are might be real SPI slaves with private DMA engines, though
it's another story).

Thus, the patch from my point of view should be kept in the same form.

Regarding to the kbuild bot warning, would you like me to resend it
fixed? Whatever the decision, I will wait for more comments and
hopefully your tags.

P.S. I dunno how it did work before, since DMA mask of slave device
basically has no effect. Perhaps first argument to allocator should be
NULL. That's only amendment I can see to this particular patch.

-- 
Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
Intel Finland Oy