Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20190326155728.5432-1-brgl@bgdev.pl> <712540aa-6927-9141-4f0c-740193d26f33@ti.com>
In-Reply-To: <712540aa-6927-9141-4f0c-740193d26f33@ti.com>
From:   Bartosz Golaszewski <brgl@bgdev.pl>
Date:   Wed, 27 Mar 2019 14:16:48 +0100
Message-ID: <CAMRc=McPzhOy9Ebp+hDC1eiSKJBf-q9gCeD_WMF5Wa4SSBR=dg@mail.gmail.com>
Subject: Re: [PATCH 0/3] ARM: davinci: ohci-da8xx: model the vbus GPIO as a
 fixed regulator
To:     Sekhar Nori <nsekhar@ti.com>
Cc:     Kevin Hilman <khilman@kernel.org>,
        Alan Stern <stern@rowland.harvard.edu>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Linux ARM <linux-arm-kernel@lists.infradead.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        linux-usb@vger.kernel.org,
        Bartosz Golaszewski <bgolaszewski@baylibre.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

=C5=9Br., 27 mar 2019 o 12:37 Sekhar Nori <nsekhar@ti.com> napisa=C5=82(a):
>
> Hi Bart,
>
> On 26/03/19 9:27 PM, Bartosz Golaszewski wrote:
> > From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> >
> > Adding the vbus GPIO support to the ohci-da8xx driver isn't really the
> > optimal solution. Rather: it should be modeled as a fixed regulator
> > in which case the driver already has support.
>
> Can you clarify "driver already has support"? You are introducing
> support to use the VBUS gpio as regulator as part of 3/3.
>

The support is there as in: if the driver can obtain the regulator, it
will enable it. The overcurrent protection does not work however and
this is what patch 3 adds. Maybe I should rework the ordering in that
I'd first add the irq thread disabling the regulator if it exists,
then the regulator fixups to board files and then remove the vbus
GPIO.

> I do see other instances of VBUS regulator being used in USB tree. But
> we just converted the driver to use VBUS and over-current GPIOs in v5.1.
> So this is a bit of "churn".
>

Yes and it's my fault - I simply converted the legacy code without
giving it enough consideration. I should have used a fixed regulator
right away, but now it's upstream and we need a follow-up series.

> Can you document why the current solution is not optimal? Is it to make
> future device-tree conversion for these boards easier? Or?
>

It's sub-optimal from the HW modeling in SW PoV - it is in fact a
regulator enabled/disabled by a GPIO. Also: it's code duplication as
currently we check if the vbus GPIO exists and then use it or check if
the regulator exists and use this as the second choice. The third
patch actually shrinks the driver.

Bart

> >
> > This series adds necessary fixups to the board files and removes the
> > vbus GPIO from the ohci driver.
>
> Thanks,
> Sekhar