Re: RFC on drivers/memory vs drivers/edac memory mapping for DDR Controller

[Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>]

On 02/01/2023 13:17, Shenhar, Talel wrote:
> Hi,
> 
> Want to consult on a topic that involve both drivers/memory and 
> drivers/edac.
> 
> * We want to introduce driver that reads DDR controller RAS register and 
> notify for ECC errors by using EDAC MC API found in drivers/edac.
> * We also want to have a capability to dynamically change DDR refresh 
> rate based on thermal values (best to be done in drivers/memory ?).
> 
> The pain point here is that both capabilities are controlled from the 
> DDR controller.
> This create issue while using 
> devm_platform_ioremap_resource*->devm_request_mem_region which prevent 
> two mapping of same area.

This could be avoided but the true problem is that you have two devices
for same memory mapping. Devicetree does not allow it and it points to
some wrong hardware representation in DTS.

> 
> It seems to be expected problem as we have 2 "framework" (edac and 
> memory) split while both aim for same HW unit.
> What is the recommended way to face such conflicts?

You now mix Devicetree and Linux drivers. You can have same IO address
space used by multiple drivers, even though it is not always good
approach (concurrent and conflicting change of same settings).

HW description is irrelevant to this.

> 
> We had several concept in mind but would love to get your point of view 
> first.

Describe hardware accurately and completely. This solves all the
problems, doesn't it? Linux drivers do not depend on it and you can make
it differently.

> 
> Things we had in mind:
> 1) map more specific region to avoid conflict (we don't need the same 
> registers on both entity so if we do very specific multiple mapping this 
> shall be resolved)
> 2) use other kernel API for mapping that doesn't do request_mem_region 
> (or use the reserve only for one of them)
> 3) have single driver (edac mc) handle also the refresh rate
> 4) export edac_mc.h and have the drivers/memory have all the needed code 
> to do both edac and refresh rate under drivers/memory

None of these address the core problem - possibly inaccurate hardware
description...

Best regards,
Krzysztof