On Tue, Jul 19, 2016 at 03:15:49PM -0700, Florian Fainelli wrote: > On 07/13/2016 03:36 PM, Mark Brown wrote: > > If it was unconditionally in the perhipheral it'd be fairly normal and > > standard but it's not, it's a completely separate and optional register > > block with a bunch of separate code in a driver which already has above > > average complexity even for the bits that belong in it. > It is not separate nor optional, the fact that the register range seems > discontiguous is just an integration choice here, but it contains bits > that are relevant exclusively to the SPI controller. Some versions of the block have it, some versions of the block don't have it. That suggests that it's optional. It is a completely separate register map, that suggests that it is an external block. > It is not exclusively an interrupt controller, there are other bits in > there are do not functionally belong into an interrupt controller driver > per-se, things like a clock enable, arbiter control and a random bit to > control MSPI flush behavior. > Oh, and please don't tell me regmap is the solution here if we need to > control both the interrupt-related bits and the other bits within this > register. That does sound system controllerish if those bits do need to be managed. Are we sure future designs won't just integrate this into the main system controller? > Considering that this block aggregates interrupt enable bits, but not > just, there is no reason for this block to change over time, and clearly > this won't be handled via a generic interrupt controller driver either. There must be *some* logic behind the way the hardware has been structured, and right now the code just isn't making any of this apparent which makes it look like it needs refactoring.
Attachment:
signature.asc
Description: PGP signature
