Re: [PATCH RFT 2/3] media: i2c: dw9719: Add DW9800W support
"Luca Weiss" <[email protected]>
| Newsgroups | org.kernel.vger.linux-media,org.kernel.vger.linux-arm-msm,org.kernel.vger.linux-devicetree,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <[email protected]> |
Hi Danila,
On Sat Aug 8, 2026 at 2:42 PM CEST, Danila Tikhonov wrote:
> Hi Griffin, Sakari,
>
> Thank you for the clarification.
>
> On 07/08/2026 17:30, Griffin Kroah-Hartman wrote:
>> Hello Sakari, Danilla,
>>
>> On 05/08/2026 13:29, Sakari Ailus wrote:
>>> Hi Danila, Griffin,
>>>
>>> On Tue, Aug 04, 2026 at 09:27:01PM +0300, Danila Tikhonov wrote:
>>>> Hi Sakari,
>>>>
>>>> Thank you for your comment.
>>>>
>>>> On 04/08/2026 11:40, Sakari Ailus wrote:
>>>>> Hi Danila,
>>>>>
>>>>> Thank you for the patch.
>>>>>
>>>>> On Sat, Aug 01, 2026 at 09:04:15PM +0300, Danila Tikhonov wrote:
>>>>>> The DW9800W uses the same chip ID and register layout as the DW9800K,
>>>>>> but requires different default VCM frequency value.
>>>>> Is this a real difference between the two models or what fits for the
>>>>> attached lens? Too bad the datasheet isn't publicly available. :-(
>>>>>
>>>>> At least the differing default and constraints for
>>>>> dongwoon,vcm-prescale
>>>>> needs to be documented in bindings.
>>>> I share your skepticism and have essentially the same concerns. Since I
>>>> do not have the DW9800K datasheet, I cannot say with certainty how it
>>>> differs from the DW9800W.
>>> Indeed. The limits should have been added for dw9800k, too, but I
>>> guess I
>>> missed this in review. :-\
>>>
>>>> In particular, the parameter referred to as the VCM frequency in the
>>>> driver is used to derive the SACT value. For the DW9800W, SACT[5:0]
>>>> defines the actuator resonance period in SAC mode and the one-step
>>>> period in LSC mode. The datasheet gives the corresponding formulas as:
>>>> - tVIB = 6.3 ms + SACT * 0.1 ms
>>>> - LSC 1-step period = 252 µs + SACT * 4 µs
>>>>
>>>> Therefore, the default value may differ between the DW9800K and DW9800W
>>>> if the formulas or timing characteristics used to calculate SACT are
>>>> different.
>>> Griffin, would you be able to comment this?
>>
>> After checking on my side, DW9800W and K are almost identical in both
>> structure
>> and register values, including how tVIB and tLSC are calculated,
>> DW9800K has an extra
>> register that can change the output current to 130mA but we don't ever
>> use that.
>>
>> We didn't have a datasheet when working on the DW9800k so we had to go
>> off of
>> the downstream driver during runtime to get our register values. The
>> datasheet is a
>> much more reliable source to be using for this instead.
>>
>> My colleague has already identified ways to fix the bindings, which we
>> will
>> tackle ourselves.
>>
> I'm glad you were able to clarify the situation, as this should help us
> handle both IC variants correctly.
>>> Another matter: the comment on dw9800k says there's no device ID. So is
>>> there a device ID that wasn't just checked earlier or is there really no
>>> device ID?
>>
>> Indeed, it turns out the DW9800K DOES have an info register, which
>> should be
>> 0xEB, that's on us for not updating it after we received more
>> documentation.
>>
> Given this, and assuming everyone agrees, I think it would make sense
> for me to leave the DW9800K untouched in the next version of this patch
> series. Since it turns out that the DW9800K has a different Chip ID, you
> can fix the Chip ID check for it separately based on the information you
> have.
>
> One point is still unclear to me, though: is the default VCM frequency
> the same for both ICs, or does it differ between them?
Like Griffin wrote, the way the driver/bindings currently handle
especially dongwoon,vcm-prescale ("vcm_freq") is completely wrong **for
DW9800K**. This wasn't done better because no datasheet was available to
us for the DW9800K at that point in time.
Sidenote, I found a datasheet for DW9718S and that one seems to be
programmed correctly. I can't comment about DW9761 and DW9719.
Please wait with adding DW9800W support until this situation has been
cleaned up by us since it's the same registers with both DW9800K/W.
Regards
Luca
>> Thanks for drawing our attention to this! Nice patch.
>>
>> Griffin
>>
> ---
> Best regards,
> Danila