Date:   Fri, 24 Feb 2023 11:55:43 +0100
Message-Id: <87r0ufs574.fsf@bloch.sibelius.xs4all.nl>
From:   Mark Kettenis <mark.kettenis@xs4all.nl>
To:     fnkl.kernel@gmail.com
Cc:     marcan@marcan.st, sven@svenpeter.dev, alyssa@rosenzweig.io,
        dmitry.torokhov@gmail.com, robh+dt@kernel.org,
        krzysztof.kozlowski+dt@linaro.org, asahi@lists.linux.dev,
        rydberg@bitmath.org, linux-arm-kernel@lists.infradead.org,
        linux-input@vger.kernel.org, devicetree@vger.kernel.org,
        linux-kernel@vger.kernel.org, fnkl.kernel@gmail.com
In-Reply-To: <20230223-z2-for-ml-v1-1-028f2b85dc15@gmail.com> (message from
        Sasha Finkelstein via B4 Relay on Fri, 24 Feb 2023 11:20:06 +0100)
Subject: Re: [PATCH RFC 1/4] dt-bindings: input: touchscreen: Add Z2 controller
 bindings.
References: <20230223-z2-for-ml-v1-0-028f2b85dc15@gmail.com> <20230223-z2-for-ml-v1-1-028f2b85dc15@gmail.com>
Precedence: bulk

> Date: Fri, 24 Feb 2023 11:20:06 +0100
> From: Sasha Finkelstein <fnkl.kernel@gmail.com>

Hi Sasha,

> Add bindings for touchscreen controllers attached using the Z2 protocol.
> Those are present in most Apple devices.
> 
> Signed-off-by: Sasha Finkelstein <fnkl.kernel@gmail.com>
> ---
>  .../input/touchscreen/apple,z2-touchscreen.yaml    | 81 ++++++++++++++++++++++
>  1 file changed, 81 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/input/touchscreen/apple,z2-touchscreen.yaml b/Documentation/devicetree/bindings/input/touchscreen/apple,z2-touchscreen.yaml
> new file mode 100644
> index 000000000000..695594494b1e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/touchscreen/apple,z2-touchscreen.yaml
> @@ -0,0 +1,81 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/input/touchscreen/apple,z2-touchscreen.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Apple touchscreens attached using the Z2 protocol.
> +
> +maintainers:
> +  - asahi@lists.linux.dev
> +  - Sasha Finkelstein <fnkl.kernel@gmail.com>
> +
> +description: A series of touschscreen controllers used in Apple products.
> +
> +allOf:
> +  - $ref: touchscreen.yaml#
> +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
> +
> +properties:
> +  compatible:
> +    const: apple,z2-touchscreen
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts-extended:
> +    maxItems: 1
> +
> +  reset-gpios:
> +    maxItems: 1
> +
> +  cs-gpios:
> +    maxItems: 1
> +
> +  firmware-name:
> +    maxItems: 1

What is the motivation for including the firmware name in the device
tree rather than constructing it in the driver like what is done for
the broadcom wireless?

In your example the firmware name includes the directory name.  I
think that is a bad idea since it makes assumptions about the
directory layout used for the firmware files on the OS level.  And in
particular, forcing the firmware to be in a subdirectory named "apple"
would be awkward for the way we handle firmware in OpenBSD.

> +
> +  apple,z2-device-name:
> +    description: The name to be used for the input device
> +    $ref: /schemas/types.yaml#/definitions/string
> +
> +  touchscreen-size-x: true
> +  touchscreen-size-y: true
> +  spi-max-frequency: true
> +
> +required:
> +  - compatible
> +  - interrupts-extended
> +  - reset-gpios
> +  - cs-gpios
> +  - firmware-name
> +  - apple,z2-device-name
> +  - touchscreen-size-x
> +  - touchscreen-size-y
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +
> +    spi {
> +            #address-cells = <1>;
> +            #size-cells = <0>;
> +
> +            touchscreen@0 {
> +                    compatible = "apple,z2-touchscreen";
> +                    reg = <0>;
> +                    spi-max-frequency = <11500000>;
> +                    reset-gpios = <&pinctrl_ap 139 0>;
> +                    cs-gpios = <&pinctrl_ap 109 0>;
> +                    interrupts-extended = <&pinctrl_ap 194 IRQ_TYPE_EDGE_FALLING>;
> +                    firmware-name = "apple/dfrmtfw-j293.bin";
> +                    touchscreen-size-x = <23045>;
> +                    touchscreen-size-y = <640>;
> +                    apple,z2-device-name = "MacBookPro17,1 Touch Bar";
> +            };
> +    };
> +
> +...
> 
> -- 
> Git-137.1)
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>