All of lore.kernel.org
 help / color / mirror / Atom feed
From: Lars Povlsen <lars.povlsen@microchip.com>
To: Adrian Hunter <adrian.hunter@intel.com>
Cc: Ulf Hansson <ulf.hansson@linaro.org>, SoC Team <soc@kernel.org>,
	"Microchip Linux Driver Support" <UNGLinuxDriver@microchip.com>,
	<linux-mmc@vger.kernel.org>, <devicetree@vger.kernel.org>,
	<linux-arm-kernel@lists.infradead.org>,
	<linux-kernel@vger.kernel.org>,
	Alexandre Belloni <alexandre.belloni@bootlin.com>,
	Lars Povlsen <lars.povlsen@microchip.com>
Subject: Re: [PATCH 2/3] sdhci: sparx5: Add Sparx5 SoC eMMC driver
Date: Wed, 20 May 2020 13:14:04 +0200	[thread overview]
Message-ID: <87wo56q2o3.fsf@soft-dev15.microsemi.net> (raw)
In-Reply-To: <87v9ktoc0h.fsf@soft-dev15.microsemi.net>


Lars Povlsen writes:

> Adrian Hunter writes:
>
>> On 13/05/20 4:31 pm, Lars Povlsen wrote:
>>> This adds the eMMC driver for the Sparx5 SoC. It is based upon the
>>> designware IP, but requires some extra initialization and quirks.
>>>
>>> Reviewed-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
>>> Signed-off-by: Lars Povlsen <lars.povlsen@microchip.com>
>>> ---
{Snip]
>>> +};
>>> +
>>> +static const struct sdhci_pltfm_data sdhci_sparx5_pdata = {
>>> +     .quirks  = 0,
>>> +     .quirks2 = SDHCI_QUIRK2_HOST_NO_CMD23 | /* Card quirk */
>>
>> If this is a card quirk then it should be in drivers/mmc/core/quirks.h not here.
>

Adrian, I had a go at changing the controller quirk to a card quirk.

Unfortunately, SDHCI_QUIRK2_HOST_NO_CMD23 does not directly translate to
MMC_QUIRK_BLK_NO_CMD23, as for 'do_rel_wr' in mmc_blk_rw_rq_prep(), it
will *still* use MMC_SET_BLOCK_COUNT (cmd23), causing the issue.

We are using a ISSI "IS004G" device, and so I have gone through the
motions of adding it to quirks.h. The comment before the list of devices
using MMC_QUIRK_BLK_NO_CMD23 suggest working around a performance issue,
which is not exactly the issue I'm seeing. I'm seeing combinations of
CMD_TOUT_ERR, DATA_CRC_ERR and DATA_END_BIT_ERR whenever a cmd23 is
issued.

I have not been able to test the controller with another eMMC device
yet, but I expect its not the controller at fault.

So, I'm a little bit in doubt of how to proceed - either keep the quirk
as a controller quirk - or make a *new* card quirk (with
SDHCI_QUIRK2_HOST_NO_CMD23 semantics)?

Anybody else have had experience with ISSI eMMC devices?

I have also tried to use DT sdhci-caps-mask, but MMC_CAP_CMD23 is not
read from the controller just (unconditionally) set in sdhci.c - so that
doesn't fly either.

Any suggestions?

> Yes, its supposedly a card quirk. I'll see to use the card quirks
> methods in place.
>

-- 
Lars Povlsen,
Microchip

WARNING: multiple messages have this Message-ID (diff)
From: Lars Povlsen <lars.povlsen@microchip.com>
To: Adrian Hunter <adrian.hunter@intel.com>
Cc: devicetree@vger.kernel.org, Ulf Hansson <ulf.hansson@linaro.org>,
	Alexandre Belloni <alexandre.belloni@bootlin.com>,
	linux-mmc@vger.kernel.org, linux-kernel@vger.kernel.org,
	Microchip Linux Driver Support <UNGLinuxDriver@microchip.com>,
	SoC Team <soc@kernel.org>,
	Lars Povlsen <lars.povlsen@microchip.com>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 2/3] sdhci: sparx5: Add Sparx5 SoC eMMC driver
Date: Wed, 20 May 2020 13:14:04 +0200	[thread overview]
Message-ID: <87wo56q2o3.fsf@soft-dev15.microsemi.net> (raw)
In-Reply-To: <87v9ktoc0h.fsf@soft-dev15.microsemi.net>


