[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAKTNdwEd5-V6REf4BDTtm37tBzWZ2baf92X3A6HvHULsUj6=Kg@mail.gmail.com>
Date: Fri, 6 Feb 2026 10:49:50 +0400
From: Alexey Charkov <alchark@...pper.net>
To: Bart Van Assche <bvanassche@....org>
Cc: Alim Akhtar <alim.akhtar@...sung.com>, Avri Altman <avri.altman@....com>,
"James E.J. Bottomley" <James.Bottomley@...senpartnership.com>,
"Martin K. Petersen" <martin.petersen@...cle.com>, Bean Huo <beanhuo@...ron.com>,
Can Guo <can.guo@....qualcomm.com>, linux-scsi@...r.kernel.org,
linux-kernel@...r.kernel.org, stable@...r.kernel.org
Subject: Re: [PATCH v2] scsi: ufs: core: Fix RPMB region size detection for
UFS 2.2
Hi Bart,
On Thu, Feb 5, 2026 at 8:08 PM Bart Van Assche <bvanassche@....org> wrote:
>
> On 2/5/26 12:30 AM, Alexey Charkov wrote:
> > @@ -5249,6 +5250,20 @@ static void ufshcd_lu_init(struct ufs_hba *hba, struct scsi_device *sdev)
> > hba->dev_info.rpmb_region_size[1] = desc_buf[RPMB_UNIT_DESC_PARAM_REGION1_SIZE];
> > hba->dev_info.rpmb_region_size[2] = desc_buf[RPMB_UNIT_DESC_PARAM_REGION2_SIZE];
> > hba->dev_info.rpmb_region_size[3] = desc_buf[RPMB_UNIT_DESC_PARAM_REGION3_SIZE];
>
> Executing the above code if (hba->dev_info.wspecversion <= 0x0220) is
> risky, isn't it?
I don't think so. On <=0x0220 this part of the descriptor (four bytes
at offset 0x13) should always return zeros, so a compliant device
shouldn't get confused, nor make the driver confused.
The spec there is worded a bit weirdly, but it does say clearly that
these are set to zero, and I can confirm it on the devices I have at
hand (a couple of Biwin and a Samsung one, all 2.2 spec).
The spec says (Section 14.1.4.6 RPMB Unit Descriptor, table entry for
offset 13h):
4 bytes. dEraseBlockSize. Value 00h. User-configurable: no. Erase
Block Size In number of Logical Blocks. For RPMB, Erase Block Size is
ignored; set to ‘0’
Not sure what was the rationale for giving this region a name at all,
as it is effectively reserved and zeroed out (and then change the name
and purpose of it in the next spec version anyway). But in this
context "Value 00h" is all that matters AFAICT :)
Shall I also add a comment to that effect?
> > + if (hba->dev_info.wspecversion <= 0x0220) {
> > + /* These older spec chips have only one RPMB region,
> > + * sized between 128 kB minimum and 16 MB maximum.
> > + * No per region size fields are provided, so get it
> > + * from the logical block count and size fields for
> > + * compatibility
> > + */
>
> Please follow the Linux kernel coding style for source code comments.
> From Documentation/process/coding-style.rst:
>
> The preferred style for long (multi-line) comments is:
>
> .. code-block:: c
>
> /*
> * This is the preferred style for multi-line
Right, the top empty line. Thanks for the pointer!
Best regards,
Alexey
Powered by blists - more mailing lists