Re: [PATCH v5] usb: misc: add USB251xB/xBi Hi-Speed Hub Controller Driver

2017-02-28 Thread Jan Lübbe 
On Mo, 2017-02-27 at 16:27 -0600, Rob Herring wrote:
> On Mon, Feb 27, 2017 at 3:48 AM, Jan Lübbe  wrote:
> > On Di, 2017-02-21 at 15:57 +0100, Richard Leitner wrote:
> >> >>> This is a lot of properties. Are you really finding a need for all of
> >> >>> them? Is this to handle h/w designers too cheap to put down the EEPROM?
> >> >>> Maybe better to just define an eeprom property in the format the h/w
> >> >>> expects.
> >> >>
> >> >>
> >> >> I need about 15 of these properties. I just exposed them all to dt 
> >> >> because I
> >> >> thought they could be useful for somebody.
> >> >>
> >> >> Yes, these are a subset of the settings which can be configured via an
> >> >> external EEPROM (By strapping some pins of the hub you can select if it
> >> >> loads its configuration from an EEPROM or receives it via SMBus).
> >> >>
> >> >> My first thought was also to define only a byte array in dt, but IMHO 
> >> >> these
> >> >> options are much more readable and convenient for everybody. I'd also be
> >> >> fine with removing the properties I don't need from dt. But that may 
> >> >> lead to
> >> >> future patches where somebody needs some of the options not already 
> >> >> exposed
> >> >> to dt.
> >> >
> >> > Okay. It's really a judgement call. If this is most of the settings,
> >> > then it's fine. If it was only a fraction of them, then maybe we'd
> >> > want to do just a byte array. Sounds like it is the former.
> >>
> >> In fact there are 6 more parameters available according to the
> >> datasheet. So how should I proceed here? Remove the one's I'm not using
> >> at the moment, leave them as they are or add the missing 6 too?
> >
> > Rob, several of these properties look more like configuration rather
> > than HW description ('skip-config', '*-id', 'manufacturer', 'product',
> > 'serial'). Is DT the right place for this? I would expect userspace to
> > provide the configuration.
> 
> Configuration can be okay. The test I use is more who needs to
> set/control these properties? Would an end-user want to control them?
> If so, then they should be sysfs controls. If they are configuration
> for a particular design, then DT is fine. Given these are all
> typically EEPROM properties, then they aren't really end-user controls
> and belong in DT.

Thanks for explaining this, your test sounds very reasonable. And if it
turns out that runtime access is useful as well, the sysfs properties
could be added as needed.

Regards,
Jan
-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5] usb: misc: add USB251xB/xBi Hi-Speed Hub Controller Driver

2017-02-27 Thread Rob Herring 
On Mon, Feb 27, 2017 at 3:48 AM, Jan Lübbe  wrote:
> On Di, 2017-02-21 at 15:57 +0100, Richard Leitner wrote:
>> >>> This is a lot of properties. Are you really finding a need for all of
>> >>> them? Is this to handle h/w designers too cheap to put down the EEPROM?
>> >>> Maybe better to just define an eeprom property in the format the h/w
>> >>> expects.
>> >>
>> >>
>> >> I need about 15 of these properties. I just exposed them all to dt 
>> >> because I
>> >> thought they could be useful for somebody.
>> >>
>> >> Yes, these are a subset of the settings which can be configured via an
>> >> external EEPROM (By strapping some pins of the hub you can select if it
>> >> loads its configuration from an EEPROM or receives it via SMBus).
>> >>
>> >> My first thought was also to define only a byte array in dt, but IMHO 
>> >> these
>> >> options are much more readable and convenient for everybody. I'd also be
>> >> fine with removing the properties I don't need from dt. But that may lead 
>> >> to
>> >> future patches where somebody needs some of the options not already 
>> >> exposed
>> >> to dt.
>> >
>> > Okay. It's really a judgement call. If this is most of the settings,
>> > then it's fine. If it was only a fraction of them, then maybe we'd
>> > want to do just a byte array. Sounds like it is the former.
>>
>> In fact there are 6 more parameters available according to the
>> datasheet. So how should I proceed here? Remove the one's I'm not using
>> at the moment, leave them as they are or add the missing 6 too?
>
> Rob, several of these properties look more like configuration rather
> than HW description ('skip-config', '*-id', 'manufacturer', 'product',
> 'serial'). Is DT the right place for this? I would expect userspace to
> provide the configuration.

