mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Francesco Dolcini <francesco@dolcini.it>
To: Frank Li <Frank.li@oss.nxp.com>
Cc: Leonardo Costa <leoreis.costa@gmail.com>,
	robh@kernel.org, krzk+dt@kernel.org, conor+dt@kernel.org,
	Frank.Li@nxp.com, s.hauer@pengutronix.de, kernel@pengutronix.de,
	festevam@gmail.com, francesco.dolcini@toradex.com,
	leonardo.costa@toradex.com, hvilleneuve@dimonoff.com,
	marex@nabladev.com, stefano.r@variscite.com,
	devicetree@vger.kernel.org, imx@lists.linux.dev,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 3/7] ARM: dts: imx6q-apalis: Add Toradex Capacitive Touch Display 10.1" LVDS
Date: Fri, 2 Oct 2026 10:18:24 +0200	[thread overview]
Message-ID: <20261002081824.GA7222@francesco-nb> (raw)
In-Reply-To: <ar6JnZp1jBvlc8bx@SMW015318>

Hello Frank,
thanks for the review

On Thu, Oct 01, 2026 at 11:26:05AM -0500, Frank Li wrote:
> On Thu, Oct 01, 2026 at 12:52:53PM -0300, Leonardo Costa wrote:
> > From: Leonardo Costa <leonardo.costa@toradex.com>
> >
> > Add a device tree overlay for the Toradex Capacitive Touch Display 10.1"
> > LVDS connected via the Apalis iMX6 LDB.
> >
> > The panel is a LogicTechno LT170410-2WHC 10.1" WXGA IPS LCD and the
> > touch input is provided by an Atmel MaxTouch capacitive touch
> > controller.
> >
> > Remove the panel-lvds node from the Apalis iMX6 dtsi, as it is an
> > external component that does not exist at the SoM level.
> >
> > The overlay is also combined with the Apalis iMX6 V1.2 Ixora Carrier
> > Board V1.2 device tree to provide a ready-to-use DTB.
> >
> > Link: https://developer.toradex.com/hardware/accessories/displays/capacitive-touch-display-101inch-lvds
> > Signed-off-by: Leonardo Costa <leonardo.costa@toradex.com>
> > ---
> >  arch/arm/boot/dts/nxp/imx/Makefile            |  6 +++
> >  ...6q-apalis-panel-cap-touch-10inch-lvds.dtso | 53 +++++++++++++++++++
> >  arch/arm/boot/dts/nxp/imx/imx6qdl-apalis.dtsi | 13 -----
> >  3 files changed, 59 insertions(+), 13 deletions(-)
> >  create mode 100644 arch/arm/boot/dts/nxp/imx/imx6q-apalis-panel-cap-touch-10inch-lvds.dtso
> 
> similar 7" case, add panel module name in file
> 
> imx6q-apalis-lvds-panel-lt170410.dtso

This does not work, sorry, the current name is the correct one, for
various reasons:

 - the product is a display made with a specific connector, touch
   controller and display and more. The actual panel is just part of it
 - the current name wholly describe the product, it's a public product
   with an official name, all of that is clearly linked in the commit
   message and comments. there is no ambiguity.
 - the same toradex accessories are not module specific, they are used
   across multiple families/carrier board. It is a whole ecosystem that is
   building on top of standardized interfaces and connectors. The same
   overlay file is available for multiple boards and in multiple SoC
   vendor directory (as of now TI and NXP, soon we are going to have
   also QCOM). Having a consistent naming scheme is important, we cannot
   call the same things differently every time.
 - there was a situation in which we did a new product revision of a
   display, specifically the "Toradex Capacitive Touch Display 10.1"
   LVDS" there are two versions. The official product name is the same,
   apart an additional version number, one is version1, the other is
   version2. They have differences, and it's not just the panel, more
   stuff changed, so having the panel name in the filename will not
   help. v2 support is already in [1], for reference.
 - the toradex naming scheme is not encoding the actual part number used
   in the product name, for example we have apalis imx6 v1.2 that uses a
   different touch/adc than previous apalis imx v1.1. The product has a
   different schematics, different BoM and so on, and there is no
   reference of the difference touch/adc in the name. You can see this
   information from the public documentation just looking at the
   version. or you can check yourself comparing the two DTS in the linux
   kernel tree.

[1]
 arch/arm64/boot/dts/ti/k3-am625-verdin-panel-cap-touch-10inch-lvds.dtso
 arch/arm64/boot/dts/ti/k3-am625-verdin-panel-cap-touch-10inch-lvds-v2.dtso


