Re: [PATCH RFT 2/3] media: i2c: dw9719: Add DW9800W support
Danila Tikhonov <[email protected]>
| Newsgroups | org.kernel.vger.linux-arm-msm,org.kernel.vger.linux-devicetree,org.kernel.vger.linux-kernel,org.kernel.vger.linux-media |
|---|---|
| Message-ID | <[email protected]> |
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? > Thanks for drawing our attention to this! Nice patch. > > Griffin > --- Best regards, Danila