Re: [PATCH 2/2] memory: tegra: Add DLA clients for Tegra234

[Thierry Reding <thierry.reding@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 3229 bytes --]

On Fri, Oct 28, 2022 at 09:07:28AM -0400, Krzysztof Kozlowski wrote:
> On 28/10/2022 09:05, Jon Hunter wrote:
> > 
> > On 28/10/2022 13:52, Krzysztof Kozlowski wrote:
> >> On 28/10/2022 08:51, Jon Hunter wrote:
> >>>
> >>> On 28/10/2022 13:46, Krzysztof Kozlowski wrote:
> >>>> On 28/10/2022 08:37, Jon Hunter wrote:
> >>>>> Add the memory clients on Tegra234 which are needed for initialising the
> >>>>> SMMU for the Deep Learning Accelerator (DLA).
> >>>>>
> >>>>> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
> >>>>> ---
> >>>>>    drivers/memory/tegra/tegra234.c | 160 ++++++++++++++++++++++++++++++++
> >>>>>    1 file changed, 160 insertions(+)
> >>>>>
> >>>>> diff --git a/drivers/memory/tegra/tegra234.c b/drivers/memory/tegra/tegra234.c
> >>>>> index a9e8fd99730f..9bdaf8af8c97 100644
> >>>>> --- a/drivers/memory/tegra/tegra234.c
> >>>>> +++ b/drivers/memory/tegra/tegra234.c
> >>>>> @@ -170,6 +170,166 @@ static const struct tegra_mc_client tegra234_mc_clients[] = {
> >>>>>    				.security = 0x504,
> >>>>>    			},
> >>>>>    		},
> >>>>> +	}, {
> >>>>> +		.id = TEGRA234_MEMORY_CLIENT_DLA0RDA,
> >>>>> +		.name = "dla0rda",
> >>>>> +		.sid = TEGRA234_SID_NVDLA0,
> >>>>
> >>>> This is now not applicable because you sent dependencies separate, mixed
> >>>> with other subsystems... Split pieces based on subsystems.
> >>>
> >>> Sorry I don't understand. This patch is dependent upon the first. I have
> >>> not sent anything separate.
> >>
> >> You sent mixed between subsystems patches adding TEGRA234_SID_NVDLA0.
> >> That one should have been sent separate so maintainers can pick it up.
> > 
> > The original patch [0] I sent had no dependencies and so Thierry picked 
> > this up. However, when adding patch 2/2 here, I noticed a typo in the 
> > definition for TEGRA234_MEMORY_CLIENT_DLA0WRB so thought I would fix 
> > this up while I am at it.
> > 
> >> Now, it's not possible for me to pick this patch.
> > 
> > Yes now you cannot simply pick this up. We have had similar problems 
> > before. I am not sure if it is easiest for Thierry to pick these up.
> > 
> 
> And there is simple solution as I said - split the patches per
> subsystem. Why the dependency was combining multiple subsystems into one?

Splitting per subsystem doesn't really work in this case. The main
problem is that the dt-bindings headers are used by multiple subsystems,
and the content is not generated in a git-friendly way. So even if we
were to split things up, at some point there would be conflicts between
the various branches that these changes end up in.

The easiest solution for this is to simply take this through a single
tree. Normally this wouldn't be a problem because the symbols defined in
these headers are only used in device trees. However, for the memory
clients it is slightly complicated by the fact that the memory
controller driver also needs access to these symbols, so it creates an
interdependency between the dt-bindings, DT and memory controller
branches.

One of the reasons why Jon originally sent the larger patch, adding all
the missing IDs in one go, was precisely so we could avoid this hassle
in the future.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]