On 07/14/2015 06:39 AM, Martin Sperl wrote: > >> On 14.07.2015, at 14:56, Stephen Warren <swarren@xxxxxxxxxxxxx> wrote: >> >> I don't care so much about the internal code details; anything there can >> be trivially changed. But please do make sure the DT correctly >> represents the HW by: >> >> * Having a separate DT node for each HW block (SPI controller, UART, and >> any shared interrupt mux/demux/controller/...) > > That is what we have with the v3 patch: > * spi1 > * spi2 > * syscon controlling the aux-enable register > (only used to enable/disable the hw block) OK, having separate nodes is great. However, I'd like to see a "semantic" driver for the shared register region rather than a "syscon". IIUC, "syscon" simply provides a stylized way for one driver to touch some shared registers directly without any semantics. I'd strongly prefer to see a real driver inside Linux rather than something that just lets drivers fiddle with the shared registers willy nilly. Still, this aspect is an internal implementation detail inside the kernel that we can change without external impact later if we need. More concerning: The bcm283x HW doesn't implement a "syscon" module, but some semantic IP block. The DT should contain a real compatible value that describes what the HW block really is, not just "syscon". We could bind the syscon driver to this compatible value if we have to for now. >> * If e.g. the SPI controller driver is going to need to manipulate the >> "shared interrupt mux/demux/controller/...", there should be a phandle >> from the SPI controller node to the "shared interrupt >> mux/demux/controller/..." node, rather than listing the shared registers >> in each "client"'s reg property. > That is not what is in the v3 release of the patch any longer. > > Each of the above dt-nodes has their own (non-overlapping) register > ranges and interrupts are enabled/disabled in the respective ranges > for spi1/spi2. Also the status register of spi1/2 contain the > corresponding flags for the reason of an interrupt - so no need > to access any of the shared aux registers for that - hence the use > of IRQF_SHARED. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
