linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RESEND][PATCH 1/2] dt-bindings: usb: add non-removable-ports hub property
@ 2020-01-24 15:25 Mans Rullgard
  2020-01-24 15:25 ` [RESEND][PATCH 2/2] usb: hub: use non-removable-ports DT property Mans Rullgard
                   ` (2 more replies)
  0 siblings, 3 replies; 13+ messages in thread
From: Mans Rullgard @ 2020-01-24 15:25 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Rob Herring, Mark Rutland
  Cc: linux-usb, devicetree, linux-kernel

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


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [RESEND][PATCH 2/2] usb: hub: use non-removable-ports DT property
  2020-01-24 15:25 [RESEND][PATCH 1/2] dt-bindings: usb: add non-removable-ports hub property Mans Rullgard
@ 2020-01-24 15:25 ` Mans Rullgard
  2020-01-28 16:51   ` Greg Kroah-Hartman
  2020-01-27 15:35 ` [RESEND][PATCH 1/2] dt-bindings: usb: add non-removable-ports hub property Rob Herring
  2020-01-28 16:53 ` Greg Kroah-Hartman
  2 siblings, 1 reply; 13+ messages in thread
From: Mans Rullgard @ 2020-01-24 15:25 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Rob Herring, Mark Rutland
  Cc: linux-usb, devicetree, linux-kernel

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


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [RESEND][PATCH 1/2] dt-bindings: usb: add non-removable-ports hub property
  2020-01-24 15:25 [RESEND][PATCH 1/2] dt-bindings: usb: add non-removable-ports hub property Mans Rullgard
  2020-01-24 15:25 ` [RESEND][PATCH 2/2] usb: hub: use non-removable-ports DT property Mans Rullgard
@ 2020-01-27 15:35 ` Rob Herring
  2020-01-27 16:56   ` Måns Rullgård
  2020-01-28 16:53 ` Greg Kroah-Hartman
  2 siblings, 1 reply; 13+ messages in thread
From: Rob Herring @ 2020-01-27 15:35 UTC (permalink / raw)
  To: Mans Rullgard
  Cc: Greg Kroah-Hartman, Mark Rutland, linux-usb, devicetree, linux-kernel

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RESEND][PATCH 1/2] dt-bindings: usb: add non-removable-ports hub property
  2020-01-27 15:35 ` [RESEND][PATCH 1/2] dt-bindings: usb: add non-removable-ports hub property Rob Herring
@ 2020-01-27 16:56   ` Måns Rullgård
  2020-01-28 13:47     ` Greg Kroah-Hartman
  0 siblings, 1 reply; 13+ messages in thread
From: Måns Rullgård @ 2020-01-27 16:56 UTC (permalink / raw)
  To: Rob Herring
  Cc: Greg Kroah-Hartman, Mark Rutland, linux-usb, devicetree, linux-kernel

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RESEND][PATCH 1/2] dt-bindings: usb: add non-removable-ports hub property
  2020-01-27 16:56   ` Måns Rullgård
@ 2020-01-28 13:47     ` Greg Kroah-Hartman
  2020-01-28 15:15       ` Måns Rullgård
  0 siblings, 1 reply; 13+ messages in thread
From: Greg Kroah-Hartman @ 2020-01-28 13:47 UTC (permalink / raw)
  To: Måns Rullgård
  Cc: Rob Herring, Mark Rutland, linux-usb, devicetree, linux-kernel

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RESEND][PATCH 1/2] dt-bindings: usb: add non-removable-ports hub property
  2020-01-28 13:47     ` Greg Kroah-Hartman
@ 2020-01-28 15:15       ` Måns Rullgård
  2020-01-28 15:28         ` Greg Kroah-Hartman
  0 siblings, 1 reply; 13+ messages in thread
From: Måns Rullgård @ 2020-01-28 15:15 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Rob Herring, Mark Rutland, linux-usb, devicetree, linux-kernel

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RESEND][PATCH 1/2] dt-bindings: usb: add non-removable-ports hub property
  2020-01-28 15:15       ` Måns Rullgård
@ 2020-01-28 15:28         ` Greg Kroah-Hartman
  2020-01-28 16:52           ` Greg Kroah-Hartman
  2020-01-28 17:27           ` Rob Herring
  0 siblings, 2 replies; 13+ messages in thread
From: Greg Kroah-Hartman @ 2020-01-28 15:28 UTC (permalink / raw)
  To: Måns Rullgård
  Cc: Rob Herring, Mark Rutland, linux-usb, devicetree, linux-kernel

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.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RESEND][PATCH 2/2] usb: hub: use non-removable-ports DT property
  2020-01-24 15:25 ` [RESEND][PATCH 2/2] usb: hub: use non-removable-ports DT property Mans Rullgard
