From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 832CFC5475B for ; Tue, 20 Feb 2024 16:12:43 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id E22AF6B0078; Tue, 20 Feb 2024 11:12:42 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id DD2BB6B007B; Tue, 20 Feb 2024 11:12:42 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id C9B196B007D; Tue, 20 Feb 2024 11:12:42 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id BAC386B0078 for ; Tue, 20 Feb 2024 11:12:42 -0500 (EST) Received: from smtpin01.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id 576C712078D for ; Tue, 20 Feb 2024 16:12:42 +0000 (UTC) X-FDA: 81812675364.01.6500894 Received: from frasgout.his.huawei.com (frasgout.his.huawei.com [185.176.79.56]) by imf21.hostedemail.com (Postfix) with ESMTP id 34AEA1C001A for ; Tue, 20 Feb 2024 16:12:38 +0000 (UTC) Authentication-Results: imf21.hostedemail.com; dkim=none; dmarc=pass (policy=quarantine) header.from=huawei.com; spf=pass (imf21.hostedemail.com: domain of jonathan.cameron@huawei.com designates 185.176.79.56 as permitted sender) smtp.mailfrom=jonathan.cameron@huawei.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1708445559; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=ccsxJstrPxfuWrt52fAHJBKE9sdZ7PogctGew5I7nsQ=; b=5qJBOzbhp+vel8AURLlNSNuTxX4nysSxmZAD3M6kF8zFXcRgv6cSUt3k9a0wSoF0XQkF7m UVVz5WseVBTID36BJgs6WYCK/SRUrVdvZl3cIKCGpMqQkQM8CE3qCWowmCFRS0ESMpydiw 99mSGWoYG+FLsnKYZiRehEXZjamXIlE= ARC-Authentication-Results: i=1; imf21.hostedemail.com; dkim=none; dmarc=pass (policy=quarantine) header.from=huawei.com; spf=pass (imf21.hostedemail.com: domain of jonathan.cameron@huawei.com designates 185.176.79.56 as permitted sender) smtp.mailfrom=jonathan.cameron@huawei.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1708445559; a=rsa-sha256; cv=none; b=Cc4SglSMaQHjiu3iPu9mfajYKQION9RjLovANRU6SD3eFVcxRXYDHbPDHVwh3SlxGnF2yh 9EidV3WSLG9JeoeeN5qwEGbcZSYZtNAuXpjQOVvj4NfaWaGzuobCwvk39aNRMwjyWa69VZ dqTLsk423VSXRzOr3mgQMlQL30sit5s= Received: from mail.maildlp.com (unknown [172.18.186.231]) by frasgout.his.huawei.com (SkyGuard) with ESMTP id 4TfPTZ58hwz6K635; Wed, 21 Feb 2024 00:08:34 +0800 (CST) Received: from lhrpeml500005.china.huawei.com (unknown [7.191.163.240]) by mail.maildlp.com (Postfix) with ESMTPS id B9642140B54; Wed, 21 Feb 2024 00:12:34 +0800 (CST) Received: from localhost (10.202.227.76) by lhrpeml500005.china.huawei.com (7.191.163.240) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.35; Tue, 20 Feb 2024 16:12:33 +0000 Date: Tue, 20 Feb 2024 16:12:32 +0000 From: Jonathan Cameron To: CC: , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , Subject: Re: [RFC PATCH v6 12/12] memory: RAS2: Add memory RAS2 driver Message-ID: <20240220161232.00000496@Huawei.com> In-Reply-To: <20240215111455.1462-13-shiju.jose@huawei.com> References: <20240215111455.1462-1-shiju.jose@huawei.com> <20240215111455.1462-13-shiju.jose@huawei.com> Organization: Huawei Technologies Research and Development (UK) Ltd. X-Mailer: Claws Mail 4.1.0 (GTK 3.24.33; x86_64-w64-mingw32) MIME-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Originating-IP: [10.202.227.76] X-ClientProxiedBy: lhrpeml500005.china.huawei.com (7.191.163.240) To lhrpeml500005.china.huawei.com (7.191.163.240) X-Rspamd-Server: rspam09 X-Rspamd-Queue-Id: 34AEA1C001A X-Stat-Signature: tg3yg3k1q4bkmwb1q73s7urecyrdi9bw X-Rspam-User: X-HE-Tag: 1708445558-991515 X-HE-Meta: U2FsdGVkX19C4c8uNywB/ylBK8gSJYooFh4P4Giaji+7T0hRdcONT1ZZnM92sRSDNAhQfWfJIKhUuDIAlla37Rc3wMnBawpf1XP/926aoY33ECZ3hSt9KjCZuDmhDq59+01hhGQFvx+yHAObrO3QOFC5C+bjn2XiMKAHo+tzBFXF+sSKaCUBsAt9hp0tswQ9DXrn39CobVRykofPSB7k3zgTkgyWHeXQoVVoNampkeSqrfLNDm5cBTM3TMKZSq7NszzkmGCPJJdZvHE6RupLP6z5t4QL3SUgIdKcmhWhT652Qnc2DsPFhaDlbh9PP53W1APDMTbW8c7g73gB7Qa+vAtezJoD1WIzmgjUb1Az3eTt5bDOSjfbObwBv+ISARJswmb/GqKIG6sAUALWoZ+n/avo44Gz/9wR/A4sG8wY8uz8XTXr1uOzUk4hs25+fVh973+SygVW5Rq5stF3e3rIL0BLpbI8fGh9laFhZMvwiWrSV8ggldVH2WFDfGxSKjkPGElWC+WRrx02GRU83LIE8EQD8l4dbzu/B+DM5E9vvWrWH2icZdCd7QmTwoMgcVtoG/xnScVoaD4SlSlgp95s3l7PHcwH2xf8ruchUTSAdGpuP/IqqF5wg4lvyzoL1Nr0V88JETKmtvwn1nn96j43uWVXo9D0Z8Gk+oAozWlXVnxCEj4AAl9eKcOpVYKRBigmFPPoOQhsqLkgSLMnvl5cNVZa2+0nc0yD9m3P77HbJ9Ay1qJrASp4txZWctJ0H0Fv83f7hKXFhrhQSM4ybPZbeBCOnyC/ohOMkLkvJUWiWVc7PPOhAhYaly+27WlGomGlRF1TL+Hs6bG82iMuq6fyreM/v/R+YA+svsrUIBXy4J78kkaVy7EDHhKxvrJH/qbUo3ADki04+7M5ohKXPWQP4qoI8MLhyDO1v0Vmhw0+BKCIXMlPRPzbWhaq/+lgYTQqp0mjUU9aaIHbwYyhS0F 2SUrCx8q h6f8xDi2g7FNGuZJXOA3c7Ys7+5wKVPncaqDVRlML8Sg1YXT+3Gv7B3HTjoF7EFxPoRcXlDBz+227e8O6/A/nfdEWtNcbHhngrzFry9XatAQMvxHQZ7xEQMtOGIxJ/i7ou/ghmml6pPib9cTv5rFbgKmegaVje+DECusSJqUzITxMwWo+7Ib9ocxNmw9Pr+yda8U4v+hJw01Vskbk3QdViXvuPycLGnJNetuKnt6QXuFDnhA9XatCRn5zit5ej8to7kWB6+ns5gcF74DJ5HD3JIniePZoy2y3wTxU X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On Thu, 15 Feb 2024 19:14:54 +0800 wrote: > From: Shiju Jose >=20 > 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. >=20 > 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. >=20 > 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. >=20 > Open Question: > Sysfs scrub control attribute "enable_background_scrub" is added for RAS2, > based on the feedback from Bill Schwartz on v4 to enable/disable the background_scrubbing in the platform as defin= ed in the > =E2=80=9CConfigure Scrub Parameters [INPUT]=E2=80=9C 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? Does anyone know what this means? IIUC patrol scrub is always background... >=20 > Signed-off-by: Shiju Jose A few minor comments inline. Rushed review as out of time for today though so may have missed stuff. > diff --git a/drivers/memory/rasf_common.c b/drivers/memory/rasf_common.c > new file mode 100644 > index 000000000000..85f67308698d > --- /dev/null > +++ b/drivers/memory/rasf_common.c > @@ -0,0 +1,269 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later > +/* > + * rasf_common.c - Common functions for memory RASF driver > + * > + * Copyright (c) 2023 HiSilicon Limited. > + * > + * This driver implements call back functions for the scrub > + * configure driver to configure the parameters of the hw patrol > + * scrubbers in the system, which exposed via the ACPI RASF/RAS2 > + * table and PCC. > + */ > + > +#define pr_fmt(fmt) "MEMORY RASF COMMON: " fmt > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include of.h Shouldn't be here in an ACPI driver! > +#include > + > +#include > +#include > + > +static int enable_write(struct rasf_context *rasf_ctx, long val) > +{ > + int ret; > + bool enable =3D val; > + > + ret =3D rasf_ctx->ops->enable_scrub(rasf_ctx, enable); > + if (ret) { > + pr_err("enable patrol scrub fail, enable=3D%d ret=3D%d\n", > + enable, ret); dev_err(rasf_ctx->dev,... > + return ret; > + } > + > + return 0; > +} > + > +/** > + * rasf_hw_scrub_is_visible() - Callback to return attribute visibility > + * @drv_data: Pointer to driver-private data structure passed > + * as argument to devm_scrub_device_register(). > + * @attr_id: Scrub attribute > + * @region_id: ID of the memory region > + * > + * Returns: 0 on success, an error otherwise > + */ > +umode_t rasf_hw_scrub_is_visible(struct device *dev, u32 attr_id, int re= gion_id) > +{ > + switch (attr_id) { > + case scrub_rate_available: > + return 0444; > + case scrub_enable: > + case scrub_enable_background_scrub: > + return 0200; > + case scrub_addr_base: > + case scrub_addr_size: > + case scrub_rate: > + return 0644; As before, I'd prefer to see this passed the current permissions then just return those rather than encoding them here and in the attributes where they may end up out of sync > + default: > + return 0; > + } > +} > + > +/** > + * rasf_hw_scrub_read_strings() - Read callback for string attributes > + * @device: Pointer to scrub device > + * @attr_id: Scrub attribute > + * @region_id: ID of the memory region > + * @buf: Pointer to the buffer for copying returned string > + * > + * Returns: 0 on success, an error otherwise > + */ > +int rasf_hw_scrub_read_strings(struct device *device, u32 attr_id, int r= egion_id, > + char *buf) dev maybe instead of device. Shorter lines and it's very common shorthand. > +{ > + struct rasf_context *rasf_ctx; > + > + rasf_ctx =3D dev_get_drvdata(device); struct rasf_context *rasf_ctx =3D dev_get_drvdata(dev); Same throughout. > + > + switch (attr_id) { > + case scrub_rate_available: > + return rate_available_read(rasf_ctx, buf); > + default: > + return -ENOTSUPP; > + } > +}