On Tue, Oct 02, 2012 at 11:41:43, Thierry Reding wrote:
> On Wed, Sep 26, 2012 at 04:57:43PM +0530, Philip, Avinash wrote:
> > Add support for device-tree binding and of_xlate for EHRWPM driver.
> > of_xlate provides EHRPWM polarity configuration from client driver
> > device-tree.
> > Also size of pwm-cells set to 3 to support pwm channel number, pwm
> > period & polarity configuration from device tree.
>
> Oh, I forgot to mention this in my reply to the previous patch, but you
> should consistently use PWM to refer to PWM devices, so the above should
> be "PWM channel number" and "PWM period & polarity". And the property is
> named "#pwm-cells".
I will correct it.
>
> > Signed-off-by: Philip, Avinash <avinashphilip@xxxxxx>
> > ---
> > :000000 100644 0000000... 05d9d63... A Documentation/devicetree/bindings/pwm/pwm-tiehrpwm.txt
> > :100644 100644 caf00fe... ae23c2b... M drivers/pwm/pwm-tiehrpwm.c
> > .../devicetree/bindings/pwm/pwm-tiehrpwm.txt | 26 +++++
> > drivers/pwm/pwm-tiehrpwm.c | 107 ++++++++++++++++++++
> > 2 files changed, 133 insertions(+), 0 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/pwm/pwm-tiehrpwm.txt b/Documentation/devicetree/bindings/pwm/pwm-tiehrpwm.txt
> > new file mode 100644
> > index 0000000..05d9d63
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/pwm/pwm-tiehrpwm.txt
> > @@ -0,0 +1,26 @@
> > +TI SOC EHRPWM based PWM controller
> > +
> > +Required properties:
> > +- compatible : Must be "ti,am33xx-ehrpwm"
> > +- #pwm-cells: Should be 3. Number of cells being used to specify PWM property.
> > + First cell specifies the per-chip index of the PWM to use, the second
> > + cell is the period cycle in nanoseconds and the third cell is the
>
> "period cycle" doesn't make much sense. It should be "period" only. This
> also applies to the previous patch.
I will correct it.
>
> > + polarity of PWM output. Polarity 0 gives normal polarity and 1 gives
> > + inversed polarity (inverse duty cycle)
>
> I don't think "inversed" exists. It should be either "inverted polarity"
> or "inverse polarity".
>
> > +- reg: physical base address and size of the registers map. For am33xx,
> > + 2 register maps are present (EHRPWM register space & PWM subsystem common
> > + config space). Order should be maintained with EHRPWM register map as first
> > + entry & PWM subsystem common config space as second entry.
> > +
> > +Optional properties:
> > +- ti,hwmods: Name of the hwmod associated to the EHRPWM:
> > + "ehrpwm<x>", <x> being the 0-based instance number from the HW spec
>
> I don't see where this property is used. There is no code in this patch
> that parses it.
This data used by omap_hwmod layer to create platform devices. This is part
of omap hwmod implementation.
>
> > +static struct pwm_device *of_ehrpwm_xlate(struct pwm_chip *chip,
> > + const struct of_phandle_args *args)
> > +{
> > + struct pwm_device *pwm;
> > +
> > + if (chip->of_pwm_n_cells < PWM_CELL_SIZE)
> > + return ERR_PTR(-EINVAL);
> > +
> > + if (args->args[0] >= chip->npwm)
> > + return ERR_PTR(-EINVAL);
> > +
> > + pwm = pwm_request_from_chip(chip, args->args[0], NULL);
> > + if (IS_ERR(pwm))
> > + return pwm;
> > +
> > + pwm_set_period(pwm, args->args[1]);
> > + pwm_set_polarity(pwm, args->args[2]);
> > + return pwm;
> > +}
>
> This is an exact duplicate of the ECAP's of_xlate(). Maybe we should
> make this part of the PWM core. If so it is probably safer to define the
> values for the third cell as flags, where the polarity is encoded in bit
> 0, and make the function handle this accordingly to allow other bits to
> be added in the future.
Custom of_xlate support is provided as suggested while the discussion of
"Adding support for configuring polarity in PWM framework".
https://lkml.org/lkml/2012/7/16/177
without custom of_xlate() support, PWM drivers has to populate
chip->of_pwm_n_cells = x;
as this is hard coded to 2 in pwm/core.c.
if (!chip->of_xlate) {
chip->of_xlate = of_pwm_simple_xlate;
chip->of_pwm_n_cells = 2;
Thanks
Avinash
>
> The same comments as for patch 1 apply to the rest of this patch as
> well.
>
> Thierry
>
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
