* [PATCH v3 1/2] mmc: core: Fix variable shadowing in mmc_route_rpmb_frames()
2025-09-11 21:06 [PATCH v3 0/2] mmc: core: RPMB code improvements Bean Huo
@ 2025-09-11 21:06 ` Bean Huo
2025-09-11 21:06 ` [PATCH v3 2/2] mmc: core: Improve RPMB frame handling code Bean Huo
2025-09-12 13:34 ` [PATCH v3 0/2] mmc: core: RPMB code improvements Ulf Hansson
2 siblings, 0 replies; 6+ messages in thread
From: Bean Huo @ 2025-09-11 21:06 UTC (permalink / raw)
To: ulf.hansson, linux-mmc, jens.wiklander, Avri.Altman; +Cc: Bean Huo, Avri Altman
From: Bean Huo <beanhuo@micron.com>
Rename the inner 'frm' variable to 'resp_frm' in the write path of
mmc_route_rpmb_frames() to avoid shadowing the outer 'frm' variable.
The function declares 'frm' at function scope pointing to the request
frame, but then redeclares another 'frm' variable inside the write
block pointing to the response frame. This shadowing makes the code
confusing and error-prone.
Using 'resp_frm' for the response frame makes the distinction clear
and improves code readability.
Fixes: 7852028a35f0 ("mmc: block: register RPMB partition with the RPMB subsystem")
Reviewed-by: Avri Altman <avri.altman@sandisk.com>
Reviewed-by: Jens Wiklander <jens.wiklander@linaro.org>
Signed-off-by: Bean Huo <beanhuo@micron.com>
---
drivers/mmc/core/block.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
index 9cc47bf94804..dd6cffc0df72 100644
--- a/drivers/mmc/core/block.c
+++ b/drivers/mmc/core/block.c
@@ -2936,15 +2936,15 @@ static int mmc_route_rpmb_frames(struct device *dev, u8 *req,
return -ENOMEM;
if (write) {
- struct rpmb_frame *frm = (struct rpmb_frame *)resp;
+ struct rpmb_frame *resp_frm = (struct rpmb_frame *)resp;
/* Send write request frame(s) */
set_idata(idata[0], MMC_WRITE_MULTIPLE_BLOCK,
1 | MMC_CMD23_ARG_REL_WR, req, req_len);
/* Send result request frame */
- memset(frm, 0, sizeof(*frm));
- frm->req_resp = cpu_to_be16(RPMB_RESULT_READ);
+ memset(resp_frm, 0, sizeof(*resp_frm));
+ resp_frm->req_resp = cpu_to_be16(RPMB_RESULT_READ);
set_idata(idata[1], MMC_WRITE_MULTIPLE_BLOCK, 1, resp,
resp_len);
--
2.34.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v3 2/2] mmc: core: Improve RPMB frame handling code
2025-09-11 21:06 [PATCH v3 0/2] mmc: core: RPMB code improvements Bean Huo
2025-09-11 21:06 ` [PATCH v3 1/2] mmc: core: Fix variable shadowing in mmc_route_rpmb_frames() Bean Huo
@ 2025-09-11 21:06 ` Bean Huo
2025-09-12 12:26 ` Avri Altman
2025-09-12 13:34 ` [PATCH v3 0/2] mmc: core: RPMB code improvements Ulf Hansson
2 siblings, 1 reply; 6+ messages in thread
From: Bean Huo @ 2025-09-11 21:06 UTC (permalink / raw)
To: ulf.hansson, linux-mmc, jens.wiklander, Avri.Altman; +Cc: Bean Huo
From: Bean Huo <beanhuo@micron.com>
Introduce RPMB_FRAME_SIZE, CHECK_SIZE_NEQ(), and CHECK_SIZE_ALIGNED()
macros to replace repetitive sizeof(struct rpmb_frame) checks in
mmc_route_rpmb_frames().
Signed-off-by: Bean Huo <beanhuo@micron.com>
---
drivers/mmc/core/block.c | 30 ++++++++++++++----------------
1 file changed, 14 insertions(+), 16 deletions(-)
diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
index dd6cffc0df72..b32eefcca4b7 100644
--- a/drivers/mmc/core/block.c
+++ b/drivers/mmc/core/block.c
@@ -121,6 +121,10 @@ struct rpmb_frame {
#define RPMB_READ_DATA 0x4 /* Read data from RPMB partition */
#define RPMB_RESULT_READ 0x5 /* Read result request (Internal) */
+#define RPMB_FRAME_SIZE sizeof(struct rpmb_frame)
+#define CHECK_SIZE_NEQ(val) ((val) != sizeof(struct rpmb_frame))
+#define CHECK_SIZE_ALIGNED(val) IS_ALIGNED((val), sizeof(struct rpmb_frame))
+
static DEFINE_MUTEX(block_mutex);
/*
@@ -2864,12 +2868,12 @@ static void set_idata(struct mmc_blk_ioc_data *idata, u32 opcode,
* The size of an RPMB frame must match what's expected by the
* hardware.
*/
- BUILD_BUG_ON(sizeof(struct rpmb_frame) != 512);
+ static_assert(!CHECK_SIZE_NEQ(512), "RPMB frame size must be 512 bytes");
idata->ic.opcode = opcode;
idata->ic.flags = MMC_RSP_R1 | MMC_CMD_ADTC;
idata->ic.write_flag = write_flag;
- idata->ic.blksz = sizeof(struct rpmb_frame);
+ idata->ic.blksz = RPMB_FRAME_SIZE;
idata->ic.blocks = buf_bytes / idata->ic.blksz;
idata->buf = buf;
idata->buf_bytes = buf_bytes;
@@ -2893,32 +2897,28 @@ static int mmc_route_rpmb_frames(struct device *dev, u8 *req,
if (IS_ERR(md->queue.card))
return PTR_ERR(md->queue.card);
- if (req_len < sizeof(*frm))
+ if (req_len < RPMB_FRAME_SIZE)
return -EINVAL;
req_type = be16_to_cpu(frm->req_resp);
switch (req_type) {
case RPMB_PROGRAM_KEY:
- if (req_len != sizeof(struct rpmb_frame) ||
- resp_len != sizeof(struct rpmb_frame))
+ if (CHECK_SIZE_NEQ(req_len) || CHECK_SIZE_NEQ(resp_len))
return -EINVAL;
write = true;
break;
case RPMB_GET_WRITE_COUNTER:
- if (req_len != sizeof(struct rpmb_frame) ||
- resp_len != sizeof(struct rpmb_frame))
+ if (CHECK_SIZE_NEQ(req_len) || CHECK_SIZE_NEQ(resp_len))
return -EINVAL;
write = false;
break;
case RPMB_WRITE_DATA:
- if (req_len % sizeof(struct rpmb_frame) ||
- resp_len != sizeof(struct rpmb_frame))
+ if (!CHECK_SIZE_ALIGNED(req_len) || CHECK_SIZE_NEQ(resp_len))
return -EINVAL;
write = true;
break;
case RPMB_READ_DATA:
- if (req_len != sizeof(struct rpmb_frame) ||
- resp_len % sizeof(struct rpmb_frame))
+ if (CHECK_SIZE_NEQ(req_len) || !CHECK_SIZE_ALIGNED(resp_len))
return -EINVAL;
write = false;
break;
@@ -2926,10 +2926,8 @@ static int mmc_route_rpmb_frames(struct device *dev, u8 *req,
return -EINVAL;
}
- if (write)
- cmd_count = 3;
- else
- cmd_count = 2;
+ /* Write operations require 3 commands, read operations require 2 */
+ cmd_count = write ? 3 : 2;
idata = alloc_idata(rpmb, cmd_count);
if (!idata)
@@ -2943,7 +2941,7 @@ static int mmc_route_rpmb_frames(struct device *dev, u8 *req,
1 | MMC_CMD23_ARG_REL_WR, req, req_len);
/* Send result request frame */
- memset(resp_frm, 0, sizeof(*resp_frm));
+ memset(resp_frm, 0, RPMB_FRAME_SIZE);
resp_frm->req_resp = cpu_to_be16(RPMB_RESULT_READ);
set_idata(idata[1], MMC_WRITE_MULTIPLE_BLOCK, 1, resp,
resp_len);
--
2.34.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v3 0/2] mmc: core: RPMB code improvements
2025-09-11 21:06 [PATCH v3 0/2] mmc: core: RPMB code improvements Bean Huo
2025-09-11 21:06 ` [PATCH v3 1/2] mmc: core: Fix variable shadowing in mmc_route_rpmb_frames() Bean Huo
2025-09-11 21:06 ` [PATCH v3 2/2] mmc: core: Improve RPMB frame handling code Bean Huo
@ 2025-09-12 13:34 ` Ulf Hansson
2025-09-12 14:58 ` Bean Huo
2 siblings, 1 reply; 6+ messages in thread
From: Ulf Hansson @ 2025-09-12 13:34 UTC (permalink / raw)
To: Bean Huo; +Cc: linux-mmc, jens.wiklander, Avri.Altman
On Thu, 11 Sept 2025 at 23:06, Bean Huo <beanhuo@iokpp.de> wrote:
>
> This patch series improves the RPMB frame handling code in the MMC block
> driver with several code quality enhancements.
>
>
> v2--v3:
> 1. Previous version was mistakenly mixed with other changes, dropped them from v3.
>
> v1--v2:
> 1. Add fix tag
> 2. Incorporate Avri's suggestions
>
> Bean Huo (2):
> mmc: core: Fix variable shadowing in mmc_route_rpmb_frames()
> mmc: core: Improve RPMB frame handling code
>
> drivers/mmc/core/block.c | 34 ++++++++++++++++------------------
> 1 file changed, 16 insertions(+), 18 deletions(-)
>
> --
> 2.34.1
>
Applied for next, but dropping the Fixes-tag from patch 1 (it's just a
nice improvement of the code, not really fixing an error, right?),
thanks!
Kind regards
Uffe
^ permalink raw reply [flat|nested] 6+ messages in thread