Configuration can be okay. The test I use is more who needs to
set/control these properties? Would an end-user want to control them?
If so, then they should be sysfs controls. If they are configuration
for a particular design, then DT is fine. Given these are all
typically EEPROM properties, then they aren't really end-user controls
and belong in DT.

Rob
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5] usb: misc: add USB251xB/xBi Hi-Speed Hub Controller Driver

2017-02-27 Thread Richard Leitner 
On 02/27/2017 10:48 AM, Jan Lübbe wrote:
> On Di, 2017-02-21 at 15:57 +0100, Richard Leitner wrote:
> This is a lot of properties. Are you really finding a need for all of
> them? Is this to handle h/w designers too cheap to put down the EEPROM?
> Maybe better to just define an eeprom property in the format the h/w
> expects.


 I need about 15 of these properties. I just exposed them all to dt because 
 I
 thought they could be useful for somebody.

 Yes, these are a subset of the settings which can be configured via an
 external EEPROM (By strapping some pins of the hub you can select if it
 loads its configuration from an EEPROM or receives it via SMBus).

 My first thought was also to define only a byte array in dt, but IMHO these
 options are much more readable and convenient for everybody. I'd also be
 fine with removing the properties I don't need from dt. But that may lead 
 to
 future patches where somebody needs some of the options not already exposed
 to dt.
>>>
>>> Okay. It's really a judgement call. If this is most of the settings,
>>> then it's fine. If it was only a fraction of them, then maybe we'd
>>> want to do just a byte array. Sounds like it is the former.
>>
>> In fact there are 6 more parameters available according to the
>> datasheet. So how should I proceed here? Remove the one's I'm not using
>> at the moment, leave them as they are or add the missing 6 too?
> 
> Rob, several of these properties look more like configuration rather
> than HW description ('skip-config', '*-id', 'manufacturer', 'product',
> 'serial'). Is DT the right place for this? I would expect userspace to
> provide the configuration.

Jan, on the one hand you're right, some (most) of the properties are
configuration parameters for the hub chip. On the other hand setting
these parameters (and therefore configuring the hub) avoids an
additional EEPROM chip and IMHO that's some kind of describing an
"imaginary HW". Isn't it? :-)

If DT is not the right place for it: Which userspace tool/procedure
would be appropriate for it? In my case the hub needs to be available
(in a configured state) when the initramfs gets executed.

Thanks for your feedback & regards,
Richard L
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5] usb: misc: add USB251xB/xBi Hi-Speed Hub Controller Driver

2017-02-27 Thread Jan Lübbe 
On Di, 2017-02-21 at 15:57 +0100, Richard Leitner wrote:
> >>> This is a lot of properties. Are you really finding a need for all of
> >>> them? Is this to handle h/w designers too cheap to put down the EEPROM?
> >>> Maybe better to just define an eeprom property in the format the h/w
> >>> expects.
> >>
> >>
> >> I need about 15 of these properties. I just exposed them all to dt because 
> >> I
> >> thought they could be useful for somebody.
> >>
> >> Yes, these are a subset of the settings which can be configured via an
> >> external EEPROM (By strapping some pins of the hub you can select if it
> >> loads its configuration from an EEPROM or receives it via SMBus).
> >>
> >> My first thought was also to define only a byte array in dt, but IMHO these
> >> options are much more readable and convenient for everybody. I'd also be
> >> fine with removing the properties I don't need from dt. But that may lead 
> >> to
> >> future patches where somebody needs some of the options not already exposed
> >> to dt.
> > 
> > Okay. It's really a judgement call. If this is most of the settings,
> > then it's fine. If it was only a fraction of them, then maybe we'd
> > want to do just a byte array. Sounds like it is the former.
> 
> In fact there are 6 more parameters available according to the
> datasheet. So how should I proceed here? Remove the one's I'm not using
> at the moment, leave them as they are or add the missing 6 too?

Rob, several of these properties look more like configuration rather
than HW description ('skip-config', '*-id', 'manufacturer', 'product',
'serial'). Is DT the right place for this? I would expect userspace to
provide the configuration.

Regards,
Jan
-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5] usb: misc: add USB251xB/xBi Hi-Speed Hub Controller Driver

