| Message ID | 20221028123741.134880-2-jonathanh@nvidia.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [1/2] dt-bindings: memory: Fix typos and definitions for Tegra | expand |
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. Best regards, Krzysztof
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. Jon
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. Now, it's not possible for me to pick this patch. Best regards, Krzysztof
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. Jon [0] https://lore.kernel.org/linux-tegra/20221003125141.123759-1-jonathanh@nvidia.com/
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? Best regards, Krzysztof
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
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, + .regs = { + .sid = { + .override = 0x5f0, + .security = 0x5f4, + }, + }, + }, { + .id = TEGRA234_MEMORY_CLIENT_DLA0FALRDB, + .name = "dla0falrdb", + .sid = TEGRA234_SID_NVDLA0, + .regs = { + .sid = { + .override = 0x5f8, + .security = 0x5fc, + }, + }, + }, { + .id = TEGRA234_MEMORY_CLIENT_DLA0WRA, + .name = "dla0wra", + .sid = TEGRA234_SID_NVDLA0, + .regs = { + .sid = { + .override = 0x600, + .security = 0x604, + }, + }, + }, { + .id = TEGRA234_MEMORY_CLIENT_DLA0RDB, + .name = "dla0rdb", + .sid = TEGRA234_SID_NVDLA0, + .regs = { + .sid = { + .override = 0x160, + .security = 0x164, + }, + }, + }, { + .id = TEGRA234_MEMORY_CLIENT_DLA0RDA1, + .name = "dla0rda1", + .sid = TEGRA234_SID_NVDLA0, + .regs = { + .sid = { + .override = 0x748, + .security = 0x74c, + }, + }, + }, { + .id = TEGRA234_MEMORY_CLIENT_DLA0FALWRB, + .name = "dla0falwrb", + .sid = TEGRA234_SID_NVDLA0, + .regs = { + .sid = { + .override = 0x608, + .security = 0x60c, + }, + }, + }, { + .id = TEGRA234_MEMORY_CLIENT_DLA0RDB1, + .name = "dla0rdb1", + .sid = TEGRA234_SID_NVDLA0, + .regs = { + .sid = { + .override = 0x168, + .security = 0x16c, + }, + }, + }, { + .id = TEGRA234_MEMORY_CLIENT_DLA0WRB, + .name = "dla0wrb", + .sid = TEGRA234_SID_NVDLA0, + .regs = { + .sid = { + .override = 0x170, + .security = 0x174, + }, + }, + }, { + .id = TEGRA234_MEMORY_CLIENT_DLA1RDA, + .name = "dla0rda", + .sid = TEGRA234_SID_NVDLA1, + .regs = { + .sid = { + .override = 0x610, + .security = 0x614, + }, + }, + }, { + .id = TEGRA234_MEMORY_CLIENT_DLA1FALRDB, + .name = "dla0falrdb", + .sid = TEGRA234_SID_NVDLA1, + .regs = { + .sid = { + .override = 0x618, + .security = 0x61c, + }, + }, + }, { + .id = TEGRA234_MEMORY_CLIENT_DLA1WRA, + .name = "dla0wra", + .sid = TEGRA234_SID_NVDLA1, + .regs = { + .sid = { + .override = 0x620, + .security = 0x624, + }, + }, + }, { + .id = TEGRA234_MEMORY_CLIENT_DLA1RDB, + .name = "dla0rdb", + .sid = TEGRA234_SID_NVDLA1, + .regs = { + .sid = { + .override = 0x178, + .security = 0x17c, + }, + }, + }, { + .id = TEGRA234_MEMORY_CLIENT_DLA1RDA1, + .name = "dla0rda1", + .sid = TEGRA234_SID_NVDLA1, + .regs = { + .sid = { + .override = 0x750, + .security = 0x754, + }, + }, + }, { + .id = TEGRA234_MEMORY_CLIENT_DLA1FALWRB, + .name = "dla0falwrb", + .sid = TEGRA234_SID_NVDLA1, + .regs = { + .sid = { + .override = 0x628, + .security = 0x62c, + }, + }, + }, { + .id = TEGRA234_MEMORY_CLIENT_DLA1RDB1, + .name = "dla0rdb1", + .sid = TEGRA234_SID_NVDLA1, + .regs = { + .sid = { + .override = 0x370, + .security = 0x374, + }, + }, + }, { + .id = TEGRA234_MEMORY_CLIENT_DLA1WRB, + .name = "dla0wrb", + .sid = TEGRA234_SID_NVDLA1, + .regs = { + .sid = { + .override = 0x378, + .security = 0x37c, + }, + }, }, };
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(+)