Re: [PATCH v3 06/19] dt-bindings: memory: tegra: Squash tegra20-gart into tegra20-mc

On 28.08.2018 13:47, Thierry Reding wrote:

On Mon, Aug 20, 2018 at 10:35:54PM +0300, Dmitry Osipenko wrote:

quoted

On 20.08.2018 22:27, Dmitry Osipenko wrote:

quoted

On 20.08.2018 22:12, Rob Herring wrote:

quoted

On Sat, Aug 18, 2018 at 06:54:17PM +0300, Dmitry Osipenko wrote:

quoted

Splitting GART and Memory Controller wasn't a good decision that was made
back in the day. Given that the GART driver hasn't ever been used by
anything in the kernel, we decided that it will be better to correct the
mistakes of the past and merge two bindings into a single one. In a result

As a result...

quoted

there is a DT ABI change for the Memory Controller that allows not to
break newer kernels using older DT by introducing a new required property,
the memory clock. Adding the new clock property also puts the tegra20-mc
binding in line with the bindings of the later Tegra generations.

I don't understand this part. It looks to me like you are breaking 
compatibility. The driver failing to probe with an old DT is okay?

Yes, DT compatibility is broken. New driver won't probe/load with the old DT,
that's what we want.

quoted

OS's like OpenSUSE use new DTs with older kernel versions, so you should 
consider how to not break them as well. I guess if all this is optional 
or has been unused, then there shouldn't be a problem.

That's interesting.. Memory Controller isn't optional, I guess we could change
compatible to "nvidia,tegra20-mc-gart".bled in kernels config by

default and driver is functional, but it's okay 

* I meant it's not optional in a sense that it's enaif MC driver will stop to probe
with older kernels as it is used only for reporting memory errors.

Yeah, we don't really regress at runtime. The errors reported by the
current driver are very rare, and even if you encounter them, they're
pretty cryptic, so I think this is one of the exceptional cases where
breaking the ABI "for the greater good" is acceptable.

It's now became apparent that factoring out EMC from MC isn't a good idea too
because MC need to interact with EMC and probably vice versa. Looks like we
should consider restructuring MC for all Tegra's.