@ 2020-01-28 16:51   ` Greg Kroah-Hartman
  0 siblings, 0 replies; 13+ messages in thread
From: Greg Kroah-Hartman @ 2020-01-28 16:51 UTC (permalink / raw)
  To: Mans Rullgard
  Cc: Rob Herring, Mark Rutland, linux-usb, devicetree, linux-kernel

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RESEND][PATCH 1/2] dt-bindings: usb: add non-removable-ports hub property
  2020-01-28 15:28         ` Greg Kroah-Hartman
@ 2020-01-28 16:52           ` Greg Kroah-Hartman
  2020-01-28 18:21             ` Rob Herring
  2020-01-28 17:27           ` Rob Herring
  1 sibling, 1 reply; 13+ messages in thread
From: Greg Kroah-Hartman @ 2020-01-28 16:52 UTC (permalink / raw)
  To: Måns Rullgård
  Cc: Rob Herring, Mark Rutland, linux-usb, devicetree, linux-kernel

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RESEND][PATCH 1/2] dt-bindings: usb: add non-removable-ports hub property
  2020-01-24 15:25 [RESEND][PATCH 1/2] dt-bindings: usb: add non-removable-ports hub property Mans Rullgard
  2020-01-24 15:25 ` [RESEND][PATCH 2/2] usb: hub: use non-removable-ports DT property Mans Rullgard
  2020-01-27 15:35 ` [RESEND][PATCH 1/2] dt-bindings: usb: add non-removable-ports hub property Rob Herring
@ 2020-01-28 16:53 ` Greg Kroah-Hartman
  2 siblings, 0 replies; 13+ messages in thread
From: Greg Kroah-Hartman @ 2020-01-28 16:53 UTC (permalink / raw)
  To: Mans Rullgard
  Cc: Rob Herring, Mark Rutland, linux-usb, devicetree, linux-kernel

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RESEND][PATCH 1/2] dt-bindings: usb: add non-removable-ports hub property
  2020-01-28 15:28         ` Greg Kroah-Hartman
  2020-01-28 16:52           ` Greg Kroah-Hartman
@ 2020-01-28 17:27           ` Rob Herring
  1 sibling, 0 replies; 13+ messages in thread
From: Rob Herring @ 2020-01-28 17:27 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Måns Rullgård, Mark Rutland, Linux USB List,
	devicetree, linux-kernel

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... :)

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RESEND][PATCH 1/2] dt-bindings: usb: add non-removable-ports hub property
  2020-01-28 16:52           ` Greg Kroah-Hartman
@ 2020-01-28 18:21             ` Rob Herring
  2020-01-30 17:06               ` Måns Rullgård
  0 siblings, 1 reply; 13+ messages in thread
From: Rob Herring @ 2020-01-28 18:21 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Måns Rullgård
  Cc: Mark Rutland, Linux USB List, devicetree, linux-kernel

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RESEND][PATCH 1/2] dt-bindings: usb: add non-removable-ports hub property
  2020-01-28 18:21             ` Rob Herring
@ 2020-01-30 17:06               ` Måns Rullgård
  0 siblings, 0 replies; 13+ messages in thread
From: Måns Rullgård @ 2020-01-30 17:06 UTC (permalink / raw)
  To: Rob Herring
  Cc: Greg Kroah-Hartman, Mark Rutland, Linux USB List, devicetree,
	linux-kernel

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2020-01-30 17:06 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-24 15:25 [RESEND][PATCH 1/2] dt-bindings: usb: add non-removable-ports hub property Mans Rullgard
2020-01-24 15:25 ` [RESEND][PATCH 2/2] usb: hub: use non-removable-ports DT property Mans Rullgard
2020-01-28 16:51   ` Greg Kroah-Hartman
2020-01-27 15:35 ` [RESEND][PATCH 1/2] dt-bindings: usb: add non-removable-ports hub property Rob Herring
2020-01-27 16:56   ` Måns Rullgård
2020-01-28 13:47     ` Greg Kroah-Hartman
2020-01-28 15:15       ` Måns Rullgård
2020-01-28 15:28         ` Greg Kroah-Hartman
2020-01-28 16:52           ` Greg Kroah-Hartman
2020-01-28 18:21             ` Rob Herring
2020-01-30 17:06               ` Måns Rullgård
2020-01-28 17:27           ` Rob Herring
2020-01-28 16:53 ` Greg Kroah-Hartman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).