Messages in this thread | | | Date | Thu, 27 Jul 2023 15:43:51 +0800 | Subject | Re: [PATCH v3 1/3] input: pm8xxx-vib: refactor to easily support new SPMI vibrator | From | Fenglin Wu <> |
| |
On 7/27/2023 3:07 PM, Krzysztof Kozlowski wrote: > On 25/07/2023 08:16, Fenglin Wu wrote: >>>> >>>> -static const struct pm8xxx_regs pm8058_regs = { >>>> - .drv_addr = 0x4A, >>>> - .drv_mask = 0xf8, >>>> - .drv_shift = 3, >>>> - .drv_en_manual_mask = 0xfc, >>>> +static struct reg_field ssbi_vib_regs[VIB_MAX_REG] = { >>> >>> Change from const to non-const is wrong. How do you support multiple >>> devices? No, this is way too fragile now. >>> >> >> The register definition is no longer used as the match data, hw_type is >> used. >> >> The last suggestion was getting the register base address from the DT >> and it has to be added into the offset of SPMI vibrator registers >> (either in the previous hard-coded format or the later the reg_filed >> data structure), so it's not appropriated to make it constant. >> >> I don't understand this question: "How do you support multiple devices?" >> For SSBI vibrator, since all the registers are fixed, and I would assume >> that there is no chance to support multiple vibrator devices on the same >> SSBI bus. If they are not on the same bus, the regmap device will be >> different while the registers definition is the same, and we are still >> able to support multiple devices, right? > > No, you have static memory. One device probes and changes static memory > to reg+=base1. Second device probes and changes the same to reg+=base2.
Thanks, got it. I can update it with following 2 options:
1) keep the register definition in 'reg_filed' data structure and make it constant, copy it to a dynamically allocated memory before adding the 'reg_base' to the '.reg' variable.
2) Define the register offsets as constant data and add the 'reg_base' to the 'reg' while using 'regmap_read()'/'regmap_write()' functions.
which one is the preferred way?
> >> The similar story for SPMI vibrators and it can support multiple devices >> if they are located on different SPMI bus, or even if they are on the >> same SPMI bus but just having different SID or PID. > > Sorry, such code cannot go in. These must stay const and you must write > driver without any static allocations or singleton-like patterns. > > > Best regards, > Krzysztof >
| |