[PATCH 00/40] TI AM437X ADC1

[PATCH 00/40] TI AM437X ADC1

[Miquel Raynal]

Hello,

This is a (fairly big) series bringing support of AM437X ADC1.
On TI AM33XX SoCs family there is an ADC that can also be connected to a
touchscreen. This hardware has been extended and is present on certain
SoCs from the AM437X family. In particular, the touchscreen has been
replaced by a magnetic card reader. In both cases, the representation is
an MFD device with two children:
- on AM33XX: the touchscreen controller and the ADC
- on AM437X: the magnetic card reader and the ADC

This series really targets small and atomic changes so that the overall
review is eased, even though it leads to a lot of rather small patches.
Here are the steps:
* Supporting the missing clock
* Translating a single text file containing the description for the
  MFD, the touchscreen and the ADC into three independent yaml files.
* Cleaning/preparing the MFD driver.
* Supporting ADC1 in the MFD driver.
* Cleaning/preparing of the ADC driver.
* Supporting ADC1 in the ADC driver.
* Updating various device trees.

Here is the full series, I can split it if the various maintainers
prefer. The clk patch can be merged alone, the MFD and the ADC changes
unfortunately are interdependent so, reviewed, the MFD maintainers will
have to create an immutable tag after applying so that ADC maintainers
can merge it. For now, this is open for reviews :) !

Thanks,
Miquèl

Miquel Raynal (40):
  clk: ti: am43xx: Add clkctrl data for am43xx ADC1
  dt-bindings: mfd: ti,am3359-tscadc: Add a yaml description for this
    MFD
  dt-bindings: touchscreen: ti,am3359-tsc: New yaml description
  dt-bindings: iio: adc: ti,am3359-adc: New yaml description
  dt-bindings: touchscreen: ti,am3359-tsc: Remove deprecated text file
  dt-bindings: mfd: ti,am3359-tscadc: Describe am4372 MFD compatible
  dt-bindings: iio: adc: ti,am3359-adc: Describe am4372 ADC compatible
  mfd: ti_am335x_tscadc: Replace license text with SPDX tag
  mfd: ti_am335x_tscadc: Fix style
  mfd: ti_am335x_tscadc: Drop extra spacing when declaring stack
    variables
  mfd: ti_am335x_tscadc: Get rid of useless gotos
  mfd: ti_am335x_tscadc: Move the driver structure allocation earlier
  mfd: ti_am335x_tscadc: Reword the comment explaining the dividers
  mfd: ti_am335x_tscadc: Don't search the tree for our clock
  mfd: ti_am335x_tscadc: Simplify divisor calculation
  mfd: ti_am335x_tscadc: Use driver data
  mfd: ti_am335x_tscadc: Mimic the probe from resume()
  mfd: ti_am335x_tscadc: Drop useless variables from the driver
    structure
  mfd: ti_am335x_tscadc: Always provide an idle configuration
  mfd: ti_am335x_tscadc: Gather the ctrl register logic at one place
  mfd: ti_am335x_tscadc: Rename the subsystem enable macro
  mfd: ti_am335x_tscadc: Add TSC prefix in certain macros
  mfd: ti_am335x_tscadc: Rename a variable
  mfd: ti_am335x_tscadc: Fix an error message
  mfd: ti_am335x_tscadc: Add a boolean to clarify the presence of a
    touchscreen
  mfd: ti_am335x_tscadc: Introduce has_tsc
  mfd: ti_am335x_tscadc: Move control register configuration
  mfd: ti_am335x_tscadc: Add ADC1/magnetic reader support
  mfd: ti_am335x_tscadc: Support the correctly spelled DT property
  iio: adc: ti_am335x_adc: Replace license text with SPDX tag
  iio: adc: ti_am335x_adc: Fix style
  iio: adc: ti_am335x_adc: Get rid of useless gotos
  iio: adc: ti_am335x_adc: Gather the checks on the delays
  iio: adc: ti_am335x_adc: Add a unit to the timeout delay
  iio: adc: ti_am335x_adc: Wait the idle state to avoid stalls
  iio: adc: ti_am335x_adc: Add the scale information
  iio: adc: ti_am335x_adc: Add the am437x compatible
  ARM: dts: am437x-cm-t43: Use a correctly spelled DT property
  ARM: dts: am43xx: Describe the magnetic reader/ADC1 hardware module
  ARM: dts: am437x-gp-evm: enable ADC1

 .../bindings/iio/adc/ti,am3359-adc.yaml       |  83 +++++++
 .../input/touchscreen/ti,am3359-tsc.yaml      |  89 +++++++
 .../bindings/input/touchscreen/ti-tsc-adc.txt |  91 -------
 .../bindings/mfd/ti,am3359-tscadc.yaml        |  80 ++++++
 arch/arm/boot/dts/am437x-cm-t43.dts           |   2 +-
 arch/arm/boot/dts/am437x-gp-evm.dts           |   8 +
 arch/arm/boot/dts/am437x-l4.dtsi              |  31 ++-
 arch/arm/boot/dts/am43xx-clocks.dtsi          |   7 +
 drivers/clk/ti/clk-43xx.c                     |   1 +
 drivers/iio/adc/ti_am335x_adc.c               | 217 +++++++++-------
 drivers/mfd/ti_am335x_tscadc.c                | 232 ++++++++++--------
 include/dt-bindings/clock/am4.h               |   1 +
 include/linux/mfd/ti_am335x_tscadc.h          |  42 ++--
 13 files changed, 582 insertions(+), 302 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/ti,am3359-adc.yaml
 create mode 100644 Documentation/devicetree/bindings/input/touchscreen/ti,am3359-tsc.yaml
 delete mode 100644 Documentation/devicetree/bindings/input/touchscreen/ti-tsc-adc.txt
 create mode 100644 Documentation/devicetree/bindings/mfd/ti,am3359-tscadc.yaml

-- 
2.27.0

[PATCH 01/40] clk: ti: am43xx: Add clkctrl data for am43xx ADC1

[Miquel Raynal]

Declare ADC1 clkctrl which feeds the magnetic-reader/ADC1 hardware
module.

Co-developed-by: Jason Reeder <jreeder@ti.com>
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Signed-off-by: Jason Reeder <jreeder@ti.com>
---
 drivers/clk/ti/clk-43xx.c       | 1 +
 include/dt-bindings/clock/am4.h | 1 +
 2 files changed, 2 insertions(+)

diff --git a/drivers/clk/ti/clk-43xx.c b/drivers/clk/ti/clk-43xx.c
index 46c0add99570..6e97a541cfd3 100644
--- a/drivers/clk/ti/clk-43xx.c
+++ b/drivers/clk/ti/clk-43xx.c
@@ -116,6 +116,7 @@ static const struct omap_clkctrl_reg_data am4_l3s_clkctrl_regs[] __initconst = {
 	{ AM4_L3S_VPFE0_CLKCTRL, NULL, CLKF_SW_SUP, "l3_gclk" },
 	{ AM4_L3S_VPFE1_CLKCTRL, NULL, CLKF_SW_SUP, "l3_gclk" },
 	{ AM4_L3S_GPMC_CLKCTRL, NULL, CLKF_SW_SUP, "l3s_gclk" },
+	{ AM4_L3S_ADC1_CLKCTRL, NULL, CLKF_SW_SUP, "l3s_gclk" },
 	{ AM4_L3S_MCASP0_CLKCTRL, NULL, CLKF_SW_SUP, "mcasp0_fck" },
 	{ AM4_L3S_MCASP1_CLKCTRL, NULL, CLKF_SW_SUP, "mcasp1_fck" },
 	{ AM4_L3S_MMC3_CLKCTRL, NULL, CLKF_SW_SUP, "mmc_clk" },
diff --git a/include/dt-bindings/clock/am4.h b/include/dt-bindings/clock/am4.h
index d961e7cb3682..4be6c5961f34 100644
--- a/include/dt-bindings/clock/am4.h
+++ b/include/dt-bindings/clock/am4.h
@@ -158,6 +158,7 @@
 #define AM4_L3S_VPFE0_CLKCTRL	AM4_L3S_CLKCTRL_INDEX(0x68)
 #define AM4_L3S_VPFE1_CLKCTRL	AM4_L3S_CLKCTRL_INDEX(0x70)
 #define AM4_L3S_GPMC_CLKCTRL	AM4_L3S_CLKCTRL_INDEX(0x220)
+#define AM4_L3S_ADC1_CLKCTRL	AM4_L3S_CLKCTRL_INDEX(0x230)
 #define AM4_L3S_MCASP0_CLKCTRL	AM4_L3S_CLKCTRL_INDEX(0x238)
 #define AM4_L3S_MCASP1_CLKCTRL	AM4_L3S_CLKCTRL_INDEX(0x240)
 #define AM4_L3S_MMC3_CLKCTRL	AM4_L3S_CLKCTRL_INDEX(0x248)
-- 
2.27.0

[PATCH 02/40] dt-bindings: mfd: ti,am3359-tscadc: Add a yaml description for this MFD

[Miquel Raynal]

There is a very light description of this MFD in a text file dedicated
to a touchscreen controller (which is one of the two children of the
MFD). Here is now a complete yaml description.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 .../bindings/mfd/ti,am3359-tscadc.yaml        | 75 +++++++++++++++++++
 1 file changed, 75 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/ti,am3359-tscadc.yaml

diff --git a/Documentation/devicetree/bindings/mfd/ti,am3359-tscadc.yaml b/Documentation/devicetree/bindings/mfd/ti,am3359-tscadc.yaml
new file mode 100644
index 000000000000..96b329508d8a
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/ti,am3359-tscadc.yaml
@@ -0,0 +1,75 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mfd/ti,am3359-tscadc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: TI AM3359 Touchscreen controller/ADC
+
+maintainers:
+  - Miquel Raynal <miquel.raynal@bootlin.com>
+
+properties:
+  compatible:
+    const: ti,am3359-tscadc
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+  clock-names:
+    const: fck
+
+  dmas:
+    items:
+      - description: DMA controller phandle and request line for FIFO0
+      - description: DMA controller phandle and request line for FIFO1
+
+  dma-names:
+    items:
+      - const: fifo0
+      - const: fifo1
+
+patternProperties:
+  "^adc$":
+    description: ADC
+
+  "^tsc$":
+    description: Touchscreen controller
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - clocks
+  - clock-names
+  - dmas
+  - dma-names
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+    tscadc: tscadc@0 {
+        compatible = "ti,am3359-tscadc";
+        reg = <0x0 0x1000>;
+        interrupts = <GIC_SPI 16 IRQ_TYPE_LEVEL_HIGH>;
+        clocks = <&adc_tsc_fck>;
+        clock-names = "fck";
+        dmas = <&edma 53 0>, <&edma 57 0>;
+        dma-names = "fifo0", "fifo1";
+        status = "disabled";
+
+        tsc {
+        };
+
+        adc {
+        };
+    };
-- 
2.27.0

Re: [PATCH 02/40] dt-bindings: mfd: ti,am3359-tscadc: Add a yaml description for this MFD

[Jonathan Cameron]

On Wed, 25 Aug 2021 17:24:40 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> There is a very light description of this MFD in a text file dedicated
> to a touchscreen controller (which is one of the two children of the
> MFD). Here is now a complete yaml description.

Make sure to call out places where you are filling in gaps in the
txt file description.  Looks like we forgot to keep that up to date as the driver
evolved. oops.

> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  .../bindings/mfd/ti,am3359-tscadc.yaml        | 75 +++++++++++++++++++
>  1 file changed, 75 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mfd/ti,am3359-tscadc.yaml
> 
> diff --git a/Documentation/devicetree/bindings/mfd/ti,am3359-tscadc.yaml b/Documentation/devicetree/bindings/mfd/ti,am3359-tscadc.yaml
> new file mode 100644
> index 000000000000..96b329508d8a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/ti,am3359-tscadc.yaml
> @@ -0,0 +1,75 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mfd/ti,am3359-tscadc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: TI AM3359 Touchscreen controller/ADC
> +
> +maintainers:
> +  - Miquel Raynal <miquel.raynal@bootlin.com>
> +
> +properties:
> +  compatible:
> +    const: ti,am3359-tscadc

text documents the more specific ti,am654-tscadc as well.
Something like

   properties:
     compatible:
       oneof:
         - const: ti,am3359-tscadc
         - items:
             - const: ti,am654-tscadc
             - const: ti,am3359-tscadc

Note this superceeds my comment on later patches about changing to enum when
introducing the new compatible. Ah well.

example for this would be something like
Documentation/devicetree/bindings/iio/adc/amlogic,meson-saradc.yam;

> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 1
> +
> +  clock-names:
> +    const: fck
> +
> +  dmas:
> +    items:
> +      - description: DMA controller phandle and request line for FIFO0
> +      - description: DMA controller phandle and request line for FIFO1
> +
> +  dma-names:
> +    items:
> +      - const: fifo0
> +      - const: fifo1
> +
> +patternProperties:
> +  "^adc$":
> +    description: ADC
> +
> +  "^tsc$":
> +    description: Touchscreen controller
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - clocks
> +  - clock-names
> +  - dmas
> +  - dma-names
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> +    tscadc: tscadc@0 {
> +        compatible = "ti,am3359-tscadc";
> +        reg = <0x0 0x1000>;
> +        interrupts = <GIC_SPI 16 IRQ_TYPE_LEVEL_HIGH>;
> +        clocks = <&adc_tsc_fck>;
> +        clock-names = "fck";
> +        dmas = <&edma 53 0>, <&edma 57 0>;
> +        dma-names = "fifo0", "fifo1";
> +        status = "disabled";
> +
> +        tsc {
> +        };
> +
> +        adc {
> +        };
> +    };

Re: [PATCH 02/40] dt-bindings: mfd: ti,am3359-tscadc: Add a yaml description for this MFD

[Miquel Raynal]

Hi Jonathan,

Jonathan Cameron <jic23@kernel.org> wrote on Mon, 30 Aug 2021 13:47:14
+0100:

> On Wed, 25 Aug 2021 17:24:40 +0200
> Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> 
> > There is a very light description of this MFD in a text file dedicated
> > to a touchscreen controller (which is one of the two children of the
> > MFD). Here is now a complete yaml description.  
> 
> Make sure to call out places where you are filling in gaps in the
> txt file description.  Looks like we forgot to keep that up to date as the driver
> evolved. oops.

Right, I always forget these links, will update them.

> 
> > 
> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > ---
> >  .../bindings/mfd/ti,am3359-tscadc.yaml        | 75 +++++++++++++++++++
> >  1 file changed, 75 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/mfd/ti,am3359-tscadc.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/mfd/ti,am3359-tscadc.yaml b/Documentation/devicetree/bindings/mfd/ti,am3359-tscadc.yaml
> > new file mode 100644
> > index 000000000000..96b329508d8a
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mfd/ti,am3359-tscadc.yaml
> > @@ -0,0 +1,75 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/mfd/ti,am3359-tscadc.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: TI AM3359 Touchscreen controller/ADC
> > +
> > +maintainers:
> > +  - Miquel Raynal <miquel.raynal@bootlin.com>
> > +
> > +properties:
> > +  compatible:
> > +    const: ti,am3359-tscadc  
> 
> text documents the more specific ti,am654-tscadc as well.

Oops, I saw it and then forgot. Thanks for checking!

> Something like
> 
>    properties:
>      compatible:
>        oneof:
>          - const: ti,am3359-tscadc
>          - items:
>              - const: ti,am654-tscadc
>              - const: ti,am3359-tscadc
> 
> Note this superceeds my comment on later patches about changing to enum when
> introducing the new compatible. Ah well.

No problem (and thanks for the 50+ reviews btw ;) )

