Re: [PATCH 3/4] input: misc: Add Qualcomm SPMI PMIC haptics driver

[Fenglin Wu <fenglin.wu@oss.qualcomm.com>]

On 6/17/2026 5:30 PM, Konrad Dybcio wrote:
> On 6/17/26 4:31 AM, Fenglin Wu wrote:
>>>> +        ret = ptn_bulk_write(h, HAP_PTN_FIFO_DIN_0_REG, &data[i], 4);
>>>> +        if (ret)
>>>> +            return ret;
>>>> +    }
>>>> +
>>>> +    for (; i < len; i++) {
>>>> +        ret = ptn_write(h, HAP_PTN_FIFO_DIN_1B_REG, (u8)data[i]);
>>>> +        if (ret)
>>>> +            return ret;
>>>> +    }
>>> So if i'm reading this right, the first loop will always write
>>> 4*(len//4) bytes and the second one will be entered at most once,
>>> to write len rem 4 bytes.. should this be an if instead?
>> I should put a comment for clarification. Here’s some background: FIFO data writing supports both 4-byte bulk writes using registers [HAP_PTN_FIFO_DIN_0_REG ... HAP_PTN_FIFO_DIN_3_REG], and 1-byte writes using the HAP_PTN_FIFO_DIN_1B_REG register. The 4-byte bulk write is more efficient, especially for waveform which has several Kb data, and it helps to reduce software latency when loading effects and reduce the delay in triggering vibration. It also helps prevent the FIFO from running dry during data refill in FIFO-empty interrupts. Typically, we use 4-byte writes for the initial 4-byte aligned data, and 1-byte writes for any trailing remainder.
>>
>> So it still needs a 'for' loop here since the remainder could be more than 1 byte.
> Right, I mentioned len rem 4 but failed to notice it's a
> single-byte write.. anyway, a comment here would be good
>
>>>> +
>>>> +    return 0;
>>>> +}
>>>> +
>>>> +/*
>>>> + * Configure the hardware FIFO memory boundary.
>>>> + * FIFO occupies addresses [0, fifo_len).
>>>> + */
>>>> +static int haptics_configure_fifo_mmap(struct qcom_haptics *h)
>>>> +{
>>>> +    u32 fifo_len, fifo_units;
>>>> +
>>>> +    /* Config all memory space for FIFO usage for now */
>>> What's the not-"for now" endgame for this?
>> The hardware supports more modes than the two currently supported in the driver. One of these, called 'PAT_MEM' mode, also shares memory space with FIFO mode. However, 'PAT_MEM' requires memory to be pre-reserved and waveform data to be pre-loaded. The entire 8K bytes of memory can be divided into partitions, and it is configurable, with FIFO mode always using the first partition [0, fifo_len], where 'fifo_len' is set via the 'MMAP_FIFO_REG' register. 'PAT_MEM' mode plays waveform using data preloaded in a memory bank defined by the registers 'PATX_MEM_START_ADDR_REG' and 'PATTERN_SPMI_PATX_LEN_REG' (they are not defined in the driver). Since PAT_MEM is mainly intended for hardware-triggered vibrations, such as a signal from a dedicated GPIO triggering a short vibration with a preloaded waveform, and although it also supports software triggers, I haven't found a suitable way to support it well into the driver under input FF framework yet. So, I am currently allocating the
>> entire 8K FIFO memory for FIFO mode only. We can adjust this later if we find a better way to incorporate 'PAT_MEM' mode into the driver.
> Sounds like a plan.
>
> For the other mode, would that GPIO trigger need any OS intervention?
> Could you speak a bit more about how that works?
>
> Konrad

I'll try to clarify the 'PAT_MEM' mode further. 'PAT_MEM' is useful for 
latency-sensitive vibrations because it preloads the waveform into a 
fixed memory bank, then it doesn't need to load the data of the effect 
in the HW before triggering the play. When playback is triggered, it 
plays the waveform from the specified memory address and length. This 
memory should be preserved, and the data is preloaded during boot. 
Unlike FIFO mode, it doesn't allow data refilling. The trigger can come 
from hardware via dedicated GPIOs—currently, three are supported, each 
mapping to a memory bank set through specific registers. Software 
configuration can be done in the bootloader or in the driver probe, but 
the 'fifo_len' should be adjusted accordingly. After setup, software 
doesn't need to manage it further, relying on the GPIO signal to 
activate the playback (for example, a pressure sensor triggering 
vibration to simulate a physical key press). The trigger can also come 
from software using SPMI commands by setting the play mode, start 
address, and data length. I previously tried using the 'FF_HAPTIC' 
effect by mapping 'hid_usage' to a predefined effect in the devicetree, 
but later I found it unsuitable since 'FF_HAPTIC' is mainly for USB HID 
touch devices and not general vibration usage. If you have any 
suggestions for supporting 'PAT_MEM' mode through the input FF 
framework, please let me know.