On 7/18/2023 2:44 PM, Dmitry Baryshkov wrote:
On Tue, 18 Jul 2023 at 09:27, Fenglin Wu <quic_fenglinw@xxxxxxxxxxx> wrote:Add support for vibrator module inside PMI632, PM7250B, PM7325B. It is very similar to vibrator inside PM8xxx but just the drive amplitude is controlled through 2 bytes registers. Signed-off-by: Fenglin Wu <quic_fenglinw@xxxxxxxxxxx> --- drivers/input/misc/pm8xxx-vibrator.c | 48 ++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/drivers/input/misc/pm8xxx-vibrator.c b/drivers/input/misc/pm8xxx-vibrator.c index 04cb87efd799..213fdfd47c7f 100644 --- a/drivers/input/misc/pm8xxx-vibrator.c +++ b/drivers/input/misc/pm8xxx-vibrator.c @@ -25,6 +25,9 @@ struct pm8xxx_regs { unsigned int drv_addr; unsigned int drv_mask; unsigned int drv_shift; + unsigned int drv_addr2; + unsigned int drv_mask2; + unsigned int drv_shift2; unsigned int drv_en_manual_mask; }; @@ -44,6 +47,42 @@ static struct pm8xxx_regs pm8916_regs = { .drv_en_manual_mask = 0, }; +static struct pm8xxx_regs pmi632_regs = { + .enable_addr = 0x5746, + .enable_mask = BIT(7), + .drv_addr = 0x5740, + .drv_mask = 0xff, + .drv_shift = 0, + .drv_addr2 = 0x5741, + .drv_mask2 = 0x0f, + .drv_shift2 = 8,I see that you are just expanding what was done for SSBI PMICs and later expanded to support pm8916. However it might be better to drop the hardcoded .drv_addr (and drv_addr2) and read address from DT instead.
Right, this is the simplest change without updating the code logic too much. If we decided to read .drv_addr and .drv_add2 from DT, we will have to read .enable_addr along with all other mask/shift for each register address from DT as well because they are not consistent from target to target. I don't know how would you suggest to add the DT properties for all of them, but if we end up to add a property for each of them, it won't be cleaner than hard-coding them.
+ .drv_en_manual_mask = 0, +}; + +static struct pm8xxx_regs pm7250b_regs = { + .enable_addr = 0x5346, + .enable_mask = BIT(7), + .drv_addr = 0x5340, + .drv_mask = 0xff, + .drv_shift = 0, + .drv_addr2 = 0x5341, + .drv_mask2 = 0x0f, + .drv_shift2 = 8, + .drv_en_manual_mask = 0, +}; + +static struct pm8xxx_regs pm7325b_regs = { + .enable_addr = 0xdf46, + .enable_mask = BIT(7), + .drv_addr = 0xdf40, + .drv_mask = 0xff, + .drv_shift = 0, + .drv_addr2 = 0xdf41, + .drv_mask2 = 0x0f, + .drv_shift2 = 8, + .drv_en_manual_mask = 0, +}; + /** * struct pm8xxx_vib - structure to hold vibrator data * @vib_input_dev: input device supporting force feedback @@ -87,6 +126,12 @@ static int pm8xxx_vib_set(struct pm8xxx_vib *vib, bool on) return rc; vib->reg_vib_drv = val; + if (regs->drv_addr2 != 0 && on) { + val = (vib->level << regs->drv_shift2) & regs->drv_mask2; + rc = regmap_write(vib->regmap, regs->drv_addr2, val); + if (rc < 0) + return rc; + } if (regs->enable_mask) rc = regmap_update_bits(vib->regmap, regs->enable_addr, @@ -242,6 +287,9 @@ 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 }, + { .compatible = "qcom,pm7250b-vib", .data = &pm7250b_regs }, + { .compatible = "qcom,pm7325b-vib", .data = &pm7325b_regs }, { } }; MODULE_DEVICE_TABLE(of, pm8xxx_vib_id_table); -- 2.25.1
