Re: [PATCH 1/9] memory: tegra: Move internal data structures into separate header

From: Dmitry Osipenko <digetx@gmail.com>
To: Thierry Reding <thierry.reding@gmail.com>
Cc: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>,
	Will Deacon <will@kernel.org>, Jon Hunter <jonathanh@nvidia.com>,
	iommu@lists.linux-foundation.org,
	Nicolin Chen <nicolinc@nvidia.com>,
	linux-tegra@vger.kernel.org, Robin Murphy <robin.murphy@arm.com>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 1/9] memory: tegra: Move internal data structures into separate header
Date: Thu, 25 Mar 2021 19:11:35 +0300	[thread overview]
Message-ID: <4f1e0859-60b9-0ded-a4b8-70e06dd5a905@gmail.com> (raw)
In-Reply-To: <YFyxtq6cwIJNr/Y5@orome.fritz.box>

25.03.2021 18:52, Thierry Reding пишет:
> On Thu, Mar 25, 2021 at 06:12:51PM +0300, Dmitry Osipenko wrote:
>> 25.03.2021 16:03, Thierry Reding пишет:
>>> From: Thierry Reding <treding@nvidia.com>
>>>
>>> From Tegra20 through Tegra210, either the GART or SMMU drivers need
>>> access to the internals of the memory controller driver because they are
>>> tightly coupled (in fact, the GART and SMMU are part of the memory
>>> controller). On later chips, a separate hardware block implements the
>>> SMMU functionality, so this is no longer needed. However, we still want
>>> to reuse some of the existing infrastructure on later chips, so split
>>> the memory controller internals into a separate header file to avoid
>>> conflicts with the implementation on newer chips.
>>>
>>> Signed-off-by: Thierry Reding <treding@nvidia.com>
>>> ---
>>>  drivers/iommu/tegra-gart.c      |  2 +-
>>>  drivers/iommu/tegra-smmu.c      |  2 +-
>>>  drivers/memory/tegra/mc.h       |  2 +-
>>>  drivers/memory/tegra/tegra186.c | 12 ++++---
>>>  include/soc/tegra/mc-internal.h | 62 +++++++++++++++++++++++++++++++++
>>>  include/soc/tegra/mc.h          | 50 --------------------------
>>>  6 files changed, 72 insertions(+), 58 deletions(-)
>>>  create mode 100644 include/soc/tegra/mc-internal.h
>>
>> What about to make T186 to re-use the existing tegra_mc struct? Seems
>> there is nothing special in that struct which doesn't fit for the newer
>> SoCs. Please notice that both SMMU and GART are already optional and all
>> the SoC differences are specified within the tegra_mc_soc. It looks to
>> me that this could be a much nicer and cleaner variant.
> 
> The problem is that much of the interesting bits in tegra_mc_soc are
> basically incompatible between the two. For instance the tegra_mc_client
> and tegra186_mc_client structures, while they have the same purpose,
> have completely different content. I didn't see a way to unify that
> without overly complicating things by making half of the fields
> basically optional on one or the other SoC generation.

The additional fields aren't problem for T20, which doesn't need most of
the fields. I'd try to go with the additional fields for now and see how
it will look like, if it will be bothering too much, then we may
consider to refactor the drivers more thoroughly (later on, in a
separate series), with a better/nicer separation and taking into account
a potential modularization support by the MC drivers.

Using a union for the exclusive fields also could work, although always
need to be extra careful with the unions.

> Maybe one option would be to split tegra_mc into a tegra_mc_common and
> then derive tegra_mc and tegra186_mc from that. That way we could share
> the common bits while still letting the chip-specific differences be
> handled separately.

But isn't tegra_mc already a superset of tegra186_mc? I think the
tegra186_mc_client is the main difference here.
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu