Re: [RESEND][PATCH 1/2] dt-bindings: usb: add non-removable-ports hub property

[Måns Rullgård]

Rob Herring <robh@xxxxxxxxxx> writes:

> n Tue, Jan 28, 2020 at 10:52 AM Greg Kroah-Hartman
> <gregkh@xxxxxxxxxxxxxxxxxxx> 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åns Rullgård wrote:
>> > > Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx> writes:
>> > >
>> > > > On Mon, Jan 27, 2020 at 04:56:15PM +0000, Måns Rullgård wrote:
>> > > >> Rob Herring <robh@xxxxxxxxxx> writes:
>> > > >>
>> > > >> > On Fri, Jan 24, 2020 at 03:25:03PM +0000, Mans Rullgard wrote:
>> > > >> >> Add a non-removable-ports property that lists the hardwired downstream
>> > > >> >> ports of a hub.  Although hubs can provide this information, they are
>> > > >> >> not always configured correctly.  An alternate means of indicating this
>> > > >> >> for built-in USB devices is thus useful.
>> > > >> >>
>> > > >> >> Signed-off-by: Mans Rullgard <mans@xxxxxxxxx>
>> > > >> >
>> > > >> > 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.txt 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 nodes 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 that imply
>> > > >> > there's some other stuff you need to describe beyond what a standard 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 instead.
>> > > >
>> > > > I said that?  I do not remember discussing this at all, when did that
>> > > > happen?
>> > >
>> > > https://lore.kernel.org/lkml/20190228155241.GC12050@xxxxxxxxx/
>> >
>> > 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.

Can you guys please agree on something or other.  I'm happy to do it
whichever way you decide, but I'd rather not waste my time making
patches that will just get rejected.

-- 
Måns Rullgård