On Mon, Apr 08, 2024 at 10:12:30PM GMT, Krzysztof Kozlowski wrote: > On 08/04/2024 17:17, Ondřej Jirman wrote: > > > > Now for things to not fail during suspend/resume based on PM callbacks > > invocation order, anx7688 driver needs to enable this regulator too, as long > > as it needs it. > > No, the I2C bus driver needs to manage it. Not one individual I2C > device. Again, why anx7688 is specific? If you next phone has anx8867, > using different driver, you also add there i2c-supply? And if it is > nxp,ptn5100 as well? Yes, that could work, if I2C core would manage this. > > > > I can put bus-supply to I2C controller node, and read it from the ANX7688 driver > > I guess, by going up a DT node. Whether that's going to be acceptable, I don't > > know. > > > > > > VCONN regulator I don't know where else to put either. It doesn't seem to belong > > anywhere. It's not something directly connected to Type-C connector, so > > not part of connector bindings, and there's nothing else I can see, other > > than anx7688 device which needs it for core functionality. > > That sounds like a GPIO, not regulator. anx7688 has GPIOs, right? On > Pinephone they go to regulator, but on FooPhone also using anx7688 they > go somewhere else, so why this anx7688 assumes this is a regulator? CC1/CC2_VCONN control pins are "GPIO" of anx7688, sort of. They have fixed purpose of switching external 5V regulator output to one of the CC pins on type-c port. I don't care what other purpose with some other firmware someone puts to those pins. It's irrelevant to the use case of anx7688 as a type-c controller/HDMI bridge, which we're describing here. VCONN regulator is an actual GPIO controlled regulator on the board, and needs to be controlled by the anx7688 driver. So that CC1/CC2_VCONN control pins driven by the firmware actually do what they're supposed to do. Not sure why it would be a business of anything else but anx7688 driver enabling this regulator, because only this driver knows and cares about this. If some other board doesn't have the need to manually enable the regulator, or doesn't have the regulator, it can simply be optional. There are also some other funky supplies in the bindings, that are not connected to the chip in any way, but need to be controlled by the driver: + vbus-supply: true + vbus-in-supply: true First one can be on the connector node instead, where the driver can fetch it from. The purpose of the second one is to link the Phone's PMIC's USB power input with the type-c controller (anx7688), to make sure the PMIC has information about how much power it can draw from external PSU. The second one can be replaced by rewriting the anx7688 driver so that it creates a power supply representing the USB PSU connected to the phone, and by linking to anx7688 DT node from x-powers,axp813-usb-power-supply via a power-supplies property, which would mean that USB input of the phone is supplied by the external USB PSU. PMIC driver can be modified to watch the power supply provided by anx7688 driver for information it detected via USB-PD and update its input current limit via a standard helper function. This is how eg. fusb302 works. Not sure if that's any better from the PoV of DT bindings, though. Because power-supplies = <&anx7688>; will not look any greater in DT bindings, IMO. It will just link the same nodes in the other direction. Anyway, the HW is that there's an external PSU (detected by type c controller) and internal USB power input, and they are connected and one has to respect the limits of the other. I guess I shouldn't be adding a device node for external PSU, since it's not part of the phone. But that's what's trully being linked on HW level. Kind regards, o. > > > > > ANX7688 chip desing doesn't have integrated VCONN mosfet switches so it always > > needs external supply + switches that are controlled by the chip itself. There's > > no sensible design where someone would not want this and the driver needs > > to get this regulator reference from somewhere. The switches are sort of an > > extension of the chip. > > > Best regards, > Krzysztof >
