2023-12-19 21:56:07

by Florian Fainelli

[permalink] [raw]
Subject: Re: [PATCH v5 2/2] mmc: add new sdhci reset sequence for brcm 74165b0

Hi Kamal,

On 12/19/2023 5:22 PM, Kamal Dasu wrote:
> From: Kamal Dasu <[email protected]>
>
> 74165b0 shall use a new sdio controller core version which
> requires a different reset sequence. For core reset we use
> sdhci_reset. For CMD and/or DATA reset added a new function
> to also enable SDHCI clocks SDHCI_CLOCK_CARD_EN
> SDHCI_CLOCK_INT_EN along with the SDHCI_RESET_CMD and/or
> SDHCI_RESET_DATA fields.
>
> Signed-off-by: Kamal Dasu <[email protected]>
> ---

[snip]

> +static void brcmstb_sdhci_reset_cmd_data(struct sdhci_host *host, u8 mask)
> +{
> + int ret;
> + u32 reg;
> + u32 new_mask = (mask & (SDHCI_RESET_CMD | SDHCI_RESET_DATA)) << 24;
> +
> + /*
> + * SDHCI_CLOCK_CONTROL register CARD_EN and CLOCK_INT_EN bits shall
> + * be set along with SOFTWARE_RESET register RESET_CMD or RESET_DATA
> + * bits, hence access SDHCI_CLOCK_CONTROL register as 32-bit register
> + */
> + new_mask |= SDHCI_CLOCK_CARD_EN | SDHCI_CLOCK_INT_EN;
> + reg = sdhci_readl(host, SDHCI_CLOCK_CONTROL);
> + sdhci_writel(host, reg | new_mask, SDHCI_CLOCK_CONTROL);
> +
> + reg = sdhci_readb(host, SDHCI_SOFTWARE_RESET);
> + ret = readb_poll_timeout(host->ioaddr + SDHCI_SOFTWARE_RESET,
> + reg, reg & mask, 10, 10000);

Does this need to be readb_poll_timeout_atomic() since this function can
be used in both atomic and non-atomic context AFAIR?
--
Florian


Attachments:
smime.p7s (4.12 kB)
S/MIME Cryptographic Signature

2023-12-20 02:07:37

by Kamal Dasu

[permalink] [raw]
Subject: Re: [PATCH v5 2/2] mmc: add new sdhci reset sequence for brcm 74165b0







> On Dec 19, 2023, at 4:55 PM, Florian Fainelli <[email protected]> wrote:
>
> Hi Kamal,
>
>> On 12/19/2023 5:22 PM, Kamal Dasu wrote:
>> From: Kamal Dasu <[email protected]>
>> 74165b0 shall use a new sdio controller core version which
>> requires a different reset sequence. For core reset we use
>> sdhci_reset. For CMD and/or DATA reset added a new function
>> to also enable SDHCI clocks SDHCI_CLOCK_CARD_EN
>> SDHCI_CLOCK_INT_EN along with the SDHCI_RESET_CMD and/or
>> SDHCI_RESET_DATA fields.
>> Signed-off-by: Kamal Dasu <[email protected]>
>> ---
>
> [snip]
>
>> +static void brcmstb_sdhci_reset_cmd_data(struct sdhci_host *host, u8 mask)
>> +{
>> + int ret;
>> + u32 reg;
>> + u32 new_mask = (mask & (SDHCI_RESET_CMD | SDHCI_RESET_DATA)) << 24;
>> +
>> + /*
>> + * SDHCI_CLOCK_CONTROL register CARD_EN and CLOCK_INT_EN bits shall
>> + * be set along with SOFTWARE_RESET register RESET_CMD or RESET_DATA
>> + * bits, hence access SDHCI_CLOCK_CONTROL register as 32-bit register
>> + */
>> + new_mask |= SDHCI_CLOCK_CARD_EN | SDHCI_CLOCK_INT_EN;
>> + reg = sdhci_readl(host, SDHCI_CLOCK_CONTROL);
>> + sdhci_writel(host, reg | new_mask, SDHCI_CLOCK_CONTROL);
>> +
>> + reg = sdhci_readb(host, SDHCI_SOFTWARE_RESET);
>> + ret = readb_poll_timeout(host->ioaddr + SDHCI_SOFTWARE_RESET,
>> + reg, reg & mask, 10, 10000);
>
> Does this need to be readb_poll_timeout_atomic() since this function can be used in both atomic and non-atomic context AFAIR?

Yes it does. Will send a v6
> --
> Florian

Kamal


Attachments:
smime.p7s (4.10 kB)
S/MIME Cryptographic Signature

2023-12-20 07:36:48

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v5 1/2] dt-bindings: mmc: brcm,sdhci-brcmstb: Add support for 74165b0

On 19/12/2023 17:22, Kamal Dasu wrote:
> From: Kamal Dasu <[email protected]>
>
> With newer sdio controller core used for 74165b0 we need to update
> the compatibility with "brcm,bcm74165b0-sdhci".
>
> Signed-off-by: Kamal Dasu <[email protected]>
> ---


Acked-by: Krzysztof Kozlowski <[email protected]>


---

This is an automated instruction, just in case, because many review tags
are being ignored. If you know the process, you can skip it (please do
not feel offended by me posting it here - no bad intentions intended).
If you do not know the process, here is a short explanation:

Please add Acked-by/Reviewed-by/Tested-by tags when posting new
versions, under or above your Signed-off-by tag. Tag is "received", when
provided in a message replied to you on the mailing list. Tools like b4
can help here. However, there's no need to repost patches *only* to add
the tags. The upstream maintainer will do that for tags received on the
version they apply.

https://elixir.bootlin.com/linux/v6.5-rc3/source/Documentation/process/submitting-patches.rst#L577

Best regards,
Krzysztof