On Mon, 2019-03-11 at 02:16 +0000, Yinbo Zhu wrote:
> From: Yinbo Zhu <yinbo.zhu@xxxxxxx>
>
> Invalid Transfer Complete (IRQSTAT[TC]) bit could be set during
> multi-write operation even when the BLK_CNT in BLKATTR register
> has not reached zero. Therefore, Transfer Complete might be
> reported twice due to this erratum since a valid Transfer Complete
> occurs when BLK_CNT reaches zero. This erratum is to fix this issue
>
> Signed-off-by: Yinbo Zhu <yinbo.zhu@xxxxxxx>
> Acked-by: Adrian Hunter <adrian.hunter@xxxxxxxxx>
> ---
> Change in v5:
> write SDHCI_INT_DATA_END to SDHCI_INT_STATUS to clear it
>
>
> +static u32 esdhc_irq(struct sdhci_host *host, u32 intmask)
> +{
> + u32 command;
> +
> + if (of_find_compatible_node(NULL, NULL,
> + "fsl,p2020-esdhc")) {
This scans the entire device tree for any nodes matching this. On
every irq. In the interrupt handler.
That's nuts!
Firstly, why scan the entire device tree! It seems like all these
of_find_compatible_node(NULL, NULL, ...) calls are wrong. Don't you
want to know if *this* host is an fsl,p2020-esdhc? Not if any of
fsl,p2020-esdhc hosts exist, even if this isn't one?
This would be far better done with:
of_device_is_compatible(dev->of_node, "fsl,fsl,p2020-esdhc")
There is a match table in the driver:
static const struct of_device_id sdhci_esdhc_of_match[] = {
{ .compatible = "fsl,ls1021a-esdhc", .data = &ls1021a_esdhc_clk},
{ .compatible = "fsl,ls1046a-esdhc", .data = &ls1046a_esdhc_clk},
and so on. What you should do is add an entry for p2020 and give it
some platform data that indicates it has these quirks.
The irq handler should not be looking at the device tree on each IRQ.
Check a bit flag in the driver's state data.
Since you know if this irq callback is needed or not before the device
is registered, it would be even more efficient to simple not use it on
a non-p2020. I.e., set ops->irq to this handler function on p2020 and
set it to NULL on other devices. That way it never even gets called.
> + command = SDHCI_GET_CMD(sdhci_readw(host,
> + SDHCI_COMMAND));
> + if (command == MMC_WRITE_MULTIPLE_BLOCK &&
> + sdhci_readw(host, SDHCI_BLOCK_COUNT) &&
> + intmask & SDHCI_INT_DATA_END) {
> + intmask &= ~SDHCI_INT_DATA_END;
> + sdhci_writel(host, SDHCI_INT_DATA_END,
> + SDHCI_INT_STATUS);
> + }
> + }
> + return intmask;
> +}
> +
> #ifdef CONFIG_PM_SLEEP
> static u32 esdhc_proctl;
> static int esdhc_of_suspend(struct device *dev)
> @@ -914,6 +934,7 @@ static const struct sdhci_ops sdhci_esdhc_be_ops = {
> .set_bus_width = esdhc_pltfm_set_bus_width,
> .reset = esdhc_reset,
> .set_uhs_signaling = esdhc_set_uhs_signaling,
> + .irq = esdhc_irq,
> };
>
> static const struct sdhci_ops sdhci_esdhc_le_ops = {
> @@ -931,6 +952,7 @@ static const struct sdhci_ops sdhci_esdhc_le_ops = {
> .set_bus_width = esdhc_pltfm_set_bus_width,
> .reset = esdhc_reset,
> .set_uhs_signaling = esdhc_set_uhs_signaling,
> + .irq = esdhc_irq,
> };
>
> static const struct sdhci_pltfm_data sdhci_esdhc_be_pdata = {
