> >> I actually think we need to define some best practices, ordered on > >> what the hardware can do. > >> > >> 1) With software control, set_brightness should do what you expect, > >> not return an error. > >> > >> 2) Without full software control, but there is a mechanism to report a > >> problem, like constant blinking, or off, do that, and return > >> -EOPNOTSUPP. > >> > >> 3) Really dumb hardware like this, set_brightness should be a NULL > >> pointer. The core returns -EOPNOTSUPP. > >> > >> The core should return this -EOPNOTSUPP to user space, but it should > >> accept the configuration change. So the user can put it into an > >> invalid state, in order to get to a valid state with further > >> configuration. > >> > > Sounds good to me. Let me come up with a RFC patch. > > > >> I don't see an easy way to let the user know what the valid states > >> are. We currently have a 10bit state. I don't think we can put all the > >> valid ones in a /sysfs file, especially when QCA8K pretty much > >> supports everything. > >> > >> Andrew > > > > Heiner > > Patch is so simple that I send it this way. What do you think? That is simpler than i expected. But i think we need to document our expectations. What do we expect an LED driver to do when it cannot support software blinking. So please could you add a comment somewhere. Maybe extend the /* *Configurable sysfs attributes: * section? Thanks Andrew