Re: [Patch V1 1/4] memory: tegra: Add support for mc interrupts

[Ashish Mhetre]

On 1/12/2022 1:59 AM, Krzysztof Kozlowski wrote:
> External email: Use caution opening links or attachments
>
>
> On 11/01/2022 19:45, Ashish Mhetre wrote:
> > Implement new structure for function related to mc interrupts.
>
> s/mc/MC/
Okay, I'll update these.
> > Move handle_irq into this structure.
> > Add support for clearing interrupts.
>
> The subject says you are adding support for MC interrupts, so before
> they were not supported at all?
Interrupt handling and error logging is not supported from Tegra186
onward. So the patches are for adding supported interrupts and logging
them from Tegra186 onward. But you are right, subject/commit message is
misleading. I'll update it in next version.

> Here you also mention clearing of interrupts - another new feature. One
> commit for refactoring (adding new structure) which does not change
> functionality, second commit for adding new feature.
> > Different question - why do you need new structure for just two function
> pointers? Why these different IRQ handling functions cannot be in
> tegra_mc_ops? To me, it's unnecessary code complexity (plus performance
> impact, but it's not that important). If this is really, really needed,
> please describe the rationale in the commit message.
clearing_interrupts() won't be needed. As pointed by Dmitry, we should
be logging early boot MC interrupts as well instead of clearing them.
Also, I'll keep handling irq inside tegra_mc_ops instead of adding new
structure.

> > Signed-off-by: Ashish Mhetre <amhetre@xxxxxxxxxx>
> > ---
> >   drivers/memory/tegra/mc.c       | 14 +++++++++++---
> >   drivers/memory/tegra/mc.h       |  1 +
> >   drivers/memory/tegra/tegra114.c |  1 +
> >   drivers/memory/tegra/tegra124.c |  2 ++
> >   drivers/memory/tegra/tegra186.c | 14 ++++++++++++++
> >   drivers/memory/tegra/tegra194.c | 12 ++++++++++++
> >   drivers/memory/tegra/tegra20.c  |  6 +++++-
> >   drivers/memory/tegra/tegra210.c |  1 +
> >   drivers/memory/tegra/tegra30.c  |  1 +
> >   include/soc/tegra/mc.h          |  7 ++++++-
> >   10 files changed, 54 insertions(+), 5 deletions(-)
>
>
> Best regards,
> Krzysztof