Re: [PATCH v3] usb: dwc3: Add support for VBUS power control

[Mike Looijmans]

Met vriendelijke groet / kind regards,

Mike Looijmans
System Expert


TOPIC Embedded Products B.V.
Materiaalweg 4, 5681 RJ Best
The Netherlands

T: +31 (0) 499 33 69 69
E: mike.looijmans@xxxxxxxxxxxxxxxxx
W: www.topicproducts.com

Please consider the environment before printing this e-mail
On 07-09-2020 09:44, Felipe Balbi wrote:
> Hi,
>
> Mike Looijmans <mike.looijmans@xxxxxxxx> writes:
> > Met vriendelijke groet / kind regards,
> >
> > Mike Looijmans
> > System Expert
> >
> >
> > TOPIC Embedded Products B.V.
> > Materiaalweg 4, 5681 RJ Best
> > The Netherlands
> >
> > T: +31 (0) 499 33 69 69
> > E: mike.looijmans@xxxxxxxxxxxxxxxxx
> > W: www.topicproducts.com
> >
> > Please consider the environment before printing this e-mail
> > On 27-07-2020 12:23, Mark Brown wrote:
> > > On Sun, Jul 26, 2020 at 09:10:39AM +0200, Mike Looijmans wrote:
> > > > On 23-07-2020 13:05, Mark Brown wrote:
> > > > > Does the device actually support running without power so that's a thing
> > > > > that can happen?  _get_optional() should only ever be used for supplies
> > > > > that may be physically absent.
> > > > It's the 5V VBUS power for the USB "plug" that's being controlled here. It
> > > > must turned on when the controller is in "host" mode. Some boards arrange
> > > > this in hardware through the PHY, and some just don't have any control at
> > > > all and have it permanently on or off. On a board where the 5V is controlled
> > > > using a GPIO line or an I2C chip, this patch is required to make it work.
> > > That sounds like the driver should not be using _get_optional() then.
> > Making it mandatory would break most (read: all except Topic's) existing
> > boards as they won't have it in their devicetree. I'm perfectly okay with
> > that, but others might disagree.
> you're perfectly okay with break all existing users of the driver?
> That's a bit harsh
It turned out that "optional" when used for regulators means the 
opposite of when used in clk context. For regulators, "optional" means 
"don't supply a dummy regulator if there's none provided". So 
get_optional will just fail when the supply isn't defined, while 
get_regulator will just return a dummy in that case.

So the v4 patch which doesn't use "_get_optional" works for both cases 
and doesn't break existing use(r)s.

--
Mike Looijmans