[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <781a97eb-ac7f-2986-9915-76f8543c61fe@ti.com>
Date: Tue, 1 Nov 2022 17:03:42 -0500
From: Andrew Davis <afd@...com>
To: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
BenoƮt Cousson <bcousson@...libre.com>,
Tony Lindgren <tony@...mide.com>, Nishanth Menon <nm@...com>,
Vignesh Raghavendra <vigneshr@...com>,
Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
<linux-omap@...r.kernel.org>, <devicetree@...r.kernel.org>,
<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/2] ARM: dts: ti: Add AM57xx GP EVM board support
On 10/15/22 10:21 AM, Krzysztof Kozlowski wrote:
> On 14/10/2022 11:26, Andrew Davis wrote:
>> The AM57xx GP EVM boards are built on top the AM57xx BeagleBoard-X15.
>> The EVM extends the BeagleBoard by adding a touchscreen, some buttons,
>> and a handful of peripheral extension slots.
>>
>> Being a plugin extension of an existing standalone board; we define
>> the am57xx-evm as a composite-DTB of the base am57xx-beagle-x15
>> and a new am57xx-evm overlay.
>>
>> Signed-off-by: Tero Kristo <t-kristo@...com>
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@...com>
>> Signed-off-by: Suman Anna <s-anna@...com>
>> Signed-off-by: Andrew Davis <afd@...com>
>> ---
>> arch/arm/boot/dts/Makefile | 2 +
>> arch/arm/boot/dts/am57xx-evm.dtso | 127 ++++++++++++++++++++++++++++++
>> 2 files changed, 129 insertions(+)
>> create mode 100644 arch/arm/boot/dts/am57xx-evm.dtso
>>
>> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
>> index 6aa7dc4db2fc..767220502021 100644
>> --- a/arch/arm/boot/dts/Makefile
>> +++ b/arch/arm/boot/dts/Makefile
>> @@ -984,10 +984,12 @@ dtb-$(CONFIG_SOC_OMAP5) += \
>> omap5-igep0050.dtb \
>> omap5-sbc-t54.dtb \
>> omap5-uevm.dtb
>> +am57xx-evm-dtbs := am57xx-beagle-x15.dtb am57xx-evm.dtbo
>> dtb-$(CONFIG_SOC_DRA7XX) += \
>> am57xx-beagle-x15.dtb \
>> am57xx-beagle-x15-revb1.dtb \
>> am57xx-beagle-x15-revc.dtb \
>> + am57xx-evm.dtb \
>> am5729-beagleboneai.dtb \
>> am57xx-cl-som-am57x.dtb \
>> am57xx-sbc-am57x.dtb \
>> diff --git a/arch/arm/boot/dts/am57xx-evm.dtso b/arch/arm/boot/dts/am57xx-evm.dtso
>> new file mode 100644
>> index 000000000000..6678aaef66ee
>> --- /dev/null
>> +++ b/arch/arm/boot/dts/am57xx-evm.dtso
>> @@ -0,0 +1,127 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * DT overlay for AM57xx GP EVM boards
>> + *
>> + * Copyright (C) 2020-2022 Texas Instruments Incorporated - https://www.ti.com/
>> + */
>> +
>> +/dts-v1/;
>> +/plugin/;
>> +
>> +#include <dt-bindings/interrupt-controller/irq.h>
>> +#include <dt-bindings/gpio/gpio.h>
>> +#include <dt-bindings/input/input.h>
>> +
>> +&{/} {
>> + compatible = "ti,am5728-evm", "ti,am572x-beagle-x15", "ti,am5728", "ti,dra742", "ti,dra74", "ti,dra7";
>
> You should start documenting these...
>
There are some odd bindings in the current txt based docs, will
need some time the fixup:
https://www.kernel.org/doc/Documentation/devicetree/bindings/arm/omap/omap.txt
I'll try converting this over to yaml after this series.
>> + model = "TI AM5728 EVM";
>> +
>> + aliases {
>> + display0 = "/display";
>> + display1 = "/connector"; // Fixme: &lcd0 and &hdmi0 could be
>> + // resolved here correcly based on
>> + // information in the base dtb symbol
>> + // table with a fix in dtc
>> + };
>> +
>> + gpio-keys {
>> + compatible = "gpio-keys";
>> +
>> + button-user1 {
>> + gpios = <&gpio2 23 GPIO_ACTIVE_LOW>;
>> + label = "USER1";
>> + linux,code = <BTN_1>;
>> + };
>> +
>> + button-user2 {
>> + gpios = <&gpio2 25 GPIO_ACTIVE_LOW>;
>> + label = "USER2";
>> + linux,code = <BTN_2>;
>> + };
>> +
>> + button-user3 {
>> + gpios = <&gpio2 28 GPIO_ACTIVE_LOW>;
>> + label = "USER3";
>> + linux,code = <BTN_3>;
>> + };
>> +
>> + button-user4 {
>> + gpios = <&gpio2 24 GPIO_ACTIVE_LOW>;
>> + label = "USER4";
>> + linux,code = <BTN_4>;
>> + };
>> +
>> + button-user5 {
>> + gpios = <&gpio2 20 GPIO_ACTIVE_LOW>;
>> + label = "USER5";
>> + linux,code = <BTN_5>;
>> + };
>> + };
>> +
>> + lcd0: display {
>> + compatible = "osddisplays,osd070t1718-19ts", "panel-dpi";
>> + backlight = <&lcd_bl>;
>> + enable-gpios = <&gpio2 5 GPIO_ACTIVE_HIGH>;
>> + label = "lcd";
>> +
>> + port {
>> + lcd_in: endpoint {
>> + remote-endpoint = <&dpi_out>;
>> + };
>> + };
>> + };
>> +
>> + lcd_bl: backlight {
>> + compatible = "pwm-backlight";
>> + brightness-levels = <0 243 245 247 249 251 252 253 255>;
>> + default-brightness-level = <8>;
>> + pwms = <&ehrpwm1 0 50000 0>;
>
> Don't you have here PWM flag?
>
The invert flag? Nope, this is not inverted. And there is no
define for none, just 0.
>> + };
>> +};
>> +
>> +&ehrpwm1 {
>> + status = "okay";
>> +};
>> +
>> +&epwmss1 {
>> + status = "okay";
>> +};
>> +
>> +&i2c5 {
>> + status = "okay";
>> + clock-frequency = <400000>;
>> +
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> +
>> + pixcir_ts@5c {
>
> No underscores in node names.
>
> Node names should be generic.
> https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
>
ACK
>> + compatible = "pixcir,pixcir_tangoc";
>> + attb-gpio = <&gpio2 4 GPIO_ACTIVE_HIGH>;
>> + interrupt-parent = <&gpio2>;
>> + interrupts = <4 0>;
>
> Use proper flags (and not NONE).
>
ACK, will fix.
Thanks,
Andrew
>> + reg = <0x5c>;
>> + reset-gpio = <&gpio2 6 GPIO_ACTIVE_HIGH>;
>> + touchscreen-size-x = <1024>;
>> + touchscreen-size-y = <600>;
> Best regards,
> Krzysztof
>
Powered by blists - more mailing lists