From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mx0a-0031df01.pphosted.com (mx0a-0031df01.pphosted.com [205.220.168.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 003AC335BDC for ; Thu, 27 Nov 2025 15:28:18 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=205.220.168.131 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1764257300; cv=none; b=PzbafYjGylUu3Ica+VZhpBFbIQQXP7mW7+42feZdAbzAvpl0iga+2wCAo6FREKaUGVQ0nyDjFe2pelJ1MmDpSt32DongW8QxZWL6ke7AI44g8CZ0wO4Fw3+pZq5A/BQXQ+1hazNPe5z2OPNCHSgS40Gr738rGp2X4khBbN9YJIs= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1764257300; c=relaxed/simple; bh=Z4GHwBbqqBLmBjW3cseRdkpTdHaLbZeRIAfeUz1ASfY=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=t9Yi6YTypInthTTR9CjHz6Fb21IVX8Dq5BL0KvuuE6lQzPJjCY1OXfcOCuzMxwG+p5LB7VaA9XYbq1H0w1k2huprmQ/CfV1IKWvFf1G7ruoOwhzjk1rdZG+uub0zIsanFwvppCLKzRNJDJAKKLvGbpe4eE/Cb+Q+NgQE+qgRc48= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=oss.qualcomm.com; spf=pass smtp.mailfrom=oss.qualcomm.com; dkim=pass (2048-bit key) header.d=qualcomm.com header.i=@qualcomm.com header.b=iHOX/0Pq; dkim=pass (2048-bit key) header.d=oss.qualcomm.com header.i=@oss.qualcomm.com header.b=EzT5nIFf; arc=none smtp.client-ip=205.220.168.131 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=oss.qualcomm.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=oss.qualcomm.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=qualcomm.com header.i=@qualcomm.com header.b="iHOX/0Pq"; dkim=pass (2048-bit key) header.d=oss.qualcomm.com header.i=@oss.qualcomm.com header.b="EzT5nIFf" Received: from pps.filterd (m0279867.ppops.net [127.0.0.1]) by mx0a-0031df01.pphosted.com (8.18.1.11/8.18.1.11) with ESMTP id 5AR8dxcc1667784 for ; Thu, 27 Nov 2025 15:28:18 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=qualcomm.com; h= cc:content-transfer-encoding:content-type:date:from:in-reply-to :message-id:mime-version:references:subject:to; s=qcppdkim1; bh= ndxdMdbzvZF/I4cZm8HOJZvbZdGMtw1Vn3ORVS+oD8Q=; b=iHOX/0Pqd+GA/PNO h2o5AeFEFuQcYL+DssWotOyw8zguwdaUEkJ87AqdDEI3vdBipRYHJJasP3LTvaAc 0KM4FttnxD+Y2oq5jEtYDFrEZS/R3FD2xdwTbqdSRKXQicRMVPGKRE7i7NjQXv5a muUsvE0SvM1x76nLSKDWRvUL9bVIOC0s8QVO7np5UDvj6c6JmkVtQ1SVI7mSB7BG vA/B775gUhUCJG6Os9p8lvnJUZa8yTfoEZytIUtMjHqg8kZ41+ViD5FBDj6e3Jok A5M2pz18qYZYGOVtpnkjTmBoAI1RUhdQ8eHzWA5Vw6QziMRRQydxlnBNjInMLFQF KoJBpw== Received: from mail-qt1-f198.google.com (mail-qt1-f198.google.com [209.85.160.198]) by mx0a-0031df01.pphosted.com (PPS) with ESMTPS id 4ap0bmv6mw-1 (version=TLSv1.3 cipher=TLS_AES_128_GCM_SHA256 bits=128 verify=NOT) for ; Thu, 27 Nov 2025 15:28:18 +0000 (GMT) Received: by mail-qt1-f198.google.com with SMTP id d75a77b69052e-4ede0bd2154so2767281cf.1 for ; Thu, 27 Nov 2025 07:28:18 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=oss.qualcomm.com; s=google; t=1764257297; x=1764862097; darn=vger.kernel.org; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=ndxdMdbzvZF/I4cZm8HOJZvbZdGMtw1Vn3ORVS+oD8Q=; b=EzT5nIFfxQq4LuqcOy9+hB+P/cc0w4sClVrZBF+5y3/K9gfi286zcn1Qu6/whLdpwZ T+DViKwvXo6YngOR5XKW2ssH9xVNaKdhXSIoHI5pU2CPWP4bhwC22gMT5dz0UTu8OzCp 9j/x2+H9xEMo8E8NPJPEVS07hZkrzxTZ1I5r1Pt6rnerb5uU5b2onmTbjRJIYOi9Jd4J hYXHbTXGHnICN9LbcDgQ0PoxKueP9u5OtOkZQqInkJhU1hswBSebEtn5h8ZcNjm2250E OyjWWxDx5KmmvOY7Jl6KQ3lQwfGRljzeGKqMcgFr2fA+3d9LEsOLLWrdEMalp6KJ6w6n ZV+w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1764257297; x=1764862097; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :x-gm-gg:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=ndxdMdbzvZF/I4cZm8HOJZvbZdGMtw1Vn3ORVS+oD8Q=; b=vPE4WT/xliHwPa+NwgSU0ixxm7SuQaZlSmGq9mq7YWBsMa+99Uw/HeD8p2mHZ5UR/2 RbCySTywIROlTrXhVBLyRY4oagFEPp3hbtrUh3LyFq7EypmuGXY3kv8UxLVSlri0bua7 DwhQU5NYi/M5SL8GMLCWO+mTzN2DTVYjWIJHbTdTeFDwtgiDgWx8cd4duyGYbb+wdJc9 KnzOIYuEEHgvmHL/QKqzeE7xx2+CDbjczdu9iqCeonHPSuN6RvQNsjTncWFWBbyndbAl h5zo5141N7yrcPf5ouUjrrV627gBIg5skyoTzUDMkPB1+AoZ6TPfBOWuzli3qFBA5fZg 4bWA== X-Forwarded-Encrypted: i=1; AJvYcCUSVSRnDvuBjhFj/vRMMme6bmJH2vMUcus2EGiKaJMhIrhzVGz2Rj3/GBeozb/el2NjEoCXofcpPomqkOAt@vger.kernel.org X-Gm-Message-State: AOJu0YzSQP7TeAehrkqOLDUAOG2q7C8khnPEo/tV8BHLMr/YOlZ1ZUFt GB+7MSfdGccLe0oVd32hNF6YsrZGIjhmt46MS7s9W41FlkMI7n+AcxH+g1R5g458l+/FFsBo5p0 cp2YfRxuWLJOvcAI42al4s8MEkyxctCR2XSK95qjE8ffeJRVEadKX4IQheQlquf74ynI/ X-Gm-Gg: ASbGncuvfarQx04TzhUSQAV+i4fFtlo6AjeFNYhm1WOVIv1VJvX9lBEVgeW1TSHkSQ+ zdb+RDJG+5AZHOSFyUKTkEPpTVQlip4AB+HLWLBKRetsQIxLr/dHOcV9bUPUd/lh3tXg34ahSr5 KoX6v1PLRQ/cK+GFF6kMh7jQYXvwG/lb+zA2n2OWXWlmbz1+YQvKu03cvHUo1idNRBo+tzV2ZaM pPT0t3vv+LWwvzHNGprSJ1jx6QRZoswDTNAOZZu7q8rfxwKCoba1+biEhD0rRP6TeygMXzJSzJ0 7alETmSulkp6DFOhkMlE1o9NuFFU0RYqo3zn6ZmEVXW+W18h4O4LKMApOKimVrVuKWG85zD29cW MzaNStZsr7tXzKmxaijIFWyEFUBPLOvV0V1JPyzFH7ZIkSHX9W4jJiWFqnVVjersoIHQ= X-Received: by 2002:a05:622a:206:b0:4ed:e2dc:dae4 with SMTP id d75a77b69052e-4ee58780ccemr236846811cf.0.1764257296976; Thu, 27 Nov 2025 07:28:16 -0800 (PST) X-Google-Smtp-Source: AGHT+IHix/8idFxBj5zD5bC/rE9LqDNBbvzU8nEEOyiI7hOsESnJb73ZJwmHLH1WFWYZ6SQ9IHWdKQ== X-Received: by 2002:a05:622a:206:b0:4ed:e2dc:dae4 with SMTP id d75a77b69052e-4ee58780ccemr236846331cf.0.1764257296373; Thu, 27 Nov 2025 07:28:16 -0800 (PST) Received: from [192.168.119.202] (078088045245.garwolin.vectranet.pl. [78.88.45.245]) by smtp.gmail.com with ESMTPSA id a640c23a62f3a-b76f59a63dfsm176945366b.37.2025.11.27.07.28.13 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 27 Nov 2025 07:28:15 -0800 (PST) Message-ID: <5d6c3dda-71cd-4684-8546-bc4918b560de@oss.qualcomm.com> Date: Thu, 27 Nov 2025 16:28:12 +0100 Precedence: bulk X-Mailing-List: linux-arm-msm@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH RFC 2/8] power: supply: Add driver for Qualcomm PMI8998 fuel gauge To: david@ixit.cz, Sebastian Reichel , Rob Herring , Krzysztof Kozlowski , Conor Dooley , Casey Connolly , Casey Connolly , Joel Selvaraj , Yassine Oudjana , Bjorn Andersson , Konrad Dybcio , Alexander Martinz , =?UTF-8?B?QmFybmFiw6FzIEN6w6ltw6Fu?= , Richard Acayan , Alexey Minnekhanov Cc: linux-pm@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org, phone-devel@vger.kernel.org References: <20251124-pmi8998_fuel_gauge-v1-0-dd3791f61478@ixit.cz> <20251124-pmi8998_fuel_gauge-v1-2-dd3791f61478@ixit.cz> Content-Language: en-US From: Konrad Dybcio In-Reply-To: <20251124-pmi8998_fuel_gauge-v1-2-dd3791f61478@ixit.cz> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Authority-Analysis: v=2.4 cv=AKyLkLWT c=1 sm=1 tr=0 ts=69286e12 cx=c_pps a=mPf7EqFMSY9/WdsSgAYMbA==:117 a=FpWmc02/iXfjRdCD7H54yg==:17 a=IkcTkHD0fZMA:10 a=6UeiqGixMTsA:10 a=s4-Qcg_JpJYA:10 a=VkNPw1HP01LnGYTKEx00:22 a=WFa1dZBpAAAA:8 a=OuZLqq7tAAAA:8 a=sfOm8-O8AAAA:8 a=V67mNnR7AAxaKjwIKscA:9 a=3ZKOabzyN94A:10 a=QEXdDO2ut3YA:10 a=dawVfQjAaf238kedN5IG:22 a=MZguhEFr_PtxzKXayD1K:22 a=AKGiAy9iJ-JzxKVHQNES:22 a=TvTJqdcANYtsRzA46cdi:22 X-Proofpoint-Spam-Details-Enc: AW1haW4tMjUxMTI3MDExNSBTYWx0ZWRfXxJmTgq9Hr5NE Wn2afeVVLkiLmaujD0eDp3YJuBkE5LKBkwYOgLNSjTTMQWb0ECgDMMCtAxWJmrz2VdTyNx9FdVD RPa6TwKxN5ToN8PhynF8d+zUJUUdxUaN270/dY6k+pp/FwXtAzOz8GmdLr2LjWUFzmjPGoNQ7u4 iJ8CZMA6jeIJketoHxeyxoW3MNaQgYDhpwU84lhagL7yfy62u120QwpMxZ2oycosc/V9bYQqisp WnUwufBt4HlJTFCToyVzh2dRF9/pnhY4DDixh6uKt+lJNEBWZlOSS5SADyyMBEDgsna79I9YKOT EvxrbU0dcTwDHDE1SenC+ZDXUZouOdOHg5qo2fWu79mBtQIDdtaDx9E1bizZ71iVakhlx9d0oic +sl+cu4uuquLkkVzFGcFpN+ze+wzKA== X-Proofpoint-GUID: d93S_XzPw5QMesOGtgHEw35dZxWqN9ov X-Proofpoint-ORIG-GUID: d93S_XzPw5QMesOGtgHEw35dZxWqN9ov X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.293,Aquarius:18.0.1121,Hydra:6.1.9,FMLib:17.12.100.49 definitions=2025-11-25_02,2025-11-27_01,2025-10-01_01 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 phishscore=0 bulkscore=0 clxscore=1015 spamscore=0 lowpriorityscore=0 priorityscore=1501 suspectscore=0 adultscore=0 malwarescore=0 impostorscore=0 classifier=typeunknown authscore=0 authtc= authcc= route=outbound adjust=0 reason=mlx scancount=1 engine=8.22.0-2510240001 definitions=main-2511270115 On 11/24/25 10:53 PM, David Heidelberg via B4 Relay wrote: > From: Joel Selvaraj > > Ths driver supports the fuel gauge hardware available on PMICs known as > 3rd generation fuel gauge hardware available on PMI8998. > > Co-developed-by: Casey Connolly > Co-developed-by: Barnabás Czémán > Signed-off-by: Barnabás Czémán > Co-developed-by: Yassine Oudjana > Signed-off-by: Yassine Oudjana > Signed-off-by: David Heidelberg > --- [...] > +/** > + * @brief pmi8998_fg_read() - Read multiple registers with regmap_bulk_read I think this won't pass kerneldoc checks (make W=1) [...] > +static int pmi8998_fg_write(struct pmi8998_fg_chip *chip, u8 *val, u16 addr, int len) > +{ > + bool sec_access = (addr & 0xff) > 0xd0; Downstream checks if the address is > 0xBA which is what you want at least for pmi8998 You can de-abbreviate this to 'secure_access' (not to be confused with 'secondary' or so). There's a locking mechanism which needs a 0xa5 byte written to the base+0xd0 register (applies to all FG peripherals with the 'last non-secure register' value possibly varying). [...] > + u8 sec_addr_val = 0xa5; > + int ret; > + > + if (((chip->base + addr) & 0xff00) == 0) The 'fuel gauge' consists of: FG_BATT_SOC @ 0x4000 (state of charge monitor) FG_BATT_INFO @ 0x4100 ("general fg minus SoC") FG_BCL @ 0x4200 (battery current limiter) FG_LMH @ 0x4300 (limits management hardware) FG_MEM_IF @ 0x4400 (DMA engine) RRADC @ 0x4500 (today handled by its own driver) and a couple other peripherals that Linux doesn't need to worry about Each one of them should have its own 'reg' entry (which is assumed to be 0x100-long), which will let you skip such interesting checks and rely on the regmap framework disallowing address spillover (or you can just then make the addr argument a u8) It would be good to keep in mind their relationship and think about how to model them together. I don't think they must all necessarily be part of a single big "fg" dt node, particularly the LMH/BCL part seems to be rather self-contained [...] > + return -EINVAL; > + > + dev_vdbg(chip->dev, "%s: Writing 0x%x to 0x%x", __func__, *val, addr); > + > + if (sec_access) { > + ret = regmap_bulk_write(chip->regmap, > + ((chip->base + addr) & 0xff00) | 0xd0, > + &sec_addr_val, 1); > + if (ret) > + return ret; > + } > + > + return regmap_bulk_write(chip->regmap, chip->base + addr, val, len); > +} > + > +/** > + * @brief pmi8998_fg_masked_write() - like pmi8998_fg_write but applies > + * a mask first. > + * > + * @param chip Pointer to chip > + * @param val Pointer to write values from > + * @param addr Address to write to > + * @param len Number of registers (bytes) to write > + * @return int 0 on success, negative errno on error > + */ > +static int pmi8998_fg_masked_write(struct pmi8998_fg_chip *chip, u16 addr, u8 mask, u8 val) > +{ > + u8 reg; > + int ret; > + > + ret = pmi8998_fg_read(chip, ®, addr, 1); > + if (ret) > + return ret; > + > + reg &= ~mask; > + reg |= val & mask; > + > + return pmi8998_fg_write(chip, ®, addr, 1); > +} > + > +/* > + * Battery status > + */ > + > +/** > + * @brief pmi8998_fg_get_capacity() - Get remaining capacity of battery > + * > + * @param chip Pointer to chip > + * @param val Pointer to store value at > + * @return int 0 on success, negative errno on error > + */ > +static int pmi8998_fg_get_capacity(struct pmi8998_fg_chip *chip, int *val) > +{ > + u8 cap[2]; > + int ret; > + > + ret = pmi8998_fg_read(chip, cap, BATT_MONOTONIC_SOC, 2); > + if (ret) { > + dev_err(chip->dev, "Failed to read capacity: %d", ret); > + return ret; > + } Downstream tries for 5 times to get this (raw) pair of values and fails if they don't match - 0x400a is a shadow register of 0x4009 and this is very much intended > + if (cap[0] != cap[1]) > + cap[0] = cap[0] < cap[1] ? cap[0] : cap[1]; > + > + *val = DIV_ROUND_CLOSEST((cap[0] - 1) * 98, 0xff - 2) + 1; 98 comes from "FULL_CAPACITY (100) - 2", 0xff denotes "FULL_SOC_RAW", i.e. the raw value of this register that corresponds to 100% (again not sure where the minus2 comes from - perhaps some rounding fixups) > + > + return 0; > +} > + > +/** > + * @brief pmi8998_fg_get_temperature() - Get temperature of battery > + * > + * @param chip Pointer to chip > + * @param val Pointer to store value at > + * @return int 0 on success, negative errno on error > + */ > +static int pmi8998_fg_get_temperature(struct pmi8998_fg_chip *chip, int *val) > +{ > + int ret, temp; > + u8 readval[2]; > + > + ret = pmi8998_fg_read(chip, readval, PARAM_ADDR_BATT_TEMP, 2); > + if (ret) { > + dev_err(chip->dev, "Failed to read temperature: %d\n", ret); > + return ret; > + } > + > + temp = ((readval[1] & BATT_TEMP_MSB_MASK) << 8) | > + (readval[0] & BATT_TEMP_LSB_MASK); > + temp = DIV_ROUND_CLOSEST(temp * 10, 4); > + > + *val = temp - 2730; > + > + return 0; > +} > + > +/** > + * @brief pmi8998_fg_get_current() - Get current being drawn from battery > + * > + * @param chip Pointer to chip > + * @param val Pointer to store value at > + * @return int 0 on success, negative errno on error > + */ > +static int pmi8998_fg_get_current(struct pmi8998_fg_chip *chip, int *val) > +{ > + s16 temp; > + u8 readval[2]; > + int ret; > + > + ret = pmi8998_fg_read(chip, readval, PARAM_ADDR_BATT_CURRENT, 2); > + if (ret) { > + dev_err(chip->dev, "Failed to read current: %d\n", ret); > + return ret; > + } > + > + /* handle rev 1 too */ PMI8998v1 has flipped the order of the registers and I would guesstimate that it wouldn't actually be present in the wild > + temp = (s16)(readval[1] << 8 | readval[0]); > + *val = div_s64((s64)temp * 488281, 1000); This is a funny way to say that this is a 2s complement-encoded 16b value, where 5 bits are reserved for the integer portion [...] > + power_supply_changed(chip->batt_psy); > + > + if (chip->status == POWER_SUPPLY_STATUS_UNKNOWN) { > + /* > + * REVISIT: Find better solution or remove current-based > + * status checking once checking is properly implemented > + * in charger drivers > + > + * Sometimes it take a while for current to stabilize, > + * so signal property change again later to make sure > + * current-based status is properly detected. > + */ On downstream, it's the charger counterpart that signals PSY_STATUS_(DIS)CHARGING Konrad