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:

quoted

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.

quoted

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