Re: [PATCH v8 0/4] edac: Add APM X-Gene SoC memory controller EDAC driver

[Loc Ho <lho-qTEPVZfXA3Y@public.gmane.org>]

Hi Borislav,

On Wed, May 6, 2015 at 11:29 AM, Borislav Petkov <bp-Gina5bIWoIWzQB+pC5nmwQ@public.gmane.org> 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-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html