Hi Sebastian, Sorry for the late reply.
On Wed, Mar 10, 2021 at 2:58 AM Sebastian Reichel <s...@kernel.org> wrote: > > Hi, > > On Mon, Mar 08, 2021 at 09:31:46PM +0800, Ray Chi wrote: > > Fix build error when CONFIG_POWER_SUPPLY is not enabled. > > > > The build error occurs in mips (cavium_octeon_defconfig). > > > > mips-linux-gnu-ld: drivers/usb/dwc3/core.o: in function `dwc3_remove': > > drivers/usb/dwc3/core.c:1657: undefined reference to `power_supply_put' > > mips-linux-gnu-ld: drivers/usb/dwc3/core.o: in function > > `dwc3_get_properties': > > drivers/usb/dwc3/core.c:1270: undefined reference to > > `power_supply_get_by_name' > > mips-linux-gnu-ld: drivers/usb/dwc3/core.o: in function `dwc3_probe': > > drivers/usb/dwc3/core.c:1632: undefined reference to `power_supply_put' > > > > Fixes: 59fa3def35de ("usb: dwc3: add a power supply for current control") > > Reported-by: Naresh Kamboju <naresh.kamb...@linaro.org> > > Signed-off-by: Ray Chi <ray...@google.com> > > --- > > While I'm fine with merging this after fixing up the subject, the > original patch for dwc3 [0] looks completly incorrect to me. > > First of all it uses wrong scale (power-supply uses uA, not mA), > so you are charging 1000x slower than expected. Then the patchset > introduces a new DT property to get the power-supply device, but > does not update the DT binding documentation and does not Cc the > DT binding maintainer. Yes, it should use uA and send this information, and I will update a patch to fix it and add the DT binding documentation. > Next the property itself looks not very > smart to me. Usually one would use a device reference, not the > Linux device name. > > Finally all existing devices solve this by registering a usb > notifier from the charger, so why are you going the other way > around? This is going to break once you want to use one of the > existing chargers with dwc3. Only the USB controller will know USB state/speed so that I think it is better to send this information from the USB side. For example: For USB high speed, charging current should be limited to 500 mA in configured state. For USB super speed, charging current should be limited to 900 mA in configured state. > > I suggest to drop/revert the whole patchset. > > [0] > https://lore.kernel.org/linux-usb/20210222115149.3606776-1-ray...@google.com/ > > -- Sebastian > > > include/linux/power_supply.h | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h > > index 81a55e974feb..6e776be5bfa0 100644 > > --- a/include/linux/power_supply.h > > +++ b/include/linux/power_supply.h > > @@ -381,8 +381,14 @@ struct power_supply_battery_info { > > extern struct atomic_notifier_head power_supply_notifier; > > extern int power_supply_reg_notifier(struct notifier_block *nb); > > extern void power_supply_unreg_notifier(struct notifier_block *nb); > > +#if IS_ENABLED(CONFIG_POWER_SUPPLY) > > extern struct power_supply *power_supply_get_by_name(const char *name); > > extern void power_supply_put(struct power_supply *psy); > > +#else > > +static inline void power_supply_put(struct power_supply *psy) {} > > +static inline struct power_supply *power_supply_get_by_name(const char > > *name) > > +{ return NULL; } > > +#endif > > #ifdef CONFIG_OF > > extern struct power_supply *power_supply_get_by_phandle(struct device_node > > *np, > > const char *property); > > -- > > 2.30.1.766.gb4fecdf3b7-goog > >