Re: [RFC PATCH v7 12/12] memory: RAS2: Add memory RAS2 driver

[Jonathan Cameron]

On Thu, 28 Mar 2024 11:23:38 -0400
Yazen Ghannam <yazen.ghannam@xxxxxxx> wrote:

> On 2/23/2024 9:37 AM, shiju.jose@xxxxxxxxxx wrote:
> > From: Shiju Jose <shiju.jose@xxxxxxxxxx>
> > 
> > Memory RAS2 driver binds to the platform device add by the ACPI RAS2
> > driver.
> > Driver registers the PCC channel for communicating with the ACPI compliant
> > platform that contains RAS2 command support in the hardware.
> > 
> > Add interface functions to support configuring the parameters of HW patrol
> > scrubs in the system, which exposed to the kernel via the RAS2 and PCC,
> > using the RAS2 commands.
> > 
> > Add support for RAS2 platform devices to register with scrub subsystem
> > driver. This enables user to configure the parameters of HW patrol scrubs,
> > which exposed to the kernel via the RAS2 table, through the scrub sysfs
> > attributes.
> > 
> > Open Question:
> > Sysfs scrub control attribute "enable_background_scrub" is added for RAS2,
> > based on the feedback from Bill Schwartz <wschwartz@xxxxxxxxxxxxxxxxxxx
> > on v4 to enable/disable the background_scrubbing in the platform as defined in the
> > “Configure Scrub Parameters [INPUT]“ field  in RAS2 Table 5.87: Parameter Block
> > Structure for PATROL_SCRUB.
> > Is it a right approach to support "enable_background_scrub" in the sysfs
> > scrub control?
> > 
> > Signed-off-by: Shiju Jose <shiju.jose@xxxxxxxxxx>
> > ---
> >  drivers/memory/Kconfig       |  14 ++
> >  drivers/memory/Makefile      |   2 +
> >  drivers/memory/ras2.c        | 364 +++++++++++++++++++++++++++++++++++
> >  drivers/memory/ras2_common.c | 282 +++++++++++++++++++++++++++
> >  include/memory/ras2.h        |  88 +++++++++
> >  5 files changed, 750 insertions(+)
> >  create mode 100644 drivers/memory/ras2.c
> >  create mode 100644 drivers/memory/ras2_common.c
> >  create mode 100755 include/memory/ras2.h
> >   
> 
> Sorry if this was already covered, but why not put the common scrub and ras2
> changes under /drivers/ras/?

Hi Yazen

Location / naming etc is definitely an open question.
Given the OCP RAS API has repeatedly come up in discussions (and that
has a much wider scope), a RAS focused location may well make sense + some
renaming to avoid being memory scrub specific.

Jonathan

> 
> Thanks,
> Yazen