| Message ID | 20240411-pm8xxx-vibrator-new-design-v9-4-7bf56cb92b28@quicinc.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add support for vibrator in multiple PMICs | expand |
On 2024/4/11 19:02, Dmitry Baryshkov wrote: > On Thu, 11 Apr 2024 at 11:32, Fenglin Wu via B4 Relay > <devnull+quic_fenglinw.quicinc.com@kernel.org> wrote: >> >> From: Fenglin Wu <quic_fenglinw@quicinc.com> >> >> Add support for a new SPMI vibrator module which is very similar >> to the vibrator module inside PM8916 but has a finer drive voltage >> step and different output voltage range, its drive level control >> is expanded across 2 registers. The vibrator module can be found >> in following Qualcomm PMICs: PMI632, PM7250B, PM7325B, PM7550BA. >> >> Signed-off-by: Fenglin Wu <quic_fenglinw@quicinc.com> >> --- >> drivers/input/misc/pm8xxx-vibrator.c | 51 +++++++++++++++++++++++++++++------- >> 1 file changed, 42 insertions(+), 9 deletions(-) >> >> diff --git a/drivers/input/misc/pm8xxx-vibrator.c b/drivers/input/misc/pm8xxx-vibrator.c >> index 2959edca8eb9..35bb6f450fd2 100644 >> --- a/drivers/input/misc/pm8xxx-vibrator.c >> +++ b/drivers/input/misc/pm8xxx-vibrator.c >> @@ -12,10 +12,10 @@ >> #include <linux/regmap.h> >> #include <linux/slab.h> >> >> -#define VIB_MAX_LEVEL_mV (3100) >> -#define VIB_MIN_LEVEL_mV (1200) >> -#define VIB_PER_STEP_mV (100) >> -#define VIB_MAX_LEVELS (VIB_MAX_LEVEL_mV - VIB_MIN_LEVEL_mV + VIB_PER_STEP_mV) >> +#define VIB_MAX_LEVEL_mV(vib) (vib->drv2_addr ? 3544 : 3100) >> +#define VIB_MIN_LEVEL_mV(vib) (vib->drv2_addr ? 1504 : 1200) >> +#define VIB_PER_STEP_mV(vib) (vib->drv2_addr ? 8 : 100) >> +#define VIB_MAX_LEVELS(vib) (VIB_MAX_LEVEL_mV(vib) - VIB_MIN_LEVEL_mV(vib) + VIB_PER_STEP_mV(vib)) >> >> #define MAX_FF_SPEED 0xff >> >> @@ -26,6 +26,9 @@ struct pm8xxx_regs { >> unsigned int drv_offset; >> unsigned int drv_mask; >> unsigned int drv_shift; >> + unsigned int drv2_offset; >> + unsigned int drv2_mask; >> + unsigned int drv2_shift; >> unsigned int drv_en_manual_mask; >> }; >> >> @@ -45,6 +48,18 @@ static struct pm8xxx_regs pm8916_regs = { >> .drv_en_manual_mask = 0, >> }; >> >> +static struct pm8xxx_regs pmi632_regs = { >> + .enable_offset = 0x46, >> + .enable_mask = BIT(7), >> + .drv_offset = 0x40, >> + .drv_mask = GENMASK(7, 0), >> + .drv_shift = 0, >> + .drv2_offset = 0x41, >> + .drv2_mask = GENMASK(3, 0), >> + .drv2_shift = 8, >> + .drv_en_manual_mask = 0, >> +}; >> + >> /** >> * struct pm8xxx_vib - structure to hold vibrator data >> * @vib_input_dev: input device supporting force feedback >> @@ -53,6 +68,7 @@ static struct pm8xxx_regs pm8916_regs = { >> * @regs: registers' info >> * @enable_addr: vibrator enable register >> * @drv_addr: vibrator drive strength register >> + * @drv2_addr: vibrator drive strength upper byte register >> * @speed: speed of vibration set from userland >> * @active: state of vibrator >> * @level: level of vibration to set in the chip >> @@ -65,6 +81,7 @@ struct pm8xxx_vib { >> const struct pm8xxx_regs *regs; >> unsigned int enable_addr; >> unsigned int drv_addr; >> + unsigned int drv2_addr; >> int speed; >> int level; >> bool active; >> @@ -82,6 +99,10 @@ static int pm8xxx_vib_set(struct pm8xxx_vib *vib, bool on) >> unsigned int val = vib->reg_vib_drv; >> const struct pm8xxx_regs *regs = vib->regs; >> >> + /* vibrator without drv2_addr needs be programmed in step increments */ > > How are these two items related? Are you using vib->drv2_addr as a > marker for 'particular generation'? In such a case please use a flag > instead. > > The rest looks good to me. > Are you suggesting to add a flag in pm8xxx_vib as a discriminator for the new generation? I actually tried to avoid that because of this comment: https://lore.kernel.org/linux-arm-msm/ZgXSBiQcBEbwF060@google.com/#t >> + if (!vib->drv2_addr) >> + vib->level /= VIB_PER_STEP_mV(vib); >> + >> if (on) >> val |= (vib->level << regs->drv_shift) & regs->drv_mask; >> else >> @@ -93,6 +114,17 @@ static int pm8xxx_vib_set(struct pm8xxx_vib *vib, bool on) >> >> vib->reg_vib_drv = val; >> >> + if (regs->drv2_mask) { >> + if (on) >> + val = (vib->level << regs->drv2_shift) & regs->drv2_mask; >> + else >> + val = 0; >> + >> + rc = regmap_write_bits(vib->regmap, vib->drv2_addr, regs->drv2_mask, val); >> + if (rc < 0) >> + return rc; >> + } >> + >> if (regs->enable_mask) >> rc = regmap_update_bits(vib->regmap, vib->enable_addr, >> regs->enable_mask, on ? regs->enable_mask : 0); >> @@ -115,17 +147,16 @@ static void pm8xxx_work_handler(struct work_struct *work) >> return; >> >> /* >> - * pmic vibrator supports voltage ranges from 1.2 to 3.1V, so >> + * pmic vibrator supports voltage ranges from MIN_LEVEL to MAX_LEVEL, so >> * scale the level to fit into these ranges. >> */ >> if (vib->speed) { >> vib->active = true; >> - vib->level = ((VIB_MAX_LEVELS * vib->speed) / MAX_FF_SPEED) + >> - VIB_MIN_LEVEL_mV; >> - vib->level /= VIB_PER_STEP_mV; >> + vib->level = VIB_MIN_LEVEL_mV(vib); >> + vib->level += mult_frac(VIB_MAX_LEVELS(vib), vib->speed, MAX_FF_SPEED); >> } else { >> vib->active = false; >> - vib->level = VIB_MIN_LEVEL_mV / VIB_PER_STEP_mV; >> + vib->level = VIB_MIN_LEVEL_mV(vib); >> } >> >> pm8xxx_vib_set(vib, vib->active); >> @@ -203,6 +234,7 @@ static int pm8xxx_vib_probe(struct platform_device *pdev) >> >> vib->enable_addr = reg_base + regs->enable_offset; >> vib->drv_addr = reg_base + regs->drv_offset; >> + vib->drv2_addr = reg_base + regs->drv2_offset; >> >> /* operate in manual mode */ >> error = regmap_read(vib->regmap, vib->drv_addr, &val); >> @@ -257,6 +289,7 @@ static const struct of_device_id pm8xxx_vib_id_table[] = { >> { .compatible = "qcom,pm8058-vib", .data = &pm8058_regs }, >> { .compatible = "qcom,pm8921-vib", .data = &pm8058_regs }, >> { .compatible = "qcom,pm8916-vib", .data = &pm8916_regs }, >> + { .compatible = "qcom,pmi632-vib", .data = &pmi632_regs }, >> { } >> }; >> MODULE_DEVICE_TABLE(of, pm8xxx_vib_id_table); >> >> -- >> 2.25.1 >> >> > >
On 4/11/2024 10:21 PM, Dmitry Baryshkov wrote: > On Thu, 11 Apr 2024 at 16:51, Fenglin Wu <quic_fenglinw@quicinc.com> wrote: >> >> >> >> On 2024/4/11 19:02, Dmitry Baryshkov wrote: >>> On Thu, 11 Apr 2024 at 11:32, Fenglin Wu via B4 Relay >>> <devnull+quic_fenglinw.quicinc.com@kernel.org> wrote: >>>> >>>> From: Fenglin Wu <quic_fenglinw@quicinc.com> >>>> >>>> Add support for a new SPMI vibrator module which is very similar >>>> to the vibrator module inside PM8916 but has a finer drive voltage >>>> step and different output voltage range, its drive level control >>>> is expanded across 2 registers. The vibrator module can be found >>>> in following Qualcomm PMICs: PMI632, PM7250B, PM7325B, PM7550BA. >>>> >>>> Signed-off-by: Fenglin Wu <quic_fenglinw@quicinc.com> >>>> --- >>>> drivers/input/misc/pm8xxx-vibrator.c | 51 +++++++++++++++++++++++++++++------- >>>> 1 file changed, 42 insertions(+), 9 deletions(-) >>>> >>>> diff --git a/drivers/input/misc/pm8xxx-vibrator.c b/drivers/input/misc/pm8xxx-vibrator.c >>>> index 2959edca8eb9..35bb6f450fd2 100644 >>>> --- a/drivers/input/misc/pm8xxx-vibrator.c >>>> +++ b/drivers/input/misc/pm8xxx-vibrator.c >>>> @@ -12,10 +12,10 @@ >>>> #include <linux/regmap.h> >>>> #include <linux/slab.h> >>>> >>>> -#define VIB_MAX_LEVEL_mV (3100) >>>> -#define VIB_MIN_LEVEL_mV (1200) >>>> -#define VIB_PER_STEP_mV (100) >>>> -#define VIB_MAX_LEVELS (VIB_MAX_LEVEL_mV - VIB_MIN_LEVEL_mV + VIB_PER_STEP_mV) >>>> +#define VIB_MAX_LEVEL_mV(vib) (vib->drv2_addr ? 3544 : 3100) >>>> +#define VIB_MIN_LEVEL_mV(vib) (vib->drv2_addr ? 1504 : 1200) >>>> +#define VIB_PER_STEP_mV(vib) (vib->drv2_addr ? 8 : 100) >>>> +#define VIB_MAX_LEVELS(vib) (VIB_MAX_LEVEL_mV(vib) - VIB_MIN_LEVEL_mV(vib) + VIB_PER_STEP_mV(vib)) >>>> >>>> #define MAX_FF_SPEED 0xff >>>> >>>> @@ -26,6 +26,9 @@ struct pm8xxx_regs { >>>> unsigned int drv_offset; >>>> unsigned int drv_mask; >>>> unsigned int drv_shift; >>>> + unsigned int drv2_offset; >>>> + unsigned int drv2_mask; >>>> + unsigned int drv2_shift; >>>> unsigned int drv_en_manual_mask; >>>> }; >>>> >>>> @@ -45,6 +48,18 @@ static struct pm8xxx_regs pm8916_regs = { >>>> .drv_en_manual_mask = 0, >>>> }; >>>> >>>> +static struct pm8xxx_regs pmi632_regs = { >>>> + .enable_offset = 0x46, >>>> + .enable_mask = BIT(7), >>>> + .drv_offset = 0x40, >>>> + .drv_mask = GENMASK(7, 0), >>>> + .drv_shift = 0, >>>> + .drv2_offset = 0x41, >>>> + .drv2_mask = GENMASK(3, 0), >>>> + .drv2_shift = 8, >>>> + .drv_en_manual_mask = 0, >>>> +}; >>>> + >>>> /** >>>> * struct pm8xxx_vib - structure to hold vibrator data >>>> * @vib_input_dev: input device supporting force feedback >>>> @@ -53,6 +68,7 @@ static struct pm8xxx_regs pm8916_regs = { >>>> * @regs: registers' info >>>> * @enable_addr: vibrator enable register >>>> * @drv_addr: vibrator drive strength register >>>> + * @drv2_addr: vibrator drive strength upper byte register >>>> * @speed: speed of vibration set from userland >>>> * @active: state of vibrator >>>> * @level: level of vibration to set in the chip >>>> @@ -65,6 +81,7 @@ struct pm8xxx_vib { >>>> const struct pm8xxx_regs *regs; >>>> unsigned int enable_addr; >>>> unsigned int drv_addr; >>>> + unsigned int drv2_addr; >>>> int speed; >>>> int level; >>>> bool active; >>>> @@ -82,6 +99,10 @@ static int pm8xxx_vib_set(struct pm8xxx_vib *vib, bool on) >>>> unsigned int val = vib->reg_vib_drv; >>>> const struct pm8xxx_regs *regs = vib->regs; >>>> >>>> + /* vibrator without drv2_addr needs be programmed in step increments */ >>> >>> How are these two items related? Are you using vib->drv2_addr as a >>> marker for 'particular generation'? In such a case please use a flag >>> instead. >>> >>> The rest looks good to me. >>> >> Are you suggesting to add a flag in pm8xxx_vib as a discriminator for >> the new generation? I actually tried to avoid that because of this comment: >> https://lore.kernel.org/linux-arm-msm/ZgXSBiQcBEbwF060@google.com/#t > > Add a flag for level being programmed in steps or in mV. Using > drv2_addr instead of such flag is a hack. > Thanks Dmitry. Does this flag look good to you? diff --git a/drivers/input/misc/pm8xxx-vibrator.c b/drivers/input/misc/pm8xxx-vibrator.c index 35bb6f450fd2..4708f441e5ac 100644 --- a/drivers/input/misc/pm8xxx-vibrator.c +++ b/drivers/input/misc/pm8xxx-vibrator.c @@ -30,6 +30,7 @@ struct pm8xxx_regs { unsigned int drv2_mask; unsigned int drv2_shift; unsigned int drv_en_manual_mask; + bool drv_in_step; }; static const struct pm8xxx_regs pm8058_regs = { @@ -37,6 +38,7 @@ static const struct pm8xxx_regs pm8058_regs = { .drv_mask = 0xf8, .drv_shift = 3, .drv_en_manual_mask = 0xfc, + .drv_in_step = true; }; static struct pm8xxx_regs pm8916_regs = { @@ -46,6 +48,7 @@ static struct pm8xxx_regs pm8916_regs = { .drv_mask = 0x1F, .drv_shift = 0, .drv_en_manual_mask = 0, + .drv_in_step = true; }; static struct pm8xxx_regs pmi632_regs = { @@ -58,6 +61,7 @@ static struct pm8xxx_regs pmi632_regs = { .drv2_mask = GENMASK(3, 0), .drv2_shift = 8, .drv_en_manual_mask = 0, + .drv_in_step = false, }; /** @@ -100,7 +104,7 @@ static int pm8xxx_vib_set(struct pm8xxx_vib *vib, bool on) const struct pm8xxx_regs *regs = vib->regs; /* vibrator without drv2_addr needs be programmed in step increments */ - if (!vib->drv2_addr) + if (regs->drv_in_step) vib->level /= VIB_PER_STEP_mV(vib); if (on) >> >>>> + if (!vib->drv2_addr) >>>> + vib->level /= VIB_PER_STEP_mV(vib); >>>> + >>>> if (on) >>>> val |= (vib->level << regs->drv_shift) & regs->drv_mask; >>>> else >>>> @@ -93,6 +114,17 @@ static int pm8xxx_vib_set(struct pm8xxx_vib *vib, bool on) >>>> >>>> vib->reg_vib_drv = val; >>>> >>>> + if (regs->drv2_mask) { >>>> + if (on) >>>> + val = (vib->level << regs->drv2_shift) & regs->drv2_mask; >>>> + else >>>> + val = 0; >>>> + >>>> + rc = regmap_write_bits(vib->regmap, vib->drv2_addr, regs->drv2_mask, val); >>>> + if (rc < 0) >>>> + return rc; >>>> + } >>>> + >>>> if (regs->enable_mask) >>>> rc = regmap_update_bits(vib->regmap, vib->enable_addr, >>>> regs->enable_mask, on ? regs->enable_mask : 0); >>>> @@ -115,17 +147,16 @@ static void pm8xxx_work_handler(struct work_struct *work) >>>> return; >>>> >>>> /* >>>> - * pmic vibrator supports voltage ranges from 1.2 to 3.1V, so >>>> + * pmic vibrator supports voltage ranges from MIN_LEVEL to MAX_LEVEL, so >>>> * scale the level to fit into these ranges. >>>> */ >>>> if (vib->speed) { >>>> vib->active = true; >>>> - vib->level = ((VIB_MAX_LEVELS * vib->speed) / MAX_FF_SPEED) + >>>> - VIB_MIN_LEVEL_mV; >>>> - vib->level /= VIB_PER_STEP_mV; >>>> + vib->level = VIB_MIN_LEVEL_mV(vib); >>>> + vib->level += mult_frac(VIB_MAX_LEVELS(vib), vib->speed, MAX_FF_SPEED); >>>> } else { >>>> vib->active = false; >>>> - vib->level = VIB_MIN_LEVEL_mV / VIB_PER_STEP_mV; >>>> + vib->level = VIB_MIN_LEVEL_mV(vib); >>>> } >>>> >>>> pm8xxx_vib_set(vib, vib->active); >>>> @@ -203,6 +234,7 @@ static int pm8xxx_vib_probe(struct platform_device *pdev) >>>> >>>> vib->enable_addr = reg_base + regs->enable_offset; >>>> vib->drv_addr = reg_base + regs->drv_offset; >>>> + vib->drv2_addr = reg_base + regs->drv2_offset; >>>> >>>> /* operate in manual mode */ >>>> error = regmap_read(vib->regmap, vib->drv_addr, &val); >>>> @@ -257,6 +289,7 @@ static const struct of_device_id pm8xxx_vib_id_table[] = { >>>> { .compatible = "qcom,pm8058-vib", .data = &pm8058_regs }, >>>> { .compatible = "qcom,pm8921-vib", .data = &pm8058_regs }, >>>> { .compatible = "qcom,pm8916-vib", .data = &pm8916_regs }, >>>> + { .compatible = "qcom,pmi632-vib", .data = &pmi632_regs }, >>>> { } >>>> }; >>>> MODULE_DEVICE_TABLE(of, pm8xxx_vib_id_table); >>>> >>>> -- >>>> 2.25.1 >>>> >>>> >>> >>> > > >
On Fri, 12 Apr 2024 at 07:05, Fenglin Wu <quic_fenglinw@quicinc.com> wrote: > > > > On 4/11/2024 10:21 PM, Dmitry Baryshkov wrote: > > On Thu, 11 Apr 2024 at 16:51, Fenglin Wu <quic_fenglinw@quicinc.com> wrote: > >> > >> > >> > >> On 2024/4/11 19:02, Dmitry Baryshkov wrote: > >>> On Thu, 11 Apr 2024 at 11:32, Fenglin Wu via B4 Relay > >>> <devnull+quic_fenglinw.quicinc.com@kernel.org> wrote: > >>>> > >>>> From: Fenglin Wu <quic_fenglinw@quicinc.com> > >>>> > >>>> Add support for a new SPMI vibrator module which is very similar > >>>> to the vibrator module inside PM8916 but has a finer drive voltage > >>>> step and different output voltage range, its drive level control > >>>> is expanded across 2 registers. The vibrator module can be found > >>>> in following Qualcomm PMICs: PMI632, PM7250B, PM7325B, PM7550BA. > >>>> > >>>> Signed-off-by: Fenglin Wu <quic_fenglinw@quicinc.com> > >>>> --- > >>>> drivers/input/misc/pm8xxx-vibrator.c | 51 +++++++++++++++++++++++++++++------- > >>>> 1 file changed, 42 insertions(+), 9 deletions(-) > >>>> > >>>> diff --git a/drivers/input/misc/pm8xxx-vibrator.c b/drivers/input/misc/pm8xxx-vibrator.c > >>>> index 2959edca8eb9..35bb6f450fd2 100644 > >>>> --- a/drivers/input/misc/pm8xxx-vibrator.c > >>>> +++ b/drivers/input/misc/pm8xxx-vibrator.c > >>>> @@ -12,10 +12,10 @@ > >>>> #include <linux/regmap.h> > >>>> #include <linux/slab.h> > >>>> > >>>> -#define VIB_MAX_LEVEL_mV (3100) > >>>> -#define VIB_MIN_LEVEL_mV (1200) > >>>> -#define VIB_PER_STEP_mV (100) > >>>> -#define VIB_MAX_LEVELS (VIB_MAX_LEVEL_mV - VIB_MIN_LEVEL_mV + VIB_PER_STEP_mV) > >>>> +#define VIB_MAX_LEVEL_mV(vib) (vib->drv2_addr ? 3544 : 3100) > >>>> +#define VIB_MIN_LEVEL_mV(vib) (vib->drv2_addr ? 1504 : 1200) > >>>> +#define VIB_PER_STEP_mV(vib) (vib->drv2_addr ? 8 : 100) > >>>> +#define VIB_MAX_LEVELS(vib) (VIB_MAX_LEVEL_mV(vib) - VIB_MIN_LEVEL_mV(vib) + VIB_PER_STEP_mV(vib)) > >>>> > >>>> #define MAX_FF_SPEED 0xff > >>>> > >>>> @@ -26,6 +26,9 @@ struct pm8xxx_regs { > >>>> unsigned int drv_offset; > >>>> unsigned int drv_mask; > >>>> unsigned int drv_shift; > >>>> + unsigned int drv2_offset; > >>>> + unsigned int drv2_mask; > >>>> + unsigned int drv2_shift; > >>>> unsigned int drv_en_manual_mask; > >>>> }; > >>>> > >>>> @@ -45,6 +48,18 @@ static struct pm8xxx_regs pm8916_regs = { > >>>> .drv_en_manual_mask = 0, > >>>> }; > >>>> > >>>> +static struct pm8xxx_regs pmi632_regs = { > >>>> + .enable_offset = 0x46, > >>>> + .enable_mask = BIT(7), > >>>> + .drv_offset = 0x40, > >>>> + .drv_mask = GENMASK(7, 0), > >>>> + .drv_shift = 0, > >>>> + .drv2_offset = 0x41, > >>>> + .drv2_mask = GENMASK(3, 0), > >>>> + .drv2_shift = 8, > >>>> + .drv_en_manual_mask = 0, > >>>> +}; > >>>> + > >>>> /** > >>>> * struct pm8xxx_vib - structure to hold vibrator data > >>>> * @vib_input_dev: input device supporting force feedback > >>>> @@ -53,6 +68,7 @@ static struct pm8xxx_regs pm8916_regs = { > >>>> * @regs: registers' info > >>>> * @enable_addr: vibrator enable register > >>>> * @drv_addr: vibrator drive strength register > >>>> + * @drv2_addr: vibrator drive strength upper byte register > >>>> * @speed: speed of vibration set from userland > >>>> * @active: state of vibrator > >>>> * @level: level of vibration to set in the chip > >>>> @@ -65,6 +81,7 @@ struct pm8xxx_vib { > >>>> const struct pm8xxx_regs *regs; > >>>> unsigned int enable_addr; > >>>> unsigned int drv_addr; > >>>> + unsigned int drv2_addr; > >>>> int speed; > >>>> int level; > >>>> bool active; > >>>> @@ -82,6 +99,10 @@ static int pm8xxx_vib_set(struct pm8xxx_vib *vib, bool on) > >>>> unsigned int val = vib->reg_vib_drv; > >>>> const struct pm8xxx_regs *regs = vib->regs; > >>>> > >>>> + /* vibrator without drv2_addr needs be programmed in step increments */ > >>> > >>> How are these two items related? Are you using vib->drv2_addr as a > >>> marker for 'particular generation'? In such a case please use a flag > >>> instead. > >>> > >>> The rest looks good to me. > >>> > >> Are you suggesting to add a flag in pm8xxx_vib as a discriminator for > >> the new generation? I actually tried to avoid that because of this comment: > >> https://lore.kernel.org/linux-arm-msm/ZgXSBiQcBEbwF060@google.com/#t > > > > Add a flag for level being programmed in steps or in mV. Using > > drv2_addr instead of such flag is a hack. > > > Thanks Dmitry. > Does this flag look good to you? Yes, this is much better from my POV > > diff --git a/drivers/input/misc/pm8xxx-vibrator.c > b/drivers/input/misc/pm8xxx-vibrator.c > index 35bb6f450fd2..4708f441e5ac 100644 > --- a/drivers/input/misc/pm8xxx-vibrator.c > +++ b/drivers/input/misc/pm8xxx-vibrator.c > @@ -30,6 +30,7 @@ struct pm8xxx_regs { > unsigned int drv2_mask; > unsigned int drv2_shift; > unsigned int drv_en_manual_mask; > + bool drv_in_step; > }; > > static const struct pm8xxx_regs pm8058_regs = { > @@ -37,6 +38,7 @@ static const struct pm8xxx_regs pm8058_regs = { > .drv_mask = 0xf8, > .drv_shift = 3, > .drv_en_manual_mask = 0xfc, > + .drv_in_step = true; > }; > > static struct pm8xxx_regs pm8916_regs = { > @@ -46,6 +48,7 @@ static struct pm8xxx_regs pm8916_regs = { > .drv_mask = 0x1F, > .drv_shift = 0, > .drv_en_manual_mask = 0, > + .drv_in_step = true; > }; > > static struct pm8xxx_regs pmi632_regs = { > @@ -58,6 +61,7 @@ static struct pm8xxx_regs pmi632_regs = { > .drv2_mask = GENMASK(3, 0), > .drv2_shift = 8, > .drv_en_manual_mask = 0, > + .drv_in_step = false, > }; > > /** > @@ -100,7 +104,7 @@ static int pm8xxx_vib_set(struct pm8xxx_vib *vib, > bool on) > const struct pm8xxx_regs *regs = vib->regs; > > /* vibrator without drv2_addr needs be programmed in step > increments */ > - if (!vib->drv2_addr) > + if (regs->drv_in_step) > vib->level /= VIB_PER_STEP_mV(vib); > > if (on) > > > >> > >>>> + if (!vib->drv2_addr) > >>>> + vib->level /= VIB_PER_STEP_mV(vib); > >>>> + > >>>> if (on) > >>>> val |= (vib->level << regs->drv_shift) & regs->drv_mask; > >>>> else > >>>> @@ -93,6 +114,17 @@ static int pm8xxx_vib_set(struct pm8xxx_vib *vib, bool on) > >>>> > >>>> vib->reg_vib_drv = val; > >>>> > >>>> + if (regs->drv2_mask) { > >>>> + if (on) > >>>> + val = (vib->level << regs->drv2_shift) & regs->drv2_mask; > >>>> + else > >>>> + val = 0; > >>>> + > >>>> + rc = regmap_write_bits(vib->regmap, vib->drv2_addr, regs->drv2_mask, val); > >>>> + if (rc < 0) > >>>> + return rc; > >>>> + } > >>>> + > >>>> if (regs->enable_mask) > >>>> rc = regmap_update_bits(vib->regmap, vib->enable_addr, > >>>> regs->enable_mask, on ? regs->enable_mask : 0); > >>>> @@ -115,17 +147,16 @@ static void pm8xxx_work_handler(struct work_struct *work) > >>>> return; > >>>> > >>>> /* > >>>> - * pmic vibrator supports voltage ranges from 1.2 to 3.1V, so > >>>> + * pmic vibrator supports voltage ranges from MIN_LEVEL to MAX_LEVEL, so > >>>> * scale the level to fit into these ranges. > >>>> */ > >>>> if (vib->speed) { > >>>> vib->active = true; > >>>> - vib->level = ((VIB_MAX_LEVELS * vib->speed) / MAX_FF_SPEED) + > >>>> - VIB_MIN_LEVEL_mV; > >>>> - vib->level /= VIB_PER_STEP_mV; > >>>> + vib->level = VIB_MIN_LEVEL_mV(vib); > >>>> + vib->level += mult_frac(VIB_MAX_LEVELS(vib), vib->speed, MAX_FF_SPEED); > >>>> } else { > >>>> vib->active = false; > >>>> - vib->level = VIB_MIN_LEVEL_mV / VIB_PER_STEP_mV; > >>>> + vib->level = VIB_MIN_LEVEL_mV(vib); > >>>> } > >>>> > >>>> pm8xxx_vib_set(vib, vib->active); > >>>> @@ -203,6 +234,7 @@ static int pm8xxx_vib_probe(struct platform_device *pdev) > >>>> > >>>> vib->enable_addr = reg_base + regs->enable_offset; > >>>> vib->drv_addr = reg_base + regs->drv_offset; > >>>> + vib->drv2_addr = reg_base + regs->drv2_offset; > >>>> > >>>> /* operate in manual mode */ > >>>> error = regmap_read(vib->regmap, vib->drv_addr, &val); > >>>> @@ -257,6 +289,7 @@ static const struct of_device_id pm8xxx_vib_id_table[] = { > >>>> { .compatible = "qcom,pm8058-vib", .data = &pm8058_regs }, > >>>> { .compatible = "qcom,pm8921-vib", .data = &pm8058_regs }, > >>>> { .compatible = "qcom,pm8916-vib", .data = &pm8916_regs }, > >>>> + { .compatible = "qcom,pmi632-vib", .data = &pmi632_regs }, > >>>> { } > >>>> }; > >>>> MODULE_DEVICE_TABLE(of, pm8xxx_vib_id_table); > >>>> > >>>> -- > >>>> 2.25.1 > >>>> > >>>> > >>> > >>> > > > > > >
diff --git a/drivers/input/misc/pm8xxx-vibrator.c b/drivers/input/misc/pm8xxx-vibrator.c index 2959edca8eb9..35bb6f450fd2 100644 --- a/drivers/input/misc/pm8xxx-vibrator.c +++ b/drivers/input/misc/pm8xxx-vibrator.c @@ -12,10 +12,10 @@ #include <linux/regmap.h> #include <linux/slab.h> -#define VIB_MAX_LEVEL_mV (3100) -#define VIB_MIN_LEVEL_mV (1200) -#define VIB_PER_STEP_mV (100) -#define VIB_MAX_LEVELS (VIB_MAX_LEVEL_mV - VIB_MIN_LEVEL_mV + VIB_PER_STEP_mV) +#define VIB_MAX_LEVEL_mV(vib) (vib->drv2_addr ? 3544 : 3100) +#define VIB_MIN_LEVEL_mV(vib) (vib->drv2_addr ? 1504 : 1200) +#define VIB_PER_STEP_mV(vib) (vib->drv2_addr ? 8 : 100) +#define VIB_MAX_LEVELS(vib) (VIB_MAX_LEVEL_mV(vib) - VIB_MIN_LEVEL_mV(vib) + VIB_PER_STEP_mV(vib)) #define MAX_FF_SPEED 0xff @@ -26,6 +26,9 @@ struct pm8xxx_regs { unsigned int drv_offset; unsigned int drv_mask; unsigned int drv_shift; + unsigned int drv2_offset; + unsigned int drv2_mask; + unsigned int drv2_shift; unsigned int drv_en_manual_mask; }; @@ -45,6 +48,18 @@ static struct pm8xxx_regs pm8916_regs = { .drv_en_manual_mask = 0, }; +static struct pm8xxx_regs pmi632_regs = { + .enable_offset = 0x46, + .enable_mask = BIT(7), + .drv_offset = 0x40, + .drv_mask = GENMASK(7, 0), + .drv_shift = 0, + .drv2_offset = 0x41, + .drv2_mask = GENMASK(3, 0), + .drv2_shift = 8, + .drv_en_manual_mask = 0, +}; + /** * struct pm8xxx_vib - structure to hold vibrator data * @vib_input_dev: input device supporting force feedback @@ -53,6 +68,7 @@ static struct pm8xxx_regs pm8916_regs = { * @regs: registers' info * @enable_addr: vibrator enable register * @drv_addr: vibrator drive strength register + * @drv2_addr: vibrator drive strength upper byte register * @speed: speed of vibration set from userland * @active: state of vibrator * @level: level of vibration to set in the chip @@ -65,6 +81,7 @@ struct pm8xxx_vib { const struct pm8xxx_regs *regs; unsigned int enable_addr; unsigned int drv_addr; + unsigned int drv2_addr; int speed; int level; bool active; @@ -82,6 +99,10 @@ static int pm8xxx_vib_set(struct pm8xxx_vib *vib, bool on) unsigned int val = vib->reg_vib_drv; const struct pm8xxx_regs *regs = vib->regs; + /* vibrator without drv2_addr needs be programmed in step increments */ + if (!vib->drv2_addr) + vib->level /= VIB_PER_STEP_mV(vib); + if (on) val |= (vib->level << regs->drv_shift) & regs->drv_mask; else @@ -93,6 +114,17 @@ static int pm8xxx_vib_set(struct pm8xxx_vib *vib, bool on) vib->reg_vib_drv = val; + if (regs->drv2_mask) { + if (on) + val = (vib->level << regs->drv2_shift) & regs->drv2_mask; + else + val = 0; + + rc = regmap_write_bits(vib->regmap, vib->drv2_addr, regs->drv2_mask, val); + if (rc < 0) + return rc; + } + if (regs->enable_mask) rc = regmap_update_bits(vib->regmap, vib->enable_addr, regs->enable_mask, on ? regs->enable_mask : 0); @@ -115,17 +147,16 @@ static void pm8xxx_work_handler(struct work_struct *work) return; /* - * pmic vibrator supports voltage ranges from 1.2 to 3.1V, so + * pmic vibrator supports voltage ranges from MIN_LEVEL to MAX_LEVEL, so * scale the level to fit into these ranges. */ if (vib->speed) { vib->active = true; - vib->level = ((VIB_MAX_LEVELS * vib->speed) / MAX_FF_SPEED) + - VIB_MIN_LEVEL_mV; - vib->level /= VIB_PER_STEP_mV; + vib->level = VIB_MIN_LEVEL_mV(vib); + vib->level += mult_frac(VIB_MAX_LEVELS(vib), vib->speed, MAX_FF_SPEED); } else { vib->active = false; - vib->level = VIB_MIN_LEVEL_mV / VIB_PER_STEP_mV; + vib->level = VIB_MIN_LEVEL_mV(vib); } pm8xxx_vib_set(vib, vib->active); @@ -203,6 +234,7 @@ static int pm8xxx_vib_probe(struct platform_device *pdev) vib->enable_addr = reg_base + regs->enable_offset; vib->drv_addr = reg_base + regs->drv_offset; + vib->drv2_addr = reg_base + regs->drv2_offset; /* operate in manual mode */ error = regmap_read(vib->regmap, vib->drv_addr, &val); @@ -257,6 +289,7 @@ static const struct of_device_id pm8xxx_vib_id_table[] = { { .compatible = "qcom,pm8058-vib", .data = &pm8058_regs }, { .compatible = "qcom,pm8921-vib", .data = &pm8058_regs }, { .compatible = "qcom,pm8916-vib", .data = &pm8916_regs }, + { .compatible = "qcom,pmi632-vib", .data = &pmi632_regs }, { } }; MODULE_DEVICE_TABLE(of, pm8xxx_vib_id_table);