Re: [PATCH v9 05/10] power: supply: bd70528: use linear ranges

From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
To: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
Cc: Mark Rutland <mark.rutland@arm.com>,
	Andrei Stefanescu <andrei.stefanescu@microchip.com>,
	Heiko Stuebner <heiko@sntech.de>,
	"Rafael J. Wysocki" <rafael@kernel.org>,
	Tony Lindgren <tony@atomide.com>,
	Linus Walleij <linus.walleij@linaro.org>,
	Brendan Higgins <brendanhiggins@google.com>,
	Liam Girdwood <lgirdwood@gmail.com>,
	Andreas Kemnade <andreas@kemnade.info>,
	"Angelo G. Del Regno" <kholk11@gmail.com>,
	Hsin-Hsiung Wang <hsin-hsiung.wang@mediatek.com>,
	linux-samsung-soc@vger.kernel.org, linux-omap@vger.kernel.org,
	Axel Lin <axel.lin@ingics.com>,
	Gregory CLEMENT <gregory.clement@bootlin.com>,
	Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>,
	Krzysztof Kozlowski <krzk@kernel.org>,
	Markus Reichl <m.reichl@fivetechno.de>,
	Bartosz Golaszewski <bgolaszewski@baylibre.com>,
	Chen-Yu Tsai <wens@csie.org>, Andy Gross <agross@kernel.org>,
	markus.laine@fi.rohmeurope.com,
	Adam Thomson <Adam.Thomson.Opensource@diasemi.com>,
	devicetree@vger.kernel.org,
	Charles Keepax <ckeepax@opensource.cirrus.com>,
	mazziesaccount@gmail.com, linux-arm-msm@vger.kernel.org,
	Richard Fitzgerald <rf@opensource.cirrus.com>,
	Mark Brown <broonie@kernel.org>,
	linux-mediatek@lists.infradead.org,
	Matthias Brugger <matthias.bgg@gmail.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Bjorn Andersson <bjorn.andersson@linaro.org>,
	linux-arm-kernel@lists.infradead.org,
	Support Opensource <support.opensource@diasemi.com>,
	Baolin Wang <baolin.wang@linaro.org>,
	Sangbeom Kim <sbkim73@samsung.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	linux-pm@vger.kernel.org, Randy Dunlap <rdunlap@infradead.org>,
	Sebastian Reichel <sre@kernel.org>,
	linux-kernel@vger.kernel.org, mikko.mutanen@fi.rohmeurope.com,
	Vinod Koul <vkoul@kernel.org>, Rob Herring <robh+dt@kernel.org>,
	patches@opensource.cirrus.com
Subject: Re: [PATCH v9 05/10] power: supply: bd70528: use linear ranges
Date: Tue, 14 Apr 2020 12:35:13 +0300	[thread overview]
Message-ID: <20200414093513.GJ34613@smile.fi.intel.com> (raw)
In-Reply-To: <20200414093243.GI34613@smile.fi.intel.com>

On Tue, Apr 14, 2020 at 12:32:43PM +0300, Andy Shevchenko wrote:
> On Tue, Apr 14, 2020 at 11:04:21AM +0300, Matti Vaittinen wrote:
> > Change the bd70528 to use common linear_range code instead of
> > implementing a copy of it in this driver.
> 
> Couple of nits below which you can take as TODO items for the future.
> (Because maintainer is fine with it according to the tags)
> 
> ...
> 
> >  config CHARGER_BD70528
> >  	tristate "ROHM bd70528 charger driver"
> >  	depends on MFD_ROHM_BD70528
> > +	select LINEAR_RANGES
> 
> >  	default n
> 
> At some point you can remove this kind of defaults (see [1] for the details).

Missed reference

[1]: commit 0192f17529fa ("clean up x86 platform driver default values")

> ...
> 
> > +static const struct linear_range current_limit_ranges[] = {
> >  	{
> >  		.min = 5,
> >  		.step = 1,
> > -		.vals = 36,
> > -		.low_sel = 0,
> 
> > +		.min_sel = 0,
> 
> Perhaps it's better to have it aligned with max_sel, i.e. be 0x00.
> Same applies to the rest of a such.
> 
> > +		.max_sel = 0x22,
> >  	},
> 
> ...
> 
> > +static const struct linear_range warm_charge_curr[] = {
> >  	{
> >  		.min = 10,
> >  		.step = 10,
> > -		.vals = 20,
> > -		.low_sel = 0,
> > +		.min_sel = 0,
> > +		.max_sel = 0x12
> 
> Perhaps leaving comma is a good thing to avoid potential churn in the future
> (if any of fields will be added here). Same applies to the reset of a such.
> 
> >  	},
> 
> -- 
> With Best Regards,
> Andy Shevchenko
> 
> 

-- 
With Best Regards,
Andy Shevchenko


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek