| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <D79CRMSWU0F1.30RZ853AES515@bootlin.com>
Date: Thu, 23 Jan 2025 10:41:57 +0100
From: "Antonin Godard" <antonin.godard@...tlin.com>
To: "Krzysztof Kozlowski" <krzk@...nel.org>, "Rob Herring"
<robh@...nel.org>, "Krzysztof Kozlowski" <krzk+dt@...nel.org>, "Conor
Dooley" <conor+dt@...nel.org>, "Shawn Guo" <shawnguo@...nel.org>, "Sascha
Hauer" <s.hauer@...gutronix.de>, "Pengutronix Kernel Team"
<kernel@...gutronix.de>, "Fabio Estevam" <festevam@...il.com>
Cc: "Thomas Petazzoni" <thomas.petazzoni@...tlin.com>,
<devicetree@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
<imx@...ts.linux.dev>, <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH 3/3] ARM: dts: imx6ul: Add Variscite Concerto board
support
Hi Krzysztof,
On Tue Jan 21, 2025 at 5:03 PM CET, Krzysztof Kozlowski wrote:
> On 21/01/2025 10:33, Antonin Godard wrote:
>> This patch adds support for the Variscite Concerto Carrier Board.
>
> Please do not use "This commit/patch/change", but imperative mood. See
> longer explanation here:
> https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95
Will do in v2.
>>
>> This Carrier-Board has the following:
>> - LVDS interface for the VLCD-CAP-GLD-LVDS 7" LCD 800 x 480 touch
>> display (not configured)
>> - USB Host + USB OTG Connector
>> - 10/100 Mbps Ethernet
>> - miniPCI-Express slot
>> - SD Card connector
>> - Audio Headphone/Line In jack connectors
>> - S-ATA
>> - On-board DMIC
>>
>> Product Page: https://www.variscite.com/product/single-board-computers/concerto-board
>>
>> This file is based on the one provided by Variscite on their own kernel,
>> but adapted for mainline.
>>
>> Signed-off-by: Antonin Godard <antonin.godard@...tlin.com>
>> ---
>> arch/arm/boot/dts/nxp/imx/Makefile | 1 +
>> .../boot/dts/nxp/imx/imx6ul-var-som-concerto.dts | 331 +++++++++++++++++++++
>> 2 files changed, 332 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/nxp/imx/Makefile b/arch/arm/boot/dts/nxp/imx/Makefile
>> index 39a153536d2a2b8f75b5fbe4332660f89442064a..94c9bc94cc8e2daa1fb3b5686b0b58db1f6678b6 100644
>> --- a/arch/arm/boot/dts/nxp/imx/Makefile
>> +++ b/arch/arm/boot/dts/nxp/imx/Makefile
>> @@ -329,6 +329,7 @@ dtb-$(CONFIG_SOC_IMX6UL) += \
>> imx6ul-tx6ul-0010.dtb \
>> imx6ul-tx6ul-0011.dtb \
>> imx6ul-tx6ul-mainboard.dtb \
>> + imx6ul-var-som-concerto.dtb \
>> imx6ull-14x14-evk.dtb \
>> imx6ull-colibri-aster.dtb \
>> imx6ull-colibri-emmc-aster.dtb \
>> diff --git a/arch/arm/boot/dts/nxp/imx/imx6ul-var-som-concerto.dts b/arch/arm/boot/dts/nxp/imx/imx6ul-var-som-concerto.dts
>> new file mode 100644
>> index 0000000000000000000000000000000000000000..4289641d94c5a72ba985f339652039dbf13da40c
>> --- /dev/null
>> +++ b/arch/arm/boot/dts/nxp/imx/imx6ul-var-som-concerto.dts
>> @@ -0,0 +1,331 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * Support for Variscite MX6 Concerto Carrier board with the VAR-SOM-MX6UL
>> + * Variscite SoM mounted on it
>> + *
>> + * Copyright 2019 Variscite Ltd.
>> + * Copyright 2025 Bootlin
>> + */
>> +
>> +#include "imx6ul-var-som.dtsi"
>> +
>> +/ {
>> + model = "Variscite VAR-SOM-MX6UL Concerto Board";
>> + compatible = "variscite,mx6concerto", "variscite,var-som-imx6ul", "fsl,imx6ul";
>> +
>> + backlight {
>> + compatible = "pwm-backlight";
>> + pwms = <&pwm4 0 20000 0>;
>> + brightness-levels = <0 4 8 16 32 64 128 255>;
>> + default-brightness-level = <6>;
>> + status = "okay";
>
> Which file disables it?
This is a mistake, I forgot to remove this when removing the parts I couldn't
test. Will remove this node in v2.
>> + };
>> +
>> + chosen {
>> + stdout-path = &uart1;
>> + };
>> +
>> + gpio-keys {
>> + compatible = "gpio-keys";
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&pinctrl_gpio_key_back>, <&pinctrl_gpio_key_wakeup>;
>> +
>> + key-back {
>> + gpios = <&gpio4 14 GPIO_ACTIVE_LOW>;
>> + linux,code = <KEY_BACK>;
>> + };
>> +
>> + key-wakeup {
>> + gpios = <&gpio5 8 GPIO_ACTIVE_LOW>;
>> + linux,code = <KEY_WAKEUP>;
>> + wakeup-source;
>> + };
>> + };
>> +
>> + leds {
>> + compatible = "gpio-leds";
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&pinctrl_gpio_leds>;
>> +
>> + gpled2 {
>
> led-0
> led-1
> led-2
Will rename this node to led-0, and set the label to "gpled2" (this is how it's
named on the schematic/datasheet).
> Are there other leds here?
Nothing else is obvious to me on the schematic. There is no gpled0 or gpled1.
>> + gpios = <&gpio1 25 GPIO_ACTIVE_HIGH>;
>> + linux,default-trigger = "heartbeat";
>
> Missing function and color
Will set the function to STATUS, color green. This is a general purpose led that
can be used for debugging purposes or as a status indicator, I think.
>> + };
>> + };
>> +};
>> +
>> +&can1 {
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&pinctrl_flexcan1>;
>> + status = "okay";
>> +};
>> +
>> +&fec1 {
>> + status = "disabled";
>> +};
>> +
>> +&fec2 {
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&pinctrl_enet2>, <&pinctrl_enet2_gpio>, <&pinctrl_enet2_mdio>;
>> + phy-mode = "rmii";
>> + phy-handle = <ðphy1>;
>> + phy-reset-gpios = <&gpio5 5 GPIO_ACTIVE_LOW>;
>> + phy-reset-duration = <100>;
>> + status = "okay";
>> +
>> + mdio {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> +
>> + ethphy1: ethernet-phy@3 {
>> + compatible = "ethernet-phy-ieee802.3-c22";
>> + micrel,rmii-reference-clock-select-25-mhz = <1>;
>> + micrel,led-mode = <0>;
>> + clocks = <&rmii_ref_clk>;
>> + clock-names = "rmii-ref";
>> + reg = <3>;
>> + };
>> + };
>> +};
>> +
>> +&i2c1 {
>> + clock-frequency = <100000>;
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&pinctrl_i2c1>;
>> + status = "okay";
>> +
>> + /* DS1337 RTC module */
>
> Drop comment, obvious. This cannot be anything else, because node name
> and compatible told that.
Will do in v2.
>> + rtc@68 {
>> + /*
>> + * To actually use this interrupt
>> + * connect pins J14.8 & J14.10 on the Concerto-Board.
>> + */
>> + compatible = "dallas,ds1337";
>> + reg = <0x68>;
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&pinctrl_rtc>;
>> + interrupt-parent = <&gpio1>;
>> + interrupts = <10 IRQ_TYPE_EDGE_FALLING>;
>> + };
>> +};
>
>
> Best regards,
> Krzysztof
Thanks for the review,
Antonin
--
Antonin Godard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
Powered by blists - more mailing lists