[PATCH 1/6] ufs: Add UFSHCD_QUIRK_BROKEN_64BIT_ADDRESS

[Marek Vasut]

On 8/16/23 00:05, Bhupesh Sharma wrote:
> On Wed, 16 Aug 2023 at 03:10, Marek Vasut <marek.vasut at mailbox.org> wrote:
>>
>> On 8/15/23 23:08, Bhupesh Sharma wrote:
>>> On Wed, 16 Aug 2023 at 02:31, Marek Vasut <marek.vasut at 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+renesas at mailbox.org>
>>>>>> ---
>>>>>> Cc: Faiz Abbas <faiz_abbas at 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.

Better use BIT() macro , can you fix that in Linux ? I'll fix it in V2 here.