Hi, On Fri, Mar 12, 2021 at 09:57:56PM +0800, Ray Chi wrote: > > 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.
Considering your programming is off by a factor 1000 I wonder how this patchset has been tested. > > 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. usb_register_notifier registers a callback to receive information from the USB subsystem. Then power-supply drivers can query specific info, e.g. call usb_phy_get_charger_current(). This is already being done by some power-supply drivers, so using one of those power-supply charger drivers in combination with dwc3 will now result in potentially racy behaviour as far as I can see. > > I suggest to drop/revert the whole patchset. -- Sebastian
signature.asc
Description: PGP signature