Thanks,
Miquèl

Re: [PATCH 02/40] dt-bindings: mfd: ti,am3359-tscadc: Add a yaml description for this MFD

[Rob Herring]

On Wed, Aug 25, 2021 at 05:24:40PM +0200, Miquel Raynal wrote:
> There is a very light description of this MFD in a text file dedicated
> to a touchscreen controller (which is one of the two children of the
> MFD). Here is now a complete yaml description.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  .../bindings/mfd/ti,am3359-tscadc.yaml        | 75 +++++++++++++++++++
>  1 file changed, 75 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mfd/ti,am3359-tscadc.yaml
> 
> diff --git a/Documentation/devicetree/bindings/mfd/ti,am3359-tscadc.yaml b/Documentation/devicetree/bindings/mfd/ti,am3359-tscadc.yaml
> new file mode 100644
> index 000000000000..96b329508d8a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/ti,am3359-tscadc.yaml
> @@ -0,0 +1,75 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mfd/ti,am3359-tscadc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: TI AM3359 Touchscreen controller/ADC
> +
> +maintainers:
> +  - Miquel Raynal <miquel.raynal@bootlin.com>
> +
> +properties:
> +  compatible:
> +    const: ti,am3359-tscadc
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 1
> +
> +  clock-names:
> +    const: fck
> +
> +  dmas:
> +    items:
> +      - description: DMA controller phandle and request line for FIFO0
> +      - description: DMA controller phandle and request line for FIFO1
> +
> +  dma-names:
> +    items:
> +      - const: fifo0
> +      - const: fifo1
> +
> +patternProperties:
> +  "^adc$":
> +    description: ADC
> +
> +  "^tsc$":
> +    description: Touchscreen controller

These are fixed strings, so move to 'properties'. They also need 'type: 
object' to indicate they are nodes.

> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - clocks
> +  - clock-names
> +  - dmas
> +  - dma-names
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> +    tscadc: tscadc@0 {
> +        compatible = "ti,am3359-tscadc";
> +        reg = <0x0 0x1000>;
> +        interrupts = <GIC_SPI 16 IRQ_TYPE_LEVEL_HIGH>;
> +        clocks = <&adc_tsc_fck>;
> +        clock-names = "fck";
> +        dmas = <&edma 53 0>, <&edma 57 0>;
> +        dma-names = "fifo0", "fifo1";
> +        status = "disabled";

Don't show 'status' in examples. It disables some checks.

> +
> +        tsc {
> +        };
> +
> +        adc {
> +        };
> +    };
> -- 
> 2.27.0
> 
> 

[PATCH 03/40] dt-bindings: touchscreen: ti,am3359-tsc: New yaml description

[Miquel Raynal]

This touchscreen controller is already described in a text file:
Documentation/devicetree/bindings/input/touchscreen/ti-tsc-adc.txt

After introducing a proper description of the MFD, this is the second
step. The file cannot be removed yet as it also contains an ADC
description.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 .../input/touchscreen/ti,am3359-tsc.yaml      | 89 +++++++++++++++++++
 1 file changed, 89 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/input/touchscreen/ti,am3359-tsc.yaml

diff --git a/Documentation/devicetree/bindings/input/touchscreen/ti,am3359-tsc.yaml b/Documentation/devicetree/bindings/input/touchscreen/ti,am3359-tsc.yaml
new file mode 100644
index 000000000000..2d4ce6d04f53
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/touchscreen/ti,am3359-tsc.yaml
@@ -0,0 +1,89 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/input/touchscreen/ti,am3359-tsc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: TI AM3359 Touchscreen controller
+
+maintainers:
+  - Miquel Raynal <miquel.raynal@bootlin.com>
+
+properties:
+  compatible:
+    const: ti,am3359-tsc
+
+  ti,wires:
+    description: Wires refer to application modes i.e. 4/5/8 wire touchscreen
+      support on the platform.
+    $ref: /schemas/types.yaml#/definitions/uint32
+    enum: [4, 5, 8]
+
+  ti,x-plate-resistance:
+    description: X plate resistance
+    $ref: /schemas/types.yaml#/definitions/uint32
+
+  ti,coordinate-readouts:
+    description: The sequencer supports a total of 16 programmable steps. Each
+      step is used to read a single coordinate. A single readout is enough but
+      multiple reads can increase the quality. A value of 5 means, 5 reads for
+      X, 5 for Y and 2 for Z (always). This utilises 12 of the 16 software steps
+      available. The remaining 4 can be used by the ADC.
+    $ref: /schemas/types.yaml#/definitions/uint32
+    minimum: 1
+    maximum: 6
+
+  ti,wire-config:
+    description: Different boards could have a different order for connecting
+      wires on touchscreen. We need to provide an 8-bit number where the
+      first four bits represent the analog lines and the next 4 bits represent
+      positive/negative terminal on that input line. Notations to represent the
+      input lines and terminals respectively are as follows, AIN0 = 0, AIN1 = 1
+      and so on untill AIN7 = 7. XP = 0, XN = 1, YP = 2, YN = 3.
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+    minItems: 4
+    maxItems: 8
+
+  ti,charge-delay:
+    description: Length of touch screen charge delay step in terms of ADC clock
+      cycles. Charge delay value should be large in order to avoid false pen-up
+      events. This value effects the overall sampling speed, hence need to be
+      kept as low as possible, while avoiding false pen-up event. Start from a
+      lower value, say 0x400, and increase value until false pen-up events are
+      avoided. The pen-up detection happens immediately after the charge step,
+      so this does in fact function as a hardware knob for adjusting the amount
+      of "settling time".
+    $ref: /schemas/types.yaml#/definitions/uint32
+
+required:
+  - compatible
+  - ti,wires
+  - ti,x-plate-resistance
+  - ti,coordinate-readouts
+  - ti,wire-config
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+    tscadc: tscadc@0 {
+        compatible = "ti,am3359-tscadc";
+        reg = <0x0 0x1000>;
+        interrupts = <GIC_SPI 16 IRQ_TYPE_LEVEL_HIGH>;
+        clocks = <&adc_tsc_fck>;
+        clock-names = "fck";
+        status = "disabled";
+        dmas = <&edma 53 0>, <&edma 57 0>;
+        dma-names = "fifo0", "fifo1";
+
+        tsc {
+            compatible = "ti,am3359-tsc";
+            ti,wires = <4>;
+            ti,x-plate-resistance = <200>;
+            ti,coordinate-readouts = <5>;
+            ti,wire-config = <0x00 0x11 0x22 0x33>;
+            ti,charge-delay = <0x400>;
+        };
+    };
-- 
2.27.0

Re: [PATCH 03/40] dt-bindings: touchscreen: ti,am3359-tsc: New yaml description

[Jonathan Cameron]

On Wed, 25 Aug 2021 17:24:41 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> This touchscreen controller is already described in a text file:
> Documentation/devicetree/bindings/input/touchscreen/ti-tsc-adc.txt
> 
> After introducing a proper description of the MFD, this is the second
> step. The file cannot be removed yet as it also contains an ADC
> description.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  .../input/touchscreen/ti,am3359-tsc.yaml      | 89 +++++++++++++++++++
>  1 file changed, 89 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/input/touchscreen/ti,am3359-tsc.yaml
> 
> diff --git a/Documentation/devicetree/bindings/input/touchscreen/ti,am3359-tsc.yaml b/Documentation/devicetree/bindings/input/touchscreen/ti,am3359-tsc.yaml
> new file mode 100644
> index 000000000000..2d4ce6d04f53
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/touchscreen/ti,am3359-tsc.yaml
> @@ -0,0 +1,89 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/input/touchscreen/ti,am3359-tsc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: TI AM3359 Touchscreen controller
> +
> +maintainers:
> +  - Miquel Raynal <miquel.raynal@bootlin.com>
> +
> +properties:
> +  compatible:
> +    const: ti,am3359-tsc
> +
> +  ti,wires:
> +    description: Wires refer to application modes i.e. 4/5/8 wire touchscreen
> +      support on the platform.
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    enum: [4, 5, 8]
> +
> +  ti,x-plate-resistance:
> +    description: X plate resistance
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +
> +  ti,coordinate-readouts:
> +    description: The sequencer supports a total of 16 programmable steps. Each
> +      step is used to read a single coordinate. A single readout is enough but
> +      multiple reads can increase the quality. A value of 5 means, 5 reads for
> +      X, 5 for Y and 2 for Z (always). This utilises 12 of the 16 software steps
> +      available. The remaining 4 can be used by the ADC.
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    minimum: 1
> +    maximum: 6
> +
> +  ti,wire-config:
> +    description: Different boards could have a different order for connecting
> +      wires on touchscreen. We need to provide an 8-bit number where the
> +      first four bits represent the analog lines and the next 4 bits represent
> +      positive/negative terminal on that input line. Notations to represent the
> +      input lines and terminals respectively are as follows, AIN0 = 0, AIN1 = 1
> +      and so on untill AIN7 = 7. XP = 0, XN = 1, YP = 2, YN = 3.
> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> +    minItems: 4
> +    maxItems: 8
> +
> +  ti,charge-delay:
> +    description: Length of touch screen charge delay step in terms of ADC clock
> +      cycles. Charge delay value should be large in order to avoid false pen-up
> +      events. This value effects the overall sampling speed, hence need to be
> +      kept as low as possible, while avoiding false pen-up event. Start from a
> +      lower value, say 0x400, and increase value until false pen-up events are
> +      avoided. The pen-up detection happens immediately after the charge step,
> +      so this does in fact function as a hardware knob for adjusting the amount
> +      of "settling time".
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +
> +required:
> +  - compatible
> +  - ti,wires

Are these all required?  Seems like the driver will work fine without some
of them and isn't doing appropriate error checking for their presence..

> +  - ti,x-plate-resistance
> +  - ti,coordinate-readouts
> +  - ti,wire-config
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> +    tscadc: tscadc@0 {
> +        compatible = "ti,am3359-tscadc";
> +        reg = <0x0 0x1000>;
> +        interrupts = <GIC_SPI 16 IRQ_TYPE_LEVEL_HIGH>;
> +        clocks = <&adc_tsc_fck>;
> +        clock-names = "fck";
> +        status = "disabled";
> +        dmas = <&edma 53 0>, <&edma 57 0>;
> +        dma-names = "fifo0", "fifo1";
> +
> +        tsc {
> +            compatible = "ti,am3359-tsc";
> +            ti,wires = <4>;
> +            ti,x-plate-resistance = <200>;
> +            ti,coordinate-readouts = <5>;
> +            ti,wire-config = <0x00 0x11 0x22 0x33>;
> +            ti,charge-delay = <0x400>;
> +        };
> +    };