2017-02-21 Thread Richard Leitner 
On 02/21/2017 03:37 PM, Rob Herring wrote:
> On Thu, Feb 16, 2017 at 12:36 AM, Richard Leitner  wrote:
>> On 02/16/2017 03:30 AM, Rob Herring wrote:
>>>
>>> On Fri, Feb 10, 2017 at 09:19:27AM +0100, Richard Leitner wrote:

 This patch adds a driver for configuration of the Microchip USB251xB/xBi
 USB 2.0 hub controller series with USB 2.0 upstream connectivity, SMBus
 configuration interface and two to four USB 2.0 downstream ports.

 Furthermore add myself as a maintainer for this driver.

 The datasheet can be found at the manufacturers website, see [1]. All
 device-tree exposed configuration features have been tested on a i.MX6
 platform with a USB2512B hub.

 [1] http://ww1.microchip.com/downloads/en/DeviceDoc/1692C.pdf

 Signed-off-by: Richard Leitner 
 ---
 CHANGES v5:
>>>
>>>
>>> V5 and the first I see it?
>>
>>
>> Just double-checked it, you (robh...@kernel.org) were on CC since v1.
> 
> Indeed. You just sent new versions faster than I get to them. New
> versions puts you at the back of my queue. Sending out 5 versions in a
> week is a lot.

Ooh. I'm sorry. It was just that I got feedback quite fast and had the
time to implement the changes.

BTW: I already sent a patch fixing most of your issues. Please just
ignore that, as not everything is fixed in there already. I'll send an
improved version end of this week.

> 
> [...]
> 
 +
 +Optional properties :
 + - reg : I2C address on the selected bus (default is <0x2C>)
>>>
>>>
>>> Why is this optional?
>>
>>
>> Due to the fact the address is hardcoded insinde the chip I thought we could
>> set it to 0x2C by default if reg is not given.
> 
> That's true for pretty much every I2C chip.
> 
>> Should it be required?
> 
> Yes. The only time it would not be present is the I2C bus is not
> physically connected. In that case though, this should be a child of
> the USB host node.

Ok.

[...]

>>>
 + - compound-device : indicated the hub is part of a compound device
 + - port-mapping-mode : enable port mapping mode
 + - string-support : enable string descriptor support (required for
 manufacturer,
 +   product and serial string configuration)
 + - non-removable-ports : Should specify the ports which have a
 non-removable
 +   device connected.
 + - sp-disabled-ports : Specifies the ports which will be self-power
 disabled
 + - bp-disabled-ports : Specifies the ports which will be bus-power
 disabled
 + - max-sp-power : Specifies the maximum current the hub consumes from an
 +   upstream port when operating as self-powered hub including the
 power
 +   consumption of a permanently attached peripheral if the hub is
 +   configured as a compound device. The value is given in mA in a 0
 - 500
 +   range (default is 2).
 + - max-bp-power : Specifies the maximum current the hub consumes from an
 +   upstream port when operating as bus-powered hub including the
 power
 +   consumption of a permanently attached peripheral if the hub is
 +   configured as a compound device. The value is given in mA in a 0
 - 500
 +   range (default is 100).
 + - max-sp-current : Specifies the maximum current the hub consumes from
 an
 +   upstream port when operating as self-powered hub EXCLUDING the
 power
 +   consumption of a permanently attached peripheral if the hub is
 +   configured as a compound device. The value is given in mA in a 0
 - 500
 +   range (default is 2).
 + - max-bp-current : Specifies the maximum current the hub consumes from
 an
 +   upstream port when operating as bus-powered hub EXCLUDING the
 power
 +   consumption of a permanently attached peripheral if the hub is
 +   configured as a compound device. The value is given in mA in a 0
 - 500
 +   range (default is 100).
 + - power-on-time : Specifies the time it takes from the time the host
 initiates
 +   the power-on sequence to a port until the port has adequate
 power. The
 +   value is given in ms in a 0 - 510 range (default is 100ms).
>>>
>>>
>>> Various properties need unit suffixes (see property-units.txt) and
>>> either be common properties or need vendor prefixes.
>>
>>
>> Ok. What exactly do you mean with common properties? I don't think it's the
>> endianness settings described in common-properties.txt, is it?
> 
> No, not common-properties.txt (maybe that needs another name). Simply
> documented in a common location that multiple device bindings can
> share. So things that would be common to USB devices or USB hubs in
> particular.
> 
> Looking through the list again, probably just the ones corresponding
> to USB descriptors should be common.

