Hi Borislav, On Wed, May 6, 2015 at 11:29 AM, Borislav Petkov <bp@xxxxxxxxx> wrote: > On Wed, May 06, 2015 at 11:12:20AM -0700, Loc Ho wrote: >> 1. Whether to have an single driver for APM EDAC or multiple instance >> of 4 different drivers. With single driver, it does not scale in the >> future when we add/remove memory controllers and CPU domains. This is > > Why doesn't it scale? Please explain this to me more verbosely. Let me explain a bit more. We have four memory controller today. Therefore, I would like to have 4 DTS node and the same driver probe function called 4 times. If there is only one driver for the entire APM EDAC, I would have to merge all the resource registers into an single DTS node and its probe function called one time. In this one driver design, what would I do in future chip or variant of the chip in which it remove or add an addition memory controller? I would have to change the driver as well as the DTS node. In the four instance probe design, all I need is to add an additional DTS node. > >> also agreed by Rob Herring from review of the DTS nodes. For L3 and >> SoC EDAC, they are less of an issue as I don't see a situation that we >> would have multiple instances. >> >> 2. With regard to the top level PCP interrupt, they are just for >> status and once configured, it will not be touch. Therefore, I keep >> the current implementation. With an single driver, there is no need to >> worry about read/modify/write as it will be guarded with an lock. For >> multiple instance, I am thinking that the xgene_edac_mc module will >> provide exported lock functions for the other drivers. > > Doing this would be a very bad design and it would be a homegrown case > only for this driver. Then other ARM drivers will appear which will do > their own locking too. No no no. No ad-hoc hackery. > What if I move the locking function into a common module to be included by the EDAC framework? Or would you prefer that I go and write an common driver that all it does is provide locking? Any suggestion as all I need is an way to share access to an CSR which can't use atomic operations? It isn't an actual interrupt controller either. -Loc -- 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
