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: <20200124152504.23411-1-mans@mansr.com> <20200127153506.GA4589@bogus>
 <yw1xy2tsvnww.fsf@mansr.com> <20200128134745.GA3048749@kroah.com>
 <yw1xpnf3vchs.fsf@mansr.com> <20200128152818.GB3437093@kroah.com> <20200128165243.GC3666045@kroah.com>
In-Reply-To: <20200128165243.GC3666045@kroah.com>
From:   Rob Herring <robh@kernel.org>
Date:   Tue, 28 Jan 2020 12:21:32 -0600
Message-ID: <CAL_JsqK1fMCrbbMdRPqVjtS0D6p4AhqjcOGoivGbT2aKN7UJWw@mail.gmail.com>
Subject: Re: [RESEND][PATCH 1/2] dt-bindings: usb: add non-removable-ports hub property
To:     Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        =?UTF-8?B?TcOlbnMgUnVsbGfDpXJk?= <mans@mansr.com>
Cc:     Mark Rutland <mark.rutland@arm.com>,
        Linux USB List <linux-usb@vger.kernel.org>,
        devicetree@vger.kernel.org,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

n Tue, Jan 28, 2020 at 10:52 AM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Tue, Jan 28, 2020 at 04:28:18PM +0100, Greg Kroah-Hartman wrote:
> > On Tue, Jan 28, 2020 at 03:15:11PM +0000, M=C3=A5ns Rullg=C3=A5rd wrote=
:
> > > Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
> > >
> > > > On Mon, Jan 27, 2020 at 04:56:15PM +0000, M=C3=A5ns Rullg=C3=A5rd w=
rote:
> > > >> Rob Herring <robh@kernel.org> writes:
> > > >>
> > > >> > On Fri, Jan 24, 2020 at 03:25:03PM +0000, Mans Rullgard wrote:
> > > >> >> Add a non-removable-ports property that lists the hardwired dow=
nstream
> > > >> >> ports of a hub.  Although hubs can provide this information, th=
ey are
> > > >> >> not always configured correctly.  An alternate means of indicat=
ing this
> > > >> >> for built-in USB devices is thus useful.
> > > >> >>
> > > >> >> Signed-off-by: Mans Rullgard <mans@mansr.com>
> > > >> >
> > > >> > I reviewed this already, but since you didn't add my reviewed-by=
, I'm
> > > >> > looking at it again and having 2nd thoughts.
> > > >> >
> > > >> >> ---
> > > >> >>  Documentation/devicetree/bindings/usb/usb-device.txt | 4 ++++
> > > >> >>  1 file changed, 4 insertions(+)
> > > >> >>
> > > >> >> diff --git a/Documentation/devicetree/bindings/usb/usb-device.t=
xt b/Documentation/devicetree/bindings/usb/usb-device.txt
> > > >> >> index 036be172b1ae..92d863cc96b6 100644
> > > >> >> --- a/Documentation/devicetree/bindings/usb/usb-device.txt
> > > >> >> +++ b/Documentation/devicetree/bindings/usb/usb-device.txt
> > > >> >> @@ -66,6 +66,10 @@ Required properties for host-controller node=
s with device nodes:
> > > >> >>  - #size-cells: shall be 0
> > > >> >>
> > > >> >>
> > > >> >> +Optional properties for hub and host-controller nodes:
> > > >> >> +- non-removable-ports: list of hardwired downstream ports
> > > >> >
> > > >> > If you have a hardwired device and need to know that, doesn't th=
at imply
> > > >> > there's some other stuff you need to describe beyond what a stan=
dard USB
> > > >> > device has. Such as a power supply that's not Vbus from the hub.
> > > >>
> > > >> I suppose there could be, but there isn't in my actual situation.
> > > >>
> > > >> > At a minimum, I think this should be a per port property.
> > > >>
> > > >> That's what I suggested first.  Greg told me to do it like this in=
stead.
> > > >
> > > > I said that?  I do not remember discussing this at all, when did th=
at
> > > > happen?
> > >
> > > https://lore.kernel.org/lkml/20190228155241.GC12050@kroah.com/
> >
> > Almost a full year ago!  Hah, I can't remember what I wrote last week.
>
> Ah, ok, all I said was "do what ACPI does here", as that's a model of
> what has already been agreed apon by a whole huge number of people and
> standardized.  No need for DT to come up with something totally
> different instead, making a mess of things :)
>
> If this is doing what ACPI does, fine, if not, it should.  It was here
> first.

That's not always possible as ACPI and DT work in different ways. The
DT (Open Firmware) USB binding originated in 1998[1]. While ancient,
that is what defines the node structure of USB hubs, ports, and
devices that we use today.

However, after a quick read of ACPI sec 9.14, I'd say what I suggested
is more aligned to ACPI than what's proposed here. Ports are child
nodes ("Device" in ACPI terms) and the properties to determine all
this are properties of the port node(s). Aligning beyond that isn't
really possible. ACPI has a standard thing (not sure what the proper
term is) called '_PLD' for describing device location which includes
'user visible' among several other things. There is no such concept in
DT to align with. What we have is the 'non-removable' property and IMO
that's what we should use here.

Rob

[1] https://www.devicetree.org/open-firmware/bindings/usb/usb-1_0.ps