Ok. But aren't for example "power-on-time" or "disabled-ports" also

Re: [PATCH v5] usb: misc: add USB251xB/xBi Hi-Speed Hub Controller Driver

2017-02-21 Thread Rob Herring 
On Thu, Feb 16, 2017 at 12:36 AM, Richard Leitner  wrote:
> On 02/16/2017 03:30 AM, Rob Herring wrote:
>>
>> On Fri, Feb 10, 2017 at 09:19:27AM +0100, Richard Leitner wrote:
>>>
>>> This patch adds a driver for configuration of the Microchip USB251xB/xBi
>>> USB 2.0 hub controller series with USB 2.0 upstream connectivity, SMBus
>>> configuration interface and two to four USB 2.0 downstream ports.
>>>
>>> Furthermore add myself as a maintainer for this driver.
>>>
>>> The datasheet can be found at the manufacturers website, see [1]. All
>>> device-tree exposed configuration features have been tested on a i.MX6
>>> platform with a USB2512B hub.
>>>
>>> [1] http://ww1.microchip.com/downloads/en/DeviceDoc/1692C.pdf
>>>
>>> Signed-off-by: Richard Leitner 
>>> ---
>>> CHANGES v5:
>>
>>
>> V5 and the first I see it?
>
>
> Just double-checked it, you (robh...@kernel.org) were on CC since v1.

Indeed. You just sent new versions faster than I get to them. New
versions puts you at the back of my queue. Sending out 5 versions in a
week is a lot.

[...]

>>> +
>>> +Optional properties :
>>> + - reg : I2C address on the selected bus (default is <0x2C>)
>>
>>
>> Why is this optional?
>
>
> Due to the fact the address is hardcoded insinde the chip I thought we could
> set it to 0x2C by default if reg is not given.

That's true for pretty much every I2C chip.

> Should it be required?

Yes. The only time it would not be present is the I2C bus is not
physically connected. In that case though, this should be a child of
the USB host node.

>>> + - skip-config : Skip Hub configuration, but only send the USB-Attach
>>> command
>>> + - vendor-id : USB Vendor ID of the hub (16 bit, default is 0x0424)
>>> + - product-id : USB Product ID of the hub (16 bit, default depends on
>>> type)
>>
>>
>> These are to set the ids or need to match what they are set too?
>
>
> These are to set the VID/PID of the Hub.
>
>>
>>> + - device-id : USB Device ID of the hub (16 bit, default is 0x0bb3)
>>> + - language-id : USB Language ID (16 bit, default is 0x)
>>> + - manufacturer : USB Manufacturer string (max 31 characters long)
>>> + - product : USB Product string (max 31 characters long)
>>> + - serial : USB Serial string (max 31 characters long)
>>> + - {bus,self}-powered : selects between self- and bus-powered operation
>>> (default
>>> +   is self-powered)
>>> + - disable-hi-speed : disable USB Hi-Speed support
>>> + - {multi,single}-tt : selects between multi- and
>>> single-transaction-translator
>>> +   (default is multi-tt)
>>> + - disable-eop : disable End of Packet generation in full-speed mode
>>> + - {ganged,individual}-sensing : select over-current sense type in
>>> self-powered
>>> +   mode (default is individual)
>>> + - {ganged,individual}-port-switching : select port power switching mode
>>> +   (default is individual)
>>> + - dynamic-power-switching : enable auto-switching from self- to
>>> bus-powered
>>> +   operation if the local power source is removed or unavailable
>>> + - oc-delay-{100us,4ms,8ms,16ms} : set over current timer delay (default
>>> is 8ms)
>>
>>
>> Make the property value be the time (e.g. oc-delay-us).
>
>
> OK.
>
>
>>
>>> + - compound-device : indicated the hub is part of a compound device
>>> + - port-mapping-mode : enable port mapping mode
>>> + - string-support : enable string descriptor support (required for
>>> manufacturer,
>>> +   product and serial string configuration)
>>> + - non-removable-ports : Should specify the ports which have a
>>> non-removable
>>> +   device connected.
>>> + - sp-disabled-ports : Specifies the ports which will be self-power
>>> disabled
>>> + - bp-disabled-ports : Specifies the ports which will be bus-power
>>> disabled
>>> + - max-sp-power : Specifies the maximum current the hub consumes from an
>>> +   upstream port when operating as self-powered hub including the
>>> power
>>> +   consumption of a permanently attached peripheral if the hub is
>>> +   configured as a compound device. The value is given in mA in a 0
>>> - 500
>>> +   range (default is 2).
>>> + - max-bp-power : Specifies the maximum current the hub consumes from an
>>> +   upstream port when operating as bus-powered hub including the
>>> power
>>> +   consumption of a permanently attached peripheral if the hub is
>>> +   configured as a compound device. The value is given in mA in a 0
>>> - 500
>>> +   range (default is 100).
>>> + - max-sp-current : Specifies the maximum current the hub consumes from
>>> an
>>> +   upstream port when operating as self-powered hub EXCLUDING the
>>> power
>>> +   consumption of a permanently attached peripheral if the hub is
>>> +   configured as a compound device. The value is given in mA in a 0
>>> - 500
>>> +   range (default is 2).
>>> + - max-bp-current : Specifies the maximum current the hub consumes from
>>> an
>>> +   upstream port

