Re: [PATCH, V4, 2/5] spi: bcm-qspi: Add SPI flash and MSPI driver

[Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 1279 bytes --]

On Fri, Jul 01, 2016 at 11:39:03AM -0400, Kamal Dasu wrote:

> > All this interrupt code and especially the fact that it's a completely
> > separate register range in the binding looks very much like it's just
> > an interrupt controller IP that's not particularly anything to do with
> > the SPI controller and should therefore be in a separate driver.  Why is
> > this part of the SPI controller driver?

> Some SoCs need this since they do not implement a separate interrupt
> controller and have dedicated l1 interrupt for spi. Also the handling
> is not generic enough to cover other ips as well in those case. Hence
> have to handle it within the driver.

That doesn't seem to match what the code is actually doing.  The
register block this is controlling is separate to the rest of the IP.
It's perfectly OK to have a driver for an interrupt controller which is
only used in one place, though you may find one of the generic ones
might be able to handle it anyway.

> >> +     default:
> >> +             break;
> >> +     }

> > We just ignore other widths?

> These are the only supported widths will make SPI_NBITS_SINGLE default.

If the user is trying to set an unsupported configuration you should
return an error rather than silently set a different configuration.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]