Received: by 2002:a25:868d:0:0:0:0:0 with SMTP id z13csp598104ybk; Fri, 15 May 2020 08:41:51 -0700 (PDT) X-Google-Smtp-Source: ABdhPJxHYCpoO5OPWuTuY5Lj8MgQXLFlEVidrVN5MR6g43z3CAewUh26s8pK6quqe7OIVfUZTUC9 X-Received: by 2002:a17:906:9518:: with SMTP id u24mr3478173ejx.137.1589557311329; Fri, 15 May 2020 08:41:51 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1589557311; cv=none; d=google.com; s=arc-20160816; b=VsLbHWpT1KMv2hctHqD5AsHoh71hSuZVSSn3hJrHmQnie9BxtY+NRIrhhcMHdGuymY cqDBy6vCw5x0CnBtmk84ZTkANN6eKfAviCKu5IeCHnIMX3Sw8hxSxUq8DJ/TDdG+Qx7e O6oCtH0frVUMth5MuV6RTfVmtVUeIEjx9Y2o5UEnhvIWH8o7SHOMm9k54WhnzGiRD6Z9 QrT5ARKnlJXt6UJp/+BO2EzrwMWH7qUGnH9QlO7rixfq5fzVppycVsIOZNtMY3GbaWBi bQ90+VmZkpJ1pESApNWY3oRiLJoLlM7GtVYoR/5/waCe0/0l4K1MCxKuJu+Vb3s7g7L7 TfSg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:in-reply-to:content-disposition :mime-version:references:message-id:subject:cc:to:from:date; bh=JDG+lBQAJD2A/z0YA+zy8W2rnntB1sE1ylhu/0kb3fI=; b=smAZtz8QmijlnNfDkcKsk3s5GxD/TxFGGYehSr+AONa/9VNOGKIcc2gBiXX155Q/Qc u6CgcA0u4hPRzlqYV8p8WswGrcwrTfQA0u6bzGSsl/WqUN9G7GrNSh5VNUVi7cojMqc+ ZkeQYclvvGcCXBziDe9v30bTvCt/RG+Ql+dG0kR0KFhUoltE5yU0mAZvQxQi/5iLfgdl DTRezpz60QN+YZzsV7hNarGxZiVdER8NCY4uCS7C5k6WpeFhtAZx5LS+8exPChKrgxCV x59oE8FyczBTbRRJ6w1W68dYWr8qs7w9KUd2GU8jlhCRA+LizoFatA+sMBXSkf7amsrX RoWA== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=collabora.com Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id k4si247569edi.24.2020.05.15.08.41.28; Fri, 15 May 2020 08:41:51 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=collabora.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726727AbgEOPhm (ORCPT + 99 others); Fri, 15 May 2020 11:37:42 -0400 Received: from bhuna.collabora.co.uk ([46.235.227.227]:45144 "EHLO bhuna.collabora.co.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726614AbgEOPhm (ORCPT ); Fri, 15 May 2020 11:37:42 -0400 Received: from [127.0.0.1] (localhost [127.0.0.1]) (Authenticated sender: evelikov) with ESMTPSA id 560F02A3375 Date: Fri, 15 May 2020 16:35:02 +0100 From: Emil Velikov To: Sebastian Reichel Cc: Sebastian Reichel , Rob Herring , Greg Kroah-Hartman , "Rafael J . Wysocki" , linux-pm@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, kernel@collabora.com Subject: Re: [PATCHv1 13/19] power: supply: sbs-battery: add POWER_SUPPLY_HEALTH_CALIBRATION_REQUIRED support Message-ID: <20200515153502.GE2836808@arch-x1c3> References: <20200513185615.508236-1-sebastian.reichel@collabora.com> <20200513185615.508236-14-sebastian.reichel@collabora.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20200513185615.508236-14-sebastian.reichel@collabora.com> Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2020/05/13, Sebastian Reichel wrote: > Add support for reporting the SBS battery's condition flag > to userspace using the new "Calibration required" health status. > > Signed-off-by: Sebastian Reichel > --- > drivers/power/supply/sbs-battery.c | 27 ++++++++++++++++++++++++--- > 1 file changed, 24 insertions(+), 3 deletions(-) > > diff --git a/drivers/power/supply/sbs-battery.c b/drivers/power/supply/sbs-battery.c > index 4fa553d61db2..2a2b926ad75c 100644 > --- a/drivers/power/supply/sbs-battery.c > +++ b/drivers/power/supply/sbs-battery.c > @@ -23,6 +23,7 @@ > > enum { > REG_MANUFACTURER_DATA, > + REG_BATTERY_MODE, > REG_TEMPERATURE, > REG_VOLTAGE, > REG_CURRENT_NOW, > @@ -94,6 +95,8 @@ static const struct chip_data { > } sbs_data[] = { > [REG_MANUFACTURER_DATA] = > SBS_DATA(POWER_SUPPLY_PROP_PRESENT, 0x00, 0, 65535), > + [REG_BATTERY_MODE] = > + SBS_DATA(-1, 0x03, 0, 65535), Fwiw I really like how neatly the driver is split into components. One thing which makes me wonder, have you considered reshuffling the sbs_data struct. In particular: - index POWER_SUPPLY_PROP, kill off the REG_ enum - sbs_get_property_index() can go, alongside a couple of unreachable paths - replace batter_mode (needs calibration) with with PROP_HEALTH + comment - perhaps even add REG_ADDR_SPEC_INFO 0x1a under POWER_SUPPLY_PROP_PRESENT - using the min/max seems wasteful, considering only one register is in s16 range while everything else is within u16 Regardless of the questions and trivial suggestions, the series looks spot on. For the lot: Reviewed-by: Emil Velikov -Emil P.S. The reg table is nearly complete only 0x01-0x07, 0x0E, 0x11, 0x1d-0x1f remain o/