Re: [PATCH v3 4/4] edac: Add support for Amazon's Annapurna Labs L2 EDAC

[Robert Richter]

On 03.09.19 11:28:41, Hawa, Hanna wrote:
> On 9/3/2019 10:27 AM, Robert Richter wrote:
> > On 15.07.19 16:24:09, Hanna Hawa wrote:
> > > Adds support for Amazon's Annapurna Labs L2 EDAC driver to detect and
> > > report L2 errors.
> > > 
> > > Signed-off-by: Hanna Hawa <hhhawa@xxxxxxxxxx>
> > > ---
> > >   MAINTAINERS               |   6 ++
> > >   drivers/edac/Kconfig      |   8 ++
> > >   drivers/edac/Makefile     |   1 +
> > >   drivers/edac/al_l2_edac.c | 187 ++++++++++++++++++++++++++++++++++++++++++++++
> > >   4 files changed, 202 insertions(+)
> > >   create mode 100644 drivers/edac/al_l2_edac.c
> > 
> >  From a brief look at it, it seems some of my comments from 2/4 apply
> > here too. Please look through it.
> 
> Thanks for your review, will look and fix on top of v5.

I have later seen your newer versions posted which haven't made it
into my inbox. But looking at the changelog my comments are still
valid. Thanks for addressing them.

>From the changelog:

"Changes since v1:
-----------------
- Split into two drivers"

This is good, but recent practice is also to have all the drivers for
the same piece of hardware in a single file, see e.g. thunderx_edac.c.
I don't know how detailed this was discussed already, but I would
prefer to join the code.

-Robert