Re: [PATCH 03/40] dt-bindings: touchscreen: ti,am3359-tsc: New yaml description

[Miquel Raynal]

Hi Jonathan,

Jonathan Cameron <jic23@kernel.org> wrote on Mon, 30 Aug 2021 14:36:10
+0100:

> On Wed, 25 Aug 2021 17:24:41 +0200
> Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> 
> > This touchscreen controller is already described in a text file:
> > Documentation/devicetree/bindings/input/touchscreen/ti-tsc-adc.txt
> > 
> > After introducing a proper description of the MFD, this is the second
> > step. The file cannot be removed yet as it also contains an ADC
> > description.
> > 
> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > ---
> >  .../input/touchscreen/ti,am3359-tsc.yaml      | 89 +++++++++++++++++++
> >  1 file changed, 89 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/input/touchscreen/ti,am3359-tsc.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/input/touchscreen/ti,am3359-tsc.yaml b/Documentation/devicetree/bindings/input/touchscreen/ti,am3359-tsc.yaml
> > new file mode 100644
> > index 000000000000..2d4ce6d04f53
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/input/touchscreen/ti,am3359-tsc.yaml
> > @@ -0,0 +1,89 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/input/touchscreen/ti,am3359-tsc.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: TI AM3359 Touchscreen controller
> > +
> > +maintainers:
> > +  - Miquel Raynal <miquel.raynal@bootlin.com>
> > +
> > +properties:
> > +  compatible:
> > +    const: ti,am3359-tsc
> > +
> > +  ti,wires:
> > +    description: Wires refer to application modes i.e. 4/5/8 wire touchscreen
> > +      support on the platform.
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    enum: [4, 5, 8]
> > +
> > +  ti,x-plate-resistance:
> > +    description: X plate resistance
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +
> > +  ti,coordinate-readouts:
> > +    description: The sequencer supports a total of 16 programmable steps. Each
> > +      step is used to read a single coordinate. A single readout is enough but
> > +      multiple reads can increase the quality. A value of 5 means, 5 reads for
> > +      X, 5 for Y and 2 for Z (always). This utilises 12 of the 16 software steps
> > +      available. The remaining 4 can be used by the ADC.
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    minimum: 1
> > +    maximum: 6
> > +
> > +  ti,wire-config:
> > +    description: Different boards could have a different order for connecting
> > +      wires on touchscreen. We need to provide an 8-bit number where the
> > +      first four bits represent the analog lines and the next 4 bits represent
> > +      positive/negative terminal on that input line. Notations to represent the
> > +      input lines and terminals respectively are as follows, AIN0 = 0, AIN1 = 1
> > +      and so on untill AIN7 = 7. XP = 0, XN = 1, YP = 2, YN = 3.
> > +    $ref: /schemas/types.yaml#/definitions/uint32-array
> > +    minItems: 4
> > +    maxItems: 8
> > +
> > +  ti,charge-delay:
> > +    description: Length of touch screen charge delay step in terms of ADC clock
> > +      cycles. Charge delay value should be large in order to avoid false pen-up
> > +      events. This value effects the overall sampling speed, hence need to be
> > +      kept as low as possible, while avoiding false pen-up event. Start from a
> > +      lower value, say 0x400, and increase value until false pen-up events are
> > +      avoided. The pen-up detection happens immediately after the charge step,
> > +      so this does in fact function as a hardware knob for adjusting the amount
> > +      of "settling time".
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +
> > +required:
> > +  - compatible
> > +  - ti,wires  
> 
> Are these all required?  Seems like the driver will work fine without some
> of them and isn't doing appropriate error checking for their presence..

Well, I don't have a touchscreen so I can't actually verify that it
works without these properties. The logic here is to simply translate
the .txt file and the .txt file states that these properties are
mandatory. But of course if people have one and can test, it would be
good to either add the proper checks to the driver or drop these
properties from the required list. But as far as I am concerned, it is
too risky to do this kind of blind change in a binding file...

Thanks,
Miquèl

Re: [PATCH 03/40] dt-bindings: touchscreen: ti,am3359-tsc: New yaml description

[Jonathan Cameron]

On Thu, 2 Sep 2021 19:45:36 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> Hi Jonathan,
> 
> Jonathan Cameron <jic23@kernel.org> wrote on Mon, 30 Aug 2021 14:36:10
> +0100:
> 
> > On Wed, 25 Aug 2021 17:24:41 +0200
> > Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> >   
> > > This touchscreen controller is already described in a text file:
> > > Documentation/devicetree/bindings/input/touchscreen/ti-tsc-adc.txt
> > > 
> > > After introducing a proper description of the MFD, this is the second
> > > step. The file cannot be removed yet as it also contains an ADC
> > > description.
> > > 
> > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > > ---
> > >  .../input/touchscreen/ti,am3359-tsc.yaml      | 89 +++++++++++++++++++
> > >  1 file changed, 89 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/input/touchscreen/ti,am3359-tsc.yaml
> > > 
> > > diff --git a/Documentation/devicetree/bindings/input/touchscreen/ti,am3359-tsc.yaml b/Documentation/devicetree/bindings/input/touchscreen/ti,am3359-tsc.yaml
> > > new file mode 100644
> > > index 000000000000..2d4ce6d04f53
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/input/touchscreen/ti,am3359-tsc.yaml
> > > @@ -0,0 +1,89 @@
> > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/input/touchscreen/ti,am3359-tsc.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: TI AM3359 Touchscreen controller
> > > +
> > > +maintainers:
> > > +  - Miquel Raynal <miquel.raynal@bootlin.com>
> > > +
> > > +properties:
> > > +  compatible:
> > > +    const: ti,am3359-tsc
> > > +
> > > +  ti,wires:
> > > +    description: Wires refer to application modes i.e. 4/5/8 wire touchscreen
> > > +      support on the platform.
> > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > > +    enum: [4, 5, 8]
> > > +
> > > +  ti,x-plate-resistance:
> > > +    description: X plate resistance
> > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > > +
> > > +  ti,coordinate-readouts:
> > > +    description: The sequencer supports a total of 16 programmable steps. Each
> > > +      step is used to read a single coordinate. A single readout is enough but
> > > +      multiple reads can increase the quality. A value of 5 means, 5 reads for
> > > +      X, 5 for Y and 2 for Z (always). This utilises 12 of the 16 software steps
> > > +      available. The remaining 4 can be used by the ADC.
> > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > > +    minimum: 1
> > > +    maximum: 6
> > > +
> > > +  ti,wire-config:
> > > +    description: Different boards could have a different order for connecting
> > > +      wires on touchscreen. We need to provide an 8-bit number where the
> > > +      first four bits represent the analog lines and the next 4 bits represent
> > > +      positive/negative terminal on that input line. Notations to represent the
> > > +      input lines and terminals respectively are as follows, AIN0 = 0, AIN1 = 1
> > > +      and so on untill AIN7 = 7. XP = 0, XN = 1, YP = 2, YN = 3.
> > > +    $ref: /schemas/types.yaml#/definitions/uint32-array
> > > +    minItems: 4
> > > +    maxItems: 8
> > > +
> > > +  ti,charge-delay:
> > > +    description: Length of touch screen charge delay step in terms of ADC clock
> > > +      cycles. Charge delay value should be large in order to avoid false pen-up
> > > +      events. This value effects the overall sampling speed, hence need to be
> > > +      kept as low as possible, while avoiding false pen-up event. Start from a
> > > +      lower value, say 0x400, and increase value until false pen-up events are
> > > +      avoided. The pen-up detection happens immediately after the charge step,
> > > +      so this does in fact function as a hardware knob for adjusting the amount
> > > +      of "settling time".
> > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > > +
> > > +required:
> > > +  - compatible
> > > +  - ti,wires    
> > 
> > Are these all required?  Seems like the driver will work fine without some
> > of them and isn't doing appropriate error checking for their presence..  
> 
> Well, I don't have a touchscreen so I can't actually verify that it
> works without these properties. The logic here is to simply translate
> the .txt file and the .txt file states that these properties are
> mandatory. But of course if people have one and can test, it would be
> good to either add the proper checks to the driver or drop these
> properties from the required list. But as far as I am concerned, it is
> too risky to do this kind of blind change in a binding file...

Fair enough!
> 
> Thanks,
> Miquèl

Re: [PATCH 03/40] dt-bindings: touchscreen: ti,am3359-tsc: New yaml description

[Rob Herring]

On Wed, Aug 25, 2021 at 05:24:41PM +0200, Miquel Raynal wrote:
> This touchscreen controller is already described in a text file:
> Documentation/devicetree/bindings/input/touchscreen/ti-tsc-adc.txt
> 
> After introducing a proper description of the MFD, this is the second
> step. The file cannot be removed yet as it also contains an ADC
> description.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  .../input/touchscreen/ti,am3359-tsc.yaml      | 89 +++++++++++++++++++
>  1 file changed, 89 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/input/touchscreen/ti,am3359-tsc.yaml
> 
> diff --git a/Documentation/devicetree/bindings/input/touchscreen/ti,am3359-tsc.yaml b/Documentation/devicetree/bindings/input/touchscreen/ti,am3359-tsc.yaml
> new file mode 100644
> index 000000000000..2d4ce6d04f53
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/touchscreen/ti,am3359-tsc.yaml
> @@ -0,0 +1,89 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/input/touchscreen/ti,am3359-tsc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: TI AM3359 Touchscreen controller
> +
> +maintainers:
> +  - Miquel Raynal <miquel.raynal@bootlin.com>
> +
> +properties:
> +  compatible:
> +    const: ti,am3359-tsc
> +
> +  ti,wires:
> +    description: Wires refer to application modes i.e. 4/5/8 wire touchscreen
> +      support on the platform.
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    enum: [4, 5, 8]
> +
> +  ti,x-plate-resistance:
> +    description: X plate resistance
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +
> +  ti,coordinate-readouts:
> +    description: The sequencer supports a total of 16 programmable steps. Each
> +      step is used to read a single coordinate. A single readout is enough but
> +      multiple reads can increase the quality. A value of 5 means, 5 reads for
> +      X, 5 for Y and 2 for Z (always). This utilises 12 of the 16 software steps
> +      available. The remaining 4 can be used by the ADC.
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    minimum: 1
> +    maximum: 6
> +
> +  ti,wire-config:
> +    description: Different boards could have a different order for connecting
> +      wires on touchscreen. We need to provide an 8-bit number where the
> +      first four bits represent the analog lines and the next 4 bits represent
> +      positive/negative terminal on that input line. Notations to represent the
> +      input lines and terminals respectively are as follows, AIN0 = 0, AIN1 = 1
> +      and so on untill AIN7 = 7. XP = 0, XN = 1, YP = 2, YN = 3.
> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> +    minItems: 4
> +    maxItems: 8
> +
> +  ti,charge-delay:
> +    description: Length of touch screen charge delay step in terms of ADC clock
> +      cycles. Charge delay value should be large in order to avoid false pen-up
> +      events. This value effects the overall sampling speed, hence need to be
> +      kept as low as possible, while avoiding false pen-up event. Start from a
> +      lower value, say 0x400, and increase value until false pen-up events are
> +      avoided. The pen-up detection happens immediately after the charge step,
> +      so this does in fact function as a hardware knob for adjusting the amount
> +      of "settling time".
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +
> +required:
> +  - compatible
> +  - ti,wires
> +  - ti,x-plate-resistance
> +  - ti,coordinate-readouts
> +  - ti,wire-config
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>

I prefer that MFDs have just 1 complete example in the MFD schema. 

> +
> +    tscadc: tscadc@0 {

Drop unused labels.

> +        compatible = "ti,am3359-tscadc";
> +        reg = <0x0 0x1000>;
> +        interrupts = <GIC_SPI 16 IRQ_TYPE_LEVEL_HIGH>;
> +        clocks = <&adc_tsc_fck>;
> +        clock-names = "fck";
> +        status = "disabled";

status, again.

> +        dmas = <&edma 53 0>, <&edma 57 0>;
> +        dma-names = "fifo0", "fifo1";
> +
> +        tsc {
> +            compatible = "ti,am3359-tsc";
> +            ti,wires = <4>;
> +            ti,x-plate-resistance = <200>;
> +            ti,coordinate-readouts = <5>;
> +            ti,wire-config = <0x00 0x11 0x22 0x33>;
> +            ti,charge-delay = <0x400>;
> +        };
> +    };
> -- 
> 2.27.0
> 
> 

Re: [PATCH 03/40] dt-bindings: touchscreen: ti,am3359-tsc: New yaml description

[Miquel Raynal]

Hi Rob,

> > +examples:
> > +  - |
> > +    #include <dt-bindings/interrupt-controller/arm-gic.h>  
> 
> I prefer that MFDs have just 1 complete example in the MFD schema. 

