Message ID | 20210511095409.9290-1-stephan@gerhold.net (mailing list archive) |
---|---|
Headers | show |
Series | iio: accel: kxcjk-1013: Add support for KX023-1025 | expand |
Hi, On 5/11/21 11:54 AM, Stephan Gerhold wrote: > KX023-1025 [1] is another accelerometer from Kionix that has lots > of additional functionality compared to KXCJK-1013. It combines the > motion interrupt functionality from KXCJK with the tap detection > from KXTF9, plus a lot more other functionality. > > This patch set does not add support for any of the extra functionality, > but makes basic functionality work with the existing kxcjk-1013 driver. > > At first, the register map for the KX023-1025 seems quite different > from the other accelerometers supported by the kxcjk-1013. > However, it turns out that at least most of the register bits > still mean the same for KX023-1025. > > This patch set refactors the kxcjk-1013 driver a little bit > to get the register addresses from a chip-specific struct. > The register bits can be re-used for all the different chips. > > The KX023-1025 is used in several smartphones from Huawei. > I tested these changes on a Huawei Ascend G7, someone else reported > they also work fine on the Huawei Honor 5X (codename "kiwi"). > > [1]: https://kionixfs.azureedge.net/en/datasheet/KX023-1025%20Specifications%20Rev%2012.0.pdf > > Stephan Gerhold (3): > dt-bindings: iio: kionix,kxcjk1013: Document kionix,kx023-1025 > iio: accel: kxcjk-1013: Refactor configuration registers into struct > iio: accel: kxcjk-1013: Add support for KX023-1025 Thanks, the entire series looks good to me: Reviewed-by: Hans de Goede <hdegoede@redhat.com> for the series. Regards, Hans
On Tue, 11 May 2021 12:44:23 +0200 Hans de Goede <hdegoede@redhat.com> wrote: > Hi, > > On 5/11/21 11:54 AM, Stephan Gerhold wrote: > > KX023-1025 [1] is another accelerometer from Kionix that has lots > > of additional functionality compared to KXCJK-1013. It combines the > > motion interrupt functionality from KXCJK with the tap detection > > from KXTF9, plus a lot more other functionality. > > > > This patch set does not add support for any of the extra functionality, > > but makes basic functionality work with the existing kxcjk-1013 driver. > > > > At first, the register map for the KX023-1025 seems quite different > > from the other accelerometers supported by the kxcjk-1013. > > However, it turns out that at least most of the register bits > > still mean the same for KX023-1025. > > > > This patch set refactors the kxcjk-1013 driver a little bit > > to get the register addresses from a chip-specific struct. > > The register bits can be re-used for all the different chips. > > > > The KX023-1025 is used in several smartphones from Huawei. > > I tested these changes on a Huawei Ascend G7, someone else reported > > they also work fine on the Huawei Honor 5X (codename "kiwi"). > > > > [1]: https://kionixfs.azureedge.net/en/datasheet/KX023-1025%20Specifications%20Rev%2012.0.pdf > > > > Stephan Gerhold (3): > > dt-bindings: iio: kionix,kxcjk1013: Document kionix,kx023-1025 > > iio: accel: kxcjk-1013: Refactor configuration registers into struct > > iio: accel: kxcjk-1013: Add support for KX023-1025 > > Thanks, the entire series looks good to me: > > Reviewed-by: Hans de Goede <hdegoede@redhat.com> > Other than those things 0-day found which would be good to tidy up for a v2 looks good to me. Thanks, Jonathan > for the series. > > Regards, > > Hans >
On Tue, May 11, 2021 at 11:54:06AM +0200, Stephan Gerhold wrote: > KX023-1025 [1] is another accelerometer from Kionix that has lots > of additional functionality compared to KXCJK-1013. It combines the > motion interrupt functionality from KXCJK with the tap detection > from KXTF9, plus a lot more other functionality. When I researched KXTF9 support it occurred to me that the -10xx part is duplicating the information in 'KXyyy' - it seems to be a project number or something. I would suggest to use just 'kx023' prefix for the code and DT but leave the full identification in the comments/description. Best Regards Michał Mirosław
Hi Michał, On Tue, May 11, 2021 at 04:28:47PM +0200, Michał Mirosław wrote: > On Tue, May 11, 2021 at 11:54:06AM +0200, Stephan Gerhold wrote: > > KX023-1025 [1] is another accelerometer from Kionix that has lots > > of additional functionality compared to KXCJK-1013. It combines the > > motion interrupt functionality from KXCJK with the tap detection > > from KXTF9, plus a lot more other functionality. > > When I researched KXTF9 support it occurred to me that the -10xx part is > duplicating the information in 'KXyyy' - it seems to be a project number > or something. I would suggest to use just 'kx023' prefix for the code > and DT but leave the full identification in the comments/description. > There do seem to be two different KXTF9 from Kionix, a KXTF9-4100 [1] and a KXTF9-2050 [2] with separate datasheets. Have you checked if there is a meaningful difference between them? In any case, I think for KX023 there is only KX023-1025, so I suppose I can omit it. I used KX023-1025 as name mostly for consistency, although I did change the convention a bit already since "kionix,kx0231025" was terribly readable. So both the current "kionix,kx023-1025" and "kionix,kx023" would be fine with me, any other opinions? Thanks! Stephan [1]: https://kionixfs.azureedge.net/en/datasheet/KXTF9-4100%20Specifications%20Rev%206.pdf [2]: https://kionixfs.azureedge.net/en/datasheet/KXTF9-2050%20Specifications%20Rev%207.pdf
On Tue, May 11, 2021 at 04:38:06PM +0200, Stephan Gerhold wrote: > Hi Michał, > > On Tue, May 11, 2021 at 04:28:47PM +0200, Michał Mirosław wrote: > > On Tue, May 11, 2021 at 11:54:06AM +0200, Stephan Gerhold wrote: > > > KX023-1025 [1] is another accelerometer from Kionix that has lots > > > of additional functionality compared to KXCJK-1013. It combines the > > > motion interrupt functionality from KXCJK with the tap detection > > > from KXTF9, plus a lot more other functionality. > > When I researched KXTF9 support it occurred to me that the -10xx part is > > duplicating the information in 'KXyyy' - it seems to be a project number > > or something. I would suggest to use just 'kx023' prefix for the code > > and DT but leave the full identification in the comments/description. > There do seem to be two different KXTF9 from Kionix, a KXTF9-4100 [1] > and a KXTF9-2050 [2] with separate datasheets. Have you checked if there > is a meaningful difference between them? I haven't compared them thoroughly, but the versions seem to differ only in power consumption (maybe a different manufacturing process?). The register sets seem the same. Best Regards Michał Mirosław
On Tue, 11 May 2021 16:50:51 +0200 Michał Mirosław <mirq-linux@rere.qmqm.pl> wrote: > On Tue, May 11, 2021 at 04:38:06PM +0200, Stephan Gerhold wrote: > > Hi Michał, > > > > On Tue, May 11, 2021 at 04:28:47PM +0200, Michał Mirosław wrote: > > > On Tue, May 11, 2021 at 11:54:06AM +0200, Stephan Gerhold wrote: > > > > KX023-1025 [1] is another accelerometer from Kionix that has lots > > > > of additional functionality compared to KXCJK-1013. It combines the > > > > motion interrupt functionality from KXCJK with the tap detection > > > > from KXTF9, plus a lot more other functionality. > > > When I researched KXTF9 support it occurred to me that the -10xx part is > > > duplicating the information in 'KXyyy' - it seems to be a project number > > > or something. I would suggest to use just 'kx023' prefix for the code > > > and DT but leave the full identification in the comments/description. > > There do seem to be two different KXTF9 from Kionix, a KXTF9-4100 [1] > > and a KXTF9-2050 [2] with separate datasheets. Have you checked if there > > is a meaningful difference between them? > > I haven't compared them thoroughly, but the versions seem to differ only > in power consumption (maybe a different manufacturing process?). The > register sets seem the same. Differ in expected Vdd supply voltage. 3.3 vs 1.8V . Looks like this has knock on effects on things like self test values. So I'd argue it's worth keeping the distinction for device tree. We could do a double compatible compatible = kionix,kx023-1024, konix,kx023; but may be too late to do that now. Jonathan > > Best Regards > Michał Mirosław