lkml.org 
[lkml]   [2023]   [Sep]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [RESEND PATCH v6 3/3] input: pm8xxx-vibrator: add new SPMI vibrator support
    On Fri, 22 Sept 2023 at 11:38, Fenglin Wu <quic_fenglinw@quicinc.com> wrote:
    >
    > Add new SPMI vibrator module which is very similar to the SPMI vibrator
    > module inside PM8916 but just has a finer drive voltage step (1mV vs
    > 100mV) hence its drive level control is expanded to across 2 registers.
    > The vibrator module can be found in Qualcomm PMIC PMI632, then following
    > PM7250B, PM7325B, PM7550BA PMICs.
    >
    > Signed-off-by: Fenglin Wu <quic_fenglinw@quicinc.com>
    > Tested-by: Luca Weiss <luca.weiss@fairphone.com> # sdm632-fairphone-fp3 (pmi632)
    > ---
    > drivers/input/misc/pm8xxx-vibrator.c | 55 +++++++++++++++++++++++++---
    > 1 file changed, 50 insertions(+), 5 deletions(-)
    >
    > diff --git a/drivers/input/misc/pm8xxx-vibrator.c b/drivers/input/misc/pm8xxx-vibrator.c
    > index d6b468324c77..990e8a9ac018 100644
    > --- a/drivers/input/misc/pm8xxx-vibrator.c
    > +++ b/drivers/input/misc/pm8xxx-vibrator.c
    > @@ -21,6 +21,13 @@
    > #define SPMI_VIB_DRV_LEVEL_MASK GENMASK(4, 0)
    > #define SPMI_VIB_DRV_SHIFT 0
    >
    > +#define SPMI_VIB_GEN2_DRV_REG 0x40
    > +#define SPMI_VIB_GEN2_DRV_MASK GENMASK(7, 0)
    > +#define SPMI_VIB_GEN2_DRV_SHIFT 0
    > +#define SPMI_VIB_GEN2_DRV2_REG 0x41
    > +#define SPMI_VIB_GEN2_DRV2_MASK GENMASK(3, 0)
    > +#define SPMI_VIB_GEN2_DRV2_SHIFT 8
    > +
    > #define SPMI_VIB_EN_REG 0x46
    > #define SPMI_VIB_EN_BIT BIT(7)
    >
    > @@ -33,12 +40,14 @@
    > enum vib_hw_type {
    > SSBI_VIB,
    > SPMI_VIB,
    > + SPMI_VIB_GEN2
    > };
    >
    > struct pm8xxx_vib_data {
    > enum vib_hw_type hw_type;
    > unsigned int enable_addr;
    > unsigned int drv_addr;
    > + unsigned int drv2_addr;
    > };
    >
    > static const struct pm8xxx_vib_data ssbi_vib_data = {
    > @@ -52,6 +61,13 @@ static const struct pm8xxx_vib_data spmi_vib_data = {
    > .drv_addr = SPMI_VIB_DRV_REG,
    > };
    >
    > +static const struct pm8xxx_vib_data spmi_vib_gen2_data = {
    > + .hw_type = SPMI_VIB_GEN2,
    > + .enable_addr = SPMI_VIB_EN_REG,
    > + .drv_addr = SPMI_VIB_GEN2_DRV_REG,
    > + .drv2_addr = SPMI_VIB_GEN2_DRV2_REG,
    > +};
    > +
    > /**
    > * struct pm8xxx_vib - structure to hold vibrator data
    > * @vib_input_dev: input device supporting force feedback
    > @@ -85,12 +101,24 @@ static int pm8xxx_vib_set(struct pm8xxx_vib *vib, bool on)
    > {
    > int rc;
    > unsigned int val = vib->reg_vib_drv;
    > - u32 mask = SPMI_VIB_DRV_LEVEL_MASK;
    > - u32 shift = SPMI_VIB_DRV_SHIFT;
    > + u32 mask, shift;
    >
    > - if (vib->data->hw_type == SSBI_VIB) {
    > +
    > + switch (vib->data->hw_type) {
    > + case SSBI_VIB:
    > mask = SSBI_VIB_DRV_LEVEL_MASK;
    > shift = SSBI_VIB_DRV_SHIFT;
    > + break;
    > + case SPMI_VIB:
    > + mask = SPMI_VIB_DRV_LEVEL_MASK;
    > + shift = SPMI_VIB_DRV_SHIFT;
    > + break;
    > + case SPMI_VIB_GEN2:
    > + mask = SPMI_VIB_GEN2_DRV_MASK;
    > + shift = SPMI_VIB_GEN2_DRV_SHIFT;
    > + break;
    > + default:
    > + return -EINVAL;

    Could you please move the switch to the previous patch? Then it would
    be more obvious that you are just adding the SPMI_VIB_GEN2 here.

    Other than that LGTM.

    > }
    >
    > if (on)
    > @@ -104,6 +132,19 @@ static int pm8xxx_vib_set(struct pm8xxx_vib *vib, bool on)
    >
    > vib->reg_vib_drv = val;
    >
    > + if (vib->data->hw_type == SPMI_VIB_GEN2) {
    > + mask = SPMI_VIB_GEN2_DRV2_MASK;
    > + shift = SPMI_VIB_GEN2_DRV2_SHIFT;
    > + if (on)
    > + val = (vib->level >> shift) & mask;
    > + else
    > + val = 0;
    > + rc = regmap_update_bits(vib->regmap,
    > + vib->reg_base + vib->data->drv2_addr, mask, val);
    > + if (rc < 0)
    > + return rc;
    > + }
    > +
    > if (vib->data->hw_type == SSBI_VIB)
    > return 0;
    >
    > @@ -128,10 +169,13 @@ static void pm8xxx_work_handler(struct work_struct *work)
    > vib->active = true;
    > vib->level = ((VIB_MAX_LEVELS * vib->speed) / MAX_FF_SPEED) +
    > VIB_MIN_LEVEL_mV;
    > - vib->level /= 100;
    > + if (vib->data->hw_type != SPMI_VIB_GEN2)
    > + vib->level /= 100;
    > } else {
    > vib->active = false;
    > - vib->level = VIB_MIN_LEVEL_mV / 100;
    > + vib->level = VIB_MIN_LEVEL_mV;
    > + if (vib->data->hw_type != SPMI_VIB_GEN2)
    > + vib->level /= 100;
    > }
    >
    > pm8xxx_vib_set(vib, vib->active);
    > @@ -266,6 +310,7 @@ static const struct of_device_id pm8xxx_vib_id_table[] = {
    > { .compatible = "qcom,pm8058-vib", .data = &ssbi_vib_data },
    > { .compatible = "qcom,pm8921-vib", .data = &ssbi_vib_data },
    > { .compatible = "qcom,pm8916-vib", .data = &spmi_vib_data },
    > + { .compatible = "qcom,pmi632-vib", .data = &spmi_vib_gen2_data },
    > { }
    > };
    > MODULE_DEVICE_TABLE(of, pm8xxx_vib_id_table);
    > --
    > 2.25.1
    >


    --
    With best wishes
    Dmitry

    \
     
     \ /
      Last update: 2023-09-23 21:08    [W:6.260 / U:0.012 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site