On 28/08/2023 08:56, Luca Weiss wrote: > Hi Krzysztof, > > On Sun Aug 27, 2023 at 2:28 PM CEST, Krzysztof Kozlowski wrote: >> This reverts commit 413821b7777d062b57f8dc66ab088ed390cbc3ec because it >> was never reviewed, was buggy (report from kernel test robot: >> https://lore.kernel.org/all/202204090333.QZXMI2tu-lkp@xxxxxxxxx/) and > > (I wouldn't say this part is accurate, the robot just didn't use a tree > with the i2c10 node already present, it was sent in an earlier patch > IIRC, but whatever) > >> used undocumented, broken bindings. Half of the properties in this >> device are questioned, thus adding DTS node causes only errors and does >> not make the device usable without the bindings and driver part: >> >> sm7225-fairphone-fp4.dtb: haptics@5a: failed to match any schema with compatible: ['awinic,aw8695'] >> sm7225-fairphone-fp4.dtb: haptics@5a: awinic,tset: b'\x12' is not of type 'object', 'array', 'boolean', 'null' >> sm7225-fairphone-fp4.dtb: haptics@5a: awinic,r-spare: b'h' is not of type 'object', 'array', 'boolean', 'null' >> >> Since bindings were abandoned (4 months since review), revert the commit >> to avoid false sense of supporting something which is not supported. > > I've been avoiding touching this topic again since I'm really not sure > how to resolve. Happens, but the DTS should not have been applied in such case. > > There's a bunch of magic registers being written to in the downstream > driver, I don't have any documentation for that so I'm not exactly sure > what I can do to make nice bindings with proper properties. > > Would you recommend just hardcoding some of these properties in the > driver, assuming they're constant for every AW8695, even though the > downstream driver has these properties in devicetree? Because of that I > assumed these properties could differ per implementation / usage of the > AW8695 in different devices. Yes, keep them in the driver. Best regards, Krzysztof