Re: [PATCH v5] usb: misc: add USB251xB/xBi Hi-Speed Hub Controller Driver

2017-02-16 Thread Greg KH 
On Thu, Feb 16, 2017 at 07:36:48AM +0100, Richard Leitner wrote:
> On 02/16/2017 03:30 AM, Rob Herring wrote:
> > On Fri, Feb 10, 2017 at 09:19:27AM +0100, Richard Leitner wrote:
> > > This patch adds a driver for configuration of the Microchip USB251xB/xBi
> > > USB 2.0 hub controller series with USB 2.0 upstream connectivity, SMBus
> > > configuration interface and two to four USB 2.0 downstream ports.
> > > 
> > > Furthermore add myself as a maintainer for this driver.
> > > 
> > > The datasheet can be found at the manufacturers website, see [1]. All
> > > device-tree exposed configuration features have been tested on a i.MX6
> > > platform with a USB2512B hub.
> > > 
> > > [1] http://ww1.microchip.com/downloads/en/DeviceDoc/1692C.pdf
> > > 
> > > Signed-off-by: Richard Leitner 
> > > ---
> > > CHANGES v5:
> > 
> > V5 and the first I see it?
> 
> Just double-checked it, you (robh...@kernel.org) were on CC since v1.
> 
> @greg-kh: I got the notification that v5 was already applied to your usb
> tree. So how should I proceed here? Send a v6 or send a separate patch
> fixing the dt issues mentioned by robh?

A separate patch fixing the issues is fine, I don't think it's worth
reverting your original patch and then adding an updated one to the
tree.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5] usb: misc: add USB251xB/xBi Hi-Speed Hub Controller Driver

2017-02-15 Thread Richard Leitner 

On 02/16/2017 03:30 AM, Rob Herring wrote:

On Fri, Feb 10, 2017 at 09:19:27AM +0100, Richard Leitner wrote:

This patch adds a driver for configuration of the Microchip USB251xB/xBi
USB 2.0 hub controller series with USB 2.0 upstream connectivity, SMBus
configuration interface and two to four USB 2.0 downstream ports.

Furthermore add myself as a maintainer for this driver.

The datasheet can be found at the manufacturers website, see [1]. All
device-tree exposed configuration features have been tested on a i.MX6
platform with a USB2512B hub.

[1] http://ww1.microchip.com/downloads/en/DeviceDoc/1692C.pdf

Signed-off-by: Richard Leitner 
---
CHANGES v5:


V5 and the first I see it?


Just double-checked it, you (robh...@kernel.org) were on CC since v1.

@greg-kh: I got the notification that v5 was already applied to your usb 
tree. So how should I proceed here? Send a v6 or send a separate patch 
fixing the dt issues mentioned by robh?





- Put includes in alphabetical order
- Fix some indentations
- Replace {set,clr}_bit_in_byte with BIT() macros
- Fix multiline comments
- Use of_property_read_u32() instead of of_get_property() where possible
- Use min_t() instead of by-hand implemented if's
- Use strlcpy and ternary operator instead of strncpy in if/else
- Remove useless & improve some other outputs
- Omit i2c remove function
- Use module_i2c_driver() instead of module_{init,exit}()
CHANGES v4:
- use utf8s_to_utf16s() instead of ascii2utf16le()
- remove ascii2utf16le() in lib/string.c again
- remove changes in drivers/usb/core/hcd.c again
  (I will post a separate patch for using utf8s_to_utf16s()
  in there too)

