Re: [PATCH v1 1/1] mmc: sdhci-acpi: Add support for NVIDIA BlueField-3 SoC

[Adrian Hunter <adrian.hunter@intel.com>]

On 15/03/21 7:00 pm, Liming Sun wrote:
> Thanks Adrian. Yes, I did consider adding this ACPI support into sdhci-of-dwcmshc.c, but not sure which one is the preferred way.
> Is this (sdhci-of-dwcmshc.c) what you recommend? I'll post the revised changes in patch v2.

Yes, that is generally preferred I think.

> 
>> -----Original Message-----
>> From: Adrian Hunter <adrian.hunter@intel.com>
>> Sent: Monday, March 15, 2021 4:34 AM
>> To: Liming Sun <limings@nvidia.com>; Ulf Hansson <ulf.hansson@linaro.org>;
>> Khalil Blaiech <kblaiech@nvidia.com>
>> Cc: linux-mmc@vger.kernel.org; linux-kernel@vger.kernel.org
>> Subject: Re: [PATCH v1 1/1] mmc: sdhci-acpi: Add support for NVIDIA
>> BlueField-3 SoC
>>
>> On 12/03/21 3:48 pm, Liming Sun wrote:
>>> This commit adds ACPI support for the BlueField-3 SoC which uses
>>> the DWC_mshc eMMC controller. The boundary check logic in static
>>> function dwcmshc_adma_write_desc() comes from sdhci-of-dwcmshc.c.
>>
>> Did you consider adding ACPI support to sdhci-of-dwcmshc.c ?
>> Other drivers have taken that approach, see sdhci-xenon.c or sdhci-iproc.c
>>
>>>
>>> Signed-off-by: Liming Sun <limings@nvidia.com>
>>> Reviewed-by: Khalil Blaiech <kblaiech@nvidia.com>
>>> ---
>>>  drivers/mmc/host/sdhci-acpi.c | 64
>> +++++++++++++++++++++++++++++++++++++++++++
>>>  1 file changed, 64 insertions(+)
>>>
>>> diff --git a/drivers/mmc/host/sdhci-acpi.c b/drivers/mmc/host/sdhci-acpi.c
>>> index 54205e3..6448e94e 100644
>>> --- a/drivers/mmc/host/sdhci-acpi.c
>>> +++ b/drivers/mmc/host/sdhci-acpi.c
>>> @@ -716,6 +716,68 @@ static int sdhci_acpi_emmc_amd_probe_slot(struct
>> platform_device *pdev,
>>>  	.priv_size	= sizeof(struct amd_sdhci_host),
>>>  };
>>>
>>> +/* Check DMA address/length boundary. */
>>> +static inline bool dwcmshc_adma_boundary_ok(dma_addr_t addr, int len)
>>> +{
>>> +	return (addr | (SZ_128M - 1)) == ((addr + len - 1) | (SZ_128M - 1));
>>> +}
>>> +
>>> +/*
>>> + * If DMA addr spans 128MB boundary, we split the DMA transfer into two
>>> + * so that each DMA transfer doesn't exceed the boundary.
>>> + */
>>> +static void dwcmshc_adma_write_desc(struct sdhci_host *host, void
>> **desc,
>>> +				    dma_addr_t addr, int len, unsigned int cmd)
>>> +{
>>> +	int tmplen, offset;
>>> +
>>> +	if (likely(!len || dwcmshc_adma_boundary_ok(addr, len))) {
>>> +		sdhci_adma_write_desc(host, desc, addr, len, cmd);
>>> +		return;
>>> +	}
>>> +
>>> +	offset = addr & (SZ_128M - 1);
>>> +	tmplen = SZ_128M - offset;
>>> +	sdhci_adma_write_desc(host, desc, addr, tmplen, cmd);
>>> +
>>> +	addr += tmplen;
>>> +	len -= tmplen;
>>> +	sdhci_adma_write_desc(host, desc, addr, len, cmd);
>>> +}
>>> +
>>> +static int sdhci_acpi_emmc_nvda_bf_probe_slot(struct platform_device
>> *pdev,
>>> +					      struct acpi_device *adev)
>>> +{
>>> +	struct sdhci_acpi_host *c = platform_get_drvdata(pdev);
>>> +	struct sdhci_host *host = c->host;
>>> +	u32 extra;
>>> +
>>> +	/* Extra adma table cnt for cross 128M boundary handling. */
>>> +	extra = DIV_ROUND_UP_ULL(dma_get_required_mask(&pdev->dev),
>> SZ_128M);
>>> +	extra = min(extra, (u32)SDHCI_MAX_SEGS);
>>> +	host->adma_table_cnt += extra;
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +static const struct sdhci_ops sdhci_acpi_ops_nvda_bf = {
>>> +	.set_clock		= sdhci_set_clock,
>>> +	.set_bus_width		= sdhci_set_bus_width,
>>> +	.set_uhs_signaling	= sdhci_set_uhs_signaling,
>>> +	.reset			= sdhci_reset,
>>> +	.adma_write_desc	= dwcmshc_adma_write_desc,
>>> +};
>>> +
>>> +static const struct sdhci_acpi_chip sdhci_acpi_chip_nvda_bf = {
>>> +	.ops = &sdhci_acpi_ops_nvda_bf,
>>> +};
>>> +
>>> +static const struct sdhci_acpi_slot sdhci_acpi_slot_nvda_bf_emmc = {
>>> +	.chip		= &sdhci_acpi_chip_nvda_bf,
>>> +	.caps		= MMC_CAP_8_BIT_DATA |
>> MMC_CAP_NONREMOVABLE,
>>> +	.probe_slot	= sdhci_acpi_emmc_nvda_bf_probe_slot,
>>> +};
>>> +
>>>  struct sdhci_acpi_uid_slot {
>>>  	const char *hid;
>>>  	const char *uid;
>>> @@ -740,6 +802,7 @@ struct sdhci_acpi_uid_slot {
>>>  	{ "QCOM8051", NULL, &sdhci_acpi_slot_qcom_sd_3v },
>>>  	{ "QCOM8052", NULL, &sdhci_acpi_slot_qcom_sd },
>>>  	{ "AMDI0040", NULL, &sdhci_acpi_slot_amd_emmc },
>>> +	{ "MLNXBF30", NULL, &sdhci_acpi_slot_nvda_bf_emmc },
>>>  	{ },
>>>  };
>>>
>>> @@ -757,6 +820,7 @@ struct sdhci_acpi_uid_slot {
>>>  	{ "QCOM8051" },
>>>  	{ "QCOM8052" },
>>>  	{ "AMDI0040" },
>>> +	{ "MLNXBF30" },
>>>  	{ },
>>>  };
>>>  MODULE_DEVICE_TABLE(acpi, sdhci_acpi_ids);
>>>
>