Re: [PATCH V4 14/15] power: supply: axp20x_battery: add support for AXP717

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On Thu, Aug 29, 2024 at 08:11:09PM GMT, Chris Morgan wrote:
> On Tue, Aug 27, 2024 at 06:24:42PM +0200, Sebastian Reichel wrote:
> > On Wed, Aug 21, 2024 at 04:54:55PM GMT, Chris Morgan wrote:
> > > +	case POWER_SUPPLY_PROP_VOLTAGE_NOW:
> > > +		ret = iio_read_channel_processed(axp20x_batt->batt_v,
> > > +						 &val->intval);
> > > +		if (ret)
> > > +			return ret;
> > > +
> > > +		/* IIO framework gives mV but Power Supply framework gives uV */
> > > +		val->intval *= 1000;
> > > +		return 0;
> > 
> > I see you followed the existing pattern for these two drivers. Can
> > you please add another patch, which converts both drivers to the
> > following pattern:
> > 
> > return iio_read_channel_processed_scale(adc_chan, &val->intval, 1000);
> 
> Would it be okay if I sent a patch series on top of this one (rather
> than rebasing now that some of these are starting to get pulled)?
> 
> It's probably a simple enough change so I don't mind.

Yes, please make this a follow-up patch and update all occurances of
the pattern in axp20x_ac_power.c, axp20x_battery.c and
axp20x_usb_power.c.

Thanks!

> > > +		 * AXP717 can go up to 4.35, 4.4, and 5.0 volts which
> > > +		 * seem too high for lithium batteries, so do not allow.
> > 
> > 4.35V and 4.4V batteries exists. You can find them when you search
> > for LiHV (Lithium High Voltage).
> 
> Do you think I should add it then? Full disclosure, I basically opted
> to not add this because while this series was written more or less
> exclusively off of the datasheet I did peek at the BSP implementation
> and I think they restricted these voltages. Again, as a fast-follow
> once these patches finish getting pulled (I can start work on it now
> though).

Feel free to ignore that information. Support can always be added later
once somebody needs to use the chip with a LiHV battery.

-- Sebastian

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Input]     [Linux Kernel]     [Linux SCSI]     [X.org]

  Powered by Linux