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

Jonathan Cameron
2024-04-02T10:17:13Z

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

> On 2/23/2024 9:37 AM, shiju.jose@huawei.com wrote:
> > From: Shiju Jose <shiju.jose@huawei.com>
> >
> > 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@amperecomputing.com
> > 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@huawei.com>
> > ---
> > 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