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

[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 <jlu@xxxxxxxxxxxxxx> 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-5555 |