Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Sun, 14 Mar 2021 23:35:13 +0100
From:   Sebastian Reichel <sre@kernel.org>
To:     Ray Chi <raychi@google.com>
Cc:     Greg KH <gregkh@linuxfoundation.org>, naresh.kamboju@linaro.org,
        Kyle Tso <kyletso@google.com>, linux-pm@vger.kernel.org,
        linux-kernel@vger.kernel.org,
        Badhri Jagan Sridharan <badhri@google.com>
Subject: Re: [PATCH] usb: dwc3: fix build error when POWER_SUPPLY is not
 enabled
Message-ID: <20210314223513.tznvhuq7phxxvjfo@earth.universe>
References: <20210308133146.3168995-1-raychi@google.com>
 <20210309185807.ka4iljasq5cmpmil@earth.universe>
 <CAPBYUsCJ3ftC4ur412rFZGeeM_kDHrCh=BVci3=8SE2eFdPcQg@mail.gmail.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
        protocol="application/pgp-signature"; boundary="r6xtjzuk4er6qt7g"
Content-Disposition: inline
In-Reply-To: <CAPBYUsCJ3ftC4ur412rFZGeeM_kDHrCh=BVci3=8SE2eFdPcQg@mail.gmail.com>
Precedence: bulk


--r6xtjzuk4er6qt7g
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

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.
>=20
> 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.
>=20
> 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
=66rom 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

--r6xtjzuk4er6qt7g
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAABCgAdFiEE72YNB0Y/i3JqeVQT2O7X88g7+poFAmBOj5MACgkQ2O7X88g7
+ppi5RAApUHg055QRJZ6hx9Eogf6zuWpbYCUC+z5bJ6WB7c9Nt4AsyAGH9GK2GvZ
phHQyxlB1Vx/AMGyOunitrkAh3qx0OZoLTJlxyY2SwTeMfVM1DIcr2s7VE//jotU
hCK0r79bYCx74aLwFVRYRKnnSlQjT/GJcs+Tx36pNdg52/Ius/tXQW/kP8DhhrHS
rL0xANsxf5TTsb5Hsv+ZCeQdYf5Ctt0ms+T3kPyHpAWcgyLP7puMhYDSb/ijkB18
pDnlpgKCG1xBPRNbSvKtoTVILYC+/BQWGn0venJn1lcjznX8JhoAD2gttQdqZWQK
8DvmOtIFUJQZz92UH1l9GBIPWqWJxSiIvewJwN0348WwTrQS9f+28fngMK0iYAWl
kBTwou5QJspoG+ydFv1VW/mdPVSBek8+T1kf2Jo+5aCrsu7XzdQc9Vrd0VaHvje9
DsrMiQDUz6Z4J9FIuzPERrGVBwVky7j1FjrK/WVuAI4Q/G3OtkLBqWEPFX+U361S
8zWUwZvbUmpHAwLadR7/Pw+9VX3dGyxR02y0uaIkXpug/oBx1dPeRMuCgqFBwrU1
iS/ggl82GiHt7f705DGz6jwZf8lGpHVDSGThA6WOF8OhYTNxtp/ZmSXgQTitoJ9j
fsbaPue/oDCAtFv6NRzRoqnSJ5PTVusVK3LDuJwrAANNsOrCNCE=
=E8+B
-----END PGP SIGNATURE-----

--r6xtjzuk4er6qt7g--