Re: [PATCH RESEND v7 0/3] Add support for vibrator in multiple PMICs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Dmitry Torokhov,

Can you help to review the series of the changes?
I knew that you left comment in patch V6 about moving shift and mask into a chip-specific data structure to avoid defining 'hw_type'. Actually 'hw_type' is added here to differentiate the SSBI vibrator module which doesn't need to read the base address from the DT and add it up in register read/write access, while SPMI vibrators in different PMICs may be the same HW but just with different base address, and we can use device compatibles instead of adding the chip-specific data structure for each of them by just updating the base address. See discussion here for more details:
https://lore.kernel.org/linux-arm-msm/20230718062639.2339589-3-quic_fenglinw@xxxxxxxxxxx/

I also responded here that having 'hw_type' would help us with a cleaner code logic instead of checking specific reg/mask for particular operations: https://lore.kernel.org/linux-arm-msm/8697d115-9aa7-2a1c-4d96-25b15adb5cca@xxxxxxxxxxx/

Let me know what's your final suggestion, if you insist that I should move the reg/mask/shift in the chip-specific data structure, I can update it even it won't be as clean as what it looks right now.
Thanks

Fenglin

On 11/8/2023 3:36 PM, Fenglin Wu via B4 Relay wrote:
Add SW support for the vibrator module inside PMI632, PM7250B, PM7325B, PM7550BA.
It is very similar to the vibrator module inside PM8916 which is supported in
pm8xxx-vib driver but just the drive amplitude is controlled with 2 registers,
and the register base offset in each PMIC is different.

Changes in v7;
   1. Fix a typo: SSBL_VIB_DRV_REG --> SSBI_VIB_DRV_REG
   2. Move the hw_type switch case in pm8xxx_vib_set() to the refactoring
      change.

Changes in v6:
   1. Add "qcom,pmi632-vib" as a standalone compatible string.

Changes in v5:
   1. Drop "qcom,spmi-vib-gen2" generic compatible string as requested
      and use device specific compatible strings only.

Changes in v4:
   1. Update to use the combination of the HW type and register offset
      as the constant match data, the register base address defined in
      'reg' property will be added when accessing SPMI registers using
      regmap APIs.
   2. Remove 'qcom,spmi-vib-gen1' generic compatible string.

Changes in v3:
   1. Refactor the driver to support different type of the vibrators with
     better flexibility by introducing the HW type with corresponding
     register fields definitions.
   2. Add 'qcom,spmi-vib-gen1' and 'qcom,spmi-vib-gen2' compatible
     strings, and add PMI632, PM7250B, PM7325B, PM7550BA as compatbile as
     spmi-vib-gen2.

Changes in v2:
   Remove the "pm7550ba-vib" compatible string as it's compatible with pm7325b.

Fenglin Wu (3):
   input: pm8xxx-vib: refactor to easily support new SPMI vibrator
   dt-bindings: input: qcom,pm8xxx-vib: add new SPMI vibrator module
   input: pm8xxx-vibrator: add new SPMI vibrator support

  .../bindings/input/qcom,pm8xxx-vib.yaml       |  16 +-
  drivers/input/misc/pm8xxx-vibrator.c          | 171 ++++++++++++------
  2 files changed, 132 insertions(+), 55 deletions(-)

--
2.25.1

---
Fenglin Wu (3):
       input: pm8xxx-vib: refactor to easily support new SPMI vibrator
       dt-bindings: input: qcom,pm8xxx-vib: add new SPMI vibrator module
       input: pm8xxx-vibrator: add new SPMI vibrator support

  .../devicetree/bindings/input/qcom,pm8xxx-vib.yaml |  16 +-
  drivers/input/misc/pm8xxx-vibrator.c               | 170 ++++++++++++++-------
  2 files changed, 131 insertions(+), 55 deletions(-)
---
base-commit: 650cda2ce25f08e8fae391b3ba6be27e7296c6a5
change-id: 20230925-pm8xxx-vibrator-62df3df46a6c

Best regards,




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux