linux-kernel - Re: [PATCH 2/4] Bluetooth: hci_qca: Don't vote for specific voltage

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]

Message-ID: <7f9a4de91f364a5f8ce707c8d8a2344d@codeaurora.org>
Date:   Mon, 21 Oct 2019 12:07:15 +0530
From:   Harish Bandi <c-hbandi@...eaurora.org>
To:     Matthias Kaehlcke <mka@...omium.org>,
        Balakrishna Godavarthi <bgodavar@...eaurora.org>
Cc:     Bjorn Andersson <bjorn.andersson@...aro.org>,
        Marcel Holtmann <marcel@...tmann.org>,
        Johan Hedberg <johan.hedberg@...il.com>,
        linux-bluetooth@...r.kernel.org, linux-kernel@...r.kernel.org,
        Jeffrey Hugo <jeffrey.l.hugo@...il.com>,
        linux-arm-msm@...r.kernel.org,
        linux-bluetooth-owner@...r.kernel.org
Subject: Re: [PATCH 2/4] Bluetooth: hci_qca: Don't vote for specific voltage

+ Bala

On 2019-10-18 23:52, Matthias Kaehlcke wrote:
> On Thu, Oct 17, 2019 at 10:24:02PM -0700, Bjorn Andersson wrote:
>> Devices with specific voltage requirements should not request voltage
>> from the driver, but instead rely on the system configuration to 
>> define
>> appropriate voltages for each rail.
>> 
>> This ensures that PMIC and board variations are accounted for, 
>> something
>> that the 0.1V range in the hci_qca driver currently tries to address.
>> But on the Lenovo Yoga C630 (with wcn3990) vddch0 is 3.1V, which means
>> the driver will fail to set the voltage.
>> 
>> Signed-off-by: Bjorn Andersson <bjorn.andersson@...aro.org>
>> ---
>>  drivers/bluetooth/hci_qca.c | 26 ++++++++------------------
>>  1 file changed, 8 insertions(+), 18 deletions(-)
>> 
>> diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
>> index c07c529b0d81..54aafcc69d06 100644
>> --- a/drivers/bluetooth/hci_qca.c
>> +++ b/drivers/bluetooth/hci_qca.c
>> @@ -130,8 +130,6 @@ enum qca_speed_type {
>>   */
>>  struct qca_vreg {
>>  	const char *name;
>> -	unsigned int min_uV;
>> -	unsigned int max_uV;
>>  	unsigned int load_uA;
>>  };
>> 
>> @@ -1332,10 +1330,10 @@ static const struct hci_uart_proto qca_proto = 
>> {
>>  static const struct qca_vreg_data qca_soc_data_wcn3990 = {
>>  	.soc_type = QCA_WCN3990,
>>  	.vregs = (struct qca_vreg []) {
>> -		{ "vddio",   1800000, 1900000,  15000  },
>> -		{ "vddxo",   1800000, 1900000,  80000  },
>> -		{ "vddrf",   1300000, 1350000,  300000 },
>> -		{ "vddch0",  3300000, 3400000,  450000 },
>> +		{ "vddio", 15000  },
>> +		{ "vddxo", 80000  },
>> +		{ "vddrf", 300000 },
>> +		{ "vddch0", 450000 },
>>  	},
>>  	.num_vregs = 4,
>>  };
>> @@ -1343,10 +1341,10 @@ static const struct qca_vreg_data 
>> qca_soc_data_wcn3990 = {
>>  static const struct qca_vreg_data qca_soc_data_wcn3998 = {
>>  	.soc_type = QCA_WCN3998,
>>  	.vregs = (struct qca_vreg []) {
>> -		{ "vddio",   1800000, 1900000,  10000  },
>> -		{ "vddxo",   1800000, 1900000,  80000  },
>> -		{ "vddrf",   1300000, 1352000,  300000 },
>> -		{ "vddch0",  3300000, 3300000,  450000 },
>> +		{ "vddio", 10000  },
>> +		{ "vddxo", 80000  },
>> +		{ "vddrf", 300000 },
>> +		{ "vddch0", 450000 },
>>  	},
>>  	.num_vregs = 4,
>>  };
>> @@ -1386,13 +1384,6 @@ static int qca_power_off(struct hci_dev *hdev)
>>  static int qca_enable_regulator(struct qca_vreg vregs,
>>  				struct regulator *regulator)
>>  {
>> -	int ret;
>> -
>> -	ret = regulator_set_voltage(regulator, vregs.min_uV,
>> -				    vregs.max_uV);
>> -	if (ret)
>> -		return ret;
>> -
>>  	return regulator_enable(regulator);
>> 
>>  }
>> @@ -1401,7 +1392,6 @@ static void qca_disable_regulator(struct 
>> qca_vreg vregs,
>>  				  struct regulator *regulator)
>>  {
>>  	regulator_disable(regulator);
>> -	regulator_set_voltage(regulator, 0, vregs.max_uV);
>> 
>>  }
> 
> This was brought up multiple times during the initial review, but
> wasn't addressed.
> 
> Reviewed-by: Matthias Kaehlcke <mka@...omium.org>