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

[Andy Shevchenko]

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@xxxxxxxxxxxxxxx>
Intel Finland Oy