| Message ID | 20250226220414.343659-5-peter.griffin@linaro.org (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | ufs-exynos fixes for gs101 | expand |
On 2/26/25 2:04 PM, Peter Griffin wrote: > - hci_writel(ufs, ilog2(DATA_UNIT_SIZE), HCI_TXPRDT_ENTRY_SIZE); > + > + if (hba->caps & UFSHCD_CAP_CRYPTO) > + val |= PRDT_PREFECT_EN; In a future patch series, please consider renaming PRDT_PREFECT_EN into PRDT_PREFECTH_EN. Thanks, Bart.
Hi Bart, Thanks for the review feedback. On Fri, 28 Feb 2025 at 19:18, Bart Van Assche <bvanassche@acm.org> wrote: > > On 2/26/25 2:04 PM, Peter Griffin wrote: > > - hci_writel(ufs, ilog2(DATA_UNIT_SIZE), HCI_TXPRDT_ENTRY_SIZE); > > + > > + if (hba->caps & UFSHCD_CAP_CRYPTO) > > + val |= PRDT_PREFECT_EN; > > In a future patch series, please consider renaming PRDT_PREFECT_EN into > PRDT_PREFECTH_EN. I was just checking the datasheet naming (it is listed as PRDT_PREFETCH_ENABLE). As well as the typo in my patch I think your reply also has a typo :) I'm assuming you would like it renamed to PRDT_PREFETCH_EN? Thanks, Peter
On Wed, Feb 26, 2025 at 10:04:12PM +0000, Peter Griffin wrote: > PRDT_PREFETCH_ENABLE[31] bit should be set when desctype field of > fmpsecurity0 register is type2 (double file encryption) or type3 > (file and disk excryption). Setting this bit enables PRDT > pre-fetching on both TXPRDT and RXPRDT. > > Signed-off-by: Peter Griffin <peter.griffin@linaro.org> I assume you mean that desctype 3 provides "support for file and disk encryption"? The driver does use desctype 3, but it only uses the "file encryption". So this confused me a bit. (BTW, in FMP terminology, "file encryption" seems to mean "use the key provided in the I/O request", and "disk encryption" seems to mean "use some key the firmware provided somehow". They can be cascaded, and the intended use cases are clearly file and disk encryption respectively, but they don't necessarily have to be used that way.) - Eric
Hi Eric, Thanks for your review feedback. On Wed, 5 Mar 2025 at 02:40, Eric Biggers <ebiggers@kernel.org> wrote: > > On Wed, Feb 26, 2025 at 10:04:12PM +0000, Peter Griffin wrote: > > PRDT_PREFETCH_ENABLE[31] bit should be set when desctype field of > > fmpsecurity0 register is type2 (double file encryption) or type3 > > (file and disk excryption). Setting this bit enables PRDT > > pre-fetching on both TXPRDT and RXPRDT. > > > > Signed-off-by: Peter Griffin <peter.griffin@linaro.org> > > I assume you mean that desctype 3 provides "support for file and disk > encryption"? Yes, the PRDT_PREFETCH_ENABLE description in the commit message I copied from the datasheet. But I can re-word it like you suggest if you think that it's clearer? I notice now there is also a typo with the word 'encryption' which I can also fix. This patch came about whilst comparing UFS SFR register dumps of upstream and downstream drivers. I noticed that PRDT_PREFETCH_ENABLE is enabled downstream but not upstream, and after checking the datasheet description it seemed like we should set this if exynos_ufs_fmp_init() completed and set CFG_DESCTYPE_3. > The driver does use desctype 3, but it only uses the "file > encryption". So this confused me a bit. (BTW, in FMP terminology, "file > encryption" seems to mean "use the key provided in the I/O request", and "disk > encryption" seems to mean "use some key the firmware provided somehow". They > can be cascaded, and the intended use cases are clearly file and disk encryption > respectively, but they don't necessarily have to be used that way.) Thanks for the additional context :) Peter
diff --git a/drivers/ufs/host/ufs-exynos.c b/drivers/ufs/host/ufs-exynos.c index 943cea569f66..27eb360458a7 100644 --- a/drivers/ufs/host/ufs-exynos.c +++ b/drivers/ufs/host/ufs-exynos.c @@ -1098,12 +1098,17 @@ static int exynos_ufs_post_link(struct ufs_hba *hba) struct exynos_ufs *ufs = ufshcd_get_variant(hba); struct phy *generic_phy = ufs->phy; struct exynos_ufs_uic_attr *attr = ufs->drv_data->uic_attr; + u32 val = ilog2(DATA_UNIT_SIZE); exynos_ufs_establish_connt(ufs); exynos_ufs_fit_aggr_timeout(ufs); hci_writel(ufs, 0xa, HCI_DATA_REORDER); - hci_writel(ufs, ilog2(DATA_UNIT_SIZE), HCI_TXPRDT_ENTRY_SIZE); + + if (hba->caps & UFSHCD_CAP_CRYPTO) + val |= PRDT_PREFECT_EN; + hci_writel(ufs, val, HCI_TXPRDT_ENTRY_SIZE); + hci_writel(ufs, ilog2(DATA_UNIT_SIZE), HCI_RXPRDT_ENTRY_SIZE); hci_writel(ufs, (1 << hba->nutrs) - 1, HCI_UTRL_NEXUS_TYPE); hci_writel(ufs, (1 << hba->nutmrs) - 1, HCI_UTMRL_NEXUS_TYPE);
PRDT_PREFETCH_ENABLE[31] bit should be set when desctype field of fmpsecurity0 register is type2 (double file encryption) or type3 (file and disk excryption). Setting this bit enables PRDT pre-fetching on both TXPRDT and RXPRDT. Signed-off-by: Peter Griffin <peter.griffin@linaro.org> --- drivers/ufs/host/ufs-exynos.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)