On Wed, 16 Aug 2023 at 03:10, Marek Vasut <marek.va...@mailbox.org> wrote: > > On 8/15/23 23:08, Bhupesh Sharma wrote: > > On Wed, 16 Aug 2023 at 02:31, Marek Vasut <marek.va...@mailbox.org> wrote: > >> > >> On 8/15/23 22:04, Bhupesh Sharma wrote: > >>> Hi Marek, > >>> > >>> On 8/14/23 5:22 AM, Marek Vasut wrote: > >>>> Add UFSHCD_QUIRK_BROKEN_64BIT_ADDRESS for host controllers which do not > >>>> support 64-bit addressing. > >>>> > >>>> Ported from Linux kernel commit > >>>> 6554400d6f66 ("scsi: ufs: core: Add UFSHCD_QUIRK_BROKEN_64BIT_ADDRESS") > >>>> with ufs_scsi_buffer_aligned() based on U-Boot generic bounce buffer. > >>>> > >>>> Signed-off-by: Marek Vasut <marek.vasut+rene...@mailbox.org> > >>>> --- > >>>> Cc: Faiz Abbas <faiz_ab...@ti.com> > >>>> --- > >>>> drivers/ufs/ufs.c | 26 ++++++++++++++++++++++++++ > >>>> drivers/ufs/ufs.h | 6 ++++++ > >>>> 2 files changed, 32 insertions(+) > >>>> > >>>> diff --git a/drivers/ufs/ufs.c b/drivers/ufs/ufs.c > >>>> index 3bf1a95e7f2..da0550d98c6 100644 > >>>> --- a/drivers/ufs/ufs.c > >>>> +++ b/drivers/ufs/ufs.c > >>>> @@ -8,6 +8,7 @@ > >>>> * Copyright (C) 2019 Texas Instruments Incorporated - > >>>> http://www.ti.com > >>>> */ > >>>> +#include <bouncebuf.h> > >>>> #include <charset.h> > >>>> #include <common.h> > >>>> #include <dm.h> > >>>> @@ -1889,6 +1890,8 @@ int ufshcd_probe(struct udevice *ufs_dev, struct > >>>> ufs_hba_ops *hba_ops) > >>>> /* Read capabilties registers */ > >>>> hba->capabilities = ufshcd_readl(hba, > >>>> REG_CONTROLLER_CAPABILITIES); > >>>> + if (hba->quirks & UFSHCD_QUIRK_BROKEN_64BIT_ADDRESS) > >>>> + hba->capabilities &= ~MASK_64_ADDRESSING_SUPPORT; > >>>> /* Get UFS version supported by the controller */ > >>>> hba->version = ufshcd_get_ufs_version(hba); > >>>> @@ -1942,8 +1945,31 @@ int ufs_scsi_bind(struct udevice *ufs_dev, > >>>> struct udevice **scsi_devp) > >>>> return ret; > >>>> } > >>>> +#if IS_ENABLED(CONFIG_BOUNCE_BUFFER) > >>>> +static int ufs_scsi_buffer_aligned(struct udevice *scsi_dev, struct > >>>> bounce_buffer *state) > >>>> +{ > >>>> +#ifdef CONFIG_PHYS_64BIT > >>>> + struct ufs_hba *hba = dev_get_uclass_priv(scsi_dev->parent); > >>>> + uintptr_t ubuf = (uintptr_t)state->user_buffer; > >>>> + size_t len = state->len_aligned; > >>>> + > >>>> + /* Check if below 32bit boundary */ > >>>> + if ((hba->quirks & UFSHCD_QUIRK_BROKEN_64BIT_ADDRESS) && > >>>> + ((ubuf >> 32) || (ubuf + len) >> 32)) { > >>>> + dev_dbg(scsi_dev, "Buffer above 32bit boundary %lx-%lx\n", > >>>> + ubuf, ubuf + len); > >>>> + return 0; > >>>> + } > >>>> +#endif > >>>> + return 1; > >>>> +} > >>>> +#endif /* CONFIG_BOUNCE_BUFFER */ > >>>> + > >>>> static struct scsi_ops ufs_ops = { > >>>> .exec = ufs_scsi_exec, > >>>> +#if IS_ENABLED(CONFIG_BOUNCE_BUFFER) > >>>> + .buffer_aligned = ufs_scsi_buffer_aligned, > >>>> +#endif /* CONFIG_BOUNCE_BUFFER */ > >>>> }; > >>>> int ufs_probe_dev(int index) > >>>> diff --git a/drivers/ufs/ufs.h b/drivers/ufs/ufs.h > >>>> index 8a38832b05f..070db0dce68 100644 > >>>> --- a/drivers/ufs/ufs.h > >>>> +++ b/drivers/ufs/ufs.h > >>>> @@ -719,6 +719,12 @@ struct ufs_hba { > >>>> */ > >>>> #define UFSHCD_QUIRK_BROKEN_LCC 0x1 > >>>> +/* > >>>> + * This quirk needs to be enabled if the host controller has > >>>> + * 64-bit addressing supported capability but it doesn't work. > >>>> + */ > >>>> +#define UFSHCD_QUIRK_BROKEN_64BIT_ADDRESS 0x2 > >>>> + > >>>> /* Virtual memory reference */ > >>>> struct utp_transfer_cmd_desc *ucdl; > >>>> struct utp_transfer_req_desc *utrdl; > >>> > >>> For this and the follow-up patch, I think implementing or at least > >>> supporting all the quirks currently supported by the Linux UFS driver > >>> makes sense as newer controllers do support these quirks. > >> > >> I disagree. There is no point in implementing quirks which cannot be > >> tested or may not even be relevant to U-Boot. Quirks which are needed > >> should be added one by one. > > I assume you agree we should not add all quirks at once ? > > > But the format you use for adding new quiks in u-boot is still in sync > > with Linux version 3.10. Can we atleast use the newer ( 1<< 7) kind of > > format for consistency. > > Feel free to update quirks to BIT() macro. > > We should enumerate the quirks in order instead of starting with BIT(7) > however.
Qualcomm UFS controllers support a few quirks from the list so the numbering can be taken care of in my patch. But since 'include/ufs/ufs_quirks.h' in Linux already uses the (1 <<7) kind of format we can use the same as it would make future backporting from Linux easier. Thanks.