On Wed, 5 Mar 2025 18:02:24 +0000
<shiju.jose@xxxxxxxxxx> wrote:
> From: Shiju Jose <shiju.jose@xxxxxxxxxx>
>
> Memory ACPI RAS2 auxiliary driver binds to the auxiliary device
> add by the ACPI RAS2 table parser.
>
> Driver uses a PCC subspace for communicating with the ACPI compliant
> platform.
>
> Device with ACPI RAS2 scrub feature registers with EDAC device driver,
> which retrieves the scrub descriptor from EDAC scrub and exposes
> the scrub control attributes for RAS2 scrub instance to userspace in
> /sys/bus/edac/devices/acpi_ras_mem0/scrubX/.
>
> Co-developed-by: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx>
> Tested-by: Daniel Ferguson <danielf@xxxxxxxxxxxxxxxxxxxxxx>
> Signed-off-by: Shiju Jose <shiju.jose@xxxxxxxxxx>
> diff --git a/Documentation/edac/scrub.rst b/Documentation/edac/scrub.rst
> index daab929cdba1..fc8dcbd13f91 100644
> --- a/Documentation/edac/scrub.rst
> +++ b/Documentation/edac/scrub.rst
> @@ -264,3 +264,76 @@ Sysfs files are documented in
...
> +1.2.4. Program background scrubbing for RAS2 device to repeat in every 21600 seconds (quarter of a day).
wrap to 80 chars. I think that is fine for titles in sphinx.
> +
> +# echo 21600 > /sys/bus/edac/devices/acpi_ras_mem0/scrub0/current_cycle_duration
> +
> +1.2.5. Start 'background scrubbing'.
> +
> +# echo 1 > /sys/bus/edac/devices/acpi_ras_mem0/scrub0/enable_background
> diff --git a/drivers/ras/acpi_ras2.c b/drivers/ras/acpi_ras2.c
> new file mode 100644
> index 000000000000..2f9317aa7b81
> --- /dev/null
> +++ b/drivers/ras/acpi_ras2.c
> @@ -0,0 +1,391 @@
> +struct acpi_ras2_ps_shared_mem {
> + struct acpi_ras2_shmem common;
> + struct acpi_ras2_patrol_scrub_param params;
> +};
...
> +static int ras2_update_patrol_scrub_params_cache(struct ras2_mem_ctx *ras2_ctx)
> +{
> + struct acpi_ras2_ps_shared_mem __iomem *ps_sm =
> + (void *)ras2_ctx->comm_addr;
Would a container_of() be better here given the type cast is doing
that with the assumption of it being first element of ps_shared_mem.
Same in other places, so maybe a macro.
> + int ret;
> +
> + ps_sm->common.set_caps[0] = RAS2_SUPPORT_HW_PARTOL_SCRUB;
> + ps_sm->params.cmd = RAS2_GET_PATROL_PARAMETERS;
...
> +
> +static int ras2_hw_scrub_set_enabled_bg(struct device *dev, void *drv_data, bool enable)
> +{
> + struct ras2_mem_ctx *ras2_ctx = drv_data;
> + struct acpi_ras2_ps_shared_mem __iomem *ps_sm =
> + (void *)ras2_ctx->comm_addr;
As above, maybe container_of appropriate as we have
a definition of what we are casting it to that has the thing
we are casting from as first element.
> + bool running;
> + int ret;
> +
...
> +
> +static int ras2_probe(struct auxiliary_device *auxdev,
> + const struct auxiliary_device_id *id)
> +{
> + struct ras2_mem_ctx *ras2_ctx = container_of(auxdev, struct ras2_mem_ctx, adev);
> + struct edac_dev_feature ras_features[RAS2_DEV_NUM_RAS_FEATURES];
Given we only have 1 RAS2 feature I'd be tempted to leave
making this flexible for some future series that adds a second one.
So maybe just have a single feature rather than array of 1.
> + char scrub_name[RAS2_SCRUB_NAME_LEN];
> + int num_ras_features = 0;
With change below this isn't needed.
> + int ret;
> +
> + if (!ras2_is_patrol_scrub_support(ras2_ctx))
> + return -EOPNOTSUPP;
> +
> + ret = ras2_update_patrol_scrub_params_cache(ras2_ctx);
> + if (ret)
> + return ret;
> +
> + snprintf(scrub_name, sizeof(scrub_name), "acpi_ras_mem%d",
> + ras2_ctx->id);
> +
> + ras_features[num_ras_features].ft_type = RAS_FEAT_SCRUB;
> + ras_features[num_ras_features].instance = ras2_ctx->instance;
> + ras_features[num_ras_features].scrub_ops = &ras2_scrub_ops;
> + ras_features[num_ras_features].ctx = ras2_ctx;
> + num_ras_features++;
As above, can also just assume this is 1 becasue it always is.
> +
> + return edac_dev_register(&auxdev->dev, scrub_name, NULL,
> + num_ras_features, ras_features);
here pass in &ras_feature after making it not be an array.
> +}
