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

From: Jan Lübbe
Date: Mon Feb 27 2017 - 05:28:21 EST


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-5555 |