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: <e0b8a451-1bce-4cc2-8793-9372dcb558da@gmail.com>
Date: Fri, 31 May 2024 14:22:14 +0200
From: Artur Weber <aweber.kernel@...il.com>
To: Krzysztof Kozlowski <krzk@...nel.org>,
 Chanwoo Choi <cw00.choi@...sung.com>
Cc: Sebastian Reichel <sre@...nel.org>, Rob Herring <robh@...nel.org>,
 Conor Dooley <conor+dt@...nel.org>, Lee Jones <lee@...nel.org>,
 Krzysztof Kozlowski <krzk+dt@...nel.org>,
 Alim Akhtar <alim.akhtar@...sung.com>, linux-pm@...r.kernel.org,
 devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-arm-kernel@...ts.infradead.org, linux-samsung-soc@...r.kernel.org,
 ~postmarketos/upstreaming@...ts.sr.ht, Henrik Grimler <henrik@...mler.se>,
 Wolfgang Wiedmeyer <wolfgit@...dmeyer.de>,
 Denis 'GNUtoo' Carikli <GNUtoo@...erdimension.org>
Subject: Re: [PATCH RFC 05/11] power: supply: max77693: Expose
 INPUT_CURRENT_LIMIT and CURRENT_MAX

On 31.05.2024 11:38, Krzysztof Kozlowski wrote:
> On 30/05/2024 10:55, Artur Weber wrote:
>> There are two charger current limit registers:
>>
>> - Fast charge current limit (which controls current going from the
>>    charger to the battery);
>> - CHGIN input current limit (which controls current going into the
>>    charger through the cable, and is managed by the CHARGER regulator).
>>
> 
> 
> 
>> +	case POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT:
>> +		ret = max77693_get_input_current_limit(chg, &val->intval);
>> +		break;
>> +	case POWER_SUPPLY_PROP_CURRENT_MAX:
>> +		ret = max77693_get_fast_charge_current(regmap, &val->intval);
>> +		break;
>>   	case POWER_SUPPLY_PROP_MODEL_NAME:
>>   		val->strval = max77693_charger_model;
>>   		break;
>> @@ -680,6 +727,11 @@ static int max77693_charger_probe(struct platform_device *pdev)
>>   	chg->dev = &pdev->dev;
>>   	chg->max77693 = max77693;
>>   
>> +	chg->regu = devm_regulator_get(chg->dev, "CHARGER");
>> +	if (IS_ERR(chg->regu))
>> +		return dev_err_probe(&pdev->dev, PTR_ERR(chg->regu),
>> +				     "failed to get charger regulator\n");\
> 
> This breaks users... and where is the binding?
Assuming "this" means "erroring out if the CHARGER regulator is not
found":

The way it works here is that the CHARGER regulator is fetched directly
from the parent max77693 device (it's defined in the regulator subnode
in DT). I suppose we could add a DT property for it, in the charger node
(like we do for the USB connector), though I don't know if anyone would
use any other regulator here than the CHARGER regulator of the max77693
regulator device. (And after all, we're using it here to modify charger
registers... maybe another point to my argument that we should be
handling all of this directly in the charger driver instead of deferring
it to a regulator.)

Best regards
Artur

[1] https://lore.kernel.org/all/20160927081344.GC4394@kozik-lap/
[2] https://lore.kernel.org/all/298d81d5-fe41-e2d1-32a7-d3dc35b0fe25@kernel.org/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