Just to be on the same line, you ask me to drop the all the lines from

	tscadc:

up to

	dma-names = ...;

and keep only the tsc node, right?

I guess I should do the same in the ADC binding then.

> 
> > +
> > +    tscadc: tscadc@0 {  
> 
> Drop unused labels.
> 
> > +        compatible = "ti,am3359-tscadc";
> > +        reg = <0x0 0x1000>;
> > +        interrupts = <GIC_SPI 16 IRQ_TYPE_LEVEL_HIGH>;
> > +        clocks = <&adc_tsc_fck>;
> > +        clock-names = "fck";
> > +        status = "disabled";  
> 
> status, again.
> 
> > +        dmas = <&edma 53 0>, <&edma 57 0>;
> > +        dma-names = "fifo0", "fifo1";
> > +
> > +        tsc {
> > +            compatible = "ti,am3359-tsc";
> > +            ti,wires = <4>;
> > +            ti,x-plate-resistance = <200>;
> > +            ti,coordinate-readouts = <5>;
> > +            ti,wire-config = <0x00 0x11 0x22 0x33>;
> > +            ti,charge-delay = <0x400>;
> > +        };
> > +    };
> > -- 
> > 2.27.0
> > 
> >   

Thanks,
Miquèl

[PATCH 04/40] dt-bindings: iio: adc: ti,am3359-adc: New yaml description

[Miquel Raynal]

This ADC was already described in a text file also containing an MFD
description an a touchscreen description:
Documentation/devicetree/bindings/input/touchscreen/ti-tsc-adc.txt
Let's add a proper description for this hardware.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 .../bindings/iio/adc/ti,am3359-adc.yaml       | 81 +++++++++++++++++++
 1 file changed, 81 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/ti,am3359-adc.yaml

diff --git a/Documentation/devicetree/bindings/iio/adc/ti,am3359-adc.yaml b/Documentation/devicetree/bindings/iio/adc/ti,am3359-adc.yaml
new file mode 100644
index 000000000000..85d9d642dc67
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/ti,am3359-adc.yaml
@@ -0,0 +1,81 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/adc/ti,am3359-adc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: TI AM3359 ADC
+
+maintainers:
+  - Miquel Raynal <miquel.raynal@bootlin.com>
+
+properties:
+  compatible:
+    const: ti,am3359-adc
+
+  '#io-channel-cells':
+    const: 1
+
+  ti,adc-channels:
+    description: List of analog inputs available for ADC. AIN0 = 0, AIN1 = 1 and
+      so on untill AIN7 = 7.
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+    minItems: 1
+    maxItems: 8
+
+  ti,chan-step-opendelay:
+    description: List of open delays for each channel of ADC in the order of
+      ti,adc-channels. The value corresponds to the number of ADC clock cycles
+      to wait after applying the step configuration registers and before sending
+      the start of ADC conversion. Maximum value is 0x3FFFF.
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+    minItems: 1
+    maxItems: 8
+
+  ti,chan-step-sampledelay:
+    description: List of sample delays for each channel of ADC in the order of
+      ti,adc-channels. The value corresponds to the number of ADC clock cycles
+      to sample (to hold start of conversion high). Maximum value is 0xFF.
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+    minItems: 1
+    maxItems: 8
+
+  ti,chan-step-avg:
+    description: Number of averages to be performed for each channel of ADC. If
+      average is 16 (this is also the maximum) then input is sampled 16 times
+      and averaged to get more accurate value. This increases the time taken by
+      ADC to generate a sample. Maximum value is 16.
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+    minItems: 1
+    maxItems: 8
+
+required:
+  - compatible
+  - '#io-channel-cells'
+  - ti,adc-channels
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+    tscadc: tscadc@0 {
+        compatible = "ti,am3359-tscadc";
+        reg = <0x0 0x1000>;
+        interrupts = <GIC_SPI 16 IRQ_TYPE_LEVEL_HIGH>;
+        clocks = <&adc_tsc_fck>;
+        clock-names = "fck";
+        status = "disabled";
+        dmas = <&edma 53 0>, <&edma 57 0>;
+        dma-names = "fifo0", "fifo1";
+
+        adc {
+            compatible = "ti,am3359-adc";
+            #io-channel-cells = <1>;
+            ti,adc-channels = <4 5 6 7>;
+            ti,chan-step-opendelay = <0x098 0x3ffff 0x098 0x0>;
+            ti,chan-step-sampledelay = <0xff 0x0 0xf 0x0>;
+            ti,chan-step-avg = <16 2 4 8>;
+        };
+    };
-- 
2.27.0

Re: [PATCH 04/40] dt-bindings: iio: adc: ti,am3359-adc: New yaml description

[Jonathan Cameron]

On Wed, 25 Aug 2021 17:24:42 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> This ADC was already described in a text file also containing an MFD
> description an a touchscreen description:
> Documentation/devicetree/bindings/input/touchscreen/ti-tsc-adc.txt
> Let's add a proper description for this hardware.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>

Hi Miquel,

We'd do this different today, but this seems to be an accurate conversion
of what we have.  I'll assume this series may well go via mfd, so

Acked-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

> ---
>  .../bindings/iio/adc/ti,am3359-adc.yaml       | 81 +++++++++++++++++++
>  1 file changed, 81 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/adc/ti,am3359-adc.yaml
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/ti,am3359-adc.yaml b/Documentation/devicetree/bindings/iio/adc/ti,am3359-adc.yaml
> new file mode 100644
> index 000000000000..85d9d642dc67
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/ti,am3359-adc.yaml
> @@ -0,0 +1,81 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/adc/ti,am3359-adc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: TI AM3359 ADC
> +
> +maintainers:
> +  - Miquel Raynal <miquel.raynal@bootlin.com>
> +
> +properties:
> +  compatible:
> +    const: ti,am3359-adc
> +
> +  '#io-channel-cells':
> +    const: 1
> +
> +  ti,adc-channels:
> +    description: List of analog inputs available for ADC. AIN0 = 0, AIN1 = 1 and
> +      so on untill AIN7 = 7.
> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> +    minItems: 1
> +    maxItems: 8
> +
> +  ti,chan-step-opendelay:
> +    description: List of open delays for each channel of ADC in the order of
> +      ti,adc-channels. The value corresponds to the number of ADC clock cycles
> +      to wait after applying the step configuration registers and before sending
> +      the start of ADC conversion. Maximum value is 0x3FFFF.
> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> +    minItems: 1
> +    maxItems: 8
> +
> +  ti,chan-step-sampledelay:
> +    description: List of sample delays for each channel of ADC in the order of
> +      ti,adc-channels. The value corresponds to the number of ADC clock cycles
> +      to sample (to hold start of conversion high). Maximum value is 0xFF.
> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> +    minItems: 1
> +    maxItems: 8
> +
> +  ti,chan-step-avg:
> +    description: Number of averages to be performed for each channel of ADC. If
> +      average is 16 (this is also the maximum) then input is sampled 16 times
> +      and averaged to get more accurate value. This increases the time taken by
> +      ADC to generate a sample. Maximum value is 16.
> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> +    minItems: 1
> +    maxItems: 8
> +
> +required:
> +  - compatible
> +  - '#io-channel-cells'
> +  - ti,adc-channels
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> +    tscadc: tscadc@0 {
> +        compatible = "ti,am3359-tscadc";
> +        reg = <0x0 0x1000>;
> +        interrupts = <GIC_SPI 16 IRQ_TYPE_LEVEL_HIGH>;
> +        clocks = <&adc_tsc_fck>;
> +        clock-names = "fck";
> +        status = "disabled";
> +        dmas = <&edma 53 0>, <&edma 57 0>;
> +        dma-names = "fifo0", "fifo1";
> +
> +        adc {
> +            compatible = "ti,am3359-adc";
> +            #io-channel-cells = <1>;
> +            ti,adc-channels = <4 5 6 7>;
> +            ti,chan-step-opendelay = <0x098 0x3ffff 0x098 0x0>;
> +            ti,chan-step-sampledelay = <0xff 0x0 0xf 0x0>;
> +            ti,chan-step-avg = <16 2 4 8>;
> +        };
> +    };

[PATCH 05/40] dt-bindings: touchscreen: ti,am3359-tsc: Remove deprecated text file

[Miquel Raynal]

Now that the three hardware peaces described in this file have been
translated in yaml in 3 dedicated files (MFD, touchscreen controller and
ADC) it is time to remove this text file.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 .../bindings/input/touchscreen/ti-tsc-adc.txt | 91 -------------------
 1 file changed, 91 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/input/touchscreen/ti-tsc-adc.txt

diff --git a/Documentation/devicetree/bindings/input/touchscreen/ti-tsc-adc.txt b/Documentation/devicetree/bindings/input/touchscreen/ti-tsc-adc.txt
deleted file mode 100644
index aad5e34965eb..000000000000
--- a/Documentation/devicetree/bindings/input/touchscreen/ti-tsc-adc.txt
+++ /dev/null
@@ -1,91 +0,0 @@
-* TI - TSC ADC (Touschscreen and analog digital converter)
-~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-
-Required properties:
-- mfd
-	compatible: Should be
-		"ti,am3359-tscadc" for AM335x/AM437x SoCs
-		"ti,am654-tscadc", "ti,am3359-tscadc" for AM654 SoCs
-- child "tsc"
-	compatible: Should be "ti,am3359-tsc".
-	ti,wires: Wires refer to application modes i.e. 4/5/8 wire touchscreen
-		  support on the platform.
-	ti,x-plate-resistance: X plate resistance
-	ti,coordinate-readouts: The sequencer supports a total of 16
-				programmable steps each step is used to
-				read a single coordinate. A single
-                                readout is enough but multiple reads can
-				increase the quality.
-				A value of 5 means, 5 reads for X, 5 for
-				Y and 2 for Z (always). This utilises 12
-				of the 16 software steps available. The
-				remaining 4 can be used by the ADC.
-	ti,wire-config: Different boards could have a different order for
-			connecting wires on touchscreen. We need to provide an
-			8 bit number where in the 1st four bits represent the
-			analog lines and the next 4 bits represent positive/
-			negative terminal on that input line. Notations to
-			represent the input lines and terminals resoectively
-			is as follows:
-			AIN0 = 0, AIN1 = 1 and so on till AIN7 = 7.
-			XP  = 0, XN = 1, YP = 2, YN = 3.
-- child "adc"
-	compatible: Should be
-		    "ti,am3359-adc" for AM335x/AM437x SoCs
-		    "ti,am654-adc", "ti,am3359-adc" for AM654 SoCs
-	ti,adc-channels: List of analog inputs available for ADC.
-			 AIN0 = 0, AIN1 = 1 and so on till AIN7 = 7.
-
-Optional properties:
-- child "tsc"
-	ti,charge-delay: Length of touch screen charge delay step in terms of
-			 ADC clock cycles. Charge delay value should be large
-			 in order to avoid false pen-up events. This value
-			 effects the overall sampling speed, hence need to be
-			 kept as low as possible, while avoiding false pen-up
-			 event. Start from a lower value, say 0x400, and
-			 increase value until false pen-up events are avoided.
-			 The pen-up detection happens immediately after the
-			 charge step, so this does in fact function as a
-			 hardware knob for adjusting the amount of "settling
-			 time".
-
-- child "adc"
-	ti,chan-step-opendelay: List of open delays for each channel of
-				ADC in the order of ti,adc-channels. The
-				value corresponds to the number of ADC
-				clock cycles to wait after applying the
-				step configuration registers and before
-				sending the start of ADC conversion.
-				Maximum value is 0x3FFFF.
-       ti,chan-step-sampledelay: List of sample delays for each channel
-				  of ADC in the order of ti,adc-channels.
-				  The value corresponds to the number of
-				  ADC clock cycles to sample (to hold
-				  start of conversion high).
-				  Maximum value is 0xFF.
-       ti,chan-step-avg: Number of averages to be performed for each
-			  channel of ADC. If average is 16 then input
-			  is sampled 16 times and averaged to get more
-			  accurate value. This increases the time taken
-			  by ADC to generate a sample. Valid range is 0
-			  average to 16 averages. Maximum value is 16.
-
-Example:
-	tscadc: tscadc@44e0d000 {
-		compatible = "ti,am3359-tscadc";
-		tsc {
-			ti,wires = <4>;
-			ti,x-plate-resistance = <200>;
-			ti,coordiante-readouts = <5>;
-			ti,wire-config = <0x00 0x11 0x22 0x33>;
-			ti,charge-delay = <0x400>;
-		};
-
-		adc {
-			ti,adc-channels = <4 5 6 7>;
-			ti,chan-step-opendelay = <0x098 0x3ffff 0x098 0x0>;
-			ti,chan-step-sampledelay = <0xff 0x0 0xf 0x0>;
-			ti,chan-step-avg = <16 2 4 8>;
-		};
-	}
-- 
2.27.0

Re: [PATCH 05/40] dt-bindings: touchscreen: ti,am3359-tsc: Remove deprecated text file

[Jonathan Cameron]

