| Message ID | 20140123115203.GV15937@n2100.arm.linux.org.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Jan 23, 2014 at 11:52:03AM +0000, Russell King - ARM Linux wrote: > On Thu, Jan 23, 2014 at 08:37:14AM +0100, Lothar Waßmann wrote: > > This wouldn't buy much without a material change to of_pwm_get(). > > The function of_parse_phandle_with_args() called by of_pwm_get() > > requires the number of args in the pwms property be greater or equal to > > the #pwm-cells property in the pwm node. Thus, the interesting case of > > having #pwm-cells = <3> without changing the existing users is > > prohibited by of_parse_phandle_with_args(). > > I really don't think that's a problem we need to be concerned with at > the moment. What we need is for the kernel to be able to parse files > with #pwm-cells = <2> with the pwms property containing two arguments, > and when they're updated to #pwm-cells = <3> with the pwms property > containing three arguments. > > Yes, that means all the board dt files need to be updated at the same > time to include the additional argument, but I don't see that as a big > problem. > > What we do need to do is to adjust the PWM parsing code such that it's > possible to use either specification without causing any side effects. > > I would test this, but as u-boot is rather fscked at the moment and the > networking has broken on my cubox-i as a result... and it seems that the > u-boot developers have pissed off cubox-i u-boot hackers soo much that > they've dropped u-boot in favour of barebox... Oh, and another reason... the u-boot video settings are totally and utterly buggered to the point that it doesn't produce correct timings, and it seems that u-boot people have zero interest in fixing that, so u-boot mainline is basically refusing to fix this - another reason to stay away from it. (1024x768 @ 60Hz produces 70Hz refresh on iMX6Q here - I've seen it produce 51Hz on iMX6S, both of which are far enough out that lots of display devices will not accept it as a valid signal.)
On Thu, Jan 23, 2014 at 11:52:03AM +0000, Russell King - ARM Linux wrote: > On Thu, Jan 23, 2014 at 08:37:14AM +0100, Lothar Waßmann wrote: > > This wouldn't buy much without a material change to of_pwm_get(). > > The function of_parse_phandle_with_args() called by of_pwm_get() > > requires the number of args in the pwms property be greater or equal to > > the #pwm-cells property in the pwm node. Thus, the interesting case of > > having #pwm-cells = <3> without changing the existing users is > > prohibited by of_parse_phandle_with_args(). > > I really don't think that's a problem we need to be concerned with at > the moment. What we need is for the kernel to be able to parse files > with #pwm-cells = <2> with the pwms property containing two arguments, > and when they're updated to #pwm-cells = <3> with the pwms property > containing three arguments. > > Yes, that means all the board dt files need to be updated at the same > time to include the additional argument, but I don't see that as a big > problem. > > What we do need to do is to adjust the PWM parsing code such that it's > possible to use either specification without causing any side effects. > > I would test this, but as u-boot is rather fscked at the moment and the > networking has broken on my cubox-i as a result... and it seems that the > u-boot developers have pissed off cubox-i u-boot hackers soo much that > they've dropped u-boot in favour of barebox... Okay, finally confirmed that this works with #pwm-cells = 2.
diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c index 2ca95042a0b9..40adbce8ef0c 100644 --- a/drivers/pwm/core.c +++ b/drivers/pwm/core.c @@ -132,14 +132,11 @@ static int pwm_device_request(struct pwm_device *pwm, const char *label) } struct pwm_device * -of_pwm_xlate_with_flags(struct pwm_chip *pc, const struct of_phandle_args *args) +of_pwm_xlate(struct pwm_chip *pc, const struct of_phandle_args *args) { struct pwm_device *pwm; - if (pc->of_pwm_n_cells < 3) - return ERR_PTR(-EINVAL); - - if (args->args[0] >= pc->npwm) + if (args->args_count < 2) return ERR_PTR(-EINVAL); pwm = pwm_request_from_chip(pc, args->args[0], NULL); @@ -148,33 +145,45 @@ of_pwm_xlate_with_flags(struct pwm_chip *pc, const struct of_phandle_args *args) pwm_set_period(pwm, args->args[1]); - if (args->args[2] & PWM_POLARITY_INVERTED) - pwm_set_polarity(pwm, PWM_POLARITY_INVERSED); - else - pwm_set_polarity(pwm, PWM_POLARITY_NORMAL); + if (args->args_count > 2) { + int err; + + if (args->args[2] & PWM_POLARITY_INVERTED) + err = pwm_set_polarity(pwm, PWM_POLARITY_INVERSED); + else + err = pwm_set_polarity(pwm, PWM_POLARITY_NORMAL); + + pwm_put(pwm); + return ERR_PTR(err); + } return pwm; } +EXPORT_SYMBOL(of_pwm_xlate); + +struct pwm_device * +of_pwm_xlate_with_flags(struct pwm_chip *pc, const struct of_phandle_args *args) +{ + if (pc->of_pwm_n_cells < 3) + return ERR_PTR(-EINVAL); + + if (args->args_count != pc->of_pwm_n_cells) + return ERR_PTR(-EINVAL); + + return of_pwm_xlate(pc, args); +} EXPORT_SYMBOL_GPL(of_pwm_xlate_with_flags); static struct pwm_device * of_pwm_simple_xlate(struct pwm_chip *pc, const struct of_phandle_args *args) { - struct pwm_device *pwm; - if (pc->of_pwm_n_cells < 2) return ERR_PTR(-EINVAL); - if (args->args[0] >= pc->npwm) + if (args->args_count != pc->of_pwm_n_cells) return ERR_PTR(-EINVAL); - pwm = pwm_request_from_chip(pc, args->args[0], NULL); - if (IS_ERR(pwm)) - return pwm; - - pwm_set_period(pwm, args->args[1]); - - return pwm; + return of_pwm_xlate(pc, args); } static void of_pwmchip_add(struct pwm_chip *chip) @@ -536,16 +545,12 @@ struct pwm_device *of_pwm_get(struct device_node *np, const char *con_id) goto put; } - if (args.args_count != pc->of_pwm_n_cells) { - pr_debug("%s: wrong #pwm-cells for %s\n", np->full_name, - args.np->full_name); - pwm = ERR_PTR(-EINVAL); - goto put; - } - pwm = pc->of_xlate(pc, &args); - if (IS_ERR(pwm)) + if (IS_ERR(pwm)) { + pr_debug("%s: of_xlate failed for %s: %d\n", np->full_name, + args.np->full_name, (int)PTR_ERR(pwm)); goto put; + } /* * If a consumer name was not given, try to look it up from the diff --git a/include/linux/pwm.h b/include/linux/pwm.h index f0feafd184a0..14a823f77c31 100644 --- a/include/linux/pwm.h +++ b/include/linux/pwm.h @@ -188,6 +188,8 @@ struct pwm_device *pwm_request_from_chip(struct pwm_chip *chip, unsigned int index, const char *label); +struct pwm_device *of_pwm_xlate(struct pwm_chip *pc, + const struct of_phandle_args *args); struct pwm_device *of_pwm_xlate_with_flags(struct pwm_chip *pc, const struct of_phandle_args *args);