CHANGES v3:
- move ascii2utf16le() to lib/string.c and also use it also for
ascii2desc in drivers/usb/core/hcd.c
- remove platform data support from usb251xb driver

CHANGES v2:
- fix max-{b,s}p-current property name
- add descriptor string handling from platform_data
- fix non-dt handling
---
 Documentation/devicetree/bindings/usb/usb251xb.txt |  83 +++
 MAINTAINERS|   8 +
 drivers/usb/misc/Kconfig   |   9 +
 drivers/usb/misc/Makefile  |   1 +
 drivers/usb/misc/usb251xb.c| 605 +
 5 files changed, 706 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/usb/usb251xb.txt
 create mode 100644 drivers/usb/misc/usb251xb.c

diff --git a/Documentation/devicetree/bindings/usb/usb251xb.txt 
b/Documentation/devicetree/bindings/usb/usb251xb.txt
new file mode 100644
index 000..0c065f7
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/usb251xb.txt
@@ -0,0 +1,83 @@
+Microchip USB 2.0 Hi-Speed Hub Controller
+
+The device node for the configuration of a Microchip USB251xB/xBi USB 2.0
+Hi-Speed Controller.
+
+Required properties :
+ - compatible : Should be "microchip,usb251xb" or one of the specific types:
+   "microchip,usb2512b", "microchip,usb2512bi", "microchip,usb2513b",
+   "microchip,usb2513bi", "microchip,usb2514b", "microchip,usb2514bi"
+ - hub-reset-gpios : Should specify the gpio for hub reset


Just "reset-gpios". And you need to define the active state.


OK.




+
+Optional properties :
+ - reg : I2C address on the selected bus (default is <0x2C>)


Why is this optional?


Due to the fact the address is hardcoded insinde the chip I thought we 
could set it to 0x2C by default if reg is not given.


Should it be required?




+ - skip-config : Skip Hub configuration, but only send the USB-Attach command
+ - vendor-id : USB Vendor ID of the hub (16 bit, default is 0x0424)
+ - product-id : USB Product ID of the hub (16 bit, default depends on type)


These are to set the ids or need to match what they are set too?


These are to set the VID/PID of the Hub.




+ - device-id : USB Device ID of the hub (16 bit, default is 0x0bb3)
+ - language-id : USB Language ID (16 bit, default is 0x)
+ - manufacturer : USB Manufacturer string (max 31 characters long)
+ - product : USB Product string (max 31 characters long)
+ - serial : USB Serial string (max 31 characters long)
+ - {bus,self}-powered : selects between self- and bus-powered operation 
(default
+   is self-powered)
+ - disable-hi-speed : disable USB Hi-Speed support
+ - {multi,single}-tt : selects between multi- and single-transaction-translator
+   (default is multi-tt)
+ - disable-eop : disable End of Packet generation in full-speed mode
+ - {ganged,individual}-sensing : select over-current sense type in self-powered
+   mode (default is individual)
+ - {ganged,individual}-port-switching : select port power switching mode
+   (default is individual)
+ - dynamic-power-switching : enable auto-switching from self- to bus-powered
+   operation if the

Re: [PATCH v5] usb: misc: add USB251xB/xBi Hi-Speed Hub Controller Driver

2017-02-15 Thread Rob Herring 
On Fri, Feb 10, 2017 at 09:19:27AM +0100, Richard Leitner wrote:
> This patch adds a driver for configuration of the Microchip USB251xB/xBi
> USB 2.0 hub controller series with USB 2.0 upstream connectivity, SMBus
> configuration interface and two to four USB 2.0 downstream ports.
> 
> Furthermore add myself as a maintainer for this driver.
> 
> The datasheet can be found at the manufacturers website, see [1]. All
> device-tree exposed configuration features have been tested on a i.MX6
> platform with a USB2512B hub.
> 
> [1] http://ww1.microchip.com/downloads/en/DeviceDoc/1692C.pdf
> 
> Signed-off-by: Richard Leitner 
> ---
> CHANGES v5:

