On 27/06/24 16:09, Jon Hunter wrote:
>
> On 27/06/2024 13:44, Christoph Hellwig wrote:
>> On Thu, Jun 27, 2024 at 01:30:03PM +0100, Jon Hunter wrote:
>>> I have been testing on both Tegra194 and Tegra234. Both of these set the
>>> above quirk. This would explain why the max_segment_size is rounded down to
>>> 65024 in the mmc_alloc_disk() function.
>>>
>>> We can check if this is needed but if it is needed then it is not clear
>>> if/how this can be fixed?
>>
>> The older kernels did this:
>>
>> if (max_size < PAGE_CACHE_SIZE) {
>> max_size = PAGE_CACHE_SIZE;
>> printk(KERN_INFO "%s: set to minimum %d\n",
>> __func__, max_size);
>> }
>>
>> q->limits.max_segment_size = max_size;
>>
>> so when these kernels actually worked despite the above warning it
>> must be ok(-ish) to just increase this value. If that is best done
>> by dropping the quirk, or changing the logic in sdhci.c is something
>> the maintainers that understand the hardware need to decide.
>>
>> The patch below gives you the pre-6.9 behavior just without the
>> boot time warning, but it might not be what was intended by the
>> quirk:
>>
>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
>> index 746f4cf7ab0338..0dc3604ac6093a 100644
>> --- a/drivers/mmc/host/sdhci.c
>> +++ b/drivers/mmc/host/sdhci.c
>> @@ -4721,12 +4721,9 @@ int sdhci_setup_host(struct sdhci_host *host)
>> * be larger than 64 KiB though.
>> */
>> if (host->flags & SDHCI_USE_ADMA) {
>> - if (host->quirks & SDHCI_QUIRK_BROKEN_ADMA_ZEROLEN_DESC) {
>> + if (host->quirks & SDHCI_QUIRK_BROKEN_ADMA_ZEROLEN_DESC)
>> host->max_adma = 65532; /* 32-bit alignment */
>> - mmc->max_seg_size = 65535;
>> - } else {
>> - mmc->max_seg_size = 65536;
>> - }
>> + mmc->max_seg_size = 65536;
>> } else {
>> mmc->max_seg_size = mmc->max_req_size;
>> }
>
>
> As a quick test I removed the quirk for Tegra194 and that did work, which achieves the same thing as the above.
>
> I guess there are two open questions here ...
>
> 1. Do we need the quirk for all the current Tegra devices? Thierry and I
> can confirm this.
> 2. For devices that need that quirk and use 64kB pages, what is the
> appropriate way to handle this for sdhci controllers?
Probably just do what the block layer was doing e.g.
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 746f4cf7ab03..6a1dea0d8d64 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -4724,6 +4724,13 @@ int sdhci_setup_host(struct sdhci_host *host)
if (host->quirks & SDHCI_QUIRK_BROKEN_ADMA_ZEROLEN_DESC) {
host->max_adma = 65532; /* 32-bit alignment */
mmc->max_seg_size = 65535;
+ /*
+ * Block layer cannot accept max_seg_size < PAGE_SIZE
+ * whereas sdhci_adma_table_pre() will anyway split the
+ * descriptor to handle size > max_adma.
+ */
+ if (mmc->max_seg_size < PAGE_SIZE)
+ mmc->max_seg_size = PAGE_SIZE;
} else {
mmc->max_seg_size = 65536;
}
