Hi, ChiYuan Huang <u0084500@xxxxxxxxx> 於 2022年10月3日 週一 上午10:00寫道： > > Sebastian Reichel <sebastian.reichel@xxxxxxxxxxxxx> 於 2022年10月2日 週日 凌晨4:58寫道： > > > > Hi, > > > > On Mon, Sep 19, 2022 at 09:11:09AM +0800, ChiYuan Huang wrote: > > > Sebastian Reichel <sebastian.reichel@xxxxxxxxxxxxx> 於 2022年9月17日 週六 上午9:19寫道： > > > > On Thu, Sep 15, 2022 at 12:30:15AM +0800, cy_huang wrote: > > > > > From: ChiYuan Huang <cy_huang@xxxxxxxxxxx> > > > > > > > > > > Document the settings exported by rt9471 charger driver through sysfs entries: > > > > > - sysoff_enable > > > > > - port_detect_enable > > > > > > > > > > Signed-off-by: ChiYuan Huang <cy_huang@xxxxxxxxxxx> > > > > > --- > > > > > Since v5: > > > > > - Recover all the change in sysfs-class-power. > > > > > - New a sysfs-class-power-rt9471 file. > > > > > - Remove 'charge_term_enable' sysfs entry, directly integrate it in > > > > > 'charge_term_current' power supply property control. > > > > > > > > > > --- > > > > > Documentation/ABI/testing/sysfs-class-power-rt9471 | 29 ++++++++++++++++++++++ > > > > > 1 file changed, 29 insertions(+) > > > > > create mode 100644 Documentation/ABI/testing/sysfs-class-power-rt9471 > > > > > > > > > > diff --git a/Documentation/ABI/testing/sysfs-class-power-rt9471 b/Documentation/ABI/testing/sysfs-class-power-rt9471 > > > > > new file mode 100644 > > > > > index 00000000..ad5b049 > > > > > --- /dev/null > > > > > +++ b/Documentation/ABI/testing/sysfs-class-power-rt9471 > > > > > @@ -0,0 +1,29 @@ > > > > > +What: /sys/class/power_supply/rt9471-*/sysoff_enable > > > > > +Date: Oct 2022 > > > > > +KernelVersion: 6.1 > > > > > +Contact: ChiYuan Huang <cy_huang@xxxxxxxxxxx> > > > > > +Description: > > > > > + This entry allows enabling the sysoff mode of rt9471 charger devices. > > > > > + If enabled and the input is removed, the internal battery FET is turned > > > > > + off to reduce the leakage from the BAT pin. See device datasheet for details. > > > > > + It's commonly used when the product enter shipping stage. > > > > > + > > > > > + Access: Read, Write > > > > > + Valid values: > > > > > + - 1: enabled > > > > > + - 0: disabled > > > > > > > > I still fail to see why this needs to be controllable at runtime. > > > > This looks like a hardware property. Are there any known products, > > > > which need this disabled? > > > It's just a switch, actually 'disabled' is not needed. > > > For the enabled case, mostly used in below scenarios > > > 1. Online testing, USB IN -> Factory testing -> write 1 to enable -> > > > USB out -> immediately VSYS off -> pack > > > 2. Offline testing no vbus -> Factory testing -> write 1 to enable -> > > > immediately VSYS off -> pack > > > > > > The 'disable" can use to cancel the shipping mode in case 1 before USB out. > > > It's more like the testing. > > > > > > Like as you said, shipping BATFET_OFF is all the hardware behavior. > > > To leave this mode after VSYS off, there're three ways > > > 1. power key pressed > > > 2. VBUS IN > > > 3. control BATFET_OFF to 0 (But it need SOC to be alive, at the time, > > > VSYS off, no one can execute this I2C command) > > > > If factory testing and preperation is the only use case, I don't > > think exposing this in sysfs and creating userspace ABI is worth > > it. Just tell factory to use i2c-dev and poke the correct registers. > > > I agree your comment if there's only this case will use it. > > So I ask our HW members about this. > They said there's still one case I didn't consider about. > It's the dual charger scenario. > If the charging process is entering CV mode, the slave charger is no > need to join the charging. > Then in common case, slave charger need to minimize the battery leakage. > And the BATFET_OFF is needed to lower the battery leakage. > > They think this sysfs entry is needed. Can this persuade you? > > > > If what you care is no need to mention 'disable', then just remove it. > > > It's fine. > > > > > > > > > +What: /sys/class/power_supply/rt9471-*/port_detect_enable > > > > > +Date: Oct 2022 > > > > > +KernelVersion: 6.1 > > > > > +Contact: ChiYuan Huang <cy_huang@xxxxxxxxxxx> > > > > > +Description: > > > > > + This entry allows enabling the USB BC12 port detect function of rt9471 charger > > > > > + devices. If enabled and VBUS is inserted, device will start to do the BC12 > > > > > + port detect and report the usb port type when port detect is done. See > > > > > + datasheet for details. Normally controlled when TypeC/USBPD port integrated. > > > > > + > > > > > + Access: Read, Write > > > > > + Valid values: > > > > > + - 1: enabled > > > > > + - 0: disabled > > > > > > > > So basically this depends on the hardware integration (e.g. it > > > > should be disabled when power source is a DC barrel jack instead > > > > of USB) and is not supposed to change at all during runtime? Then > > > > the information wether it needs to be enabled should be derived > > > > from the device tree. > > > > > > It's a switching charger integrates OTG boost. > > > For the case 'DC Jack', there's no need to use this kind of product. > > > > > > With typec integration, at most time, it still need bc12 to be enabled > > > by default. Just in some case, like as power role swap (SNK -> SRC -> SNK), > > > to automatically identify the USB port, this may interrupt USB communication. > > > > > > So as my understanding, keep it enabled by default, just in some case, > > > it my need to control at runtime. > > > > This should be part of the description. You can drop the sentence > > "Normally controlled when TypeC/USBPD port integrated.", since > > that's hard to comprehend. Instead add the information that this > > is supposed to be always enabled, but can be disabled to avoid > > USB link loss (?) when doing a USB PD role swap. > > > Thanks for the comment. > > I'll rewrite it as below > 'It's supposed to be always enabled, but can be disabled to avoid usb > link interruption especially when doing a USBPD 'power' role swap.' > > Sorry, due to the long reply period, I already sent v6/v7/v8 to fix > some coding like as missing header and irq wakeup check. > Actually only missing header change is needed. > Please ignore these noise. > > And after the sysfs reviewing is finished, you can just review the revision v9. I'm preparing the final patch. 2 more weeks past, now only one question is left for rt9471 ABI document. After my explanation, is 'to keep sysoff_enable" acceptible? Or no more discussion, just delete it? > Thanks. > > -- Sebastian