V5 and the first I see it?

>   - Put includes in alphabetical order
>   - Fix some indentations
>   - Replace {set,clr}_bit_in_byte with BIT() macros
>   - Fix multiline comments
>   - Use of_property_read_u32() instead of of_get_property() where possible
>   - Use min_t() instead of by-hand implemented if's
>   - Use strlcpy and ternary operator instead of strncpy in if/else
>   - Remove useless & improve some other outputs
>   - Omit i2c remove function
>   - Use module_i2c_driver() instead of module_{init,exit}()
> CHANGES v4:
>   - use utf8s_to_utf16s() instead of ascii2utf16le()
>   - remove ascii2utf16le() in lib/string.c again
>   - remove changes in drivers/usb/core/hcd.c again
> (I will post a separate patch for using utf8s_to_utf16s()
> in there too)
> 
> CHANGES v3:
>   - move ascii2utf16le() to lib/string.c and also use it also for
>   ascii2desc in drivers/usb/core/hcd.c
>   - remove platform data support from usb251xb driver
> 
> CHANGES v2:
>   - fix max-{b,s}p-current property name
>   - add descriptor string handling from platform_data
>   - fix non-dt handling
> ---
>  Documentation/devicetree/bindings/usb/usb251xb.txt |  83 +++
>  MAINTAINERS|   8 +
>  drivers/usb/misc/Kconfig   |   9 +
>  drivers/usb/misc/Makefile  |   1 +
>  drivers/usb/misc/usb251xb.c| 605 
> +
>  5 files changed, 706 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/usb/usb251xb.txt
>  create mode 100644 drivers/usb/misc/usb251xb.c
> 
> diff --git a/Documentation/devicetree/bindings/usb/usb251xb.txt 
> b/Documentation/devicetree/bindings/usb/usb251xb.txt
> new file mode 100644
> index 000..0c065f7
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/usb251xb.txt
> @@ -0,0 +1,83 @@
> +Microchip USB 2.0 Hi-Speed Hub Controller
> +
> +The device node for the configuration of a Microchip USB251xB/xBi USB 2.0
> +Hi-Speed Controller.
> +
> +Required properties :
> + - compatible : Should be "microchip,usb251xb" or one of the specific types:
> + "microchip,usb2512b", "microchip,usb2512bi", "microchip,usb2513b",
> + "microchip,usb2513bi", "microchip,usb2514b", "microchip,usb2514bi"
> + - hub-reset-gpios : Should specify the gpio for hub reset

Just "reset-gpios". And you need to define the active state.

> +
> +Optional properties :
> + - reg : I2C address on the selected bus (default is <0x2C>)

Why is this optional? 

> + - skip-config : Skip Hub configuration, but only send the USB-Attach command
> + - vendor-id : USB Vendor ID of the hub (16 bit, default is 0x0424)
> + - product-id : USB Product ID of the hub (16 bit, default depends on type)

These are to set the ids or need to match what they are set too?

> + - device-id : USB Device ID of the hub (16 bit, default is 0x0bb3)
> + - language-id : USB Language ID (16 bit, default is 0x)
> + - manufacturer : USB Manufacturer string (max 31 characters long)
> + - product : USB Product string (max 31 characters long)
> + - serial : USB Serial string (max 31 characters long)
> + - {bus,self}-powered : selects between self- and bus-powered operation 
> (default
> + is self-powered)
> + - disable-hi-speed : disable USB Hi-Speed support
> + - {multi,single}-tt : selects between multi- and 
> single-transaction-translator
> + (default is multi-tt)
> + - disable-eop : disable End of Packet generation in full-speed mode
> + - {ganged,individual}-sensing : select over-current sense type in 
> self-powered
> + mode (default is individual)
> + - {ganged,individual}-port-switching : select port power switching mode
> + (default is individual)
> + - dynamic-power-switching : enable auto-switching from self- to bus-powered
> + operation if the local power source is removed or unavailable
> + - oc-delay-{100us,4ms,8ms,16ms} : set over current timer delay (default is 
> 8ms)

Make the property value be the time (e.g. oc-delay-us).

> + - compound-device : indicated the hub is part of a compound device
> + - port-mapping-mode : enable port mapping mode
> + - string-support : enable string descriptor support (required for 
>