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

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

[Mans Rullgard]

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@mansr.com>
---
 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
+
+
 Example:
 
 &usb1 {	/* host controller */
-- 
2.25.0

[RESEND][PATCH 2/2] usb: hub: use non-removable-ports DT property

[Mans Rullgard]

Mark any ports listed in the non-removable-ports DT property as
hardwired. This is useful for boards with built-in USB devices
that cannot be (or have not been) marked as fixed in hardware.

Signed-off-by: Mans Rullgard <mans@mansr.com>
---
 drivers/usb/core/hub.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 4ac74b354801..97f8f15fb632 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -1366,6 +1366,9 @@ static int hub_configure(struct usb_hub *hub,
 	unsigned unit_load;
 	unsigned full_load;
 	unsigned maxchild;
+	struct property *prop;
+	const __be32 *cur;
+	u32 val;
 
 	hub->buffer = kmalloc(sizeof(*hub->buffer), GFP_KERNEL);
 	if (!hub->buffer) {
@@ -1667,6 +1670,19 @@ static int hub_configure(struct usb_hub *hub,
 		}
 	}
 
+	of_property_for_each_u32(hub_dev->of_node, "non-removable-ports",
+				 prop, cur, val) {
+		if (val < 1 || val > hdev->maxchild) {
+			dev_warn(hub_dev,
+				 "bad port number %u in non-removable-ports\n",
+				 val);
+			continue;
+		}
+
+		hub->ports[val - 1]->connect_type =
+			USB_PORT_CONNECT_TYPE_HARD_WIRED;
+	}
+
 	usb_hub_adjust_deviceremovable(hdev, hub->descriptor);
 
 	hub_activate(hub, HUB_INIT);
-- 
2.25.0

Re: [RESEND][PATCH 2/2] usb: hub: use non-removable-ports DT property

[Greg Kroah-Hartman]

On Fri, Jan 24, 2020 at 03:25:04PM +0000, Mans Rullgard wrote:
> Mark any ports listed in the non-removable-ports DT property as
> hardwired. This is useful for boards with built-in USB devices
> that cannot be (or have not been) marked as fixed in hardware.
> 
> Signed-off-by: Mans Rullgard <mans@mansr.com>
> ---
>  drivers/usb/core/hub.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> index 4ac74b354801..97f8f15fb632 100644
> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c
> @@ -1366,6 +1366,9 @@ static int hub_configure(struct usb_hub *hub,
>  	unsigned unit_load;
>  	unsigned full_load;
>  	unsigned maxchild;
> +	struct property *prop;
> +	const __be32 *cur;
> +	u32 val;
>  
>  	hub->buffer = kmalloc(sizeof(*hub->buffer), GFP_KERNEL);
>  	if (!hub->buffer) {
> @@ -1667,6 +1670,19 @@ static int hub_configure(struct usb_hub *hub,
>  		}
>  	}
>  
> +	of_property_for_each_u32(hub_dev->of_node, "non-removable-ports",
> +				 prop, cur, val) {
> +		if (val < 1 || val > hdev->maxchild) {
> +			dev_warn(hub_dev,
> +				 "bad port number %u in non-removable-ports\n",
> +				 val);
> +			continue;
> +		}
> +
> +		hub->ports[val - 1]->connect_type =
> +			USB_PORT_CONNECT_TYPE_HARD_WIRED;
> +	}
> +

Is this doing the same thing that ACPI does?  It doesn't seem like it,
why not do this in usb_hub_adjust_deviceremovable()?  Or wherever ACPI
does this...

thanks,

greg k-h

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

[Rob Herring]

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@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.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.

At a minimum, I think this should be a per port property. Though really, 
I think this should just be implied by describing the device in DT. I'm 
not sure if there's a case for hotpluggable devices described in DT. 
Maybe with overlays.

Rob

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

[Måns Rullgård]

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 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@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.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.

> Though really, I think this should just be implied by describing the
> device in DT. I'm not sure if there's a case for hotpluggable devices
> described in DT.  Maybe with overlays.

That's also an option.  Greg, what do you think?

-- 
Måns Rullgård

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

[Greg Kroah-Hartman]

On Mon, Jan 27, 2020 at 04:56:15PM +0000, Måns Rullgård wrote:
> 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 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@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.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?

> > Though really, I think this should just be implied by describing the
> > device in DT. I'm not sure if there's a case for hotpluggable devices
> > described in DT.  Maybe with overlays.
> 
> That's also an option.  Greg, what do you think?

I have no idea, sorry, I'm totally lost here...

greg k-h

> 
> -- 
> Måns Rullgård

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

[Måns Rullgård]

Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:

> On Mon, Jan 27, 2020 at 04:56:15PM +0000, Måns Rullgård wrote:
>> 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 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@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.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@kroah.com/

>> > Though really, I think this should just be implied by describing the
>> > device in DT. I'm not sure if there's a case for hotpluggable devices
>> > described in DT.  Maybe with overlays.
>> 
>> That's also an option.  Greg, what do you think?
>
> I have no idea, sorry, I'm totally lost here...

Background: I need to differentiate between on-board and external USB
devices on a few boards.  Although hubs can indicate the removable
status of each port, the configuration options are often limited and may
not be capable of describing the actual wiring.  Also, if a device is
hard-wired directly to a host port, there is no way of indicating this.

While I could match the full device path using per-board lists, I'd
prefer a generic solution.  To this end, it is necessary to add the
ability for DT to supply this information.  Three variants have been
discussed:

1. Add a "non-removable" property to the USB device node similar to how
   it's done for MMC.

2. Add a "non-removable-ports" property to the hub node.  Apparently
   ACPI can supply the information in this manner.

3. Make any USB device with a DT node implicitly non-removable.

Either one will work for me.

-- 
Måns Rullgård

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

[Greg Kroah-Hartman]

On Tue, Jan 28, 2020 at 03:15:11PM +0000, Måns Rullgård wrote:
> Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
> 
> > On Mon, Jan 27, 2020 at 04:56:15PM +0000, Måns Rullgård wrote:
> >> 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 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@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.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@kroah.com/

Almost a full year ago!  Hah, I can't remember what I wrote last week.

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

[Greg Kroah-Hartman]

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@linuxfoundation.org> writes:
> > 
> > > On Mon, Jan 27, 2020 at 04:56:15PM +0000, Måns Rullgård wrote:
> > >> 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 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@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.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@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.

thanks,

greg k-h

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

[Rob Herring]

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åns Rullgård wrote:
> > > Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
> > >
> > > > On Mon, Jan 27, 2020 at 04:56:15PM +0000, Måns Rullgård wrote:
> > > >> 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 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@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.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@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

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

[Måns Rullgård]

Rob Herring <robh@kernel.org> writes:

> 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åns Rullgård wrote:
>> > > Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
>> > >
>> > > > On Mon, Jan 27, 2020 at 04:56:15PM +0000, Måns Rullgård wrote:
>> > > >> 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 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@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.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@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.

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

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

[Rob Herring]

On Tue, Jan 28, 2020 at 9:28 AM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Tue, Jan 28, 2020 at 03:15:11PM +0000, Måns Rullgård wrote:
> > Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
> >
> > > On Mon, Jan 27, 2020 at 04:56:15PM +0000, Måns Rullgård wrote:
> > >> 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 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@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.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@kroah.com/
>
> Almost a full year ago!  Hah, I can't remember what I wrote last week.

I was typing up the same thing... :)

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

[Greg Kroah-Hartman]

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@mansr.com>
> ---
>  Documentation/devicetree/bindings/usb/usb-device.txt | 4 ++++
>  1 file changed, 4 insertions(+)

Why is this a RESEND?  What changed from v1?

thanks,

greg k-h