Lars Povlsen writes:

> Adrian Hunter writes:
>
>> On 13/05/20 4:31 pm, Lars Povlsen wrote:
>>> This adds the eMMC driver for the Sparx5 SoC. It is based upon the
>>> designware IP, but requires some extra initialization and quirks.
>>>
>>> Reviewed-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
>>> Signed-off-by: Lars Povlsen <lars.povlsen@microchip.com>
>>> ---
{Snip]
>>> +};
>>> +
>>> +static const struct sdhci_pltfm_data sdhci_sparx5_pdata = {
>>> +     .quirks  = 0,
>>> +     .quirks2 = SDHCI_QUIRK2_HOST_NO_CMD23 | /* Card quirk */
>>
>> If this is a card quirk then it should be in drivers/mmc/core/quirks.h not here.
>

Adrian, I had a go at changing the controller quirk to a card quirk.

Unfortunately, SDHCI_QUIRK2_HOST_NO_CMD23 does not directly translate to
MMC_QUIRK_BLK_NO_CMD23, as for 'do_rel_wr' in mmc_blk_rw_rq_prep(), it
will *still* use MMC_SET_BLOCK_COUNT (cmd23), causing the issue.

We are using a ISSI "IS004G" device, and so I have gone through the
motions of adding it to quirks.h. The comment before the list of devices
using MMC_QUIRK_BLK_NO_CMD23 suggest working around a performance issue,
which is not exactly the issue I'm seeing. I'm seeing combinations of
CMD_TOUT_ERR, DATA_CRC_ERR and DATA_END_BIT_ERR whenever a cmd23 is
issued.

I have not been able to test the controller with another eMMC device
yet, but I expect its not the controller at fault.

So, I'm a little bit in doubt of how to proceed - either keep the quirk
as a controller quirk - or make a *new* card quirk (with
SDHCI_QUIRK2_HOST_NO_CMD23 semantics)?

Anybody else have had experience with ISSI eMMC devices?

I have also tried to use DT sdhci-caps-mask, but MMC_CAP_CMD23 is not
read from the controller just (unconditionally) set in sdhci.c - so that
doesn't fly either.

Any suggestions?

> Yes, its supposedly a card quirk. I'll see to use the card quirks
> methods in place.
>

-- 
Lars Povlsen,
Microchip

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2020-05-20 11:14 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-13 13:31 [PATCH 0/3] mmc: Adding support for Microchip Sparx5 SoC Lars Povlsen
2020-05-13 13:31 ` Lars Povlsen
2020-05-13 13:31 ` [PATCH 1/3] dt-bindings: mmc: Add Sparx5 SDHCI controller bindings Lars Povlsen
2020-05-13 13:31   ` Lars Povlsen
2020-05-14 13:03   ` Rob Herring
2020-05-14 13:03     ` Rob Herring
2020-05-15 15:50     ` Lars Povlsen
2020-05-15 15:50       ` Lars Povlsen
2020-05-13 13:31 ` [PATCH 2/3] sdhci: sparx5: Add Sparx5 SoC eMMC driver Lars Povlsen
2020-05-13 13:31   ` Lars Povlsen
2020-05-17 12:58   ` Adrian Hunter
2020-05-17 12:58     ` Adrian Hunter
2020-05-18  8:58     ` Lars Povlsen
2020-05-18  8:58       ` Lars Povlsen
2020-05-20 11:14       ` Lars Povlsen [this message]
2020-05-20 11:14         ` Lars Povlsen
2020-05-24 19:26         ` Adrian Hunter
2020-05-24 19:26           ` Adrian Hunter
2020-05-25 14:26           ` Lars Povlsen
2020-05-25 14:26             ` Lars Povlsen
2020-05-29 14:11             ` Lars Povlsen
2020-05-29 14:11               ` Lars Povlsen
2020-05-13 13:31 ` [PATCH 3/3] arm64: dts: sparx5: Add Sparx5 eMMC support Lars Povlsen
2020-05-13 13:31   ` Lars Povlsen

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=87wo56q2o3.fsf@soft-dev15.microsemi.net \
    --to=lars.povlsen@microchip.com \
    --cc=UNGLinuxDriver@microchip.com \
    --cc=adrian.hunter@intel.com \
    --cc=alexandre.belloni@bootlin.com \
    --cc=devicetree@vger.kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mmc@vger.kernel.org \
    --cc=soc@kernel.org \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.