From: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
To: Fenglin Wu <quic_fenglinw@quicinc.com>
Cc: linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org,
krzysztof.kozlowski+dt@linaro.org, robh+dt@kernel.org,
agross@kernel.org, andersson@kernel.org,
Konrad Dybcio <konrad.dybcio@linaro.org>,
Dmitry Torokhov <dmitry.torokhov@gmail.com>,
linux-input@vger.kernel.org, quic_collinsd@quicinc.com,
quic_subbaram@quicinc.com, quic_kamalw@quicinc.com,
jestar@qti.qualcomm.com, Luca Weiss <luca.weiss@fairphone.com>
Subject: Re: [RESEND PATCH v6 3/3] input: pm8xxx-vibrator: add new SPMI vibrator support
Date: Sat, 23 Sep 2023 22:07:40 +0300 [thread overview]
Message-ID: <CAA8EJpoW8DJOTVHBu9_+BQs5DtxyJu3xrCfDNyYHn2MeHZHV4w@mail.gmail.com> (raw)
In-Reply-To: <20230922083801.3056724-4-quic_fenglinw@quicinc.com>
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
next prev parent reply other threads:[~2023-09-23 19:07 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-09-22 8:37 [RESEND PATCH v6 0/3] Add support for vibrator in multiple PMICs Fenglin Wu
2023-09-22 8:37 ` [RESEND PATCH v6 1/3] input: pm8xxx-vib: refactor to easily support new SPMI vibrator Fenglin Wu
2023-09-23 19:05 ` Dmitry Baryshkov
2023-09-25 2:52 ` Fenglin Wu
2023-09-22 8:38 ` [RESEND PATCH v6 2/3] dt-bindings: input: qcom,pm8xxx-vib: add new SPMI vibrator module Fenglin Wu
2023-09-22 8:38 ` [RESEND PATCH v6 3/3] input: pm8xxx-vibrator: add new SPMI vibrator support Fenglin Wu
2023-09-23 19:07 ` Dmitry Baryshkov [this message]
2023-09-25 2:54 ` Fenglin Wu
2023-09-30 16:17 ` Dmitry Torokhov
2023-10-09 4:01 ` Fenglin Wu
2023-10-25 9:54 ` Fenglin Wu
2024-03-28 6:52 ` Fenglin Wu
2024-03-28 20:24 ` Dmitry Torokhov
-- strict thread matches above, loose matches on Subject: below --
2023-08-28 5:32 [RESEND PATCH v6 0/3] Add support for vibrator in multiple PMICs Fenglin Wu
2023-08-28 5:32 ` [RESEND PATCH v6 3/3] input: pm8xxx-vibrator: add new SPMI vibrator support Fenglin Wu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAA8EJpoW8DJOTVHBu9_+BQs5DtxyJu3xrCfDNyYHn2MeHZHV4w@mail.gmail.com \
--to=dmitry.baryshkov@linaro.org \
--cc=agross@kernel.org \
--cc=andersson@kernel.org \
--cc=dmitry.torokhov@gmail.com \
--cc=jestar@qti.qualcomm.com \
--cc=konrad.dybcio@linaro.org \
--cc=krzysztof.kozlowski+dt@linaro.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-input@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=luca.weiss@fairphone.com \
--cc=quic_collinsd@quicinc.com \
--cc=quic_fenglinw@quicinc.com \
--cc=quic_kamalw@quicinc.com \
--cc=quic_subbaram@quicinc.com \
--cc=robh+dt@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.