Re: [PATCH 1/3] ARM: dts: sun4i: Allow to use the PH6 pin for GPIO on pcDuino1/2

[Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 2558 bytes --]

On Mon, Oct 05, 2015 at 08:39:40AM +0300, Siarhei Siamashka wrote:
> On Mon, 5 Oct 2015 09:55:28 +0800
> Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org> wrote:
> 
> > On Mon, Oct 5, 2015 at 2:58 AM, Siarhei Siamashka
> > <siarhei.siamashka-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> > > The pcDuino1 board does not use any power switches at all for its
> > > two USB host ports and the VBUS pins are always connected to 5V.
> > >
> > > The pcDuino2 board uses the RT9701GB power switch for its single
> > > USB host port, but the USB_EN pin (PD2) is pulled up with a 10K
> > > resistor. So that the USB power is still enabled by default even
> > > if nobody bothers to configure the PD2 pin or runs the pcDuino1
> > > firmware.
> > 
> > Seems like it would be better if you had a regulator controlled
> > by PD2. At least can shut down VBUS power when it wants to?
> 
> That's a good question.
> 
> Describing the regulator controlled by PD2 in the dts file is surely
> the right solution for pcDuino2 boards. But in the case of using this
> dts for pcDuino1, the kernel would think that it can shut down VBUS
> power, while in fact this is not true.

I do agree that this is the right solution for the pcduino1, but it's
definitely not the right one for the pcduino 2 then.

Declaring this as a regulator isn't just meant for the USB to be
working, it's also meant so that it keeps working. If the pin is not
claimed by anyone, the userspace and / or some other driver, will
actually be able to grab that pin and do whatever it wants with it,
effectively fiddling with the VBUS supply behind the USB driver's
back.

It also allows to disable the regulator if VBUS isn't going to be
used, for example because the driver has not be compiled in, or that
it's actually a module that might (or might not) be loaded later.

Finally, it also allows to keep track of who consumes what amount of
power in the system, which is a nice plus.

> The RT9701GB switch also provides the current limiting feature in
> addition to the ability to enable/disable the VBUS power. Probably
> this was a real reason why it was added to the board.
> 
> Everything boils down to the question whether we want to have a
> common dts file for pcDuino1 and pcDuino2 or decide to split them.

You don't have to split them, if this is really the only difference,
just create a new dts for the pcduino2 that includes the first one,
and add the supply there.

Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com