On Wed, 25 Aug 2021 17:24:43 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> Now that the three hardware peaces described in this file have been
> translated in yaml in 3 dedicated files (MFD, touchscreen controller and
> ADC) it is time to remove this text file.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>

Acked-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

Nice to have another one of these go away :)
> ---
>  .../bindings/input/touchscreen/ti-tsc-adc.txt | 91 -------------------
>  1 file changed, 91 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/input/touchscreen/ti-tsc-adc.txt
> 
> diff --git a/Documentation/devicetree/bindings/input/touchscreen/ti-tsc-adc.txt b/Documentation/devicetree/bindings/input/touchscreen/ti-tsc-adc.txt
> deleted file mode 100644
> index aad5e34965eb..000000000000
> --- a/Documentation/devicetree/bindings/input/touchscreen/ti-tsc-adc.txt
> +++ /dev/null
> @@ -1,91 +0,0 @@
> -* TI - TSC ADC (Touschscreen and analog digital converter)
> -~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> -
> -Required properties:
> -- mfd
> -	compatible: Should be
> -		"ti,am3359-tscadc" for AM335x/AM437x SoCs
> -		"ti,am654-tscadc", "ti,am3359-tscadc" for AM654 SoCs
> -- child "tsc"
> -	compatible: Should be "ti,am3359-tsc".
> -	ti,wires: Wires refer to application modes i.e. 4/5/8 wire touchscreen
> -		  support on the platform.
> -	ti,x-plate-resistance: X plate resistance
> -	ti,coordinate-readouts: The sequencer supports a total of 16
> -				programmable steps each step is used to
> -				read a single coordinate. A single
> -                                readout is enough but multiple reads can
> -				increase the quality.
> -				A value of 5 means, 5 reads for X, 5 for
> -				Y and 2 for Z (always). This utilises 12
> -				of the 16 software steps available. The
> -				remaining 4 can be used by the ADC.
> -	ti,wire-config: Different boards could have a different order for
> -			connecting wires on touchscreen. We need to provide an
> -			8 bit number where in the 1st four bits represent the
> -			analog lines and the next 4 bits represent positive/
> -			negative terminal on that input line. Notations to
> -			represent the input lines and terminals resoectively
> -			is as follows:
> -			AIN0 = 0, AIN1 = 1 and so on till AIN7 = 7.
> -			XP  = 0, XN = 1, YP = 2, YN = 3.
> -- child "adc"
> -	compatible: Should be
> -		    "ti,am3359-adc" for AM335x/AM437x SoCs
> -		    "ti,am654-adc", "ti,am3359-adc" for AM654 SoCs
> -	ti,adc-channels: List of analog inputs available for ADC.
> -			 AIN0 = 0, AIN1 = 1 and so on till AIN7 = 7.
> -
> -Optional properties:
> -- child "tsc"
> -	ti,charge-delay: Length of touch screen charge delay step in terms of
> -			 ADC clock cycles. Charge delay value should be large
> -			 in order to avoid false pen-up events. This value
> -			 effects the overall sampling speed, hence need to be
> -			 kept as low as possible, while avoiding false pen-up
> -			 event. Start from a lower value, say 0x400, and
> -			 increase value until false pen-up events are avoided.
> -			 The pen-up detection happens immediately after the
> -			 charge step, so this does in fact function as a
> -			 hardware knob for adjusting the amount of "settling
> -			 time".
> -
> -- child "adc"
> -	ti,chan-step-opendelay: List of open delays for each channel of
> -				ADC in the order of ti,adc-channels. The
> -				value corresponds to the number of ADC
> -				clock cycles to wait after applying the
> -				step configuration registers and before
> -				sending the start of ADC conversion.
> -				Maximum value is 0x3FFFF.
> -       ti,chan-step-sampledelay: List of sample delays for each channel
> -				  of ADC in the order of ti,adc-channels.
> -				  The value corresponds to the number of
> -				  ADC clock cycles to sample (to hold
> -				  start of conversion high).
> -				  Maximum value is 0xFF.
> -       ti,chan-step-avg: Number of averages to be performed for each
> -			  channel of ADC. If average is 16 then input
> -			  is sampled 16 times and averaged to get more
> -			  accurate value. This increases the time taken
> -			  by ADC to generate a sample. Valid range is 0
> -			  average to 16 averages. Maximum value is 16.
> -
> -Example:
> -	tscadc: tscadc@44e0d000 {
> -		compatible = "ti,am3359-tscadc";
> -		tsc {
> -			ti,wires = <4>;
> -			ti,x-plate-resistance = <200>;
> -			ti,coordiante-readouts = <5>;
> -			ti,wire-config = <0x00 0x11 0x22 0x33>;
> -			ti,charge-delay = <0x400>;
> -		};
> -
> -		adc {
> -			ti,adc-channels = <4 5 6 7>;
> -			ti,chan-step-opendelay = <0x098 0x3ffff 0x098 0x0>;
> -			ti,chan-step-sampledelay = <0xff 0x0 0xf 0x0>;
> -			ti,chan-step-avg = <16 2 4 8>;
> -		};
> -	}

Re: [PATCH 05/40] dt-bindings: touchscreen: ti,am3359-tsc: Remove deprecated text file

[Rob Herring]

On Wed, 25 Aug 2021 17:24:43 +0200, Miquel Raynal wrote:
> Now that the three hardware peaces described in this file have been
> translated in yaml in 3 dedicated files (MFD, touchscreen controller and
> ADC) it is time to remove this text file.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  .../bindings/input/touchscreen/ti-tsc-adc.txt | 91 -------------------
>  1 file changed, 91 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/input/touchscreen/ti-tsc-adc.txt
> 

Acked-by: Rob Herring <robh@kernel.org>

[PATCH 06/40] dt-bindings: mfd: ti,am3359-tscadc: Describe am4372 MFD compatible

[Miquel Raynal]

A more recent version of the am3359 ADC/Touchscreen controller is
available on certain SoCs of the am437x family. This IP has evolved a
little bit and deserves its own compatible.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 .../devicetree/bindings/mfd/ti,am3359-tscadc.yaml          | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/mfd/ti,am3359-tscadc.yaml b/Documentation/devicetree/bindings/mfd/ti,am3359-tscadc.yaml
index 96b329508d8a..2240299dcf2f 100644
--- a/Documentation/devicetree/bindings/mfd/ti,am3359-tscadc.yaml
+++ b/Documentation/devicetree/bindings/mfd/ti,am3359-tscadc.yaml
@@ -11,7 +11,9 @@ maintainers:
 
 properties:
   compatible:
-    const: ti,am3359-tscadc
+    oneOf:
+      - const: ti,am3359-tscadc
+      - const: ti,am4372-magadc
 
   reg:
     maxItems: 1
@@ -42,6 +44,9 @@ patternProperties:
   "^tsc$":
     description: Touchscreen controller
 
+  "^mag$":
+    description: Magnetic reader
+
 required:
   - compatible
   - reg
-- 
2.27.0

Re: [PATCH 06/40] dt-bindings: mfd: ti,am3359-tscadc: Describe am4372 MFD compatible

[Jonathan Cameron]

On Wed, 25 Aug 2021 17:24:44 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> A more recent version of the am3359 ADC/Touchscreen controller is
> available on certain SoCs of the am437x family. This IP has evolved a
> little bit and deserves its own compatible.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  .../devicetree/bindings/mfd/ti,am3359-tscadc.yaml          | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/mfd/ti,am3359-tscadc.yaml b/Documentation/devicetree/bindings/mfd/ti,am3359-tscadc.yaml
> index 96b329508d8a..2240299dcf2f 100644
> --- a/Documentation/devicetree/bindings/mfd/ti,am3359-tscadc.yaml
> +++ b/Documentation/devicetree/bindings/mfd/ti,am3359-tscadc.yaml
> @@ -11,7 +11,9 @@ maintainers:
>  
>  properties:
>    compatible:
> -    const: ti,am3359-tscadc
> +    oneOf:
> +      - const: ti,am3359-tscadc
> +      - const: ti,am4372-magadc
       enum:
         - ti,am3359-tscadc
         - ti,am4372-magadc

is a little bit cleaner and means the same thing.
>  
>    reg:
>      maxItems: 1
> @@ -42,6 +44,9 @@ patternProperties:
>    "^tsc$":
>      description: Touchscreen controller
>  
> +  "^mag$":
> +    description: Magnetic reader
> +
>  required:
>    - compatible
>    - reg

[PATCH 07/40] dt-bindings: iio: adc: ti,am3359-adc: Describe am4372 ADC compatible

[Miquel Raynal]

A more recent version of the am3359 ADC is available on certain SoCs of
the am437x family. This IP has evolved a little bit and deserves its own
compatible.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 Documentation/devicetree/bindings/iio/adc/ti,am3359-adc.yaml | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/iio/adc/ti,am3359-adc.yaml b/Documentation/devicetree/bindings/iio/adc/ti,am3359-adc.yaml
index 85d9d642dc67..34743371ee18 100644
--- a/Documentation/devicetree/bindings/iio/adc/ti,am3359-adc.yaml
+++ b/Documentation/devicetree/bindings/iio/adc/ti,am3359-adc.yaml
@@ -11,7 +11,9 @@ maintainers:
 
 properties:
   compatible:
-    const: ti,am3359-adc
+    oneOf:
+      - const: ti,am3359-adc
+      - const: ti,am4372-adc
 
   '#io-channel-cells':
     const: 1
-- 
2.27.0

Re: [PATCH 07/40] dt-bindings: iio: adc: ti,am3359-adc: Describe am4372 ADC compatible

[Jonathan Cameron]

On Wed, 25 Aug 2021 17:24:45 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> A more recent version of the am3359 ADC is available on certain SoCs of
> the am437x family. This IP has evolved a little bit and deserves its own
> compatible.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  Documentation/devicetree/bindings/iio/adc/ti,am3359-adc.yaml | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/ti,am3359-adc.yaml b/Documentation/devicetree/bindings/iio/adc/ti,am3359-adc.yaml
> index 85d9d642dc67..34743371ee18 100644
> --- a/Documentation/devicetree/bindings/iio/adc/ti,am3359-adc.yaml
> +++ b/Documentation/devicetree/bindings/iio/adc/ti,am3359-adc.yaml
> @@ -11,7 +11,9 @@ maintainers:
>  
>  properties:
>    compatible:
> -    const: ti,am3359-adc
> +    oneOf:
> +      - const: ti,am3359-adc
> +      - const: ti,am4372-adc

enum

With that change
Acked-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

>  
>    '#io-channel-cells':
>      const: 1

[PATCH 08/40] mfd: ti_am335x_tscadc: Replace license text with SPDX tag

[Miquel Raynal]

Drop the text license and replace it with an equivalent SPDX license tag
identifier.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mfd/ti_am335x_tscadc.c | 10 +---------
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/drivers/mfd/ti_am335x_tscadc.c b/drivers/mfd/ti_am335x_tscadc.c
index 0e6e25308190..45262ca7734a 100644
--- a/drivers/mfd/ti_am335x_tscadc.c
+++ b/drivers/mfd/ti_am335x_tscadc.c
@@ -1,16 +1,8 @@
+// SPDX-License-Identifier: GPL-2.0-only
 /*
  * TI Touch Screen / ADC MFD driver
  *
  * Copyright (C) 2012 Texas Instruments Incorporated - https://www.ti.com/
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public License as
- * published by the Free Software Foundation version 2.
- *
- * This program is distributed "as is" WITHOUT ANY WARRANTY of any
- * kind, whether express or implied; without even the implied warranty
- * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
  */
 
 #include <linux/module.h>
-- 
2.27.0

Re: [PATCH 08/40] mfd: ti_am335x_tscadc: Replace license text with SPDX tag

[Jonathan Cameron]

On Wed, 25 Aug 2021 17:24:46 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> Drop the text license and replace it with an equivalent SPDX license tag
> identifier.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Matches with the MODULE_LICENSE(GPL) so make sense to me.

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

I wonder how this one managed to escape being updated before?  Ah well..

> ---
>  drivers/mfd/ti_am335x_tscadc.c | 10 +---------
>  1 file changed, 1 insertion(+), 9 deletions(-)
> 
> diff --git a/drivers/mfd/ti_am335x_tscadc.c b/drivers/mfd/ti_am335x_tscadc.c
> index 0e6e25308190..45262ca7734a 100644
> --- a/drivers/mfd/ti_am335x_tscadc.c
> +++ b/drivers/mfd/ti_am335x_tscadc.c
> @@ -1,16 +1,8 @@
> +// SPDX-License-Identifier: GPL-2.0-only
>  /*
>   * TI Touch Screen / ADC MFD driver
>   *
>   * Copyright (C) 2012 Texas Instruments Incorporated - https://www.ti.com/
> - *
> - * This program is free software; you can redistribute it and/or
> - * modify it under the terms of the GNU General Public License as
> - * published by the Free Software Foundation version 2.
> - *
> - * This program is distributed "as is" WITHOUT ANY WARRANTY of any
> - * kind, whether express or implied; without even the implied warranty
> - * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> - * GNU General Public License for more details.
>   */
>  
>  #include <linux/module.h>

