Sebastian Reichel [off-list ref] 於 2021年1月16日 週六 下午6:12寫道：

Hi,

On Mon, Jan 11, 2021 at 08:15:33PM +0800, Gene Chen wrote:

quoted

Sebastian Reichel [off-list ref] 於 2021年1月7日 週四 上午4:16寫道：

quoted

quoted

+     last_usb_type = mci->psy_usb_type;
+     /* Plug in */
+     ret = regmap_read(mci->regmap, MT6360_PMU_USB_STATUS1, &usb_status);
+     if (ret < 0)
+             goto out;
+     usb_status &= MT6360_USB_STATUS_MASK;
+     usb_status >>= MT6360_USB_STATUS_SHFT;
+     switch (usb_status) {
+     case MT6360_CHG_TYPE_UNDER_GOING:
+             dev_info(mci->dev, "%s: under going...\n", __func__);
+             goto out;

IDK what this is supposed to tell me. Do you mean "detection in
progress"? Also why is this info level? I would expect either
debug (assuming it happens regularly and is normal) or warning
(assuming it should not happen).

When handling attach interrupt and cable plug out at the same
time, HW change register status. So we don' need to handle this
attach interrupt at this case.

So this is basically for debouncing? I suggest adding a comment:

/* Received attach IRQ followed by detach event, so nothing to do */
dev_dbg(mci->dev, "under going...\n");
goto out;

Sorry I have a little misunderstand.
under going is "detect in progress".
Attachi irq will trigger when power ready(vbus=5V) and bc12
chargedetection done.
Another irq, Detachi, is indicated power not ready(vbus=0V) and which
is be masked.
So, if the usb status is not SDP/NONSTD/CDP/DCP, the result can be
ignored. (e.q. NO VBUS/Under going/BC12 disabled/Reserved address)

[...]

quoted

quoted

quoted

+     config.dev = &pdev->dev;
+     config.regmap = mci->regmap;
+     mci->otg_rdev = devm_regulator_register(&pdev->dev, &mt6360_otg_rdesc,
+                                             &config);
+     if (IS_ERR(mci->otg_rdev))
+             return PTR_ERR(mci->otg_rdev);
+
+     ret = mt6360_sysfs_create_group(mci);
+     if (ret) {
+             dev_err(&pdev->dev,
+                     "%s: Failed to create sysfs attrs\n", __func__);
+             return ret;
+     }

Use charger_cfg.attr_grp to register custom sysfs group for
power-supply devices. Otherwise your code is racy (udev may not pick
up the sysfs attributes). Also custom sysfs attributes need to be
documented in Documentation/ABI/testing/sysfs-class-power-<driver>.

Looking at the attributes you are planning to expose, I don't think they
are suitable for sysfs anyways. Looks more like a debug interface, which
should go into debugfs instead. But it's hard to tell without any documentation
being provided :)

ACK, I will change to charger_cfg.attr_grp.
I assumed the charger algorithm thread is in user space, and take
control by sysfs node from charger device, like bq24190.c.
Should I change to debugfs?

It's hard to tell without knowing more about the attributes
your are trying to expose. In debugfs we have relaxed ABI rules,
so it's easier to adopt naming e.t.c. later.

I briefly classify the whole attributes. There are either unused, or
can be replaced by POWER_SUPPLY PROPERTY,
so I will remove unuse part.

HIZ = VBUS_IN high impedance mode.
VMIVR = Maximum input voltage regulation. Let input power can provide
at the predetermined voltage level.
(like POWER_SUPPLY_PROP_INPUT_VOLTAGE_LIMIT)
SYSREG = Config system minimum regulation voltage.
OTG_OC = maximum current of battery boost OTG 5V.
ICHG = maximum Charging current. (like
POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT)
IEOC = Like POWER_SUPPLY_PROP_CHARGE_TERM_CURRENT
VOREG = Input voltage regulation. (like
POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE)
LBP = Low battery protection for battery boost OTG 5V.
VPREC = Pre-charge volatge level. (maybe can add new prop
POWER_SUPPLY_PROP_PRECHARGE_VOLTAGE)
TE = Charge termination enable/disable.
CHG_WDT_EN = Charger Watch dog timer enable/disable.
CHG_WDT = Charger Watch dog timer, 8/40/80/160s.
WT_FC = Fast charge Timer, 4~20hr.
BAT_COMP = Battery IR compensation resistor setting.
VCLAMP = Battery IR compensation maximum voltage clamp.
USBCHGEN = USB charger detection flow enable/disable.
CHG_EN = Battery charging enable/disable.
CHRDET_EXT = VBUS_IN is between VBUS_UV_TH(3.7V) and VBUS_OV_TH(10.5V)

-- Sebastian

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel