On Thu, Nov 12, 2015 at 01:52:36PM +0100, Thierry Reding wrote:
> On Fri, Nov 13, 2015 at 12:28:04AM +0800, Qipeng Zha wrote:
> > For Broxton PWM controller, base unit is defined as 8bit integer
> > and 14bit fraction, so need to update base unit setting to output
> > wave with right frequency.
> > a) add scaler for each board setting;
> > b) remove validity check of base unit for special board, let pwm
> > user to handle this;
> >
> > Signed-off-by: Qipeng Zha <[email protected]>
> > ---
> > drivers/pwm/pwm-lpss.c | 20 +++++++++++---------
> > drivers/pwm/pwm-lpss.h | 1 +
> > 2 files changed, 12 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/pwm/pwm-lpss.c b/drivers/pwm/pwm-lpss.c
> > index 2504410..5a907db 100644
> > --- a/drivers/pwm/pwm-lpss.c
> > +++ b/drivers/pwm/pwm-lpss.c
> > @@ -14,6 +14,7 @@
> > */
> >
> > #include <linux/io.h>
> > +#include <linux/time.h>
> > #include <linux/kernel.h>
> > #include <linux/module.h>
> > #include <linux/pm_runtime.h>
> > @@ -24,11 +25,9 @@
> > #define PWM_ENABLE BIT(31)
> > #define PWM_SW_UPDATE BIT(30)
> > #define PWM_BASE_UNIT_SHIFT 8
> > -#define PWM_BASE_UNIT_MASK 0x00ffff00
> > +#define PWM_BASE_UNIT_MASK 0x3fffff00
>
> Isn't this going to potentially write reserved bits on non-Broxton
> platforms? Previously the upper 8 bits were masked out, but now only the
> upper 2 bits are masked out. What about the other 6?
>
> Perhaps it'd be better to parameterize the mask in a way similar to the
> scaler value?
Or call the field "base_unit_bits" which holds 16 for BSW/BYT and 22 for
BXT, and calculate both scaler and mask from that in pwm_lpss_config()?
Actually I think we should make the driver private structure look like
this:
struct pwm_lpss_chip {
struct pwm_chip chip;
void __iomem *regs;
const struct pwm_lpss_boardinfo *info;
};
and then set ->info to point to the platform data in probe().
pwm_lpss_config() can then refer lpwm->info->base_unit_bits and
lpwm->info->clk_rate.
> Also it's unclear to me how critical this is. Initial patches for
> Broxton support were merged into Linus' tree yesterday. Presumably they
> had received some testing before, but nobody can't have noticed or this
> bug wouldn't exist in the patches that were merged. Does this break any
> existing setups and hence should go into v4.4 along with the initial
> Broxton support?
Nobody outside Intel should have Broxtons yet so it should not break any
existing users.
--
To unsubscribe from this list: send the line "unsubscribe linux-pwm" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
