More RK817 questions

[Peter Geis]

On Mon, Oct 10, 2022 at 8:47 PM Chris Morgan <macromorgan at hotmail.com> wrote:
>
> On Mon, Oct 10, 2022 at 08:33:48PM +0100, Philip C wrote:
> > Hi Peter, Chris and Maya

Good Evening,

> >
> > I'm not going to bother with the rockchip list because I think that you are
> > the only three that will be interested in this.

I have added the Rockchip mailing list so they can get a heads up about this.

> >
> > I did a bit of hacking of the 4.19 BSP kernel on my device to see how the 5V
> > rail of the USB port works, and I am pretty sure that I understand it now.
> >
> > I think that /power/supply/rk817_charger.c registers itself as an extcon
> > with /phy/rockchip/phy-rockchip-inno-inno-usb2.c for notification when the
> > state of usb2phy0 changes.
> >
> > Specifically within rk817_charger.c there is a function
> > rk817_charge_host_evt_worker which reacts to the notifications from the phy
> > and runs rk817_charge_set_otg_state(charge, USB_OTG_POWER_ON)
> >
> > This won't be true of all devices, only those (like mine) that have the
> > EXTCON=<&usb2phy0> option set in dts for the RK817.

I suggest you look closely at how this was implemented in mainline.
You'll notice we do things significantly differently.

> >
> > I have done enough experimentation with my device to prove that if I upset
> > this particular mechanism that that 5V is not applied to the USB port when
> > it should be, so I really do think that this is how it works in BSP.
> >
> > Looking in mainline is interesting, because the concept of another driver
> > registering with phy-rockchip-inno-usb2.c for updates when the phy state
> > changes are still there, all be it with different messages.
> >
> > So now for my questions:-
> >
> > Do you folks have any plans to write something into mainline rk817_charger.c
> > to register with the phy for notifications and switch on the OTG power?
>
> Not yet, but it's not a bad idea. Honestly I wonder if that's why my OTG USB
> doesn't work on mainline... because I don't have the extcon set up properly.

OTG works when set up correctly, look to the pinenote for how to set
it up. It also works on soquartz when attached to a correct carrier
board. There are a few kinks that needed to be worked out with the phy
driver, they have landed in next and should trickle down to the stable
kernels.

>
> >
> > If not then would it be productive if I have a go myself?
>
> Absolutely, let me know if you have questions. I might get around to it one day
> but not in the next few weeks. I have a large stack of U-Boot technical debt I
> have to pay.

You really should have included the Rockchip, USB, and MFD mailing
lists on this. That way they could tell you that the existing
infrastructure does everything it needs to do. Downstream does a lot
of naughty things to hack around limitations of hardware and the
limitations of the coders. Mainline requires everything to be
compliant with the various standards applicable, even if that means
some things aren't capable of working to their full potential. It
pushes for manufacturers to comply with the standards of the systems
they implement.

>
> >
> > Do you consider rk817_charger.c the right place to have such code or should
> > I be putting it somewhere else?
>
> The rk817_charger.c driver in mainline currently only checks (via an IRQ) if
> the power state has changed, if it has been plugged in or unplugged. Presumably
> without looking at the phy driver you could always check the phy state when
> this changes and then take the appropriate action? Otherwise we could set up
> another IRQ or workqueue of some kind in the charger driver to listen for events
> from the extcon.
>
> The only gotcha is this *might* actually be something the main mfd rk808 driver
> needs to do or the rk808 regulator driver (since I believe you need to simultaneously
> shut off the BOOST regulator when you switch the OTG switch). Mark Brown/Broonie might
> have some insight into that as he's the main guy for mfd drivers.

The otg_switch is literally that, a switch. It exists to permit you to
switch the voltage generated by the boost converter on and off to the
usb_otg port when necessary without stopping the boost converter,
which would kill anything else powered by it. The phy subsystem and
the regulator subsystem handle this without the awful hacks downstream
implemented. If you would like some help with getting your dts to
work, you are welcome to reach out to the mailing list, or to me
directly.

>
> >
> > thanks, Philip

Very Respectfully,
Peter Geis