Frank: in general the names are clearly linked to the official product
name, and this applies also to other patches in which you commented
about the names, not planning to reply to every single one.

> > diff --git a/arch/arm/boot/dts/nxp/imx/imx6q-apalis-panel-cap-touch-10inch-lvds.dtso b/arch/arm/boot/dts/nxp/imx/imx6q-apalis-panel-cap-touch-10inch-lvds.dtso
> > new file mode 100644
> > index 0000000000000..a84114e1d3fba
> > --- /dev/null
> > +++ b/arch/arm/boot/dts/nxp/imx/imx6q-apalis-panel-cap-touch-10inch-lvds.dtso
> > @@ -0,0 +1,53 @@
> > +// SPDX-License-Identifier: GPL-2.0-only OR MIT
> > +/*
> > + * Copyright (c) Toradex
> > + *
> > + * Toradex Capacitive Touch Display 10.1" connected via Apalis iMX6 LDB
> > + * on carrier boards with a Toradex standard LVDS display connector.
> > + *
> > + * https://docs.toradex.com/105952-10-1-inch-lvds-capacitive-touch-display-1280x800-datasheet.pdf
> > + * https://developer.toradex.com/hardware/accessories/displays/capacitive-touch-display-101inch-lvds
> > + * https://www.toradex.com/accessories/capacitive-touch-display-10.1-inch-lvds
> > + */
> > +
> > +/dts-v1/;
> > +/plugin/;
> > +
> > +&{/} {
> > +	panel-lvds {
> > +		compatible = "logictechno,lt170410-2whc";
> > +		backlight = <&backlight>;
> > +		power-supply = <&reg_3v3_sw>;
> 
> use name reg_lvds_panel, it help improve reusablity.

I disagree.

The regulator should be the one that is physically used on
the board. The DTS *must* describe the HW as accurately as possible, we
are not supposed to invent non existing regulator and more in general
non existing HW.

I see your need to avoid duplication, and I agree with it. But an
accurate HW description and the user experience trumps this need. 
And I insist on the user experience, what we are doing is for someone to
use, we should not make the life of people hard because we decide on
non-descriptive or inaccurate names.


Francesco


  reply	other threads:[~2026-10-02  8:18 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-01 15:52 [PATCH 0/7] ARM: dts: Add Apalis iMX6 overlays Leonardo Costa
2026-10-01 15:52 ` [PATCH 1/7] ARM: dts: imx6q-apalis: Add HDMI Overlay Leonardo Costa
2026-10-01 16:11   ` Frank Li
2026-10-01 15:52 ` [PATCH 2/7] ARM: dts: imx6q-apalis-ixora: Add 3.3V_SW power regulator Leonardo Costa
2026-10-01 15:52 ` [PATCH 3/7] ARM: dts: imx6q-apalis: Add Toradex Capacitive Touch Display 10.1" LVDS Leonardo Costa
2026-10-01 16:26   ` Frank Li
2026-10-02  8:18     ` Francesco Dolcini [this message]
2026-10-02 16:28       ` Frank Li
2026-10-05  7:53         ` Francesco Dolcini
2026-10-01 15:52 ` [PATCH 4/7] ARM: dts: imx6q-apalis: Add Toradex Capacitive Touch Display 7" Parallel Leonardo Costa
2026-10-01 15:52 ` [PATCH 5/7] ARM: dts: imx6q-apalis: Add Toradex Resistive " Leonardo Costa
2026-10-01 16:19   ` Frank Li
2026-10-01 15:52 ` [PATCH 6/7] ARM: dts: imx6q-apalis: Add NAU8822 Bridge Tied Load Leonardo Costa
2026-10-01 15:52 ` [PATCH 7/7] ARM: dts: imx6q-apalis: Add Toradex OV5640 CSI Cameras Leonardo Costa
2026-10-01 16:21   ` Frank Li

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20261002081824.GA7222@francesco-nb \
    --to=francesco@dolcini.it \
    --cc=Frank.Li@nxp.com \
    --cc=Frank.li@oss.nxp.com \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=festevam@gmail.com \
    --cc=francesco.dolcini@toradex.com \
    --cc=hvilleneuve@dimonoff.com \
    --cc=imx@lists.linux.dev \
    --cc=kernel@pengutronix.de \
    --cc=krzk+dt@kernel.org \
    --cc=leonardo.costa@toradex.com \
    --cc=leoreis.costa@gmail.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=marex@nabladev.com \
    --cc=robh@kernel.org \
    --cc=s.hauer@pengutronix.de \
    --cc=stefano.r@variscite.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®