linux-kernel - Re: [PATCH] mmc: dw_mmc: Change signal voltage error to dev

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 14 Oct 2014 09:27:24 -0700
From:	Doug Anderson <dianders@...omium.org>
To:	Alim Akhtar <alim.akhtar@...il.com>
Cc:	Ulf Hansson <ulf.hansson@...aro.org>,
	Seungwon Jeon <tgih.jun@...sung.com>,
	Jaehoon Chung <jh80.chung@...sung.com>,
	Addy Ke <addy.ke@...k-chips.com>,
	Sonny Rao <sonnyrao@...omium.org>,
	Alim Akhtar <alim.akhtar@...sung.com>,
	Chris Ball <chris@...ntf.net>,
	"linux-mmc@...r.kernel.org" <linux-mmc@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] mmc: dw_mmc: Change signal voltage error to dev_dbg()

Alim,

On Tue, Oct 14, 2014 at 5:02 AM, Alim Akhtar <alim.akhtar@...il.com> wrote:
>> --- a/drivers/mmc/host/dw_mmc.c
>> +++ b/drivers/mmc/host/dw_mmc.c
>> @@ -1075,7 +1075,7 @@ static int dw_mci_switch_voltage(struct mmc_host *mmc, struct mmc_ios *ios)
>>                 ret = regulator_set_voltage(mmc->supply.vqmmc, min_uv, max_uv);
>>
>>                 if (ret) {
>> -                       dev_err(&mmc->class_dev,
>> +                       dev_dbg(&mmc->class_dev,
>>                                          "Regulator set error %d: %d - %d\n",
>>                                          ret, min_uv, max_uv);
>>                         return ret;
> Well, I am ok with this but this info is very useful, what if PMIC
> failed to actually set the voltage? may be because of some PMIC driver
> bug or i2c driver bug? Ofcourse this can be found by turning MMC_DEBUG
> ON, but is that worth in this case. Or is there a way to print that,
> this failure is because of a regulator re-try?
> your thoughts?

I think that the regulator framework and the i2c framework are
supposed to be reliable.  If they aren't reliable there will be lots
of places that will have problems.  I think that you _could_:

* In your regulator driver print an error when an i2c transfer fails.

* In your regulator driver print an error if some unexpected event
happens (like a regulator reports that the voltage didn't actually
change).

That would get you want you want, right?  ...but an error here doesn't
belong and that's pretty much determined by (28f92b5 mmc: core: Try
other signal levels during power up).  That patch wants to be able to
try several different voltage levels and if we print an error in that
case then it's going to be very confusing to the user.

-Doug
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/