Hi, After some more thought, a small change in plans. On Tue, May 31, 2016 at 06:10:59PM -0700, Brian Norris wrote: > On Sat, May 28, 2016 at 10:00:45PM -0700, Gwendal Grignou wrote: > > Instead of using device tree, assuming you have firmware control, > > another way could be to add a firmware feature: > > I do have firmware control, but I don't think that will be too necessary > actually. > > > for instance, there is one EC_FEATURE_PWM_FAN, the fan PWM, one for > > the keyboard lightning as well. (see num ec_feature_code) > > By adding one more, you let cros_ec_dev load the platform driver for > > you, it works even if the machine does not use device tree. > > I think we can actually get this without doing the EC_FEATURE_* thing > (which notably is not in upstream, BTW), nor by requiring a separate > node with the "google,cros-ec-pwm" property, but instead by running a > sample EC_CMD_PWM_GET_DUTY command on indeces [0, 255], stopping at the > first INVAL_PARAM failure (if we stop at 0, then we have no PWM API at > all). > > But that still leaves the problem of mapping PWMs to consumer devices. > The phandle translation is very helpful for our DT-based systems, but > there isn't a really nice equivalent for non-DT ones. I see struct > pwm_lookup, which looks like it could do some of what we want, but we'd > still either need to encode a ton of board-specific information in the > kernel, or else start exposing PWMs via the non-EC_PWM_TYPE_GENERIC > methods (see the new enum ec_pwm_type, where we can see > EC_PWM_TYPE_KB_LIGHT and EC_PWM_TYPE_DISPLAY_LIGHT). I think the way that DT systems and non-DT systems work means that we really want to address this in different ways. DT has nice phandles, which naturally work well with index-based addressing, while non-DT has the much inferior pwm_lookup stuff, which we'd have to encode directly in the drivers, and which would probably work out better with the TYPE-based addressing. So if/when we want to work on the non-DT case, we'll just need to extend this driver to support either method. But as we're interested in DT right now, I plan to only implement the DT method. > Anyway, along this line, perhaps it makes sense to: > > (a) drop the "google,cros-ec-pwm" property (via the probe method I > described above) So I will be doing (a) still... > (b) drop the separate node for "google,cros-ec-pwm", since the presence > of this feature can be detected by the same methods as in (a) > > leaving the only DT binding change to be to: > > (c) add an optional #pwm-cells property to the cros-ec node > (Documentation/devicetree/bindings/mfd/cros-ec.txt) so that we can > still utilize the nice PWM of_xlate stuff (and its corresponding pwms = > <...> property for consumer devices) ...but I'm not doing (b) or (c). Will send v2 shortly. Regards, Brian -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
