On Aug 28, 2016, at 4:56 AM, Florian Fainelli <f.fainelli@xxxxxxxxx> wrote: > > Le 26/08/2016 à 21:02, Jaedon Shin a écrit : >> Hi Florian, >> >> On Aug 26, 2016, at 1:41 AM, Florian Fainelli <f.fainelli@xxxxxxxxx> wrote: >>> >>> On 08/19/2016 07:05 AM, Jaedon Shin wrote: >>>> Hi Ulf, >>>> >>>>> On Aug 19, 2016, at 10:44 PM, Ulf Hansson <ulf.hansson@xxxxxxxxxx> wrote: >>>>> >>>>> On 19 August 2016 at 04:25, Jaedon Shin <jaedon.shin@xxxxxxxxx> wrote: >>>>>> Hi Alan, >>>>>> >>>>>> On Aug 18, 2016, at 11:27 PM, Alan Cooper <alcooperx@xxxxxxxxx> wrote: >>>>>>> >>>>>>> It would be better to make this a MIPS only setting because this issue >>>>>>> only exists for MIPS chips and some newer ARM chips will support 64 >>>>>>> bit DMA. >>>>>>> Also, since there's been a general effort to reduce the use QUIRKs, >>>>>>> you could clear the SDHCI_CAN_64BIT in CAPS1 instead of using the >>>>>>> QUIRK. >>>>>>> >>>>>>> @@ -101,6 +101,9 @@ static int sdhci_brcmstb_probe(struct platform_device *pdev) >>>>>>> host->caps1 = sdhci_readl(host, SDHCI_CAPABILITIES_1); >>>>>>> host->caps1 &= ~(SDHCI_SUPPORT_SDR50 | SDHCI_SUPPORT_SDR104 | >>>>>>> SDHCI_SUPPORT_DDR50); >>>>>>> +#if defined(CONFIG_MIPS) >>>>>>> + host->caps1 &= ~SDHCI_CAN_64BIT; >>>>>>> +#endif >>>>>>> host->quirks |= SDHCI_QUIRK_MISSING_CAPS | >>>>>>> SDHCI_QUIRK_BROKEN_TIMEOUT_VAL; >>>>>> >>>>>> It's better to me, but we should use host->cap instead of host->cap1. I will update >>>>>> patch with your comment. >>>>> >>>>> Please, then also send this to the public linux-mmc list. >>>>> >>>>> Kind regards >>>>> Uffe >>>>> >>>> >>>> I'm sorry I could not add the public linux-mmc list this mail thread, but >>>> I have already sent the updated patch with linux-mmc. >>>> >>>> https://patchwork.kernel.org/patch/9289189/ >>> >>> Humm, is not this one of these cases where we would expect the >>> compatible string to dictacte whether enabling 64_BIT_DMA makes sense or >>> not? >>> >>> The patch is technically correct though. > > Hi Jaedon, > >> >> Yes, It's right way that uses host->cap according to the previous discussion >> for this driver and commit 5eaa7476f937 ("mmc: sdhci: Allow CAPS check for >> SDHCI_CAN_64BIT to use overridden caps"). >> >> If the 64bit ARM chipsets have own compatible string, the patch like as below >> >> if (of_device_is_compatible(pdev->dev.of_node, "brcm,bcm7425-sdhci")) >> host->caps &= ~SDHCI_CAN_64BIT; >> >> Could you tell me the some newer 64bit ARM chipsets have possible own compatible >> string? > > All ARM 32-bit brcmstb chips are LPAE capable, which means that the > SDHCI controller may have to deal with bus addresses larger than > 32-bits, so we always need SDHCI_CAN_64BIT to be set for that to happen > and work correctly. > > On ARM 64-bit brcmstb chips, we may not have enough memory such that the > SDHCI controller needs to deal with > 32-bits bus addresses, but same > thing, this may happen and the controller is fully capable of, so we > also need SDHCI_CAN_64BIT. > > In both cases, the controller should be fully operational with > 32-bits > physical addresses. > > On BMIPS chips, we should probably clear SDHCI_CAN_64BIT because AFAIR, > it really is broken (Al, can you confirm?), but at the same time, the > DMA-API should never hand us buffers which exceed the 32-bits bus > address boundary because of the processor and chip memory map > limitations anyway, is that what you encountered though? > > At the moment, brcm,bcm7425-sdhci is used across all 3 types of SoCs > (BMIPS, ARM and ARM64) while we should probably allocate a new one for > ARM and newer and then we could reliably base the clearing of > SDHCI_CAN_64BIT based on brcm,bcm7425-sdhci. > > Finally, Arnd's suggestions of using "dma-ranges" is fine, but I do not > think we quite need this here because we really need to advertise the > right set of capabilities based on the generation/version of the > controller deployed in specific chips. > > I would like to have Al's feedback on this, since he wrote the driver ;) > > Thanks! > -- > Florian Thanks for your detailed replay, and I understand why it must use a compatible string for all brcmstb chips. I'm sending the bumped patch for BMIPS quickly. Thanks, Jaedon-- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
