On Thu, 10 Jul 2025 at 09:57, Avri Altman Avri.Altman@sandisk.com wrote:
/* * Current limit switch is only defined for SDR50,SDR104, and DDR50 @@ -575,33 +574,24 @@ static int sd_set_current_limit(struct
mmc_card *card, u8 *status)
max_current = sd_get_host_max_current(card->host);Looking at the implementation of sd_get_host_max_current(), it's very
limiting.
For example, if we are using MMC_VDD_34_35 or MMC_VDD_35_36, the function returns 0. Maybe this is good enough based upon those host drivers that actually sets host->max_current_180|300|330, but it kind of
looks wrong to me.
I think we should re-work this interface to let us retrieve the maximum current from the host in a more flexible way. What we are really looking for is a value in Watt instead, I think. Don't get me wrong, this deserved it's own standalone patch on top of
$subject patch.
I still need to consult internally, but Yes - I agree. Ultimately however, CMD6 expects us to fill the current limit value, so
multiplying by voltage and dividing it back seems superfluous.
How about adding to missing vdd and treat them as max_current_330, like in
sdhci_get_vdd_value?
Maybe something like:
+/*
- Get host's max current setting at its current voltage
+normalized to 3.6
- volt which is the voltage in which the card defines its limits
+*/ static u32 sd_host_normalized_max_current(struct mmc_host *host) {
u32 voltage, max_current;voltage = 1 << host->ios.vdd;switch (voltage) {case MMC_VDD_165_195:max_current = host->max_current_180 * 180 / 360;break;case MMC_VDD_29_30:case MMC_VDD_30_31:max_current = host->max_current_300 * 300 / 360;break;case MMC_VDD_32_33:case MMC_VDD_33_34:case MMC_VDD_34_35:case MMC_VDD_35_36:max_current = host->max_current_330 * 330 / 360;break;default:max_current = 0;}return max_current;+}
I think it's way better than the current implementation in sd_get_host_max_current().
Still, I still think it's weird to have three different variables in the host, max_current_180, max_current_300 and max_current_330. That seems like an SDHCI specific thing to me, unless I am mistaken.
I would rather see a more flexible interface where we move away from using host->max_current_180|300|330 entirely and have a function that host->returns the supported limit (whatever unit we decide). Maybe it also makes sense to provide some additional helpers from the core that host drivers can call, to fetch/translate the values it should provide for this.
+Adrian
IIUC, you are looking for a host->max_power to replace the above.
No, the new function/callback should provide us the value immediately, rather than having it stored in the host struct.
However, giver that: a) there is no power class in SD like in mmc, and the card needs to be set to a power-limit b) the platform supported voltages can be either be given via DT as well as hard-coded and it's shared with mmc, and c) the platform supported max current is either read from the sdhci register as well as can be hard-coded I am not sure if and where we should set
this max_power member, but I am open for suggestions.
I will certainly be host specific, so we need to have a host ops for it. Depending on how the host is powering the card, it may need to do different things to get the max-current/max-power for the currently selected voltage-level for vcc/vdd.
I can take a stab and post a draft for it. I will keep you posted.
That would be great. Thanks a lot.
Thanks, Avri
[...]
Kind regards Uffe