From: Adrian Hunter <adrian.hunter@intel.com>
To: Sowjanya Komatineni <skomatineni@nvidia.com>,
Ritesh Harjani <riteshh@codeaurora.org>,
"ulf.hansson@linaro.org" <ulf.hansson@linaro.org>,
"robh+dt@kernel.org" <robh+dt@kernel.org>,
"mark.rutland@arm.com" <mark.rutland@arm.com>,
Asutosh Das <asutoshd@codeaurora.org>
Cc: "thierry.reding@gmail.com" <thierry.reding@gmail.com>,
Jonathan Hunter <jonathanh@nvidia.com>,
Aniruddha Tvs Rao <anrao@nvidia.com>,
"linux-tegra@vger.kernel.org" <linux-tegra@vger.kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"linux-mmc@vger.kernel.org" <linux-mmc@vger.kernel.org>,
"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>
Subject: Re: [PATCH V1 07/11] mmc: cqhci: add quirk for setting DCMD CMD_TIMING
Date: Fri, 8 Mar 2019 14:29:32 +0200 [thread overview]
Message-ID: <1fa6f529-2cb8-0108-2041-99ad9b39498f@intel.com> (raw)
In-Reply-To: <BYAPR12MB33984D80A404EF919F3A86A0C24C0@BYAPR12MB3398.namprd12.prod.outlook.com>
On 7/03/19 8:16 PM, Sowjanya Komatineni wrote:
>
>> On 3/6/2019 6:30 PM, Adrian Hunter wrote:
>>> On 2/03/19 7:20 AM, Sowjanya Komatineni wrote:
>>>> This patch adds a quirk for setting CMD_TIMING to 1 in descriptor for
>>>> DCMD with R1B response type to allow the command to be sent to device
>>>> during data activity or busy time.
>>>>
>>>> Tegra186 CQHCI host has bug where it selects DATA_PRESENT_SELECT to 1
>>>> by CQHCI controller for DCMDs with R1B response type and since DCMD
>>>> does not trigger any data transfer, DCMD task complete happens
>>>> leaving the DATA FSM of host controller in wait state for data.
>>>>
>>>> This effects the data transfer task issued after R1B DCMD task and no
>>>> interrupt is generated for the data transfer task.
>>>>
>>>> SW WAR for this issue is to set CMD_TIMING bit to 1 in DCMD task
>>>> descriptor and as DCMD task descriptor preparation is done by cqhci
>>>> driver, this patch adds cqequirk to handle this.
>>>>
>>>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
>>>> ---
>>>> drivers/mmc/host/cqhci.c | 5 ++++-
>>>> drivers/mmc/host/cqhci.h | 1 +
>>>> 2 files changed, 5 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/mmc/host/cqhci.c b/drivers/mmc/host/cqhci.c
>>>> index a8af682a9182..b34c07125f32 100644
>>>> --- a/drivers/mmc/host/cqhci.c
>>>> +++ b/drivers/mmc/host/cqhci.c
>>>> @@ -521,7 +521,10 @@ static void cqhci_prep_dcmd_desc(struct mmc_host *mmc,
>>>> } else {
>>>> if (mrq->cmd->flags & MMC_RSP_R1B) {
>>>> resp_type = 0x3;
>>>> - timing = 0x0;
>>>> + if (cq_host->quirks & CQHCI_QUIRK_CMD_TIMING_R1B_DCMD)
>>>> + timing = 0x1;
>>>> + else
>>>> + timing = 0x0;
>>> I was thinking it would be nice if there was a generic way for drivers
>>> to make changes to descriptors before a task is started. Currently
>>> there is
>>> host->ops->write_l() which would make it possible by checking for
>>> host->ops->CQHCI_TDBR
>>> register and, in this case, the DCMD tag. We would need to export
>>> get_desc(), perhaps rename it cqhci_get_desc() and put it in cqhci.h
>>> since it is an inline function.
>>
>> We take spin_lock_irqsave after the descriptor is prepared and before writing to TDBR.
>> Not sure but tomorrow this may become a limitation for drivers to make changes to descriptors if they edit descriptors in host->ops->write_l() call.
>> Though in this case it is not required here.
>>
>>>
>>> Alternatively we could add host->ops for descriptor preparation.
>>
>> Both ways sounds good to me.
>> But maybe adding a host->ops for descriptor preparation is better way to go,
>> since that will be the right interface exposed to make changes to
>> descriptors.
>>
>
> DCMD descriptor attributes remain same for any host and also task parameters as QBR need to be enabled with DCMD.
> So I believe it should be ok if we just add callback to allow hosts to update command parameters of DCMD descriptor only thru cqhci_host_ops.
That sounds fine to me. Maybe do that for the next version of this patchset.
>
> Also, don’t see any requirement for host specific Task parameter updates in Task descriptor so not sure if there is any need to provide callback for task descriptor data preparation to hosts.
> Please confirm.
>
>>>
>>> What do people think?
>>>
>>>> } else {
>>>> resp_type = 0x2;
>>>> timing = 0x1;
>>>> diff --git a/drivers/mmc/host/cqhci.h b/drivers/mmc/host/cqhci.h
>>>> index 9e68286a07b4..f96d8565cc07 100644
>>>> --- a/drivers/mmc/host/cqhci.h
>>>> +++ b/drivers/mmc/host/cqhci.h
>>>> @@ -170,6 +170,7 @@ struct cqhci_host {
>>>>
>>>> u32 quirks;
>>>> #define CQHCI_QUIRK_SHORT_TXFR_DESC_SZ 0x1
>>>> +#define CQHCI_QUIRK_CMD_TIMING_R1B_DCMD 0x2
>>>>
>>>> bool enabled;
>>>> bool halted;
>>>>
next prev parent reply other threads:[~2019-03-08 12:29 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-03-02 5:20 [PATCH V1 01/11] mmc: tegra: fix ddr signaling for non-ddr modes Sowjanya Komatineni
2019-03-02 5:20 ` [PATCH V1 02/11] mmc: sdhci: allow host to specify maximum tuning loops Sowjanya Komatineni
2019-03-08 11:50 ` Adrian Hunter
2019-03-02 5:20 ` [PATCH V1 03/11] mmc: sdhci: add support for post tuning process Sowjanya Komatineni
2019-03-08 11:55 ` Adrian Hunter
2019-03-02 5:20 ` [PATCH V1 04/11] mmc: tegra: update hw " Sowjanya Komatineni
2019-03-08 12:07 ` Adrian Hunter
2019-03-02 5:20 ` [PATCH V1 05/11] dt-bindings: mmc: tegra: document Tegra194 compatible string Sowjanya Komatineni
2019-03-02 5:20 ` [PATCH V1 06/11] arm64: tegra: fix default tap and trim values Sowjanya Komatineni
2019-03-02 5:20 ` [PATCH V1 07/11] mmc: cqhci: add quirk for setting DCMD CMD_TIMING Sowjanya Komatineni
2019-03-06 13:00 ` Adrian Hunter
2019-03-07 2:43 ` Ritesh Harjani
2019-03-07 18:16 ` Sowjanya Komatineni
2019-03-08 12:29 ` Adrian Hunter [this message]
2019-03-09 5:14 ` Sowjanya Komatineni
2019-03-13 2:31 ` Ritesh Harjani
2019-03-13 9:56 ` Hunter, Adrian
2019-03-13 15:47 ` Sowjanya Komatineni
2019-03-02 5:20 ` [PATCH V1 08/11] mmc: tegra: add Tegra186 WAR for CQE Sowjanya Komatineni
2019-03-02 5:20 ` [PATCH V1 09/11] mmc: cqhci: add CQHCI_SSC1 register CBC field mask Sowjanya Komatineni
2019-03-08 12:14 ` Adrian Hunter
2019-03-02 5:20 ` [PATCH V1 10/11] mmc: tegra: fix CQE resume sequence Sowjanya Komatineni
2019-03-08 12:59 ` Adrian Hunter
2019-03-02 5:20 ` [PATCH V1 11/11] arm64: tegra: enable command queue for tegra186 sdmmc4 Sowjanya Komatineni
2019-03-07 21:31 ` [PATCH V1 01/11] mmc: tegra: fix ddr signaling for non-ddr modes Jon Hunter
2019-03-08 11:44 ` Adrian Hunter
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1fa6f529-2cb8-0108-2041-99ad9b39498f@intel.com \
--to=adrian.hunter@intel.com \
--cc=anrao@nvidia.com \
--cc=asutoshd@codeaurora.org \
--cc=devicetree@vger.kernel.org \
--cc=jonathanh@nvidia.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mmc@vger.kernel.org \
--cc=linux-tegra@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=riteshh@codeaurora.org \
--cc=robh+dt@kernel.org \
--cc=skomatineni@nvidia.com \
--cc=thierry.reding@gmail.com \
--cc=ulf.hansson@linaro.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).