On 5/2/23 18:13, Tim Harvey wrote:
On Mon, May 1, 2023 at 11:51 PM Eugen Hristev
<eugen.hris...@collabora.com> wrote:
regulator_set_enable_if_allowed already handles cases when the
regulator is already enabled, or already disabled, and does not
treat these as errors.
With this change, the driver can work correctly even if the regulator
is already taken or already disabled by another consumer.
Signed-off-by: Eugen Hristev <eugen.hris...@collabora.com>
---
Hi Tim,
I have not tested this as I do not have a mx6 board. can you please
try it ?
Thanks,
Eugen
Eugen,
This does resolve the issue that occurs after your refcount series [1].
However after thinking about this more and seeing Marek's responses
this wasn't an issue of multiple consumers sharing the same regulator.
Instead this particular issue was that the vbus regulator is getting
enabled twice without being disabled. Adding a print in
regulator_set_enable shows me:
--- a/drivers/power/regulator/regulator-uclass.c
+++ b/drivers/power/regulator/regulator-uclass.c
@@ -165,6 +165,7 @@ int regulator_set_enable(struct udevice *dev, bool enable)
struct dm_regulator_uclass_plat *uc_pdata;
int ret, old_enable = 0;
+printf("%s %s %s\n", __func__, dev->name, enable ? "enable" : "disable");
if (!ops || !ops->set_enable)
return -ENOSYS;
u-boot=> usb start
starting USB...
Bus usb@32e40000: regulator_set_enable regulator-usb-otg1 enable
^^^ from ehci_usb_probe
Bus usb@32e50000: regulator_set_enable regulator-usb-otg2 enable
^^^ from ehci_usb_probe
regulator_set_enable regulator-usb-otg2 enable
^^^ from mx6_init_after_reset - 2nd enable without a disable
So while I think this patch does make sense to cover the case where a
regulator could be shared
Does such a case really still exist after the discovery you made above ?
there probably is a follow-on patch needed
to balance the regulator calls (unrelated to your series).
Marek,
Should vbus regulator enable really be in init_after_reset call?
Yes, but I am not entirely convinced the VBUS should be enabled in
ehci_usb_probe(), because in ehci_usb_probe() resp. in ehci_register()
which is called at the end, we might detect that the port is configured
as PERIPHERAL and we don't want to enable VBUS in that case .
So I suspect regulator_set_enable() should rather be dropped from
ehci_usb_probe() . What do you think ?
