* [PATCH v4 0/3] iio: adc: mt6360: Add ADC driver for MT6360
@ 2020-09-15 17:36 Gene Chen
2020-09-15 17:36 ` [PATCH v4 1/3] dt-bindings: iio: adc: add bindings doc for MT6360 ADC Gene Chen
` (2 more replies)
0 siblings, 3 replies; 13+ messages in thread
From: Gene Chen @ 2020-09-15 17:36 UTC (permalink / raw)
To: jic23, matthias.bgg
Cc: knaack.h, lars, pmeerw, linux-iio, linux-arm-kernel,
linux-mediatek, linux-kernel, gene_chen, Wilma.Wu, shufan_lee,
cy_huang, benjamin.chao
This patch series add MT6360 ADC support contains driver, testing document
and binding document
Gene Chen (2)
dt-bindings: iio: adc: add bindings doc for MT6360 ADC
Documentation: ABI: testing: mt6360: Add ADC sysfs guideline
iio: adc: mt6360: Add ADC driver for MT6360
Documentation/ABI/testing/sysfs-bus-iio-adc-mt6360 | 83 ++
Documentation/devicetree/bindings/iio/adc/mediatek,mt6360.yaml | 32
drivers/iio/adc/Kconfig | 11
drivers/iio/adc/Makefile | 1
drivers/iio/adc/mt6360-adc.c | 357 ++++++++++
5 files changed, 484 insertions(+)
changelogs between v1 & v2
- adc: use IIO_CHAN_INFO_PROCESSED only
- adc: use devm_iio_triggered_buffer_setup
- adc: use use s64 to record timestamp
changelogs between v2 & v3
- Rearrange include file order by alphabet
- Set line length constraint below 100
- Add Document for testing adc sysfs node guideline
- Set compiler 64 bit aligned when handle iio timestamp
changelogs between v3 & v4
- Fix sysfs guideline description
- Replace iio channel processed by raw/scale/offset
- Add comment of read adc flow for special HW design
^ permalink raw reply [flat|nested] 13+ messages in thread* [PATCH v4 1/3] dt-bindings: iio: adc: add bindings doc for MT6360 ADC 2020-09-15 17:36 [PATCH v4 0/3] iio: adc: mt6360: Add ADC driver for MT6360 Gene Chen @ 2020-09-15 17:36 ` Gene Chen 2020-09-17 17:33 ` Jonathan Cameron 2020-09-15 17:36 ` [PATCH v4 2/3] Documentation: ABI: testing: mt6360: Add ADC sysfs guideline Gene Chen 2020-09-15 17:36 ` [PATCH v4 3/3] iio: adc: mt6360: Add ADC driver for MT6360 Gene Chen 2 siblings, 1 reply; 13+ messages in thread From: Gene Chen @ 2020-09-15 17:36 UTC (permalink / raw) To: jic23, matthias.bgg Cc: knaack.h, lars, pmeerw, linux-iio, linux-arm-kernel, linux-mediatek, linux-kernel, gene_chen, Wilma.Wu, shufan_lee, cy_huang, benjamin.chao From: Gene Chen <gene_chen@richtek.com> This change adds the binding doc for the MT6360 ADC. Signed-off-by: Gene Chen <gene_chen@richtek.com> --- .../bindings/iio/adc/mediatek,mt6360.yaml | 32 ++++++++++++++++++++++ 1 file changed, 32 insertions(+) create mode 100644 Documentation/devicetree/bindings/iio/adc/mediatek,mt6360.yaml diff --git a/Documentation/devicetree/bindings/iio/adc/mediatek,mt6360.yaml b/Documentation/devicetree/bindings/iio/adc/mediatek,mt6360.yaml new file mode 100644 index 0000000..2fa2fe7 --- /dev/null +++ b/Documentation/devicetree/bindings/iio/adc/mediatek,mt6360.yaml @@ -0,0 +1,32 @@ +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/iio/adc/mediatek,mt6360.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Mediatek MT6360 and similar ADCs + +maintainers: + - Gene Chen <gene_chen@richtek.com> + +description: | + Family of simple ADCs with i2c interface and internal references. + +properties: + compatible: + const: mediatek,mt6360-adc + + "#io-channel-cells": + const: 1 + +required: + - compatible + - "#io-channel-cells" + +examples: + - | + adc { + compatible = "mediatek,mt6360-adc"; + #io-channel-cells = <1>; + }; +... -- 2.7.4 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v4 1/3] dt-bindings: iio: adc: add bindings doc for MT6360 ADC 2020-09-15 17:36 ` [PATCH v4 1/3] dt-bindings: iio: adc: add bindings doc for MT6360 ADC Gene Chen @ 2020-09-17 17:33 ` Jonathan Cameron 0 siblings, 0 replies; 13+ messages in thread From: Jonathan Cameron @ 2020-09-17 17:33 UTC (permalink / raw) To: Gene Chen Cc: matthias.bgg, knaack.h, lars, pmeerw, linux-iio, linux-arm-kernel, linux-mediatek, linux-kernel, gene_chen, Wilma.Wu, shufan_lee, cy_huang, benjamin.chao, Rob Herring, devicetree On Wed, 16 Sep 2020 01:36:07 +0800 Gene Chen <gene.chen.richtek@gmail.com> wrote: > From: Gene Chen <gene_chen@richtek.com> > > This change adds the binding doc for the MT6360 ADC. > > Signed-off-by: Gene Chen <gene_chen@richtek.com> Hi Gene A few things inline I missed before. Ideally this wants a device-tree ack which it isn't likely to get without cc'ing the binding Maintainers. I've added Rob and the list. Thanks, Jonathan > --- > .../bindings/iio/adc/mediatek,mt6360.yaml | 32 ++++++++++++++++++++++ > 1 file changed, 32 insertions(+) > create mode 100644 Documentation/devicetree/bindings/iio/adc/mediatek,mt6360.yaml > > diff --git a/Documentation/devicetree/bindings/iio/adc/mediatek,mt6360.yaml b/Documentation/devicetree/bindings/iio/adc/mediatek,mt6360.yaml > new file mode 100644 > index 0000000..2fa2fe7 > --- /dev/null > +++ b/Documentation/devicetree/bindings/iio/adc/mediatek,mt6360.yaml > @@ -0,0 +1,32 @@ > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/iio/adc/mediatek,mt6360.yaml# I think this should probably match the compatible as should the file name. (sorry missed this on previous review!) > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Mediatek MT6360 and similar ADCs > + > +maintainers: > + - Gene Chen <gene_chen@richtek.com> > + > +description: | > + Family of simple ADCs with i2c interface and internal references. > + > +properties: > + compatible: > + const: mediatek,mt6360-adc > + > + "#io-channel-cells": > + const: 1 > + > +required: > + - compatible > + - "#io-channel-cells" As it's fresh in my mind from forgetting it myself, can we add additionalProperties: false ? > + > +examples: > + - | > + adc { > + compatible = "mediatek,mt6360-adc"; > + #io-channel-cells = <1>; > + }; > +... ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v4 2/3] Documentation: ABI: testing: mt6360: Add ADC sysfs guideline 2020-09-15 17:36 [PATCH v4 0/3] iio: adc: mt6360: Add ADC driver for MT6360 Gene Chen 2020-09-15 17:36 ` [PATCH v4 1/3] dt-bindings: iio: adc: add bindings doc for MT6360 ADC Gene Chen @ 2020-09-15 17:36 ` Gene Chen 2020-09-17 17:42 ` Jonathan Cameron 2020-09-15 17:36 ` [PATCH v4 3/3] iio: adc: mt6360: Add ADC driver for MT6360 Gene Chen 2 siblings, 1 reply; 13+ messages in thread From: Gene Chen @ 2020-09-15 17:36 UTC (permalink / raw) To: jic23, matthias.bgg Cc: knaack.h, lars, pmeerw, linux-iio, linux-arm-kernel, linux-mediatek, linux-kernel, gene_chen, Wilma.Wu, shufan_lee, cy_huang, benjamin.chao From: Gene Chen <gene_chen@richtek.com> Add ABI documentation for mt6360 ADC sysfs interfaces. Signed-off-by: Gene Chen <gene_chen@richtek.com> --- Documentation/ABI/testing/sysfs-bus-iio-adc-mt6360 | 83 ++++++++++++++++++++++ 1 file changed, 83 insertions(+) create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-adc-mt6360 diff --git a/Documentation/ABI/testing/sysfs-bus-iio-adc-mt6360 b/Documentation/ABI/testing/sysfs-bus-iio-adc-mt6360 new file mode 100644 index 0000000..4b1c270 --- /dev/null +++ b/Documentation/ABI/testing/sysfs-bus-iio-adc-mt6360 @@ -0,0 +1,83 @@ +What: /sys/bus/iio/devices/iio:deviceX/in_voltage_USBID_input +KernelVersion: 5.8.0 +Contact: gene_chen@richtek.com +Description: + Indicated MT6360 USBID ADC which connected to connector ID pin. + Reading returns voltage in uV + +What: /sys/bus/iio/devices/iio:deviceX/in_voltage_VBUSDIV5_input +KernelVersion: 5.8.0 +Contact: gene_chen@richtek.com +Description: + Indicated MT6360 VBUS ADC with high accuracy + Reading returns voltage in uV + +What: /sys/bus/iio/devices/iio:deviceX/in_voltage_VBUSDIV2_input +KernelVersion: 5.8.0 +Contact: gene_chen@richtek.com +Description: + Indicated MT6360 VBUS ADC with low accuracy + Reading returns voltage in uV + +What: /sys/bus/iio/devices/iio:deviceX/in_voltage_VSYS_input +KernelVersion: 5.8.0 +Contact: gene_chen@richtek.com +Description: + Indicated MT6360 VSYS ADC + Reading returns voltage in uV + +What: /sys/bus/iio/devices/iio:deviceX/in_voltage_VBAT_input +KernelVersion: 5.8.0 +Contact: gene_chen@richtek.com +Description: + Indicated MT6360 VBAT ADC + Reading returns voltage in uV + +What: /sys/bus/iio/devices/iio:deviceX/in_current_IBUS_input +KernelVersion: 5.8.0 +Contact: gene_chen@richtek.com +Description: + Indicated MT6360 IBUS ADC + Reading returns current in uA + +What: /sys/bus/iio/devices/iio:deviceX/in_current_IBAT_input +KernelVersion: 5.8.0 +Contact: gene_chen@richtek.com +Description: + Indicated MT6360 IBAT ADC + Reading returns current in uA + +What: /sys/bus/iio/devices/iio:deviceX/in_voltage_CHG_VDDP_input +KernelVersion: 5.8.0 +Contact: gene_chen@richtek.com +Description: + Indicated MT6360 CHG_VDDP ADC + Reading returns voltage in uV + +What: /sys/bus/iio/devices/iio:deviceX/in_temp_TEMP_JC_input +KernelVersion: 5.8.0 +Contact: gene_chen@richtek.com +Description: + Indicated MT6360 IC junction temperature + Reading returns temperature in degree + +What: /sys/bus/iio/devices/iio:deviceX/in_voltage_VREF_TS_input +KernelVersion: 5.8.0 +Contact: gene_chen@richtek.com +Description: + Indicated MT6360 VREF_TS ADC + Reading returns voltage in uV + +What: /sys/bus/iio/devices/iio:deviceX/in_voltage_TS_input +KernelVersion: 5.8.0 +Contact: gene_chen@richtek.com +Description: + Indicated MT6360 TS ADC + Reading returns voltage in uV + +What: /sys/bus/iio/devices/iio:deviceX/timestamp +KernelVersion: 5.8.0 +Contact: gene_chen@richtek.com +Description: + Indicated MT6360 timestamp + Reading returns current timestamp in ms -- 2.7.4 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v4 2/3] Documentation: ABI: testing: mt6360: Add ADC sysfs guideline 2020-09-15 17:36 ` [PATCH v4 2/3] Documentation: ABI: testing: mt6360: Add ADC sysfs guideline Gene Chen @ 2020-09-17 17:42 ` Jonathan Cameron 2020-09-18 7:21 ` Gene Chen 0 siblings, 1 reply; 13+ messages in thread From: Jonathan Cameron @ 2020-09-17 17:42 UTC (permalink / raw) To: Gene Chen Cc: matthias.bgg, knaack.h, lars, pmeerw, linux-iio, linux-arm-kernel, linux-mediatek, linux-kernel, gene_chen, Wilma.Wu, shufan_lee, cy_huang, benjamin.chao, Cristian Pop On Wed, 16 Sep 2020 01:36:08 +0800 Gene Chen <gene.chen.richtek@gmail.com> wrote: > From: Gene Chen <gene_chen@richtek.com> > > Add ABI documentation for mt6360 ADC sysfs interfaces. > > Signed-off-by: Gene Chen <gene_chen@richtek.com> Would you consider using the proposed label attribute for channels? https://lore.kernel.org/linux-iio/20200916132115.81795-1-cristian.pop@analog.com/T/#u I'm hoping that will remove the need to have ext name used in the majority of cases and would like to know if it would work for you? It may not work for this particular case of course. Other comments inline. > --- > Documentation/ABI/testing/sysfs-bus-iio-adc-mt6360 | 83 ++++++++++++++++++++++ > 1 file changed, 83 insertions(+) > create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-adc-mt6360 > > diff --git a/Documentation/ABI/testing/sysfs-bus-iio-adc-mt6360 b/Documentation/ABI/testing/sysfs-bus-iio-adc-mt6360 > new file mode 100644 > index 0000000..4b1c270 > --- /dev/null > +++ b/Documentation/ABI/testing/sysfs-bus-iio-adc-mt6360 > @@ -0,0 +1,83 @@ > +What: /sys/bus/iio/devices/iio:deviceX/in_voltage_USBID_input The mixture of case is a bit ugly. Could we do in_voltage_usbin_input? > +KernelVersion: 5.8.0 > +Contact: gene_chen@richtek.com > +Description: > + Indicated MT6360 USBID ADC which connected to connector ID pin. > + Reading returns voltage in uV > + > +What: /sys/bus/iio/devices/iio:deviceX/in_voltage_VBUSDIV5_input > +KernelVersion: 5.8.0 > +Contact: gene_chen@richtek.com > +Description: > + Indicated MT6360 VBUS ADC with high accuracy > + Reading returns voltage in uV Why would we ever read the low accuracy version? > + > +What: /sys/bus/iio/devices/iio:deviceX/in_voltage_VBUSDIV2_input > +KernelVersion: 5.8.0 > +Contact: gene_chen@richtek.com > +Description: > + Indicated MT6360 VBUS ADC with low accuracy > + Reading returns voltage in uV > + > +What: /sys/bus/iio/devices/iio:deviceX/in_voltage_VSYS_input > +KernelVersion: 5.8.0 > +Contact: gene_chen@richtek.com > +Description: > + Indicated MT6360 VSYS ADC > + Reading returns voltage in uV > + > +What: /sys/bus/iio/devices/iio:deviceX/in_voltage_VBAT_input > +KernelVersion: 5.8.0 > +Contact: gene_chen@richtek.com > +Description: > + Indicated MT6360 VBAT ADC > + Reading returns voltage in uV > + > +What: /sys/bus/iio/devices/iio:deviceX/in_current_IBUS_input > +KernelVersion: 5.8.0 > +Contact: gene_chen@richtek.com > +Description: > + Indicated MT6360 IBUS ADC > + Reading returns current in uA Given voltage and current are already clear from the channel type, could we avoid the repetition? in_current_bus_input perhaps? > + > +What: /sys/bus/iio/devices/iio:deviceX/in_current_IBAT_input > +KernelVersion: 5.8.0 > +Contact: gene_chen@richtek.com > +Description: > + Indicated MT6360 IBAT ADC > + Reading returns current in uA > + > +What: /sys/bus/iio/devices/iio:deviceX/in_voltage_CHG_VDDP_input > +KernelVersion: 5.8.0 > +Contact: gene_chen@richtek.com > +Description: > + Indicated MT6360 CHG_VDDP ADC > + Reading returns voltage in uV > + > +What: /sys/bus/iio/devices/iio:deviceX/in_temp_TEMP_JC_input > +KernelVersion: 5.8.0 > +Contact: gene_chen@richtek.com > +Description: > + Indicated MT6360 IC junction temperature > + Reading returns temperature in degree > + > +What: /sys/bus/iio/devices/iio:deviceX/in_voltage_VREF_TS_input > +KernelVersion: 5.8.0 > +Contact: gene_chen@richtek.com > +Description: > + Indicated MT6360 VREF_TS ADC > + Reading returns voltage in uV > + > +What: /sys/bus/iio/devices/iio:deviceX/in_voltage_TS_input > +KernelVersion: 5.8.0 > +Contact: gene_chen@richtek.com > +Description: > + Indicated MT6360 TS ADC > + Reading returns voltage in uV > + > +What: /sys/bus/iio/devices/iio:deviceX/timestamp > +KernelVersion: 5.8.0 > +Contact: gene_chen@richtek.com > +Description: > + Indicated MT6360 timestamp > + Reading returns current timestamp in ms That's an odd bit of ABI. Why would we want to read the current timestamp from sysfs? Timestamps in IIO also tend to be in nano seconds. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v4 2/3] Documentation: ABI: testing: mt6360: Add ADC sysfs guideline 2020-09-17 17:42 ` Jonathan Cameron @ 2020-09-18 7:21 ` Gene Chen 2020-09-18 8:03 ` Jonathan Cameron 0 siblings, 1 reply; 13+ messages in thread From: Gene Chen @ 2020-09-18 7:21 UTC (permalink / raw) To: Jonathan Cameron Cc: Matthias Brugger, knaack.h, lars, pmeerw, linux-iio, linux-arm Mailing List, moderated list:ARM/Mediatek SoC support, Linux Kernel Mailing List, Gene Chen, Wilma.Wu, shufan_lee, cy_huang, benjamin.chao, Cristian Pop Jonathan Cameron <jic23@kernel.org> 於 2020年9月18日 週五 上午1:43寫道： > > On Wed, 16 Sep 2020 01:36:08 +0800 > Gene Chen <gene.chen.richtek@gmail.com> wrote: > > > From: Gene Chen <gene_chen@richtek.com> > > > > Add ABI documentation for mt6360 ADC sysfs interfaces. > > > > Signed-off-by: Gene Chen <gene_chen@richtek.com> > Would you consider using the proposed label attribute for channels? > > https://lore.kernel.org/linux-iio/20200916132115.81795-1-cristian.pop@analog.com/T/#u > > I'm hoping that will remove the need to have ext name used in the majority of > cases and would like to know if it would work for you? > It may not work for this particular case of course. > > Other comments inline. > because of ADC layout is fixed, I can't switch channel to specific purpose for userspace. > > --- > > Documentation/ABI/testing/sysfs-bus-iio-adc-mt6360 | 83 ++++++++++++++++++++++ > > 1 file changed, 83 insertions(+) > > create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-adc-mt6360 > > > > diff --git a/Documentation/ABI/testing/sysfs-bus-iio-adc-mt6360 b/Documentation/ABI/testing/sysfs-bus-iio-adc-mt6360 > > new file mode 100644 > > index 0000000..4b1c270 > > --- /dev/null > > +++ b/Documentation/ABI/testing/sysfs-bus-iio-adc-mt6360 > > @@ -0,0 +1,83 @@ > > +What: /sys/bus/iio/devices/iio:deviceX/in_voltage_USBID_input > > > The mixture of case is a bit ugly. Could we do > in_voltage_usbin_input? > ACK > > +KernelVersion: 5.8.0 > > +Contact: gene_chen@richtek.com > > +Description: > > + Indicated MT6360 USBID ADC which connected to connector ID pin. > > + Reading returns voltage in uV > > + > > +What: /sys/bus/iio/devices/iio:deviceX/in_voltage_VBUSDIV5_input > > > +KernelVersion: 5.8.0 > > +Contact: gene_chen@richtek.com > > +Description: > > + Indicated MT6360 VBUS ADC with high accuracy > > + Reading returns voltage in uV > > Why would we ever read the low accuracy version? > > > + > > +What: /sys/bus/iio/devices/iio:deviceX/in_voltage_VBUSDIV2_input > > +KernelVersion: 5.8.0 > > +Contact: gene_chen@richtek.com > > +Description: > > + Indicated MT6360 VBUS ADC with low accuracy > > + Reading returns voltage in uV > > + > > +What: /sys/bus/iio/devices/iio:deviceX/in_voltage_VSYS_input > > +KernelVersion: 5.8.0 > > +Contact: gene_chen@richtek.com > > +Description: > > + Indicated MT6360 VSYS ADC > > + Reading returns voltage in uV > > + > > +What: /sys/bus/iio/devices/iio:deviceX/in_voltage_VBAT_input > > +KernelVersion: 5.8.0 > > +Contact: gene_chen@richtek.com > > +Description: > > + Indicated MT6360 VBAT ADC > > + Reading returns voltage in uV > > + > > +What: /sys/bus/iio/devices/iio:deviceX/in_current_IBUS_input > > +KernelVersion: 5.8.0 > > +Contact: gene_chen@richtek.com > > +Description: > > + Indicated MT6360 IBUS ADC > > + Reading returns current in uA > Given voltage and current are already clear from the channel type, > could we avoid the repetition? > > in_current_bus_input perhaps? > ACK > > + > > +What: /sys/bus/iio/devices/iio:deviceX/in_current_IBAT_input > > +KernelVersion: 5.8.0 > > +Contact: gene_chen@richtek.com > > +Description: > > + Indicated MT6360 IBAT ADC > > + Reading returns current in uA > > + > > +What: /sys/bus/iio/devices/iio:deviceX/in_voltage_CHG_VDDP_input > > +KernelVersion: 5.8.0 > > +Contact: gene_chen@richtek.com > > +Description: > > + Indicated MT6360 CHG_VDDP ADC > > + Reading returns voltage in uV > > + > > +What: /sys/bus/iio/devices/iio:deviceX/in_temp_TEMP_JC_input > > +KernelVersion: 5.8.0 > > +Contact: gene_chen@richtek.com > > +Description: > > + Indicated MT6360 IC junction temperature > > + Reading returns temperature in degree > > + > > +What: /sys/bus/iio/devices/iio:deviceX/in_voltage_VREF_TS_input > > +KernelVersion: 5.8.0 > > +Contact: gene_chen@richtek.com > > +Description: > > + Indicated MT6360 VREF_TS ADC > > + Reading returns voltage in uV > > + > > +What: /sys/bus/iio/devices/iio:deviceX/in_voltage_TS_input > > +KernelVersion: 5.8.0 > > +Contact: gene_chen@richtek.com > > +Description: > > + Indicated MT6360 TS ADC > > + Reading returns voltage in uV > > + > > +What: /sys/bus/iio/devices/iio:deviceX/timestamp > > +KernelVersion: 5.8.0 > > +Contact: gene_chen@richtek.com > > +Description: > > + Indicated MT6360 timestamp > > + Reading returns current timestamp in ms > > That's an odd bit of ABI. Why would we want to read the current timestamp from > sysfs? Timestamps in IIO also tend to be in nano seconds. > > > > ACK, I will remove this. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v4 2/3] Documentation: ABI: testing: mt6360: Add ADC sysfs guideline 2020-09-18 7:21 ` Gene Chen @ 2020-09-18 8:03 ` Jonathan Cameron 2020-09-18 10:33 ` Gene Chen 0 siblings, 1 reply; 13+ messages in thread From: Jonathan Cameron @ 2020-09-18 8:03 UTC (permalink / raw) To: Gene Chen Cc: Jonathan Cameron, Matthias Brugger, knaack.h, lars, pmeerw, linux-iio, linux-arm Mailing List, moderated list:ARM/Mediatek SoC support, Linux Kernel Mailing List, Gene Chen, Wilma.Wu, shufan_lee, cy_huang, benjamin.chao, Cristian Pop On Fri, 18 Sep 2020 15:21:44 +0800 Gene Chen <gene.chen.richtek@gmail.com> wrote: > Jonathan Cameron <jic23@kernel.org> 於 2020年9月18日 週五 上午1:43寫道： > > > > On Wed, 16 Sep 2020 01:36:08 +0800 > > Gene Chen <gene.chen.richtek@gmail.com> wrote: > > > > > From: Gene Chen <gene_chen@richtek.com> > > > > > > Add ABI documentation for mt6360 ADC sysfs interfaces. > > > > > > Signed-off-by: Gene Chen <gene_chen@richtek.com> > > Would you consider using the proposed label attribute for channels? > > > > https://lore.kernel.org/linux-iio/20200916132115.81795-1-cristian.pop@analog.com/T/#u > > > > I'm hoping that will remove the need to have ext name used in the majority of > > cases and would like to know if it would work for you? > > It may not work for this particular case of course. > > > > Other comments inline. > > > > because of ADC layout is fixed, I can't switch channel to specific > purpose for userspace. That patch set doesn't allow userspace to change the purpose. It provides a *_label attribute for each channel to allow for identification of the channel. That can be provided by ACPI / DT or can be provided by the driver itself. The advantage is that it removes the nasty freeform parsing that is needed to work out the filenames. > > > > --- > > > Documentation/ABI/testing/sysfs-bus-iio-adc-mt6360 | 83 ++++++++++++++++++++++ > > > 1 file changed, 83 insertions(+) > > > create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-adc-mt6360 > > > > > > diff --git a/Documentation/ABI/testing/sysfs-bus-iio-adc-mt6360 b/Documentation/ABI/testing/sysfs-bus-iio-adc-mt6360 > > > new file mode 100644 > > > index 0000000..4b1c270 > > > --- /dev/null > > > +++ b/Documentation/ABI/testing/sysfs-bus-iio-adc-mt6360 > > > @@ -0,0 +1,83 @@ > > > +What: /sys/bus/iio/devices/iio:deviceX/in_voltage_USBID_input > > > > > > The mixture of case is a bit ugly. Could we do > > in_voltage_usbin_input? > > > > ACK > > > > +KernelVersion: 5.8.0 > > > +Contact: gene_chen@richtek.com > > > +Description: > > > + Indicated MT6360 USBID ADC which connected to connector ID pin. > > > + Reading returns voltage in uV > > > + > > > +What: /sys/bus/iio/devices/iio:deviceX/in_voltage_VBUSDIV5_input > > > > > +KernelVersion: 5.8.0 > > > +Contact: gene_chen@richtek.com > > > +Description: > > > + Indicated MT6360 VBUS ADC with high accuracy > > > + Reading returns voltage in uV > > > > Why would we ever read the low accuracy version? > > > > > + > > > +What: /sys/bus/iio/devices/iio:deviceX/in_voltage_VBUSDIV2_input > > > +KernelVersion: 5.8.0 > > > +Contact: gene_chen@richtek.com > > > +Description: > > > + Indicated MT6360 VBUS ADC with low accuracy > > > + Reading returns voltage in uV > > > + > > > +What: /sys/bus/iio/devices/iio:deviceX/in_voltage_VSYS_input > > > +KernelVersion: 5.8.0 > > > +Contact: gene_chen@richtek.com > > > +Description: > > > + Indicated MT6360 VSYS ADC > > > + Reading returns voltage in uV > > > + > > > +What: /sys/bus/iio/devices/iio:deviceX/in_voltage_VBAT_input > > > +KernelVersion: 5.8.0 > > > +Contact: gene_chen@richtek.com > > > +Description: > > > + Indicated MT6360 VBAT ADC > > > + Reading returns voltage in uV > > > + > > > +What: /sys/bus/iio/devices/iio:deviceX/in_current_IBUS_input > > > +KernelVersion: 5.8.0 > > > +Contact: gene_chen@richtek.com > > > +Description: > > > + Indicated MT6360 IBUS ADC > > > + Reading returns current in uA > > Given voltage and current are already clear from the channel type, > > could we avoid the repetition? > > > > in_current_bus_input perhaps? > > > > ACK > > > > + > > > +What: /sys/bus/iio/devices/iio:deviceX/in_current_IBAT_input > > > +KernelVersion: 5.8.0 > > > +Contact: gene_chen@richtek.com > > > +Description: > > > + Indicated MT6360 IBAT ADC > > > + Reading returns current in uA > > > + > > > +What: /sys/bus/iio/devices/iio:deviceX/in_voltage_CHG_VDDP_input > > > +KernelVersion: 5.8.0 > > > +Contact: gene_chen@richtek.com > > > +Description: > > > + Indicated MT6360 CHG_VDDP ADC > > > + Reading returns voltage in uV > > > + > > > +What: /sys/bus/iio/devices/iio:deviceX/in_temp_TEMP_JC_input > > > +KernelVersion: 5.8.0 > > > +Contact: gene_chen@richtek.com > > > +Description: > > > + Indicated MT6360 IC junction temperature > > > + Reading returns temperature in degree > > > + > > > +What: /sys/bus/iio/devices/iio:deviceX/in_voltage_VREF_TS_input > > > +KernelVersion: 5.8.0 > > > +Contact: gene_chen@richtek.com > > > +Description: > > > + Indicated MT6360 VREF_TS ADC > > > + Reading returns voltage in uV > > > + > > > +What: /sys/bus/iio/devices/iio:deviceX/in_voltage_TS_input > > > +KernelVersion: 5.8.0 > > > +Contact: gene_chen@richtek.com > > > +Description: > > > + Indicated MT6360 TS ADC > > > + Reading returns voltage in uV > > > + > > > +What: /sys/bus/iio/devices/iio:deviceX/timestamp > > > +KernelVersion: 5.8.0 > > > +Contact: gene_chen@richtek.com > > > +Description: > > > + Indicated MT6360 timestamp > > > + Reading returns current timestamp in ms > > > > That's an odd bit of ABI. Why would we want to read the current timestamp from > > sysfs? Timestamps in IIO also tend to be in nano seconds. > > > > > > > > > > ACK, I will remove this. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v4 2/3] Documentation: ABI: testing: mt6360: Add ADC sysfs guideline 2020-09-18 8:03 ` Jonathan Cameron @ 2020-09-18 10:33 ` Gene Chen 2020-09-18 15:12 ` Jonathan Cameron 0 siblings, 1 reply; 13+ messages in thread From: Gene Chen @ 2020-09-18 10:33 UTC (permalink / raw) To: Jonathan Cameron Cc: Jonathan Cameron, Matthias Brugger, knaack.h, lars, pmeerw, linux-iio, linux-arm Mailing List, moderated list:ARM/Mediatek SoC support, Linux Kernel Mailing List, Gene Chen, Wilma.Wu, shufan_lee, cy_huang, benjamin.chao, Cristian Pop Jonathan Cameron <Jonathan.Cameron@huawei.com> 於 2020年9月18日 週五 下午4:05寫道： > > On Fri, 18 Sep 2020 15:21:44 +0800 > Gene Chen <gene.chen.richtek@gmail.com> wrote: > > > Jonathan Cameron <jic23@kernel.org> 於 2020年9月18日 週五 上午1:43寫道： > > > > > > On Wed, 16 Sep 2020 01:36:08 +0800 > > > Gene Chen <gene.chen.richtek@gmail.com> wrote: > > > > > > > From: Gene Chen <gene_chen@richtek.com> > > > > > > > > Add ABI documentation for mt6360 ADC sysfs interfaces. > > > > > > > > Signed-off-by: Gene Chen <gene_chen@richtek.com> > > > Would you consider using the proposed label attribute for channels? > > > > > > https://lore.kernel.org/linux-iio/20200916132115.81795-1-cristian.pop@analog.com/T/#u > > > > > > I'm hoping that will remove the need to have ext name used in the majority of > > > cases and would like to know if it would work for you? > > > It may not work for this particular case of course. > > > > > > Other comments inline. > > > > > > > because of ADC layout is fixed, I can't switch channel to specific > > purpose for userspace. > > That patch set doesn't allow userspace to change the purpose. It provides > a *_label attribute for each channel to allow for identification of the channel. > That can be provided by ACPI / DT or can be provided by the driver itself. > The advantage is that it removes the nasty freeform parsing that is needed > to work out the filenames. > May I ask how to get this patch for test the labels? I supposed userspace catch meanings by iio device sysfs node name. The label defined in DT means it can be modified. But actually shouldn't. > > > > > > --- > > > > Documentation/ABI/testing/sysfs-bus-iio-adc-mt6360 | 83 ++++++++++++++++++++++ > > > > 1 file changed, 83 insertions(+) > > > > create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-adc-mt6360 > > > > > > > > diff --git a/Documentation/ABI/testing/sysfs-bus-iio-adc-mt6360 b/Documentation/ABI/testing/sysfs-bus-iio-adc-mt6360 > > > > new file mode 100644 > > > > index 0000000..4b1c270 > > > > --- /dev/null > > > > +++ b/Documentation/ABI/testing/sysfs-bus-iio-adc-mt6360 > > > > @@ -0,0 +1,83 @@ > > > > +What: /sys/bus/iio/devices/iio:deviceX/in_voltage_USBID_input > > > > > > > > > The mixture of case is a bit ugly. Could we do > > > in_voltage_usbin_input? > > > > > > > ACK > > > > > > +KernelVersion: 5.8.0 > > > > +Contact: gene_chen@richtek.com > > > > +Description: > > > > + Indicated MT6360 USBID ADC which connected to connector ID pin. > > > > + Reading returns voltage in uV > > > > + > > > > +What: /sys/bus/iio/devices/iio:deviceX/in_voltage_VBUSDIV5_input > > > > > > > +KernelVersion: 5.8.0 > > > > +Contact: gene_chen@richtek.com > > > > +Description: > > > > + Indicated MT6360 VBUS ADC with high accuracy > > > > + Reading returns voltage in uV > > > > > > Why would we ever read the low accuracy version? > > > VBUSDIV5 with lower accuracy(+-75mA) higher measure range(1~22V) VBUSDIV2 with higher accracy (+-30mA) lower measure range(1~9.76V) I will fix the description > > > > + > > > > +What: /sys/bus/iio/devices/iio:deviceX/in_voltage_VBUSDIV2_input > > > > +KernelVersion: 5.8.0 > > > > +Contact: gene_chen@richtek.com > > > > +Description: > > > > + Indicated MT6360 VBUS ADC with low accuracy > > > > + Reading returns voltage in uV > > > > + > > > > +What: /sys/bus/iio/devices/iio:deviceX/in_voltage_VSYS_input > > > > +KernelVersion: 5.8.0 > > > > +Contact: gene_chen@richtek.com > > > > +Description: > > > > + Indicated MT6360 VSYS ADC > > > > + Reading returns voltage in uV > > > > + > > > > +What: /sys/bus/iio/devices/iio:deviceX/in_voltage_VBAT_input > > > > +KernelVersion: 5.8.0 > > > > +Contact: gene_chen@richtek.com > > > > +Description: > > > > + Indicated MT6360 VBAT ADC > > > > + Reading returns voltage in uV > > > > + > > > > +What: /sys/bus/iio/devices/iio:deviceX/in_current_IBUS_input > > > > +KernelVersion: 5.8.0 > > > > +Contact: gene_chen@richtek.com > > > > +Description: > > > > + Indicated MT6360 IBUS ADC > > > > + Reading returns current in uA > > > Given voltage and current are already clear from the channel type, > > > could we avoid the repetition? > > > > > > in_current_bus_input perhaps? > > > > > > > ACK > > > > > > + > > > > +What: /sys/bus/iio/devices/iio:deviceX/in_current_IBAT_input > > > > +KernelVersion: 5.8.0 > > > > +Contact: gene_chen@richtek.com > > > > +Description: > > > > + Indicated MT6360 IBAT ADC > > > > + Reading returns current in uA > > > > + > > > > +What: /sys/bus/iio/devices/iio:deviceX/in_voltage_CHG_VDDP_input > > > > +KernelVersion: 5.8.0 > > > > +Contact: gene_chen@richtek.com > > > > +Description: > > > > + Indicated MT6360 CHG_VDDP ADC > > > > + Reading returns voltage in uV > > > > + > > > > +What: /sys/bus/iio/devices/iio:deviceX/in_temp_TEMP_JC_input > > > > +KernelVersion: 5.8.0 > > > > +Contact: gene_chen@richtek.com > > > > +Description: > > > > + Indicated MT6360 IC junction temperature > > > > + Reading returns temperature in degree > > > > + > > > > +What: /sys/bus/iio/devices/iio:deviceX/in_voltage_VREF_TS_input > > > > +KernelVersion: 5.8.0 > > > > +Contact: gene_chen@richtek.com > > > > +Description: > > > > + Indicated MT6360 VREF_TS ADC > > > > + Reading returns voltage in uV > > > > + > > > > +What: /sys/bus/iio/devices/iio:deviceX/in_voltage_TS_input > > > > +KernelVersion: 5.8.0 > > > > +Contact: gene_chen@richtek.com > > > > +Description: > > > > + Indicated MT6360 TS ADC > > > > + Reading returns voltage in uV > > > > + > > > > +What: /sys/bus/iio/devices/iio:deviceX/timestamp > > > > +KernelVersion: 5.8.0 > > > > +Contact: gene_chen@richtek.com > > > > +Description: > > > > + Indicated MT6360 timestamp > > > > + Reading returns current timestamp in ms > > > > > > That's an odd bit of ABI. Why would we want to read the current timestamp from > > > sysfs? Timestamps in IIO also tend to be in nano seconds. > > > > > > > > > > > > > > > > ACK, I will remove this. > > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v4 2/3] Documentation: ABI: testing: mt6360: Add ADC sysfs guideline 2020-09-18 10:33 ` Gene Chen @ 2020-09-18 15:12 ` Jonathan Cameron 0 siblings, 0 replies; 13+ messages in thread From: Jonathan Cameron @ 2020-09-18 15:12 UTC (permalink / raw) To: Gene Chen Cc: Jonathan Cameron, Matthias Brugger, knaack.h, lars, pmeerw, linux-iio, linux-arm Mailing List, moderated list:ARM/Mediatek SoC support, Linux Kernel Mailing List, Gene Chen, Wilma.Wu, shufan_lee, cy_huang, benjamin.chao, Cristian Pop On Fri, 18 Sep 2020 18:33:06 +0800 Gene Chen <gene.chen.richtek@gmail.com> wrote: > Jonathan Cameron <Jonathan.Cameron@huawei.com> 於 2020年9月18日 週五 下午4:05寫道： > > > > On Fri, 18 Sep 2020 15:21:44 +0800 > > Gene Chen <gene.chen.richtek@gmail.com> wrote: > > > > > Jonathan Cameron <jic23@kernel.org> 於 2020年9月18日 週五 上午1:43寫道： > > > > > > > > On Wed, 16 Sep 2020 01:36:08 +0800 > > > > Gene Chen <gene.chen.richtek@gmail.com> wrote: > > > > > > > > > From: Gene Chen <gene_chen@richtek.com> > > > > > > > > > > Add ABI documentation for mt6360 ADC sysfs interfaces. > > > > > > > > > > Signed-off-by: Gene Chen <gene_chen@richtek.com> > > > > Would you consider using the proposed label attribute for channels? > > > > > > > > https://lore.kernel.org/linux-iio/20200916132115.81795-1-cristian.pop@analog.com/T/#u > > > > > > > > I'm hoping that will remove the need to have ext name used in the majority of > > > > cases and would like to know if it would work for you? > > > > It may not work for this particular case of course. > > > > > > > > Other comments inline. > > > > > > > > > > because of ADC layout is fixed, I can't switch channel to specific > > > purpose for userspace. > > > > That patch set doesn't allow userspace to change the purpose. It provides > > a *_label attribute for each channel to allow for identification of the channel. > > That can be provided by ACPI / DT or can be provided by the driver itself. > > The advantage is that it removes the nasty freeform parsing that is needed > > to work out the filenames. > > > > May I ask how to get this patch for test the labels? You should be able to use the link above then click on "raw" next to the from line. That will download you a patch that you can apply with git am. > I supposed userspace catch meanings by iio device sysfs node name. That is the idea. > The label defined in DT means it can be modified. But actually shouldn't. In your case, I agree that they are fixed in the hardware so the driver should provide a read_label callback that simply prints the relevant const string when requested for a particular channel. For more general devices, the idea is that DT or similar can provide naming to indicate that a particular board uses this channel to measure the bus voltage or things like that. Here we don't need that flexibility. Jonathan > > > > > > > > > --- > > > > > Documentation/ABI/testing/sysfs-bus-iio-adc-mt6360 | 83 ++++++++++++++++++++++ > > > > > 1 file changed, 83 insertions(+) > > > > > create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-adc-mt6360 > > > > > > > > > > diff --git a/Documentation/ABI/testing/sysfs-bus-iio-adc-mt6360 b/Documentation/ABI/testing/sysfs-bus-iio-adc-mt6360 > > > > > new file mode 100644 > > > > > index 0000000..4b1c270 > > > > > --- /dev/null > > > > > +++ b/Documentation/ABI/testing/sysfs-bus-iio-adc-mt6360 > > > > > @@ -0,0 +1,83 @@ > > > > > +What: /sys/bus/iio/devices/iio:deviceX/in_voltage_USBID_input > > > > > > > > > > > > The mixture of case is a bit ugly. Could we do > > > > in_voltage_usbin_input? > > > > > > > > > > ACK > > > > > > > > +KernelVersion: 5.8.0 > > > > > +Contact: gene_chen@richtek.com > > > > > +Description: > > > > > + Indicated MT6360 USBID ADC which connected to connector ID pin. > > > > > + Reading returns voltage in uV > > > > > + > > > > > +What: /sys/bus/iio/devices/iio:deviceX/in_voltage_VBUSDIV5_input > > > > > > > > > +KernelVersion: 5.8.0 > > > > > +Contact: gene_chen@richtek.com > > > > > +Description: > > > > > + Indicated MT6360 VBUS ADC with high accuracy > > > > > + Reading returns voltage in uV > > > > > > > > Why would we ever read the low accuracy version? > > > > > > VBUSDIV5 with lower accuracy(+-75mA) higher measure range(1~22V) > VBUSDIV2 with higher accracy (+-30mA) lower measure range(1~9.76V) > I will fix the description Great. > > > > > > + > > > > > +What: /sys/bus/iio/devices/iio:deviceX/in_voltage_VBUSDIV2_input > > > > > +KernelVersion: 5.8.0 > > > > > +Contact: gene_chen@richtek.com > > > > > +Description: > > > > > + Indicated MT6360 VBUS ADC with low accuracy > > > > > + Reading returns voltage in uV > > > > > + > > > > > +What: /sys/bus/iio/devices/iio:deviceX/in_voltage_VSYS_input > > > > > +KernelVersion: 5.8.0 > > > > > +Contact: gene_chen@richtek.com > > > > > +Description: > > > > > + Indicated MT6360 VSYS ADC > > > > > + Reading returns voltage in uV > > > > > + > > > > > +What: /sys/bus/iio/devices/iio:deviceX/in_voltage_VBAT_input > > > > > +KernelVersion: 5.8.0 > > > > > +Contact: gene_chen@richtek.com > > > > > +Description: > > > > > + Indicated MT6360 VBAT ADC > > > > > + Reading returns voltage in uV > > > > > + > > > > > +What: /sys/bus/iio/devices/iio:deviceX/in_current_IBUS_input > > > > > +KernelVersion: 5.8.0 > > > > > +Contact: gene_chen@richtek.com > > > > > +Description: > > > > > + Indicated MT6360 IBUS ADC > > > > > + Reading returns current in uA > > > > Given voltage and current are already clear from the channel type, > > > > could we avoid the repetition? > > > > > > > > in_current_bus_input perhaps? > > > > > > > > > > ACK > > > > > > > > + > > > > > +What: /sys/bus/iio/devices/iio:deviceX/in_current_IBAT_input > > > > > +KernelVersion: 5.8.0 > > > > > +Contact: gene_chen@richtek.com > > > > > +Description: > > > > > + Indicated MT6360 IBAT ADC > > > > > + Reading returns current in uA > > > > > + > > > > > +What: /sys/bus/iio/devices/iio:deviceX/in_voltage_CHG_VDDP_input > > > > > +KernelVersion: 5.8.0 > > > > > +Contact: gene_chen@richtek.com > > > > > +Description: > > > > > + Indicated MT6360 CHG_VDDP ADC > > > > > + Reading returns voltage in uV > > > > > + > > > > > +What: /sys/bus/iio/devices/iio:deviceX/in_temp_TEMP_JC_input > > > > > +KernelVersion: 5.8.0 > > > > > +Contact: gene_chen@richtek.com > > > > > +Description: > > > > > + Indicated MT6360 IC junction temperature > > > > > + Reading returns temperature in degree > > > > > + > > > > > +What: /sys/bus/iio/devices/iio:deviceX/in_voltage_VREF_TS_input > > > > > +KernelVersion: 5.8.0 > > > > > +Contact: gene_chen@richtek.com > > > > > +Description: > > > > > + Indicated MT6360 VREF_TS ADC > > > > > + Reading returns voltage in uV > > > > > + > > > > > +What: /sys/bus/iio/devices/iio:deviceX/in_voltage_TS_input > > > > > +KernelVersion: 5.8.0 > > > > > +Contact: gene_chen@richtek.com > > > > > +Description: > > > > > + Indicated MT6360 TS ADC > > > > > + Reading returns voltage in uV > > > > > + > > > > > +What: /sys/bus/iio/devices/iio:deviceX/timestamp > > > > > +KernelVersion: 5.8.0 > > > > > +Contact: gene_chen@richtek.com > > > > > +Description: > > > > > + Indicated MT6360 timestamp > > > > > + Reading returns current timestamp in ms > > > > > > > > That's an odd bit of ABI. Why would we want to read the current timestamp from > > > > sysfs? Timestamps in IIO also tend to be in nano seconds. > > > > > > > > > > > > > > > > > > > > > > ACK, I will remove this. > > > > ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v4 3/3] iio: adc: mt6360: Add ADC driver for MT6360 2020-09-15 17:36 [PATCH v4 0/3] iio: adc: mt6360: Add ADC driver for MT6360 Gene Chen 2020-09-15 17:36 ` [PATCH v4 1/3] dt-bindings: iio: adc: add bindings doc for MT6360 ADC Gene Chen 2020-09-15 17:36 ` [PATCH v4 2/3] Documentation: ABI: testing: mt6360: Add ADC sysfs guideline Gene Chen @ 2020-09-15 17:36 ` Gene Chen 2020-09-17 17:53 ` Jonathan Cameron 2 siblings, 1 reply; 13+ messages in thread From: Gene Chen @ 2020-09-15 17:36 UTC (permalink / raw) To: jic23, matthias.bgg Cc: knaack.h, lars, pmeerw, linux-iio, linux-arm-kernel, linux-mediatek, linux-kernel, gene_chen, Wilma.Wu, shufan_lee, cy_huang, benjamin.chao From: Gene Chen <gene_chen@richtek.com> Add MT6360 ADC driver include Charger Current, Voltage, and Temperature. Signed-off-by: Gene Chen <gene_chen@richtek.com> --- drivers/iio/adc/Kconfig | 11 ++ drivers/iio/adc/Makefile | 1 + drivers/iio/adc/mt6360-adc.c | 357 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 369 insertions(+) create mode 100644 drivers/iio/adc/mt6360-adc.c diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig index 66d9cc0..8d36b66 100644 --- a/drivers/iio/adc/Kconfig +++ b/drivers/iio/adc/Kconfig @@ -703,6 +703,17 @@ config MCP3911 This driver can also be built as a module. If so, the module will be called mcp3911. +config MEDIATEK_MT6360_ADC + tristate "Mediatek MT6360 ADC driver" + depends on MFD_MT6360 + select IIO_BUFFER + select IIO_TRIGGERED_BUFFER + help + Say Y here to enable MT6360 ADC support. + Integrated for System Monitoring includes + is used in smartphones and tablets and supports a 11 channel + general purpose ADC. + config MEDIATEK_MT6577_AUXADC tristate "MediaTek AUXADC driver" depends on ARCH_MEDIATEK || COMPILE_TEST diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile index 90f94ad..5fca90a 100644 --- a/drivers/iio/adc/Makefile +++ b/drivers/iio/adc/Makefile @@ -65,6 +65,7 @@ obj-$(CONFIG_MAX9611) += max9611.o obj-$(CONFIG_MCP320X) += mcp320x.o obj-$(CONFIG_MCP3422) += mcp3422.o obj-$(CONFIG_MCP3911) += mcp3911.o +obj-$(CONFIG_MEDIATEK_MT6360_ADC) += mt6360-adc.o obj-$(CONFIG_MEDIATEK_MT6577_AUXADC) += mt6577_auxadc.o obj-$(CONFIG_MEN_Z188_ADC) += men_z188_adc.o obj-$(CONFIG_MESON_SARADC) += meson_saradc.o diff --git a/drivers/iio/adc/mt6360-adc.c b/drivers/iio/adc/mt6360-adc.c new file mode 100644 index 0000000..b256d0a --- /dev/null +++ b/drivers/iio/adc/mt6360-adc.c @@ -0,0 +1,357 @@ +// SPDX-License-Identifier: GPL-2.0 + +#include <linux/delay.h> +#include <linux/irq.h> +#include <linux/kernel.h> +#include <linux/ktime.h> +#include <linux/module.h> +#include <linux/mutex.h> +#include <linux/platform_device.h> +#include <linux/regmap.h> + +#include <linux/iio/buffer.h> +#include <linux/iio/iio.h> +#include <linux/iio/trigger_consumer.h> +#include <linux/iio/triggered_buffer.h> + +#define MT6360_REG_PMUCHGCTRL3 0x313 +#define MT6360_REG_PMUADCCFG 0x356 +#define MT6360_REG_PMUADCIDLET 0x358 +#define MT6360_REG_PMUADCRPT1 0x35A + +/* PMUCHGCTRL3 0x313 */ +#define MT6360_AICR_MASK GENMASK(7, 2) +#define MT6360_AICR_SHFT 2 +#define MT6360_AICR_400MA 0x6 +/* PMUADCCFG 0x356 */ +#define MT6360_ADCEN_MASK 0x8000 +/* PMUADCRPT1 0x35A */ +#define MT6360_PREFERCH_MASK GENMASK(7, 4) +#define MT6360_PREFERCH_SHFT 4 +#define MT6360_RPTCH_MASK GENMASK(3, 0) +#define MT6360_NO_PREFER 15 + +/* Time in ms */ +#define ADC_WAIT_TIME_MS 25 + +enum { + MT6360_CHAN_USBID = 0, + MT6360_CHAN_VBUSDIV5, + MT6360_CHAN_VBUSDIV2, + MT6360_CHAN_VSYS, + MT6360_CHAN_VBAT, + MT6360_CHAN_IBUS, + MT6360_CHAN_IBAT, + MT6360_CHAN_CHG_VDDP, + MT6360_CHAN_TEMP_JC, + MT6360_CHAN_VREF_TS, + MT6360_CHAN_TS, + MT6360_CHAN_MAX +}; + +struct mt6360_adc_data { + struct device *dev; + struct regmap *regmap; + struct mutex adc_lock; + ktime_t last_off_timestamps[MT6360_CHAN_MAX]; +}; + +static int mt6360_adc_read_channel(struct mt6360_adc_data *mad, int channel, int *val) +{ + __be16 adc_enable; + u8 rpt[3]; + ktime_t start_t, predict_end_t; + unsigned int pre_wait_time; + int ret; + + mutex_lock(&mad->adc_lock); + + /* Select the preferred ADC channel */ + ret = regmap_update_bits(mad->regmap, MT6360_REG_PMUADCRPT1, MT6360_PREFERCH_MASK, + channel << MT6360_PREFERCH_SHFT); + if (ret) + goto out_adc_lock; + + adc_enable = cpu_to_be16(MT6360_ADCEN_MASK | BIT(channel)); + ret = regmap_raw_write(mad->regmap, MT6360_REG_PMUADCCFG, (void *)&adc_enable, + sizeof(__be16)); + if (ret) + goto out_adc_lock; + + start_t = ktime_get(); + predict_end_t = ktime_add_ms(mad->last_off_timestamps[channel], 2 * ADC_WAIT_TIME_MS); + + if (ktime_after(start_t, predict_end_t)) + pre_wait_time = ADC_WAIT_TIME_MS; + else + pre_wait_time = 3 * ADC_WAIT_TIME_MS; + + msleep(pre_wait_time); + + while (true) { + ret = regmap_raw_read(mad->regmap, MT6360_REG_PMUADCRPT1, rpt, sizeof(rpt)); + if (ret) + goto out_adc_conv; + + /* + * There are two functions, ZCV and TypeC OTP, running ADC VBAT and TS in + * background, and ADC samples are taken on a fixed frequency no matter read the + * previous one or not. + * To avoid conflict, We set minimum time threshold after enable ADC and + * check report channel is the same. + * The worst case is run the same ADC twice and background function is also running, + * ADC conversion sequence is desire channel before start ADC, background ADC, + * desire channel after start ADC. + * So the minimum correct data is three times of typical conversion time. + */ + if ((rpt[0] & MT6360_RPTCH_MASK) == channel) + break; + + msleep(ADC_WAIT_TIME_MS); + } + + /* rpt[1]: ADC_HI_BYTE, rpt[2]: ADC_LOW_BYTE */ + *val = be16_to_cpup((__be16 *)(rpt + 1)); + ret = IIO_VAL_INT; + +out_adc_conv: + /* Only keep ADC enable */ + adc_enable = cpu_to_be16(MT6360_ADCEN_MASK); + regmap_raw_write(mad->regmap, MT6360_REG_PMUADCCFG, (void *)&adc_enable, sizeof(__be16)); + mad->last_off_timestamps[channel] = ktime_get(); + /* Config prefer channel to NO_PREFER */ + regmap_update_bits(mad->regmap, MT6360_REG_PMUADCRPT1, MT6360_PREFERCH_MASK, + MT6360_NO_PREFER << MT6360_PREFERCH_SHFT); +out_adc_lock: + mutex_unlock(&mad->adc_lock); + + return ret; +} + +static int mt6360_adc_read_scale(struct mt6360_adc_data *mad, int channel, int *val, int *val2) +{ + unsigned int regval; + int ret; + + switch (channel) { + case MT6360_CHAN_USBID: + fallthrough; + case MT6360_CHAN_VSYS: + fallthrough; + case MT6360_CHAN_VBAT: + fallthrough; + case MT6360_CHAN_CHG_VDDP: + fallthrough; + case MT6360_CHAN_VREF_TS: + fallthrough; + case MT6360_CHAN_TS: + *val = 1250; + return IIO_VAL_INT; + case MT6360_CHAN_VBUSDIV5: + *val = 6250; + return IIO_VAL_INT; + case MT6360_CHAN_VBUSDIV2: + fallthrough; + case MT6360_CHAN_IBUS: + fallthrough; + case MT6360_CHAN_IBAT: + *val = 2500; + + if (channel == MT6360_CHAN_IBUS) { + /* IBUS will be affected by input current limit for the different Ron */ + /* Check whether the config is <400mA or not */ + ret = regmap_read(mad->regmap, MT6360_REG_PMUCHGCTRL3, ®val); + if (ret) + return ret; + + regval = (regval & MT6360_AICR_MASK) >> MT6360_AICR_SHFT; + if (regval < MT6360_AICR_400MA) + *val = 1900; + } + + return IIO_VAL_INT; + case MT6360_CHAN_TEMP_JC: + *val = 105; + *val2 = 100; + return IIO_VAL_FRACTIONAL; + } + + return -EINVAL; +} + +static int mt6360_adc_read_offset(struct mt6360_adc_data *mad, int channel, int *val) +{ + *val = (channel == MT6360_CHAN_TEMP_JC) ? -80 : 0; + return IIO_VAL_INT; + +} + +static int mt6360_adc_read_raw(struct iio_dev *iio_dev, const struct iio_chan_spec *chan, + int *val, int *val2, long mask) +{ + struct mt6360_adc_data *mad = iio_priv(iio_dev); + + switch (mask) { + case IIO_CHAN_INFO_RAW: + return mt6360_adc_read_channel(mad, chan->channel, val); + case IIO_CHAN_INFO_SCALE: + return mt6360_adc_read_scale(mad, chan->channel, val, val2); + case IIO_CHAN_INFO_OFFSET: + return mt6360_adc_read_offset(mad, chan->channel, val); + } + + return -EINVAL; +} + +static const struct iio_info mt6360_adc_iio_info = { + .read_raw = mt6360_adc_read_raw, +}; + +#define MT6360_ADC_CHAN(_idx, _type) { \ + .type = _type, \ + .channel = MT6360_CHAN_##_idx, \ + .scan_index = MT6360_CHAN_##_idx, \ + .extend_name = #_idx, \ + .datasheet_name = #_idx, \ + .scan_type = { \ + .sign = 'u', \ + .realbits = 16, \ + .storagebits = 16, \ + .endianness = IIO_CPU, \ + }, \ + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \ + BIT(IIO_CHAN_INFO_SCALE) | \ + BIT(IIO_CHAN_INFO_OFFSET), \ +} + +static const struct iio_chan_spec mt6360_adc_channels[] = { + MT6360_ADC_CHAN(USBID, IIO_VOLTAGE), + MT6360_ADC_CHAN(VBUSDIV5, IIO_VOLTAGE), + MT6360_ADC_CHAN(VBUSDIV2, IIO_VOLTAGE), + MT6360_ADC_CHAN(VSYS, IIO_VOLTAGE), + MT6360_ADC_CHAN(VBAT, IIO_VOLTAGE), + MT6360_ADC_CHAN(IBUS, IIO_CURRENT), + MT6360_ADC_CHAN(IBAT, IIO_CURRENT), + MT6360_ADC_CHAN(CHG_VDDP, IIO_VOLTAGE), + MT6360_ADC_CHAN(TEMP_JC, IIO_TEMP), + MT6360_ADC_CHAN(VREF_TS, IIO_VOLTAGE), + MT6360_ADC_CHAN(TS, IIO_VOLTAGE), + IIO_CHAN_SOFT_TIMESTAMP(MT6360_CHAN_MAX), +}; + +static irqreturn_t mt6360_adc_trigger_handler(int irq, void *p) +{ + struct iio_poll_func *pf = p; + struct iio_dev *indio_dev = pf->indio_dev; + struct mt6360_adc_data *mad = iio_priv(indio_dev); + struct { + u16 values[MT6360_CHAN_MAX]; + int64_t timestamp; + } data; + int i = 0, bit, val, ret; + + for_each_set_bit(bit, indio_dev->active_scan_mask, indio_dev->masklength) { + ret = mt6360_adc_read_channel(mad, bit, &val); + if (ret < 0) { + dev_warn(&indio_dev->dev, "Failed to get channel %d conversion val\n", bit); + goto out; + } + + data.values[i++] = val; + } + iio_push_to_buffers_with_timestamp(indio_dev, &data, iio_get_time_ns(indio_dev)); +out: + iio_trigger_notify_done(indio_dev->trig); + + return IRQ_HANDLED; +} + +static inline int mt6360_adc_reset(struct mt6360_adc_data *info) +{ + __be16 adc_enable; + ktime_t all_off_time; + int i, ret; + + /* Clear ADC idle wait time to 0 */ + ret = regmap_write(info->regmap, MT6360_REG_PMUADCIDLET, 0); + if (ret) + return ret; + + /* Only keep ADC enable, but keep all channels off */ + adc_enable = cpu_to_be16(MT6360_ADCEN_MASK); + ret = regmap_raw_write(info->regmap, MT6360_REG_PMUADCCFG, (void *)&adc_enable, + sizeof(__be16)); + if (ret) + return ret; + + /* Reset all channel off time to the current one */ + all_off_time = ktime_get(); + for (i = 0; i < MT6360_CHAN_MAX; i++) + info->last_off_timestamps[i] = all_off_time; + + return 0; +} + +static int mt6360_adc_probe(struct platform_device *pdev) +{ + struct mt6360_adc_data *mad; + struct regmap *regmap; + struct iio_dev *indio_dev; + int ret; + + regmap = dev_get_regmap(pdev->dev.parent, NULL); + if (!regmap) { + dev_err(&pdev->dev, "Failed to get parent regmap\n"); + return -ENODEV; + } + + indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*mad)); + if (!indio_dev) + return -ENOMEM; + + mad = iio_priv(indio_dev); + mad->dev = &pdev->dev; + mad->regmap = regmap; + mutex_init(&mad->adc_lock); + + ret = mt6360_adc_reset(mad); + if (ret < 0) { + dev_err(&pdev->dev, "Failed to reset adc\n"); + return ret; + } + + indio_dev->name = dev_name(&pdev->dev); + indio_dev->dev.parent = &pdev->dev; + indio_dev->info = &mt6360_adc_iio_info; + indio_dev->modes = INDIO_DIRECT_MODE; + indio_dev->channels = mt6360_adc_channels; + indio_dev->num_channels = ARRAY_SIZE(mt6360_adc_channels); + + ret = devm_iio_triggered_buffer_setup(&pdev->dev, indio_dev, NULL, + mt6360_adc_trigger_handler, NULL); + if (ret) { + dev_err(&pdev->dev, "Failed to allocate iio trigger buffer\n"); + return ret; + } + + return devm_iio_device_register(&pdev->dev, indio_dev); +} + +static const struct of_device_id __maybe_unused mt6360_adc_of_id[] = { + { .compatible = "mediatek,mt6360-adc", }, + {} +}; +MODULE_DEVICE_TABLE(of, mt6360_adc_of_id); + +static struct platform_driver mt6360_adc_driver = { + .driver = { + .name = "mt6360-adc", + .of_match_table = mt6360_adc_of_id, + }, + .probe = mt6360_adc_probe, +}; +module_platform_driver(mt6360_adc_driver); + +MODULE_AUTHOR("Gene Chen <gene_chen@richtek.com>"); +MODULE_DESCRIPTION("MT6360 ADC Driver"); +MODULE_LICENSE("GPL v2"); -- 2.7.4 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v4 3/3] iio: adc: mt6360: Add ADC driver for MT6360 2020-09-15 17:36 ` [PATCH v4 3/3] iio: adc: mt6360: Add ADC driver for MT6360 Gene Chen @ 2020-09-17 17:53 ` Jonathan Cameron 2020-09-18 8:04 ` Gene Chen 0 siblings, 1 reply; 13+ messages in thread From: Jonathan Cameron @ 2020-09-17 17:53 UTC (permalink / raw) To: Gene Chen Cc: matthias.bgg, knaack.h, lars, pmeerw, linux-iio, linux-arm-kernel, linux-mediatek, linux-kernel, gene_chen, Wilma.Wu, shufan_lee, cy_huang, benjamin.chao On Wed, 16 Sep 2020 01:36:09 +0800 Gene Chen <gene.chen.richtek@gmail.com> wrote: > From: Gene Chen <gene_chen@richtek.com> > > Add MT6360 ADC driver include Charger Current, Voltage, and > Temperature. > > Signed-off-by: Gene Chen <gene_chen@richtek.com> Comments inline. > --- > drivers/iio/adc/Kconfig | 11 ++ > drivers/iio/adc/Makefile | 1 + > drivers/iio/adc/mt6360-adc.c | 357 +++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 369 insertions(+) > create mode 100644 drivers/iio/adc/mt6360-adc.c > > diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig > index 66d9cc0..8d36b66 100644 > --- a/drivers/iio/adc/Kconfig > +++ b/drivers/iio/adc/Kconfig > @@ -703,6 +703,17 @@ config MCP3911 > This driver can also be built as a module. If so, the module will be > called mcp3911. > > +config MEDIATEK_MT6360_ADC > + tristate "Mediatek MT6360 ADC driver" > + depends on MFD_MT6360 > + select IIO_BUFFER > + select IIO_TRIGGERED_BUFFER > + help > + Say Y here to enable MT6360 ADC support. > + Integrated for System Monitoring includes > + is used in smartphones and tablets and supports a 11 channel > + general purpose ADC. > + > config MEDIATEK_MT6577_AUXADC > tristate "MediaTek AUXADC driver" > depends on ARCH_MEDIATEK || COMPILE_TEST > diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile > index 90f94ad..5fca90a 100644 > --- a/drivers/iio/adc/Makefile > +++ b/drivers/iio/adc/Makefile > @@ -65,6 +65,7 @@ obj-$(CONFIG_MAX9611) += max9611.o > obj-$(CONFIG_MCP320X) += mcp320x.o > obj-$(CONFIG_MCP3422) += mcp3422.o > obj-$(CONFIG_MCP3911) += mcp3911.o > +obj-$(CONFIG_MEDIATEK_MT6360_ADC) += mt6360-adc.o > obj-$(CONFIG_MEDIATEK_MT6577_AUXADC) += mt6577_auxadc.o > obj-$(CONFIG_MEN_Z188_ADC) += men_z188_adc.o > obj-$(CONFIG_MESON_SARADC) += meson_saradc.o > diff --git a/drivers/iio/adc/mt6360-adc.c b/drivers/iio/adc/mt6360-adc.c > new file mode 100644 > index 0000000..b256d0a > --- /dev/null > +++ b/drivers/iio/adc/mt6360-adc.c > @@ -0,0 +1,357 @@ > +// SPDX-License-Identifier: GPL-2.0 > + > +#include <linux/delay.h> > +#include <linux/irq.h> > +#include <linux/kernel.h> > +#include <linux/ktime.h> > +#include <linux/module.h> > +#include <linux/mutex.h> > +#include <linux/platform_device.h> > +#include <linux/regmap.h> > + > +#include <linux/iio/buffer.h> > +#include <linux/iio/iio.h> > +#include <linux/iio/trigger_consumer.h> > +#include <linux/iio/triggered_buffer.h> > + > +#define MT6360_REG_PMUCHGCTRL3 0x313 > +#define MT6360_REG_PMUADCCFG 0x356 > +#define MT6360_REG_PMUADCIDLET 0x358 > +#define MT6360_REG_PMUADCRPT1 0x35A > + > +/* PMUCHGCTRL3 0x313 */ > +#define MT6360_AICR_MASK GENMASK(7, 2) > +#define MT6360_AICR_SHFT 2 > +#define MT6360_AICR_400MA 0x6 > +/* PMUADCCFG 0x356 */ > +#define MT6360_ADCEN_MASK 0x8000 > +/* PMUADCRPT1 0x35A */ > +#define MT6360_PREFERCH_MASK GENMASK(7, 4) > +#define MT6360_PREFERCH_SHFT 4 > +#define MT6360_RPTCH_MASK GENMASK(3, 0) > +#define MT6360_NO_PREFER 15 > + > +/* Time in ms */ > +#define ADC_WAIT_TIME_MS 25 > + > +enum { > + MT6360_CHAN_USBID = 0, > + MT6360_CHAN_VBUSDIV5, > + MT6360_CHAN_VBUSDIV2, > + MT6360_CHAN_VSYS, > + MT6360_CHAN_VBAT, > + MT6360_CHAN_IBUS, > + MT6360_CHAN_IBAT, > + MT6360_CHAN_CHG_VDDP, > + MT6360_CHAN_TEMP_JC, > + MT6360_CHAN_VREF_TS, > + MT6360_CHAN_TS, > + MT6360_CHAN_MAX > +}; > + > +struct mt6360_adc_data { > + struct device *dev; > + struct regmap *regmap; > + struct mutex adc_lock; Please add a comment to document what the scope of the lock is. > + ktime_t last_off_timestamps[MT6360_CHAN_MAX]; > +}; > + > +static int mt6360_adc_read_channel(struct mt6360_adc_data *mad, int channel, int *val) > +{ > + __be16 adc_enable; > + u8 rpt[3]; > + ktime_t start_t, predict_end_t; > + unsigned int pre_wait_time; > + int ret; > + > + mutex_lock(&mad->adc_lock); > + > + /* Select the preferred ADC channel */ > + ret = regmap_update_bits(mad->regmap, MT6360_REG_PMUADCRPT1, MT6360_PREFERCH_MASK, > + channel << MT6360_PREFERCH_SHFT); > + if (ret) > + goto out_adc_lock; > + > + adc_enable = cpu_to_be16(MT6360_ADCEN_MASK | BIT(channel)); > + ret = regmap_raw_write(mad->regmap, MT6360_REG_PMUADCCFG, (void *)&adc_enable, Shouldn't be any need to cast a pointer explicitly to void * > + sizeof(__be16)); sizeof(adc_enable) preferred. > + if (ret) > + goto out_adc_lock; > + > + start_t = ktime_get(); > + predict_end_t = ktime_add_ms(mad->last_off_timestamps[channel], 2 * ADC_WAIT_TIME_MS); > + > + if (ktime_after(start_t, predict_end_t)) > + pre_wait_time = ADC_WAIT_TIME_MS; > + else > + pre_wait_time = 3 * ADC_WAIT_TIME_MS; > + > + msleep(pre_wait_time); > + > + while (true) { > + ret = regmap_raw_read(mad->regmap, MT6360_REG_PMUADCRPT1, rpt, sizeof(rpt)); > + if (ret) > + goto out_adc_conv; > + > + /* > + * There are two functions, ZCV and TypeC OTP, running ADC VBAT and TS in > + * background, and ADC samples are taken on a fixed frequency no matter read the > + * previous one or not. > + * To avoid conflict, We set minimum time threshold after enable ADC and > + * check report channel is the same. > + * The worst case is run the same ADC twice and background function is also running, > + * ADC conversion sequence is desire channel before start ADC, background ADC, > + * desire channel after start ADC. > + * So the minimum correct data is three times of typical conversion time. > + */ > + if ((rpt[0] & MT6360_RPTCH_MASK) == channel) > + break; > + > + msleep(ADC_WAIT_TIME_MS); > + } > + > + /* rpt[1]: ADC_HI_BYTE, rpt[2]: ADC_LOW_BYTE */ > + *val = be16_to_cpup((__be16 *)(rpt + 1)); To be entirely safe, probably need that to be an unaligned read? > + ret = IIO_VAL_INT; > + > +out_adc_conv: > + /* Only keep ADC enable */ > + adc_enable = cpu_to_be16(MT6360_ADCEN_MASK); > + regmap_raw_write(mad->regmap, MT6360_REG_PMUADCCFG, (void *)&adc_enable, sizeof(__be16)); sizeof(adc_enable) > + mad->last_off_timestamps[channel] = ktime_get(); > + /* Config prefer channel to NO_PREFER */ > + regmap_update_bits(mad->regmap, MT6360_REG_PMUADCRPT1, MT6360_PREFERCH_MASK, > + MT6360_NO_PREFER << MT6360_PREFERCH_SHFT); > +out_adc_lock: > + mutex_unlock(&mad->adc_lock); > + > + return ret; > +} > + > +static int mt6360_adc_read_scale(struct mt6360_adc_data *mad, int channel, int *val, int *val2) > +{ > + unsigned int regval; > + int ret; > + > + switch (channel) { > + case MT6360_CHAN_USBID: > + fallthrough; I don't think we need fallthrough for cases with nothing in them. > + case MT6360_CHAN_VSYS: > + fallthrough; > + case MT6360_CHAN_VBAT: > + fallthrough; > + case MT6360_CHAN_CHG_VDDP: > + fallthrough; > + case MT6360_CHAN_VREF_TS: > + fallthrough; > + case MT6360_CHAN_TS: > + *val = 1250; > + return IIO_VAL_INT; > + case MT6360_CHAN_VBUSDIV5: > + *val = 6250; > + return IIO_VAL_INT; > + case MT6360_CHAN_VBUSDIV2: > + fallthrough; > + case MT6360_CHAN_IBUS: > + fallthrough; > + case MT6360_CHAN_IBAT: > + *val = 2500; > + > + if (channel == MT6360_CHAN_IBUS) { > + /* IBUS will be affected by input current limit for the different Ron */ > + /* Check whether the config is <400mA or not */ > + ret = regmap_read(mad->regmap, MT6360_REG_PMUCHGCTRL3, ®val); > + if (ret) > + return ret; > + > + regval = (regval & MT6360_AICR_MASK) >> MT6360_AICR_SHFT; > + if (regval < MT6360_AICR_400MA) > + *val = 1900; > + } > + > + return IIO_VAL_INT; > + case MT6360_CHAN_TEMP_JC: > + *val = 105; > + *val2 = 100; > + return IIO_VAL_FRACTIONAL; > + } > + > + return -EINVAL; > +} > + > +static int mt6360_adc_read_offset(struct mt6360_adc_data *mad, int channel, int *val) > +{ > + *val = (channel == MT6360_CHAN_TEMP_JC) ? -80 : 0; > + return IIO_VAL_INT; > + > +} > + > +static int mt6360_adc_read_raw(struct iio_dev *iio_dev, const struct iio_chan_spec *chan, > + int *val, int *val2, long mask) > +{ > + struct mt6360_adc_data *mad = iio_priv(iio_dev); > + > + switch (mask) { > + case IIO_CHAN_INFO_RAW: > + return mt6360_adc_read_channel(mad, chan->channel, val); > + case IIO_CHAN_INFO_SCALE: > + return mt6360_adc_read_scale(mad, chan->channel, val, val2); > + case IIO_CHAN_INFO_OFFSET: > + return mt6360_adc_read_offset(mad, chan->channel, val); > + } > + > + return -EINVAL; > +} > + > +static const struct iio_info mt6360_adc_iio_info = { > + .read_raw = mt6360_adc_read_raw, > +}; > + > +#define MT6360_ADC_CHAN(_idx, _type) { \ > + .type = _type, \ > + .channel = MT6360_CHAN_##_idx, \ > + .scan_index = MT6360_CHAN_##_idx, \ > + .extend_name = #_idx, \ > + .datasheet_name = #_idx, \ > + .scan_type = { \ > + .sign = 'u', \ > + .realbits = 16, \ > + .storagebits = 16, \ > + .endianness = IIO_CPU, \ > + }, \ > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \ Docs in patch 1 need to reflect this change. Currently they have processed channels. > + BIT(IIO_CHAN_INFO_SCALE) | \ > + BIT(IIO_CHAN_INFO_OFFSET), \ > +} > + > +static const struct iio_chan_spec mt6360_adc_channels[] = { > + MT6360_ADC_CHAN(USBID, IIO_VOLTAGE), > + MT6360_ADC_CHAN(VBUSDIV5, IIO_VOLTAGE), > + MT6360_ADC_CHAN(VBUSDIV2, IIO_VOLTAGE), > + MT6360_ADC_CHAN(VSYS, IIO_VOLTAGE), > + MT6360_ADC_CHAN(VBAT, IIO_VOLTAGE), > + MT6360_ADC_CHAN(IBUS, IIO_CURRENT), > + MT6360_ADC_CHAN(IBAT, IIO_CURRENT), > + MT6360_ADC_CHAN(CHG_VDDP, IIO_VOLTAGE), > + MT6360_ADC_CHAN(TEMP_JC, IIO_TEMP), > + MT6360_ADC_CHAN(VREF_TS, IIO_VOLTAGE), > + MT6360_ADC_CHAN(TS, IIO_VOLTAGE), > + IIO_CHAN_SOFT_TIMESTAMP(MT6360_CHAN_MAX), > +}; > + > +static irqreturn_t mt6360_adc_trigger_handler(int irq, void *p) > +{ > + struct iio_poll_func *pf = p; > + struct iio_dev *indio_dev = pf->indio_dev; > + struct mt6360_adc_data *mad = iio_priv(indio_dev); > + struct { > + u16 values[MT6360_CHAN_MAX]; There is a hole in here I think? (MT6360_CHAN_MAX is 11?) If so we need to explicitly memset the structure to avoid any risk of kernel data leakage to userspace. > + int64_t timestamp; > + } data; I guess we know this is on a platform with 64bit alignment for int64_t's (unlike x86_64 for example) > + int i = 0, bit, val, ret; > + > + for_each_set_bit(bit, indio_dev->active_scan_mask, indio_dev->masklength) { > + ret = mt6360_adc_read_channel(mad, bit, &val); > + if (ret < 0) { > + dev_warn(&indio_dev->dev, "Failed to get channel %d conversion val\n", bit); > + goto out; > + } > + > + data.values[i++] = val; > + } > + iio_push_to_buffers_with_timestamp(indio_dev, &data, iio_get_time_ns(indio_dev)); > +out: > + iio_trigger_notify_done(indio_dev->trig); > + > + return IRQ_HANDLED; > +} > + > +static inline int mt6360_adc_reset(struct mt6360_adc_data *info) > +{ > + __be16 adc_enable; > + ktime_t all_off_time; > + int i, ret; > + > + /* Clear ADC idle wait time to 0 */ > + ret = regmap_write(info->regmap, MT6360_REG_PMUADCIDLET, 0); > + if (ret) > + return ret; > + > + /* Only keep ADC enable, but keep all channels off */ > + adc_enable = cpu_to_be16(MT6360_ADCEN_MASK); > + ret = regmap_raw_write(info->regmap, MT6360_REG_PMUADCCFG, (void *)&adc_enable, > + sizeof(__be16)); > + if (ret) > + return ret; > + > + /* Reset all channel off time to the current one */ > + all_off_time = ktime_get(); > + for (i = 0; i < MT6360_CHAN_MAX; i++) > + info->last_off_timestamps[i] = all_off_time; > + > + return 0; > +} > + > +static int mt6360_adc_probe(struct platform_device *pdev) > +{ > + struct mt6360_adc_data *mad; > + struct regmap *regmap; > + struct iio_dev *indio_dev; > + int ret; > + > + regmap = dev_get_regmap(pdev->dev.parent, NULL); > + if (!regmap) { > + dev_err(&pdev->dev, "Failed to get parent regmap\n"); > + return -ENODEV; > + } > + > + indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*mad)); > + if (!indio_dev) > + return -ENOMEM; > + > + mad = iio_priv(indio_dev); > + mad->dev = &pdev->dev; > + mad->regmap = regmap; > + mutex_init(&mad->adc_lock); > + > + ret = mt6360_adc_reset(mad); > + if (ret < 0) { > + dev_err(&pdev->dev, "Failed to reset adc\n"); > + return ret; > + } > + > + indio_dev->name = dev_name(&pdev->dev); > + indio_dev->dev.parent = &pdev->dev; > + indio_dev->info = &mt6360_adc_iio_info; > + indio_dev->modes = INDIO_DIRECT_MODE; > + indio_dev->channels = mt6360_adc_channels; > + indio_dev->num_channels = ARRAY_SIZE(mt6360_adc_channels); > + > + ret = devm_iio_triggered_buffer_setup(&pdev->dev, indio_dev, NULL, > + mt6360_adc_trigger_handler, NULL); > + if (ret) { > + dev_err(&pdev->dev, "Failed to allocate iio trigger buffer\n"); > + return ret; > + } > + > + return devm_iio_device_register(&pdev->dev, indio_dev); > +} > + > +static const struct of_device_id __maybe_unused mt6360_adc_of_id[] = { > + { .compatible = "mediatek,mt6360-adc", }, > + {} > +}; > +MODULE_DEVICE_TABLE(of, mt6360_adc_of_id); > + > +static struct platform_driver mt6360_adc_driver = { > + .driver = { > + .name = "mt6360-adc", > + .of_match_table = mt6360_adc_of_id, > + }, > + .probe = mt6360_adc_probe, > +}; > +module_platform_driver(mt6360_adc_driver); > + > +MODULE_AUTHOR("Gene Chen <gene_chen@richtek.com>"); > +MODULE_DESCRIPTION("MT6360 ADC Driver"); > +MODULE_LICENSE("GPL v2"); ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v4 3/3] iio: adc: mt6360: Add ADC driver for MT6360 2020-09-17 17:53 ` Jonathan Cameron @ 2020-09-18 8:04 ` Gene Chen 2020-09-19 13:20 ` Jonathan Cameron 0 siblings, 1 reply; 13+ messages in thread From: Gene Chen @ 2020-09-18 8:04 UTC (permalink / raw) To: Jonathan Cameron Cc: Matthias Brugger, knaack.h, lars, pmeerw, linux-iio, linux-arm Mailing List, moderated list:ARM/Mediatek SoC support, Linux Kernel Mailing List, Gene Chen, Wilma.Wu, shufan_lee, cy_huang, benjamin.chao Jonathan Cameron <jic23@kernel.org> 於 2020年9月18日 週五 上午1:53寫道： > > On Wed, 16 Sep 2020 01:36:09 +0800 > Gene Chen <gene.chen.richtek@gmail.com> wrote: > > > From: Gene Chen <gene_chen@richtek.com> > > > > Add MT6360 ADC driver include Charger Current, Voltage, and > > Temperature. > > > > Signed-off-by: Gene Chen <gene_chen@richtek.com> > Comments inline. > > > --- > > drivers/iio/adc/Kconfig | 11 ++ > > drivers/iio/adc/Makefile | 1 + > > drivers/iio/adc/mt6360-adc.c | 357 +++++++++++++++++++++++++++++++++++++++++++ > > 3 files changed, 369 insertions(+) > > create mode 100644 drivers/iio/adc/mt6360-adc.c > > > > diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig > > index 66d9cc0..8d36b66 100644 > > --- a/drivers/iio/adc/Kconfig > > +++ b/drivers/iio/adc/Kconfig > > @@ -703,6 +703,17 @@ config MCP3911 > > This driver can also be built as a module. If so, the module will be > > called mcp3911. > > > > +config MEDIATEK_MT6360_ADC > > + tristate "Mediatek MT6360 ADC driver" > > + depends on MFD_MT6360 > > + select IIO_BUFFER > > + select IIO_TRIGGERED_BUFFER > > + help > > + Say Y here to enable MT6360 ADC support. > > + Integrated for System Monitoring includes > > + is used in smartphones and tablets and supports a 11 channel > > + general purpose ADC. > > + > > config MEDIATEK_MT6577_AUXADC > > tristate "MediaTek AUXADC driver" > > depends on ARCH_MEDIATEK || COMPILE_TEST > > diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile > > index 90f94ad..5fca90a 100644 > > --- a/drivers/iio/adc/Makefile > > +++ b/drivers/iio/adc/Makefile > > @@ -65,6 +65,7 @@ obj-$(CONFIG_MAX9611) += max9611.o > > obj-$(CONFIG_MCP320X) += mcp320x.o > > obj-$(CONFIG_MCP3422) += mcp3422.o > > obj-$(CONFIG_MCP3911) += mcp3911.o > > +obj-$(CONFIG_MEDIATEK_MT6360_ADC) += mt6360-adc.o > > obj-$(CONFIG_MEDIATEK_MT6577_AUXADC) += mt6577_auxadc.o > > obj-$(CONFIG_MEN_Z188_ADC) += men_z188_adc.o > > obj-$(CONFIG_MESON_SARADC) += meson_saradc.o > > diff --git a/drivers/iio/adc/mt6360-adc.c b/drivers/iio/adc/mt6360-adc.c > > new file mode 100644 > > index 0000000..b256d0a > > --- /dev/null > > +++ b/drivers/iio/adc/mt6360-adc.c > > @@ -0,0 +1,357 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > + > > +#include <linux/delay.h> > > +#include <linux/irq.h> > > +#include <linux/kernel.h> > > +#include <linux/ktime.h> > > +#include <linux/module.h> > > +#include <linux/mutex.h> > > +#include <linux/platform_device.h> > > +#include <linux/regmap.h> > > + > > +#include <linux/iio/buffer.h> > > +#include <linux/iio/iio.h> > > +#include <linux/iio/trigger_consumer.h> > > +#include <linux/iio/triggered_buffer.h> > > + > > +#define MT6360_REG_PMUCHGCTRL3 0x313 > > +#define MT6360_REG_PMUADCCFG 0x356 > > +#define MT6360_REG_PMUADCIDLET 0x358 > > +#define MT6360_REG_PMUADCRPT1 0x35A > > + > > +/* PMUCHGCTRL3 0x313 */ > > +#define MT6360_AICR_MASK GENMASK(7, 2) > > +#define MT6360_AICR_SHFT 2 > > +#define MT6360_AICR_400MA 0x6 > > +/* PMUADCCFG 0x356 */ > > +#define MT6360_ADCEN_MASK 0x8000 > > +/* PMUADCRPT1 0x35A */ > > +#define MT6360_PREFERCH_MASK GENMASK(7, 4) > > +#define MT6360_PREFERCH_SHFT 4 > > +#define MT6360_RPTCH_MASK GENMASK(3, 0) > > +#define MT6360_NO_PREFER 15 > > + > > +/* Time in ms */ > > +#define ADC_WAIT_TIME_MS 25 > > + > > +enum { > > + MT6360_CHAN_USBID = 0, > > + MT6360_CHAN_VBUSDIV5, > > + MT6360_CHAN_VBUSDIV2, > > + MT6360_CHAN_VSYS, > > + MT6360_CHAN_VBAT, > > + MT6360_CHAN_IBUS, > > + MT6360_CHAN_IBAT, > > + MT6360_CHAN_CHG_VDDP, > > + MT6360_CHAN_TEMP_JC, > > + MT6360_CHAN_VREF_TS, > > + MT6360_CHAN_TS, > > + MT6360_CHAN_MAX > > +}; > > + > > +struct mt6360_adc_data { > > + struct device *dev; > > + struct regmap *regmap; > > + struct mutex adc_lock; > > Please add a comment to document what the scope of the lock is. > ACK > > + ktime_t last_off_timestamps[MT6360_CHAN_MAX]; > > +}; > > + > > +static int mt6360_adc_read_channel(struct mt6360_adc_data *mad, int channel, int *val) > > +{ > > + __be16 adc_enable; > > + u8 rpt[3]; > > + ktime_t start_t, predict_end_t; > > + unsigned int pre_wait_time; > > + int ret; > > + > > + mutex_lock(&mad->adc_lock); > > + > > + /* Select the preferred ADC channel */ > > + ret = regmap_update_bits(mad->regmap, MT6360_REG_PMUADCRPT1, MT6360_PREFERCH_MASK, > > + channel << MT6360_PREFERCH_SHFT); > > + if (ret) > > + goto out_adc_lock; > > + > > + adc_enable = cpu_to_be16(MT6360_ADCEN_MASK | BIT(channel)); > > + ret = regmap_raw_write(mad->regmap, MT6360_REG_PMUADCCFG, (void *)&adc_enable, > > Shouldn't be any need to cast a pointer explicitly to void * > ACK > > + sizeof(__be16)); > > sizeof(adc_enable) preferred. > ACK > > + if (ret) > > + goto out_adc_lock; > > + > > + start_t = ktime_get(); > > + predict_end_t = ktime_add_ms(mad->last_off_timestamps[channel], 2 * ADC_WAIT_TIME_MS); > > + > > + if (ktime_after(start_t, predict_end_t)) > > + pre_wait_time = ADC_WAIT_TIME_MS; > > + else > > + pre_wait_time = 3 * ADC_WAIT_TIME_MS; > > + > > + msleep(pre_wait_time); > > + > > + while (true) { > > + ret = regmap_raw_read(mad->regmap, MT6360_REG_PMUADCRPT1, rpt, sizeof(rpt)); > > + if (ret) > > + goto out_adc_conv; > > + > > + /* > > + * There are two functions, ZCV and TypeC OTP, running ADC VBAT and TS in > > + * background, and ADC samples are taken on a fixed frequency no matter read the > > + * previous one or not. > > + * To avoid conflict, We set minimum time threshold after enable ADC and > > + * check report channel is the same. > > + * The worst case is run the same ADC twice and background function is also running, > > + * ADC conversion sequence is desire channel before start ADC, background ADC, > > + * desire channel after start ADC. > > + * So the minimum correct data is three times of typical conversion time. > > + */ > > + if ((rpt[0] & MT6360_RPTCH_MASK) == channel) > > + break; > > + > > + msleep(ADC_WAIT_TIME_MS); > > + } > > + > > + /* rpt[1]: ADC_HI_BYTE, rpt[2]: ADC_LOW_BYTE */ > > + *val = be16_to_cpup((__be16 *)(rpt + 1)); > > To be entirely safe, probably need that to be an unaligned read? > Maybe I can change to "*val = rpt[1] << 8 | rpt[2];". It's more abvious. > > + ret = IIO_VAL_INT; > > + > > +out_adc_conv: > > + /* Only keep ADC enable */ > > + adc_enable = cpu_to_be16(MT6360_ADCEN_MASK); > > + regmap_raw_write(mad->regmap, MT6360_REG_PMUADCCFG, (void *)&adc_enable, sizeof(__be16)); > > sizeof(adc_enable) > ACK > > + mad->last_off_timestamps[channel] = ktime_get(); > > + /* Config prefer channel to NO_PREFER */ > > + regmap_update_bits(mad->regmap, MT6360_REG_PMUADCRPT1, MT6360_PREFERCH_MASK, > > + MT6360_NO_PREFER << MT6360_PREFERCH_SHFT); > > +out_adc_lock: > > + mutex_unlock(&mad->adc_lock); > > + > > + return ret; > > +} > > + > > +static int mt6360_adc_read_scale(struct mt6360_adc_data *mad, int channel, int *val, int *val2) > > +{ > > + unsigned int regval; > > + int ret; > > + > > + switch (channel) { > > + case MT6360_CHAN_USBID: > > + fallthrough; > > I don't think we need fallthrough for cases > with nothing in them. > Every channel needs set " *val = 2500" for scale. Do you mean change as below? switch (channel) { case MT6360_CHAN_USBID: case MT6360_CHAN_VSYS: case MT6360_CHAN_VBAT: case MT6360_CHAN_CHG_VDDP: case MT6360_CHAN_VREF_TS: fallthrough; case MT6360_CHAN_TS: *val = 1250; return IIO_VAL_INT; > > + case MT6360_CHAN_USBID: > > + case MT6360_CHAN_VSYS: > > + fallthrough; > > + case MT6360_CHAN_VBAT: > > + fallthrough; > > + case MT6360_CHAN_CHG_VDDP: > > + fallthrough; > > + case MT6360_CHAN_VREF_TS: > > + fallthrough; > > + case MT6360_CHAN_TS: > > + *val = 1250; > > + return IIO_VAL_INT; > > + case MT6360_CHAN_VBUSDIV5: > > + *val = 6250; > > + return IIO_VAL_INT; > > + case MT6360_CHAN_VBUSDIV2: > > + fallthrough; > > + case MT6360_CHAN_IBUS: > > + fallthrough; > > + case MT6360_CHAN_IBAT: > > + *val = 2500; > > + > > + if (channel == MT6360_CHAN_IBUS) { > > + /* IBUS will be affected by input current limit for the different Ron */ > > + /* Check whether the config is <400mA or not */ > > + ret = regmap_read(mad->regmap, MT6360_REG_PMUCHGCTRL3, ®val); > > + if (ret) > > + return ret; > > + > > + regval = (regval & MT6360_AICR_MASK) >> MT6360_AICR_SHFT; > > + if (regval < MT6360_AICR_400MA) > > + *val = 1900; > > + } > > + > > + return IIO_VAL_INT; > > + case MT6360_CHAN_TEMP_JC: > > + *val = 105; > > + *val2 = 100; > > + return IIO_VAL_FRACTIONAL; > > + } > > + > > + return -EINVAL; > > +} > > + > > +static int mt6360_adc_read_offset(struct mt6360_adc_data *mad, int channel, int *val) > > +{ > > + *val = (channel == MT6360_CHAN_TEMP_JC) ? -80 : 0; > > + return IIO_VAL_INT; > > + > > +} > > + > > +static int mt6360_adc_read_raw(struct iio_dev *iio_dev, const struct iio_chan_spec *chan, > > + int *val, int *val2, long mask) > > +{ > > + struct mt6360_adc_data *mad = iio_priv(iio_dev); > > + > > + switch (mask) { > > + case IIO_CHAN_INFO_RAW: > > + return mt6360_adc_read_channel(mad, chan->channel, val); > > + case IIO_CHAN_INFO_SCALE: > > + return mt6360_adc_read_scale(mad, chan->channel, val, val2); > > + case IIO_CHAN_INFO_OFFSET: > > + return mt6360_adc_read_offset(mad, chan->channel, val); > > + } > > + > > + return -EINVAL; > > +} > > + > > +static const struct iio_info mt6360_adc_iio_info = { > > + .read_raw = mt6360_adc_read_raw, > > +}; > > + > > +#define MT6360_ADC_CHAN(_idx, _type) { \ > > + .type = _type, \ > > + .channel = MT6360_CHAN_##_idx, \ > > + .scan_index = MT6360_CHAN_##_idx, \ > > + .extend_name = #_idx, \ > > + .datasheet_name = #_idx, \ > > + .scan_type = { \ > > + .sign = 'u', \ > > + .realbits = 16, \ > > + .storagebits = 16, \ > > + .endianness = IIO_CPU, \ > > + }, \ > > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \ > > Docs in patch 1 need to reflect this change. Currently they have > processed channels. > ACK > > + BIT(IIO_CHAN_INFO_SCALE) | \ > > + BIT(IIO_CHAN_INFO_OFFSET), \ > > +} > > + > > +static const struct iio_chan_spec mt6360_adc_channels[] = { > > + MT6360_ADC_CHAN(USBID, IIO_VOLTAGE), > > + MT6360_ADC_CHAN(VBUSDIV5, IIO_VOLTAGE), > > + MT6360_ADC_CHAN(VBUSDIV2, IIO_VOLTAGE), > > + MT6360_ADC_CHAN(VSYS, IIO_VOLTAGE), > > + MT6360_ADC_CHAN(VBAT, IIO_VOLTAGE), > > + MT6360_ADC_CHAN(IBUS, IIO_CURRENT), > > + MT6360_ADC_CHAN(IBAT, IIO_CURRENT), > > + MT6360_ADC_CHAN(CHG_VDDP, IIO_VOLTAGE), > > + MT6360_ADC_CHAN(TEMP_JC, IIO_TEMP), > > + MT6360_ADC_CHAN(VREF_TS, IIO_VOLTAGE), > > + MT6360_ADC_CHAN(TS, IIO_VOLTAGE), > > + IIO_CHAN_SOFT_TIMESTAMP(MT6360_CHAN_MAX), > > +}; > > + > > +static irqreturn_t mt6360_adc_trigger_handler(int irq, void *p) > > +{ > > + struct iio_poll_func *pf = p; > > + struct iio_dev *indio_dev = pf->indio_dev; > > + struct mt6360_adc_data *mad = iio_priv(indio_dev); > > + struct { > > + u16 values[MT6360_CHAN_MAX]; > > There is a hole in here I think? (MT6360_CHAN_MAX is 11?) > If so we need to explicitly memset the structure to avoid any > risk of kernel data leakage to userspace. > > > + int64_t timestamp; > > + } data; > > I guess we know this is on a platform with 64bit alignment for int64_t's > (unlike x86_64 for example) > Do you mean change as below? struct { u16 values[MT6360_CHAN_MAX]; int64_t timestamp; __aligned(8) } data; > > + int i = 0, bit, val, ret; > > + > > + for_each_set_bit(bit, indio_dev->active_scan_mask, indio_dev->masklength) { > > + ret = mt6360_adc_read_channel(mad, bit, &val); > > + if (ret < 0) { > > + dev_warn(&indio_dev->dev, "Failed to get channel %d conversion val\n", bit); > > + goto out; > > + } > > + > > + data.values[i++] = val; > > + } > > + iio_push_to_buffers_with_timestamp(indio_dev, &data, iio_get_time_ns(indio_dev)); > > +out: > > + iio_trigger_notify_done(indio_dev->trig); > > + > > + return IRQ_HANDLED; > > +} > > + > > +static inline int mt6360_adc_reset(struct mt6360_adc_data *info) > > +{ > > + __be16 adc_enable; > > + ktime_t all_off_time; > > + int i, ret; > > + > > + /* Clear ADC idle wait time to 0 */ > > + ret = regmap_write(info->regmap, MT6360_REG_PMUADCIDLET, 0); > > + if (ret) > > + return ret; > > + > > + /* Only keep ADC enable, but keep all channels off */ > > + adc_enable = cpu_to_be16(MT6360_ADCEN_MASK); > > + ret = regmap_raw_write(info->regmap, MT6360_REG_PMUADCCFG, (void *)&adc_enable, > > + sizeof(__be16)); > > + if (ret) > > + return ret; > > + > > + /* Reset all channel off time to the current one */ > > + all_off_time = ktime_get(); > > + for (i = 0; i < MT6360_CHAN_MAX; i++) > > + info->last_off_timestamps[i] = all_off_time; > > + > > + return 0; > > +} > > + > > +static int mt6360_adc_probe(struct platform_device *pdev) > > +{ > > + struct mt6360_adc_data *mad; > > + struct regmap *regmap; > > + struct iio_dev *indio_dev; > > + int ret; > > + > > + regmap = dev_get_regmap(pdev->dev.parent, NULL); > > + if (!regmap) { > > + dev_err(&pdev->dev, "Failed to get parent regmap\n"); > > + return -ENODEV; > > + } > > + > > + indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*mad)); > > + if (!indio_dev) > > + return -ENOMEM; > > + > > + mad = iio_priv(indio_dev); > > + mad->dev = &pdev->dev; > > + mad->regmap = regmap; > > + mutex_init(&mad->adc_lock); > > + > > + ret = mt6360_adc_reset(mad); > > + if (ret < 0) { > > + dev_err(&pdev->dev, "Failed to reset adc\n"); > > + return ret; > > + } > > + > > + indio_dev->name = dev_name(&pdev->dev); > > + indio_dev->dev.parent = &pdev->dev; > > + indio_dev->info = &mt6360_adc_iio_info; > > + indio_dev->modes = INDIO_DIRECT_MODE; > > + indio_dev->channels = mt6360_adc_channels; > > + indio_dev->num_channels = ARRAY_SIZE(mt6360_adc_channels); > > + > > + ret = devm_iio_triggered_buffer_setup(&pdev->dev, indio_dev, NULL, > > + mt6360_adc_trigger_handler, NULL); > > + if (ret) { > > + dev_err(&pdev->dev, "Failed to allocate iio trigger buffer\n"); > > + return ret; > > + } > > + > > + return devm_iio_device_register(&pdev->dev, indio_dev); > > +} > > + > > +static const struct of_device_id __maybe_unused mt6360_adc_of_id[] = { > > + { .compatible = "mediatek,mt6360-adc", }, > > + {} > > +}; > > +MODULE_DEVICE_TABLE(of, mt6360_adc_of_id); > > + > > +static struct platform_driver mt6360_adc_driver = { > > + .driver = { > > + .name = "mt6360-adc", > > + .of_match_table = mt6360_adc_of_id, > > + }, > > + .probe = mt6360_adc_probe, > > +}; > > +module_platform_driver(mt6360_adc_driver); > > + > > +MODULE_AUTHOR("Gene Chen <gene_chen@richtek.com>"); > > +MODULE_DESCRIPTION("MT6360 ADC Driver"); > > +MODULE_LICENSE("GPL v2"); > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v4 3/3] iio: adc: mt6360: Add ADC driver for MT6360 2020-09-18 8:04 ` Gene Chen @ 2020-09-19 13:20 ` Jonathan Cameron 0 siblings, 0 replies; 13+ messages in thread From: Jonathan Cameron @ 2020-09-19 13:20 UTC (permalink / raw) To: Gene Chen Cc: Matthias Brugger, knaack.h, lars, pmeerw, linux-iio, linux-arm Mailing List, moderated list:ARM/Mediatek SoC support, Linux Kernel Mailing List, Gene Chen, Wilma.Wu, shufan_lee, cy_huang, benjamin.chao On Fri, 18 Sep 2020 16:04:43 +0800 Gene Chen <gene.chen.richtek@gmail.com> wrote: > Jonathan Cameron <jic23@kernel.org> 於 2020年9月18日 週五 上午1:53寫道： > > > > On Wed, 16 Sep 2020 01:36:09 +0800 > > Gene Chen <gene.chen.richtek@gmail.com> wrote: > > > > > From: Gene Chen <gene_chen@richtek.com> > > > > > > Add MT6360 ADC driver include Charger Current, Voltage, and > > > Temperature. > > > > > > Signed-off-by: Gene Chen <gene_chen@richtek.com> > > Comments inline. ... > > > + if (ret) > > > + goto out_adc_lock; > > > + > > > + start_t = ktime_get(); > > > + predict_end_t = ktime_add_ms(mad->last_off_timestamps[channel], 2 * ADC_WAIT_TIME_MS); > > > + > > > + if (ktime_after(start_t, predict_end_t)) > > > + pre_wait_time = ADC_WAIT_TIME_MS; > > > + else > > > + pre_wait_time = 3 * ADC_WAIT_TIME_MS; > > > + > > > + msleep(pre_wait_time); > > > + > > > + while (true) { > > > + ret = regmap_raw_read(mad->regmap, MT6360_REG_PMUADCRPT1, rpt, sizeof(rpt)); > > > + if (ret) > > > + goto out_adc_conv; > > > + > > > + /* > > > + * There are two functions, ZCV and TypeC OTP, running ADC VBAT and TS in > > > + * background, and ADC samples are taken on a fixed frequency no matter read the > > > + * previous one or not. > > > + * To avoid conflict, We set minimum time threshold after enable ADC and > > > + * check report channel is the same. > > > + * The worst case is run the same ADC twice and background function is also running, > > > + * ADC conversion sequence is desire channel before start ADC, background ADC, > > > + * desire channel after start ADC. > > > + * So the minimum correct data is three times of typical conversion time. > > > + */ > > > + if ((rpt[0] & MT6360_RPTCH_MASK) == channel) > > > + break; > > > + > > > + msleep(ADC_WAIT_TIME_MS); > > > + } > > > + > > > + /* rpt[1]: ADC_HI_BYTE, rpt[2]: ADC_LOW_BYTE */ > > > + *val = be16_to_cpup((__be16 *)(rpt + 1)); > > > > To be entirely safe, probably need that to be an unaligned read? > > > > Maybe I can change to "*val = rpt[1] << 8 | rpt[2];". > It's more abvious. That would definitely be safe so do that. > > > > + ret = IIO_VAL_INT; > > > + > > > +out_adc_conv: > > > + /* Only keep ADC enable */ > > > + adc_enable = cpu_to_be16(MT6360_ADCEN_MASK); > > > + regmap_raw_write(mad->regmap, MT6360_REG_PMUADCCFG, (void *)&adc_enable, sizeof(__be16)); > > > > sizeof(adc_enable) > > > > ACK > > > > + mad->last_off_timestamps[channel] = ktime_get(); > > > + /* Config prefer channel to NO_PREFER */ > > > + regmap_update_bits(mad->regmap, MT6360_REG_PMUADCRPT1, MT6360_PREFERCH_MASK, > > > + MT6360_NO_PREFER << MT6360_PREFERCH_SHFT); > > > +out_adc_lock: > > > + mutex_unlock(&mad->adc_lock); > > > + > > > + return ret; > > > +} > > > + > > > +static int mt6360_adc_read_scale(struct mt6360_adc_data *mad, int channel, int *val, int *val2) > > > +{ > > > + unsigned int regval; > > > + int ret; > > > + > > > + switch (channel) { > > > + case MT6360_CHAN_USBID: > > > + fallthrough; > > > > I don't think we need fallthrough for cases > > with nothing in them. > > > > Every channel needs set " *val = 2500" for scale. > Do you mean change as below? > > switch (channel) { > case MT6360_CHAN_USBID: > case MT6360_CHAN_VSYS: > case MT6360_CHAN_VBAT: > case MT6360_CHAN_CHG_VDDP: > case MT6360_CHAN_VREF_TS: > fallthrough; Don't need this fallthrough. fallthrough is only needed if something is done in the case statement. I believe the checker is clever enough to assume that a set of case statements with nothing inbetween them are deliberate. > case MT6360_CHAN_TS: > *val = 1250; > return IIO_VAL_INT; > ... > > > + > > > +static irqreturn_t mt6360_adc_trigger_handler(int irq, void *p) > > > +{ > > > + struct iio_poll_func *pf = p; > > > + struct iio_dev *indio_dev = pf->indio_dev; > > > + struct mt6360_adc_data *mad = iio_priv(indio_dev); > > > + struct { > > > + u16 values[MT6360_CHAN_MAX]; > > > > There is a hole in here I think? (MT6360_CHAN_MAX is 11?) > > If so we need to explicitly memset the structure to avoid any > > risk of kernel data leakage to userspace. Make sure you deal with this in v5! > > > > > + int64_t timestamp; > > > + } data; > > > > I guess we know this is on a platform with 64bit alignment for int64_t's > > (unlike x86_64 for example) > > > > Do you mean change as below? > struct { > u16 values[MT6360_CHAN_MAX]; > int64_t timestamp; __aligned(8) > } data; You can do that, or we can rely on the fact this part is never used on a platform where that isn't guaranteed anyway. > > > > + int i = 0, bit, val, ret; ... ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2020-09-19 13:20 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-09-15 17:36 [PATCH v4 0/3] iio: adc: mt6360: Add ADC driver for MT6360 Gene Chen 2020-09-15 17:36 ` [PATCH v4 1/3] dt-bindings: iio: adc: add bindings doc for MT6360 ADC Gene Chen 2020-09-17 17:33 ` Jonathan Cameron 2020-09-15 17:36 ` [PATCH v4 2/3] Documentation: ABI: testing: mt6360: Add ADC sysfs guideline Gene Chen 2020-09-17 17:42 ` Jonathan Cameron 2020-09-18 7:21 ` Gene Chen 2020-09-18 8:03 ` Jonathan Cameron 2020-09-18 10:33 ` Gene Chen 2020-09-18 15:12 ` Jonathan Cameron 2020-09-15 17:36 ` [PATCH v4 3/3] iio: adc: mt6360: Add ADC driver for MT6360 Gene Chen 2020-09-17 17:53 ` Jonathan Cameron 2020-09-18 8:04 ` Gene Chen 2020-09-19 13:20 ` Jonathan Cameron
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).