Re: [PATCH 4/6] mmc: sh_mobile_sdhi: Add UHS-I mode support

[Kuninori Morimoto <kuninori.morimoto.gx@xxxxxxxxxxx>]

Hi Ben

> Implement voltage switch, supporting modes up to SDR-50.
> 
> Based on work by Shinobu Uehara, Rob Taylor, William Towle and Ian Molton.
> 
> Signed-off-by: Ben Hutchings <ben.hutchings@xxxxxxxxxxxxxxx>
> ---

I have 1 concern about .start_signal_voltage_switch return value

> +static int sh_mobile_sdhi_start_signal_voltage_switch(
> +	struct tmio_mmc_host *host, unsigned char signal_voltage)
> +{
> +	struct sh_mobile_sdhi *priv = host_to_priv(host);
> +	struct pinctrl_state *state;
> +	int min_uV, max_uV;
> +	int ret;
> +
> +	if (IS_ERR(priv->pinctrl) || IS_ERR(host->mmc->supply.vqmmc))
> +		return -EOPNOTSUPP;
(snip)
> @@ -239,6 +293,10 @@ static int sh_mobile_sdhi_probe(struct platform_device *pdev)
>  	host->clk_enable	= sh_mobile_sdhi_clk_enable;
>  	host->clk_disable	= sh_mobile_sdhi_clk_disable;
>  	host->multi_io_quirk	= sh_mobile_sdhi_multi_io_quirk;
> +
> +	host->start_signal_voltage_switch
> +				= sh_mobile_sdhi_start_signal_voltage_switch;
> +

This sh_mobile_sdhi_start_signal_voltage_switch() will return -EOPNOTSUPP
if IS_ERR(xx) cases, and it will be used on .tmio_mmc_start_signal_voltage_switch /
mmc_set_signal_voltage. These will think it is error and goes to error case or try again.
But, not supported is not error ?
Maybe we need this kind of code somewhere ? (ex. ALSA SoC has similar method)

	/* EOPNOTSUPP is not error */
	if (ret == -EOPNOTSUPP)
		ret = 0;

I have no objection if my concern never happen.


--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html