Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.38 as permitted sender) client-ip=23.128.96.38;
MIME-Version: 1.0
References: <20231201091034.936441-1-jorge@foundries.io>
In-Reply-To: <20231201091034.936441-1-jorge@foundries.io>
From:   Linus Walleij <linus.walleij@linaro.org>
Date:   Fri, 1 Dec 2023 10:28:52 +0100
Message-ID: <CACRpkdaMnphBxvqpmvNG_72O02Umny3qdxfyrKdA4BsDwZmtfg@mail.gmail.com>
Subject: Re: [PATCH] mmc: rpmb: fixes pause retune on all RPMB partitions.
To:     Jorge Ramirez-Ortiz <jorge@foundries.io>
Cc:     ulf.hansson@linaro.org, CLoehle@hyperstone.com,
        adrian.hunter@intel.com, jinpu.wang@ionos.com, hare@suse.de,
        beanhuo@micron.com, asuk4.q@gmail.com, yangyingliang@huawei.com,
        yibin.ding@unisoc.com, victor.shih@genesyslogic.com.tw,
        linux-mmc@vger.kernel.org, linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Hi Jorge,

thanks for your patch!

On Fri, Dec 1, 2023 at 10:10=E2=80=AFAM Jorge Ramirez-Ortiz <jorge@foundrie=
s.io> wrote:

> When RPMB was converted to a character device, it added support for
> multiple RPMB partitions (Commit 97548575bef3 ("mmc: block: Convert RPMB
> to a character device").
>
> One of the changes in this commit was transforming the variable
> target_part defined in __mmc_blk_ioctl_cmd into a bitmask.
>
> This inadvertedly regressed the validation check done in
> mmc_blk_part_switch_pre() and mmc_blk_part_switch_post().
>
> This commit fixes that regression.
>
> Fixes: 97548575bef3 ("mmc: block: Convert RPMB to a character device")
> Signed-off-by: Jorge Ramirez-Ortiz <jorge@foundries.io>

My bug :/
Shouldn't we also add Cc: stable@vger.kernel.org?


> +       const unsigned int mask =3D EXT_CSD_PART_CONFIG_ACC_RPMB;
>         int ret =3D 0;
>
> -       if (part_type =3D=3D EXT_CSD_PART_CONFIG_ACC_RPMB) {
> +       if (part_type & mask =3D=3D mask) {

That looks complex, can't we just:

if (part_type & EXT_CSD_PART_CONFIG_ACC_RPMB)?

> +       const unsigned int mask =3D EXT_CSD_PART_CONFIG_ACC_RPMB;
>         int ret =3D 0;
>
> -       if (part_type =3D=3D EXT_CSD_PART_CONFIG_ACC_RPMB) {
> +       if (part_type & mask =3D=3D mask) {

Dito here.

Yours,
Linus Walleij