HI,
On 3/16/20 10:07 AM, Adrian Hunter wrote:
On 14/03/20 3:59 pm, Hans de Goede wrote:
Hi,
On 3/9/20 10:26 AM, Adrian Hunter wrote:
Thanks for doing this. A couple of questions below.
<snip>
@@ -823,17 +856,59 @@ static int sdhci_acpi_remove(struct platform_device
*pdev)
return 0;
}
+static void __maybe_unused sdhci_acpi_reset_signal_voltage_if_needed(
+ struct device *dev)
+{
+ struct sdhci_acpi_host *c = dev_get_drvdata(dev);
+ struct sdhci_host *host = c->host;
+
+ if (c->reset_signal_volt_on_suspend &&
+ host->mmc_host_ops.start_signal_voltage_switch ==
+ intel_start_signal_voltage_switch &&
This creates a unexpected dependency on
host->mmc_host_ops.start_signal_voltage_switch. Is it really necessary?
Well we are directly invoking the intel_dsm here, although the
DMI match should only happen on a system which is using an
Intel SDHCI controller, I thought it would be better to check for
that rather then just assuming it.
Also see the:
+ struct intel_host *intel_host = sdhci_acpi_priv(c);
Line, doing this on a non Intel SDHCI ACPI controller would be bad.
Then you need to add a comment to intel_probe_slot() to explain that
sdhci_acpi_reset_signal_voltage_if_needed() is dependent on:
host->mmc_host_ops.start_signal_voltage_switch =
intel_start_signal_voltage_switch;
For v3 which I'm preparing now, I've decided that adding an "is_intel"
bool to sdhci_acpi_host is a cleaner way to deal with this.
Regards,
Hans