[PATCH 09/40] mfd: ti_am335x_tscadc: Fix style

[Miquel Raynal]

These are mostly deffects reported by checkpatch.pl.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mfd/ti_am335x_tscadc.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/mfd/ti_am335x_tscadc.c b/drivers/mfd/ti_am335x_tscadc.c
index 45262ca7734a..540c6dcef541 100644
--- a/drivers/mfd/ti_am335x_tscadc.c
+++ b/drivers/mfd/ti_am335x_tscadc.c
@@ -140,15 +140,17 @@ static	int ti_tscadc_probe(struct platform_device *pdev)
 		adc_channels++;
 		if (val > 7) {
 			dev_err(&pdev->dev, " PIN numbers are 0..7 (not %d)\n",
-					val);
+				val);
 			return -EINVAL;
 		}
 	}
+
 	total_channels = tsc_wires + adc_channels;
 	if (total_channels > 8) {
 		dev_err(&pdev->dev, "Number of i/p channels more than 8\n");
 		return -EINVAL;
 	}
+
 	if (total_channels == 0) {
 		dev_err(&pdev->dev, "Need atleast one channel.\n");
 		return -EINVAL;
@@ -170,8 +172,9 @@ static	int ti_tscadc_probe(struct platform_device *pdev)
 	if (err < 0) {
 		dev_err(&pdev->dev, "no irq ID is specified.\n");
 		goto ret;
-	} else
+	} else {
 		tscadc->irq = err;
+	}
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	tscadc->tscadc_base = devm_ioremap_resource(&pdev->dev, res);
@@ -180,7 +183,8 @@ static	int ti_tscadc_probe(struct platform_device *pdev)
 
 	tscadc->tscadc_phys_base = res->start;
 	tscadc->regmap = devm_regmap_init_mmio(&pdev->dev,
-			tscadc->tscadc_base, &tscadc_regmap_config);
+					       tscadc->tscadc_base,
+					       &tscadc_regmap_config);
 	if (IS_ERR(tscadc->regmap)) {
 		dev_err(&pdev->dev, "regmap init failed\n");
 		err = PTR_ERR(tscadc->regmap);
-- 
2.27.0

Re: [PATCH 09/40] mfd: ti_am335x_tscadc: Fix style

[Jonathan Cameron]

On Wed, 25 Aug 2021 17:24:47 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> These are mostly deffects reported by checkpatch.pl.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>

look sensible to me.

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

> ---
>  drivers/mfd/ti_am335x_tscadc.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mfd/ti_am335x_tscadc.c b/drivers/mfd/ti_am335x_tscadc.c
> index 45262ca7734a..540c6dcef541 100644
> --- a/drivers/mfd/ti_am335x_tscadc.c
> +++ b/drivers/mfd/ti_am335x_tscadc.c
> @@ -140,15 +140,17 @@ static	int ti_tscadc_probe(struct platform_device *pdev)
>  		adc_channels++;
>  		if (val > 7) {
>  			dev_err(&pdev->dev, " PIN numbers are 0..7 (not %d)\n",
> -					val);
> +				val);
>  			return -EINVAL;
>  		}
>  	}
> +
>  	total_channels = tsc_wires + adc_channels;
>  	if (total_channels > 8) {
>  		dev_err(&pdev->dev, "Number of i/p channels more than 8\n");
>  		return -EINVAL;
>  	}
> +
>  	if (total_channels == 0) {
>  		dev_err(&pdev->dev, "Need atleast one channel.\n");
>  		return -EINVAL;
> @@ -170,8 +172,9 @@ static	int ti_tscadc_probe(struct platform_device *pdev)
>  	if (err < 0) {
>  		dev_err(&pdev->dev, "no irq ID is specified.\n");
>  		goto ret;
> -	} else
> +	} else {
>  		tscadc->irq = err;
> +	}
>  
>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>  	tscadc->tscadc_base = devm_ioremap_resource(&pdev->dev, res);
> @@ -180,7 +183,8 @@ static	int ti_tscadc_probe(struct platform_device *pdev)
>  
>  	tscadc->tscadc_phys_base = res->start;
>  	tscadc->regmap = devm_regmap_init_mmio(&pdev->dev,
> -			tscadc->tscadc_base, &tscadc_regmap_config);
> +					       tscadc->tscadc_base,
> +					       &tscadc_regmap_config);
>  	if (IS_ERR(tscadc->regmap)) {
>  		dev_err(&pdev->dev, "regmap init failed\n");
>  		err = PTR_ERR(tscadc->regmap);

[PATCH 10/40] mfd: ti_am335x_tscadc: Drop extra spacing when declaring stack variables

[Miquel Raynal]

Many variables will be updated (renamed, dropped, added) in the upcoming
changes, so let's simplify the style to avoid messing with spaces over
and over again.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mfd/ti_am335x_tscadc.c | 28 +++++++++++++---------------
 1 file changed, 13 insertions(+), 15 deletions(-)

diff --git a/drivers/mfd/ti_am335x_tscadc.c b/drivers/mfd/ti_am335x_tscadc.c
index 540c6dcef541..f0824732badf 100644
--- a/drivers/mfd/ti_am335x_tscadc.c
+++ b/drivers/mfd/ti_am335x_tscadc.c
@@ -113,18 +113,16 @@ static void tscadc_idle_config(struct ti_tscadc_dev *tscadc)
 
 static	int ti_tscadc_probe(struct platform_device *pdev)
 {
-	struct ti_tscadc_dev	*tscadc;
-	struct resource		*res;
-	struct clk		*clk;
-	struct device_node	*node;
-	struct mfd_cell		*cell;
-	struct property         *prop;
-	const __be32            *cur;
-	u32			val;
-	int			err, ctrl;
-	int			clock_rate;
-	int			tsc_wires = 0, adc_channels = 0, total_channels;
-	int			readouts = 0;
+	struct ti_tscadc_dev *tscadc;
+	struct device_node *node;
+	struct property *prop;
+	struct mfd_cell *cell;
+	struct resource *res;
+	const __be32 *cur;
+	struct clk *clk;
+	u32 val;
+	int tsc_wires = 0, adc_channels = 0, readouts = 0;
+	int total_channels, clock_rate, ctrl, err;
 
 	if (!pdev->dev.of_node) {
 		dev_err(&pdev->dev, "Could not find valid DT data.\n");
@@ -278,7 +276,7 @@ static	int ti_tscadc_probe(struct platform_device *pdev)
 
 static int ti_tscadc_remove(struct platform_device *pdev)
 {
-	struct ti_tscadc_dev	*tscadc = platform_get_drvdata(pdev);
+	struct ti_tscadc_dev *tscadc = platform_get_drvdata(pdev);
 
 	regmap_write(tscadc->regmap, REG_SE, 0x00);
 
@@ -297,7 +295,7 @@ static int __maybe_unused ti_tscadc_can_wakeup(struct device *dev, void *data)
 
 static int __maybe_unused tscadc_suspend(struct device *dev)
 {
-	struct ti_tscadc_dev	*tscadc = dev_get_drvdata(dev);
+	struct ti_tscadc_dev *tscadc = dev_get_drvdata(dev);
 
 	regmap_write(tscadc->regmap, REG_SE, 0x00);
 	if (device_for_each_child(dev, NULL, ti_tscadc_can_wakeup)) {
@@ -315,7 +313,7 @@ static int __maybe_unused tscadc_suspend(struct device *dev)
 
 static int __maybe_unused tscadc_resume(struct device *dev)
 {
-	struct ti_tscadc_dev	*tscadc = dev_get_drvdata(dev);
+	struct ti_tscadc_dev *tscadc = dev_get_drvdata(dev);
 	u32 ctrl;
 
 	pm_runtime_get_sync(dev);
-- 
2.27.0

Re: [PATCH 10/40] mfd: ti_am335x_tscadc: Drop extra spacing when declaring stack variables

[Jonathan Cameron]

On Wed, 25 Aug 2021 17:24:48 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> Many variables will be updated (renamed, dropped, added) in the upcoming
> changes, so let's simplify the style to avoid messing with spaces over
> and over again.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
I agree with dropping the spaces, but not so much with reordering the elements.
Whilst it obviously doesn't make any difference to padding etc, it does
require more thought that I'd want to expend reviewing a patch like this
to establish whether there was a semantic meaning to the original order that
we are disrupting.

Jonathan


> ---
>  drivers/mfd/ti_am335x_tscadc.c | 28 +++++++++++++---------------
>  1 file changed, 13 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/mfd/ti_am335x_tscadc.c b/drivers/mfd/ti_am335x_tscadc.c
> index 540c6dcef541..f0824732badf 100644
> --- a/drivers/mfd/ti_am335x_tscadc.c
> +++ b/drivers/mfd/ti_am335x_tscadc.c
> @@ -113,18 +113,16 @@ static void tscadc_idle_config(struct ti_tscadc_dev *tscadc)
>  
>  static	int ti_tscadc_probe(struct platform_device *pdev)
>  {
> -	struct ti_tscadc_dev	*tscadc;
> -	struct resource		*res;
> -	struct clk		*clk;
> -	struct device_node	*node;
> -	struct mfd_cell		*cell;
> -	struct property         *prop;
> -	const __be32            *cur;
> -	u32			val;
> -	int			err, ctrl;
> -	int			clock_rate;
> -	int			tsc_wires = 0, adc_channels = 0, total_channels;
> -	int			readouts = 0;
> +	struct ti_tscadc_dev *tscadc;
> +	struct device_node *node;
> +	struct property *prop;
> +	struct mfd_cell *cell;
> +	struct resource *res;
> +	const __be32 *cur;
> +	struct clk *clk;
> +	u32 val;
> +	int tsc_wires = 0, adc_channels = 0, readouts = 0;
> +	int total_channels, clock_rate, ctrl, err;
>  
>  	if (!pdev->dev.of_node) {
>  		dev_err(&pdev->dev, "Could not find valid DT data.\n");
> @@ -278,7 +276,7 @@ static	int ti_tscadc_probe(struct platform_device *pdev)
>  
>  static int ti_tscadc_remove(struct platform_device *pdev)
>  {
> -	struct ti_tscadc_dev	*tscadc = platform_get_drvdata(pdev);
> +	struct ti_tscadc_dev *tscadc = platform_get_drvdata(pdev);
>  
>  	regmap_write(tscadc->regmap, REG_SE, 0x00);
>  
> @@ -297,7 +295,7 @@ static int __maybe_unused ti_tscadc_can_wakeup(struct device *dev, void *data)
>  
>  static int __maybe_unused tscadc_suspend(struct device *dev)
>  {
> -	struct ti_tscadc_dev	*tscadc = dev_get_drvdata(dev);
> +	struct ti_tscadc_dev *tscadc = dev_get_drvdata(dev);
>  
>  	regmap_write(tscadc->regmap, REG_SE, 0x00);
>  	if (device_for_each_child(dev, NULL, ti_tscadc_can_wakeup)) {
> @@ -315,7 +313,7 @@ static int __maybe_unused tscadc_suspend(struct device *dev)
>  
>  static int __maybe_unused tscadc_resume(struct device *dev)
>  {
> -	struct ti_tscadc_dev	*tscadc = dev_get_drvdata(dev);
> +	struct ti_tscadc_dev *tscadc = dev_get_drvdata(dev);
>  	u32 ctrl;
>  
>  	pm_runtime_get_sync(dev);

[PATCH 11/40] mfd: ti_am335x_tscadc: Get rid of useless gotos

[Miquel Raynal]

Gotos jumping to a return statement are not really useful, drop them.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mfd/ti_am335x_tscadc.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/mfd/ti_am335x_tscadc.c b/drivers/mfd/ti_am335x_tscadc.c
index f0824732badf..8e86b766b619 100644
--- a/drivers/mfd/ti_am335x_tscadc.c
+++ b/drivers/mfd/ti_am335x_tscadc.c
@@ -169,7 +169,7 @@ static	int ti_tscadc_probe(struct platform_device *pdev)
 	err = platform_get_irq(pdev, 0);
 	if (err < 0) {
 		dev_err(&pdev->dev, "no irq ID is specified.\n");
-		goto ret;
+		return err;
 	} else {
 		tscadc->irq = err;
 	}
@@ -185,8 +185,7 @@ static	int ti_tscadc_probe(struct platform_device *pdev)
 					       &tscadc_regmap_config);
 	if (IS_ERR(tscadc->regmap)) {
 		dev_err(&pdev->dev, "regmap init failed\n");
-		err = PTR_ERR(tscadc->regmap);
-		goto ret;
+		return PTR_ERR(tscadc->regmap);
 	}
 
 	spin_lock_init(&tscadc->reg_lock);
@@ -270,7 +269,7 @@ static	int ti_tscadc_probe(struct platform_device *pdev)
 err_disable_clk:
 	pm_runtime_put_sync(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
-ret:
+
 	return err;
 }
 
-- 
2.27.0

Re: [PATCH 11/40] mfd: ti_am335x_tscadc: Get rid of useless gotos

[Jonathan Cameron]

On Wed, 25 Aug 2021 17:24:49 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> Gotos jumping to a return statement are not really useful, drop them.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>

Acked-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

As a side note, I'm not seeing the expected calls to of_put_node() for
the child nodes.  So we are leaking some stuff on those calls.

Fixing that might make this patch no longer correct.

> ---
>  drivers/mfd/ti_am335x_tscadc.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/mfd/ti_am335x_tscadc.c b/drivers/mfd/ti_am335x_tscadc.c
> index f0824732badf..8e86b766b619 100644
> --- a/drivers/mfd/ti_am335x_tscadc.c
> +++ b/drivers/mfd/ti_am335x_tscadc.c
> @@ -169,7 +169,7 @@ static	int ti_tscadc_probe(struct platform_device *pdev)
>  	err = platform_get_irq(pdev, 0);
>  	if (err < 0) {
>  		dev_err(&pdev->dev, "no irq ID is specified.\n");
> -		goto ret;
> +		return err;
>  	} else {
>  		tscadc->irq = err;
>  	}
> @@ -185,8 +185,7 @@ static	int ti_tscadc_probe(struct platform_device *pdev)
>  					       &tscadc_regmap_config);
>  	if (IS_ERR(tscadc->regmap)) {
>  		dev_err(&pdev->dev, "regmap init failed\n");
> -		err = PTR_ERR(tscadc->regmap);
> -		goto ret;
> +		return PTR_ERR(tscadc->regmap);
>  	}
>  
>  	spin_lock_init(&tscadc->reg_lock);
> @@ -270,7 +269,7 @@ static	int ti_tscadc_probe(struct platform_device *pdev)
>  err_disable_clk:
>  	pm_runtime_put_sync(&pdev->dev);
>  	pm_runtime_disable(&pdev->dev);
> -ret:
> +
>  	return err;
>  }
>  

[PATCH 12/40] mfd: ti_am335x_tscadc: Move the driver structure allocation earlier

[Miquel Raynal]

Allocating the driver structure should be done earlier in the probe so
that we can used its members from the beginning.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mfd/ti_am335x_tscadc.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/mfd/ti_am335x_tscadc.c b/drivers/mfd/ti_am335x_tscadc.c
index 8e86b766b619..0fef35e1e64b 100644
--- a/drivers/mfd/ti_am335x_tscadc.c
+++ b/drivers/mfd/ti_am335x_tscadc.c
@@ -124,6 +124,13 @@ static	int ti_tscadc_probe(struct platform_device *pdev)
 	int tsc_wires = 0, adc_channels = 0, readouts = 0;
 	int total_channels, clock_rate, ctrl, err;
 
+	/* Allocate memory for device */
+	tscadc = devm_kzalloc(&pdev->dev, sizeof(*tscadc), GFP_KERNEL);
+	if (!tscadc)
+		return -ENOMEM;
+
+	tscadc->dev = &pdev->dev;
+
 	if (!pdev->dev.of_node) {
 		dev_err(&pdev->dev, "Could not find valid DT data.\n");
 		return -EINVAL;
@@ -159,13 +166,6 @@ static	int ti_tscadc_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
-	/* Allocate memory for device */
-	tscadc = devm_kzalloc(&pdev->dev, sizeof(*tscadc), GFP_KERNEL);
-	if (!tscadc)
-		return -ENOMEM;
-
-	tscadc->dev = &pdev->dev;
-
 	err = platform_get_irq(pdev, 0);
 	if (err < 0) {
 		dev_err(&pdev->dev, "no irq ID is specified.\n");
-- 
2.27.0

Re: [PATCH 12/40] mfd: ti_am335x_tscadc: Move the driver structure allocation earlier

[Jonathan Cameron]

On Wed, 25 Aug 2021 17:24:50 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> Allocating the driver structure should be done earlier in the probe so
> that we can used its members from the beginning.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
No problem with this, but I would expect either this patch or the one
immediately after it to actually take advantage of this being
available earlier in probe().
As things stand it's not clear to a reviewer that this is actually
a useful bit of refactoring.

> ---
>  drivers/mfd/ti_am335x_tscadc.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/mfd/ti_am335x_tscadc.c b/drivers/mfd/ti_am335x_tscadc.c
> index 8e86b766b619..0fef35e1e64b 100644
> --- a/drivers/mfd/ti_am335x_tscadc.c
> +++ b/drivers/mfd/ti_am335x_tscadc.c
> @@ -124,6 +124,13 @@ static	int ti_tscadc_probe(struct platform_device *pdev)
>  	int tsc_wires = 0, adc_channels = 0, readouts = 0;
>  	int total_channels, clock_rate, ctrl, err;
>  
> +	/* Allocate memory for device */
> +	tscadc = devm_kzalloc(&pdev->dev, sizeof(*tscadc), GFP_KERNEL);
> +	if (!tscadc)
> +		return -ENOMEM;
> +
> +	tscadc->dev = &pdev->dev;
> +
>  	if (!pdev->dev.of_node) {
>  		dev_err(&pdev->dev, "Could not find valid DT data.\n");
>  		return -EINVAL;
> @@ -159,13 +166,6 @@ static	int ti_tscadc_probe(struct platform_device *pdev)
>  		return -EINVAL;
>  	}
>  
> -	/* Allocate memory for device */
> -	tscadc = devm_kzalloc(&pdev->dev, sizeof(*tscadc), GFP_KERNEL);
> -	if (!tscadc)
> -		return -ENOMEM;
> -
> -	tscadc->dev = &pdev->dev;
> -
>  	err = platform_get_irq(pdev, 0);
>  	if (err < 0) {
>  		dev_err(&pdev->dev, "no irq ID is specified.\n");

[PATCH 13/40] mfd: ti_am335x_tscadc: Reword the comment explaining the dividers

[Miquel Raynal]

The comment misses the main information which is that we assume that a
sample takes 15 ADC clock cycles to be generated. Let's take the
occasion to rework a little bit this comment.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mfd/ti_am335x_tscadc.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/mfd/ti_am335x_tscadc.c b/drivers/mfd/ti_am335x_tscadc.c
index 0fef35e1e64b..3afe22680973 100644
--- a/drivers/mfd/ti_am335x_tscadc.c
+++ b/drivers/mfd/ti_am335x_tscadc.c
@@ -195,12 +195,12 @@ static	int ti_tscadc_probe(struct platform_device *pdev)
 	pm_runtime_get_sync(&pdev->dev);
 
 	/*
-	 * The TSC_ADC_Subsystem has 2 clock domains
-	 * OCP_CLK and ADC_CLK.
-	 * The ADC clock is expected to run at target of 3MHz,
-	 * and expected to capture 12-bit data at a rate of 200 KSPS.
-	 * The TSC_ADC_SS controller design assumes the OCP clock is
-	 * at least 6x faster than the ADC clock.
+	 * The TSC_ADC_Subsystem has 2 clock domains: OCP_CLK and ADC_CLK.
+	 * ADCs produce a 12-bit sample every 15 ADC_CLK cycles.
+	 * am33xx ADCs expect to capture 200ksps.
+	 * We need the ADC clocks to run at 3MHz.
+	 * This frequency is valid since TSC_ADC_SS controller design
+	 * assumes the OCP clock is at least 6x faster than the ADC clock.
 	 */
 	clk = devm_clk_get(&pdev->dev, "adc_tsc_fck");
 	if (IS_ERR(clk)) {
-- 
2.27.0

Re: [PATCH 13/40] mfd: ti_am335x_tscadc: Reword the comment explaining the dividers

[Jonathan Cameron]

On Wed, 25 Aug 2021 17:24:51 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> The comment misses the main information which is that we assume that a
> sample takes 15 ADC clock cycles to be generated. Let's take the
> occasion to rework a little bit this comment.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Fair enough. It does seem a little clearer to me after your rewording.

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

> ---
>  drivers/mfd/ti_am335x_tscadc.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/mfd/ti_am335x_tscadc.c b/drivers/mfd/ti_am335x_tscadc.c
> index 0fef35e1e64b..3afe22680973 100644
> --- a/drivers/mfd/ti_am335x_tscadc.c
> +++ b/drivers/mfd/ti_am335x_tscadc.c
> @@ -195,12 +195,12 @@ static	int ti_tscadc_probe(struct platform_device *pdev)
>  	pm_runtime_get_sync(&pdev->dev);
>  
>  	/*
> -	 * The TSC_ADC_Subsystem has 2 clock domains
> -	 * OCP_CLK and ADC_CLK.
> -	 * The ADC clock is expected to run at target of 3MHz,
> -	 * and expected to capture 12-bit data at a rate of 200 KSPS.
> -	 * The TSC_ADC_SS controller design assumes the OCP clock is
> -	 * at least 6x faster than the ADC clock.
> +	 * The TSC_ADC_Subsystem has 2 clock domains: OCP_CLK and ADC_CLK.
> +	 * ADCs produce a 12-bit sample every 15 ADC_CLK cycles.
> +	 * am33xx ADCs expect to capture 200ksps.
> +	 * We need the ADC clocks to run at 3MHz.
> +	 * This frequency is valid since TSC_ADC_SS controller design
> +	 * assumes the OCP clock is at least 6x faster than the ADC clock.
>  	 */
>  	clk = devm_clk_get(&pdev->dev, "adc_tsc_fck");
>  	if (IS_ERR(clk)) {

[PATCH 14/40] mfd: ti_am335x_tscadc: Don't search the tree for our clock

[Miquel Raynal]

There is a single clock available in our node, which is named
"fck". The clock handler then points to adc_tsc_fck but no need to point
directly to it and do a full tree search.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mfd/ti_am335x_tscadc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mfd/ti_am335x_tscadc.c b/drivers/mfd/ti_am335x_tscadc.c
index 3afe22680973..d27f8917272d 100644
--- a/drivers/mfd/ti_am335x_tscadc.c
+++ b/drivers/mfd/ti_am335x_tscadc.c
@@ -202,7 +202,7 @@ static	int ti_tscadc_probe(struct platform_device *pdev)
 	 * This frequency is valid since TSC_ADC_SS controller design
 	 * assumes the OCP clock is at least 6x faster than the ADC clock.
 	 */
-	clk = devm_clk_get(&pdev->dev, "adc_tsc_fck");
+	clk = devm_clk_get(&pdev->dev, NULL);
 	if (IS_ERR(clk)) {
 		dev_err(&pdev->dev, "failed to get TSC fck\n");
 		err = PTR_ERR(clk);
-- 
2.27.0

[PATCH 15/40] mfd: ti_am335x_tscadc: Simplify divisor calculation

[Miquel Raynal]

Let's rewrite this on a single line, it does not hurt the readability
and saves a useless temporary variable.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mfd/ti_am335x_tscadc.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/mfd/ti_am335x_tscadc.c b/drivers/mfd/ti_am335x_tscadc.c
index d27f8917272d..3231ccbbad02 100644
--- a/drivers/mfd/ti_am335x_tscadc.c
+++ b/drivers/mfd/ti_am335x_tscadc.c
@@ -122,7 +122,7 @@ static	int ti_tscadc_probe(struct platform_device *pdev)
 	struct clk *clk;
 	u32 val;
 	int tsc_wires = 0, adc_channels = 0, readouts = 0;
-	int total_channels, clock_rate, ctrl, err;
+	int total_channels, ctrl, err;
 
 	/* Allocate memory for device */
 	tscadc = devm_kzalloc(&pdev->dev, sizeof(*tscadc), GFP_KERNEL);
@@ -208,11 +208,8 @@ static	int ti_tscadc_probe(struct platform_device *pdev)
 		err = PTR_ERR(clk);
 		goto err_disable_clk;
 	}
-	clock_rate = clk_get_rate(clk);
-	tscadc->clk_div = clock_rate / ADC_CLK;
 
-	/* TSCADC_CLKDIV needs to be configured to the value minus 1 */
-	tscadc->clk_div--;
+	tscadc->clk_div = (clk_get_rate(clk) / ADC_CLK) - 1;
 	regmap_write(tscadc->regmap, REG_CLKDIV, tscadc->clk_div);
 
 	/* Set the control register bits */
-- 
2.27.0

Re: [PATCH 15/40] mfd: ti_am335x_tscadc: Simplify divisor calculation

[Jonathan Cameron]

On Wed, 25 Aug 2021 17:24:53 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> Let's rewrite this on a single line, it does not hurt the readability
> and saves a useless temporary variable.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

> ---
>  drivers/mfd/ti_am335x_tscadc.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/mfd/ti_am335x_tscadc.c b/drivers/mfd/ti_am335x_tscadc.c
> index d27f8917272d..3231ccbbad02 100644
> --- a/drivers/mfd/ti_am335x_tscadc.c
> +++ b/drivers/mfd/ti_am335x_tscadc.c
> @@ -122,7 +122,7 @@ static	int ti_tscadc_probe(struct platform_device *pdev)
>  	struct clk *clk;
>  	u32 val;
>  	int tsc_wires = 0, adc_channels = 0, readouts = 0;
> -	int total_channels, clock_rate, ctrl, err;
> +	int total_channels, ctrl, err;
>  
>  	/* Allocate memory for device */
>  	tscadc = devm_kzalloc(&pdev->dev, sizeof(*tscadc), GFP_KERNEL);
> @@ -208,11 +208,8 @@ static	int ti_tscadc_probe(struct platform_device *pdev)
>  		err = PTR_ERR(clk);
>  		goto err_disable_clk;
>  	}
> -	clock_rate = clk_get_rate(clk);
> -	tscadc->clk_div = clock_rate / ADC_CLK;
>  
> -	/* TSCADC_CLKDIV needs to be configured to the value minus 1 */
> -	tscadc->clk_div--;
> +	tscadc->clk_div = (clk_get_rate(clk) / ADC_CLK) - 1;
>  	regmap_write(tscadc->regmap, REG_CLKDIV, tscadc->clk_div);
>  
>  	/* Set the control register bits */

[PATCH 16/40] mfd: ti_am335x_tscadc: Use driver data

[Miquel Raynal]

So far every sub-cell parameter in this driver was hardcoded: cell name,
cell compatible, specific clock name and desired clock frequency.

As we are about to introduce support for ADC1/magnetic reader, we need a
bit of flexibility. Let's add a driver data structure which will contain
these information.

Co-developed-by: Jason Reeder <jreeder@ti.com>
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Signed-off-by: Jason Reeder <jreeder@ti.com>
---
 drivers/mfd/ti_am335x_tscadc.c       | 20 +++++++++++++++-----
 include/linux/mfd/ti_am335x_tscadc.h |  9 +++++++++
 2 files changed, 24 insertions(+), 5 deletions(-)

diff --git a/drivers/mfd/ti_am335x_tscadc.c b/drivers/mfd/ti_am335x_tscadc.c
index 3231ccbbad02..0cea44b4bd1a 100644
--- a/drivers/mfd/ti_am335x_tscadc.c
+++ b/drivers/mfd/ti_am335x_tscadc.c
@@ -136,6 +136,8 @@ static	int ti_tscadc_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
+	tscadc->data = of_device_get_match_data(&pdev->dev);
+
 	node = of_get_child_by_name(pdev->dev.of_node, "tsc");
 	of_property_read_u32(node, "ti,wires", &tsc_wires);
 	of_property_read_u32(node, "ti,coordiante-readouts", &readouts);
@@ -209,7 +211,7 @@ static	int ti_tscadc_probe(struct platform_device *pdev)
 		goto err_disable_clk;
 	}
 
-	tscadc->clk_div = (clk_get_rate(clk) / ADC_CLK) - 1;
+	tscadc->clk_div = (clk_get_rate(clk) / tscadc->data->target_clk_rate) - 1;
 	regmap_write(tscadc->regmap, REG_CLKDIV, tscadc->clk_div);
 
 	/* Set the control register bits */
@@ -238,8 +240,8 @@ static	int ti_tscadc_probe(struct platform_device *pdev)
 	if (tsc_wires > 0) {
 		tscadc->tsc_cell = tscadc->used_cells;
 		cell = &tscadc->cells[tscadc->used_cells++];
-		cell->name = "TI-am335x-tsc";
-		cell->of_compatible = "ti,am3359-tsc";
+		cell->name = tscadc->data->name_tscmag;
+		cell->of_compatible = tscadc->data->compat_tscmag;
 		cell->platform_data = &tscadc;
 		cell->pdata_size = sizeof(tscadc);
 	}
@@ -248,8 +250,8 @@ static	int ti_tscadc_probe(struct platform_device *pdev)
 	if (adc_channels > 0) {
 		tscadc->adc_cell = tscadc->used_cells;
 		cell = &tscadc->cells[tscadc->used_cells++];
-		cell->name = "TI-am335x-adc";
-		cell->of_compatible = "ti,am3359-adc";
+		cell->name = tscadc->data->name_adc;
+		cell->of_compatible = tscadc->data->compat_adc;
 		cell->platform_data = &tscadc;
 		cell->pdata_size = sizeof(tscadc);
 	}
@@ -335,6 +337,14 @@ static int __maybe_unused tscadc_resume(struct device *dev)
 
 static SIMPLE_DEV_PM_OPS(tscadc_pm_ops, tscadc_suspend, tscadc_resume);
 
+static const struct ti_tscadc_data tscdata = {
+	.name_tscmag = "TI-am335x-tsc",
+	.compat_tscmag = "ti,am3359-tsc",
+	.name_adc = "TI-am335x-adc",
+	.compat_adc = "ti,am3359-adc",
+	.target_clk_rate = ADC_CLK,
+};
+
 static const struct of_device_id ti_tscadc_dt_ids[] = {
 	{ .compatible = "ti,am3359-tscadc", },
 	{ }
diff --git a/include/linux/mfd/ti_am335x_tscadc.h b/include/linux/mfd/ti_am335x_tscadc.h
index ffc091b77633..0f581c15d95a 100644
--- a/include/linux/mfd/ti_am335x_tscadc.h
+++ b/include/linux/mfd/ti_am335x_tscadc.h
@@ -162,11 +162,20 @@
 
 #define TSCADC_CELLS		2
 
+struct ti_tscadc_data {
+	char *name_tscmag;
+	char *compat_tscmag;
+	char *name_adc;
+	char *compat_adc;
+	unsigned int target_clk_rate;
+};
+
 struct ti_tscadc_dev {
 	struct device *dev;
 	struct regmap *regmap;
 	void __iomem *tscadc_base;
 	phys_addr_t tscadc_phys_base;
+	const struct ti_tscadc_data *data;
 	int irq;
 	int used_cells;	/* 1-2 */
 	int tsc_wires;
-- 
2.27.0

Re: [PATCH 16/40] mfd: ti_am335x_tscadc: Use driver data

[Jonathan Cameron]

On Wed, 25 Aug 2021 17:24:54 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> So far every sub-cell parameter in this driver was hardcoded: cell name,
> cell compatible, specific clock name and desired clock frequency.
> 
> As we are about to introduce support for ADC1/magnetic reader, we need a
> bit of flexibility. Let's add a driver data structure which will contain
> these information.
> 
> Co-developed-by: Jason Reeder <jreeder@ti.com>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> Signed-off-by: Jason Reeder <jreeder@ti.com>
> ---
>  drivers/mfd/ti_am335x_tscadc.c       | 20 +++++++++++++++-----
>  include/linux/mfd/ti_am335x_tscadc.h |  9 +++++++++
>  2 files changed, 24 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/mfd/ti_am335x_tscadc.c b/drivers/mfd/ti_am335x_tscadc.c
> index 3231ccbbad02..0cea44b4bd1a 100644
> --- a/drivers/mfd/ti_am335x_tscadc.c
> +++ b/drivers/mfd/ti_am335x_tscadc.c
> @@ -136,6 +136,8 @@ static	int ti_tscadc_probe(struct platform_device *pdev)
>  		return -EINVAL;
>  	}
>  
> +	tscadc->data = of_device_get_match_data(&pdev->dev);

As mentioned in later patch, I can't see that you actually wired this up here.
match_data doesn't seem to be set.

Otherwise looks fine to me.

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>



> +
>  	node = of_get_child_by_name(pdev->dev.of_node, "tsc");
>  	of_property_read_u32(node, "ti,wires", &tsc_wires);
>  	of_property_read_u32(node, "ti,coordiante-readouts", &readouts);
> @@ -209,7 +211,7 @@ static	int ti_tscadc_probe(struct platform_device *pdev)
>  		goto err_disable_clk;
>  	}
>  
> -	tscadc->clk_div = (clk_get_rate(clk) / ADC_CLK) - 1;
> +	tscadc->clk_div = (clk_get_rate(clk) / tscadc->data->target_clk_rate) - 1;
>  	regmap_write(tscadc->regmap, REG_CLKDIV, tscadc->clk_div);
>  
>  	/* Set the control register bits */
> @@ -238,8 +240,8 @@ static	int ti_tscadc_probe(struct platform_device *pdev)
>  	if (tsc_wires > 0) {
>  		tscadc->tsc_cell = tscadc->used_cells;
>  		cell = &tscadc->cells[tscadc->used_cells++];
> -		cell->name = "TI-am335x-tsc";
> -		cell->of_compatible = "ti,am3359-tsc";
> +		cell->name = tscadc->data->name_tscmag;
> +		cell->of_compatible = tscadc->data->compat_tscmag;
>  		cell->platform_data = &tscadc;
>  		cell->pdata_size = sizeof(tscadc);
>  	}
> @@ -248,8 +250,8 @@ static	int ti_tscadc_probe(struct platform_device *pdev)
>  	if (adc_channels > 0) {
>  		tscadc->adc_cell = tscadc->used_cells;
>  		cell = &tscadc->cells[tscadc->used_cells++];
> -		cell->name = "TI-am335x-adc";
> -		cell->of_compatible = "ti,am3359-adc";
> +		cell->name = tscadc->data->name_adc;
> +		cell->of_compatible = tscadc->data->compat_adc;
>  		cell->platform_data = &tscadc;
>  		cell->pdata_size = sizeof(tscadc);
>  	}
> @@ -335,6 +337,14 @@ static int __maybe_unused tscadc_resume(struct device *dev)
>  
>  static SIMPLE_DEV_PM_OPS(tscadc_pm_ops, tscadc_suspend, tscadc_resume);
>  
> +static const struct ti_tscadc_data tscdata = {
> +	.name_tscmag = "TI-am335x-tsc",
> +	.compat_tscmag = "ti,am3359-tsc",
> +	.name_adc = "TI-am335x-adc",
> +	.compat_adc = "ti,am3359-adc",
> +	.target_clk_rate = ADC_CLK,
> +};
> +
>  static const struct of_device_id ti_tscadc_dt_ids[] = {
>  	{ .compatible = "ti,am3359-tscadc", },
>  	{ }
> diff --git a/include/linux/mfd/ti_am335x_tscadc.h b/include/linux/mfd/ti_am335x_tscadc.h
> index ffc091b77633..0f581c15d95a 100644
> --- a/include/linux/mfd/ti_am335x_tscadc.h
> +++ b/include/linux/mfd/ti_am335x_tscadc.h
> @@ -162,11 +162,20 @@
>  
>  #define TSCADC_CELLS		2
>  
> +struct ti_tscadc_data {
> +	char *name_tscmag;
> +	char *compat_tscmag;
> +	char *name_adc;
> +	char *compat_adc;
> +	unsigned int target_clk_rate;
> +};
> +
>  struct ti_tscadc_dev {
>  	struct device *dev;
>  	struct regmap *regmap;
>  	void __iomem *tscadc_base;
>  	phys_addr_t tscadc_phys_base;
> +	const struct ti_tscadc_data *data;
>  	int irq;
>  	int used_cells;	/* 1-2 */
>  	int tsc_wires;

[PATCH 17/40] mfd: ti_am335x_tscadc: Mimic the probe from resume()

[Miquel Raynal]

Use the number of touchscreen wires (tsc_wires) instead of the flag
indicating a touchscreen cell (tsc_cell) to mimic the logic from the
probe.

Besides keeping a certain harmony in the driver, the main benefit is
that we are going to simplify the number of entries in the main MFD
structure and tsc_cell can now be removed more easily.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mfd/ti_am335x_tscadc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mfd/ti_am335x_tscadc.c b/drivers/mfd/ti_am335x_tscadc.c
index 0cea44b4bd1a..c25c7f5c101b 100644
--- a/drivers/mfd/ti_am335x_tscadc.c
+++ b/drivers/mfd/ti_am335x_tscadc.c
@@ -320,7 +320,7 @@ static int __maybe_unused tscadc_resume(struct device *dev)
 	ctrl = CNTRLREG_STEPCONFIGWRT |	CNTRLREG_STEPID;
 	regmap_write(tscadc->regmap, REG_CTRL, ctrl);
 
-	if (tscadc->tsc_cell != -1) {
+	if (tscadc->tsc_wires > 0) {
 		if (tscadc->tsc_wires == 5)
 			ctrl |= CNTRLREG_5WIRE | CNTRLREG_TSCENB;
 		else
-- 
2.27.0

[PATCH 18/40] mfd: ti_am335x_tscadc: Drop useless variables from the driver structure

[Miquel Raynal]

Keeping the count of tsc_cells and adc_cells is completely redundant, we
can derive this information from other variables. Plus, these variables
are not used anywhere else now. Let's get rid of them.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mfd/ti_am335x_tscadc.c       | 15 ++++-----------
 include/linux/mfd/ti_am335x_tscadc.h |  3 ---
 2 files changed, 4 insertions(+), 14 deletions(-)

diff --git a/drivers/mfd/ti_am335x_tscadc.c b/drivers/mfd/ti_am335x_tscadc.c
index c25c7f5c101b..258199be4c6a 100644
--- a/drivers/mfd/ti_am335x_tscadc.c
+++ b/drivers/mfd/ti_am335x_tscadc.c
@@ -121,7 +121,7 @@ static	int ti_tscadc_probe(struct platform_device *pdev)
 	const __b