Re: [PATCH v2 4/5] arm64: dts: add description for solidrun am642 som and evaluation board

From: Josua Mayer <josua@solid-run.com>
To: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>,
	Nishanth Menon <nm@ti.com>, Vignesh Raghavendra <vigneshr@ti.com>,
	Tero Kristo <kristo@kernel.org>, Rob Herring <robh+dt@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Conor Dooley <conor+dt@kernel.org>,
	Alessandro Zummo <a.zummo@towertech.it>,
	Alexandre Belloni <alexandre.belloni@bootlin.com>
Cc: Yazan Shhady <yazan.shhady@solid-run.com>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-rtc@vger.kernel.org" <linux-rtc@vger.kernel.org>
Subject: Re: [PATCH v2 4/5] arm64: dts: add description for solidrun am642 som and evaluation board
Date: Tue, 16 Jan 2024 13:29:45 +0000	[thread overview]
Message-ID: <2cd24d3d-40a2-435e-bf60-bf6af24128b8@solid-run.com> (raw)
In-Reply-To: <5e0e6a28-58bb-4f28-aee7-28b05efe14b3@solid-run.com>

Am 15.01.24 um 11:32 schrieb Josua Mayer:
> Am 15.01.24 um 11:21 schrieb Krzysztof Kozlowski:
>> On 15/01/2024 11:05, Josua Mayer wrote:
>>> +
>>> +		ti,pruss-gp-mux-sel = <2>,	/* MII mode */
>>> +				      <2>,
>>> +				      <2>,
>>> +				      <2>,	/* MII mode */
>>> +				      <2>,
>>> +				      <2>;
This property is described in remoteproc/ti,pru-consumer.yaml
without explanation what each numeric value means.|
SoC TRM provides these names: "GP", "EnDAT", "MII", "SD".
Is it okay to keep number here, or are named constants needed?
Additionally, this array better to do in single line?:

/* configurei nternal mux for mii mode */
ti,pruss-gp-mux-sel = <2>, <2>, <2>, <2>, <2>, <2>;

>>> +
>>> +		ti,mii-g-rt = <&icssg1_mii_g_rt>;
>>> +		ti,mii-rt = <&icssg1_mii_rt>;
>>> +		ti,iep = <&icssg1_iep0>, <&icssg1_iep1>;
>>> +
>>> +		interrupt-parent = <&icssg1_intc>;
>>> +		interrupts = <24 0 2>, <25 1 3>;
>>>>>> None of these are typical interrupt constants/flags?
"pruss-intc" interrupt controller driver has a special xlate function,
taking 3 integer arguments: event, channel, host.

I came up with the below description:
/*
* icssg subsystem interrupt controller can be programmed
* for routing any of 64 predefined subsytem-internal events
* (documented in TRM) to one of 20 host interrupts.
* Some host interrupts are device-wide, others special
* purpose.
* Mapping is done via one of 20 channels - channel number
* decides processing priority (0 = highest).
*
*
* Map pru-internal interrupt #8 (24) via channel 0 to
* host-side pru interrupt #0 (2) (gic 246 / "host_intr0");
* and pru-internal interrupt #9 via channel 0 to
* host-side pru interrupt #1 (3) (gic 247 / "host_intr1").
*/

I feel it is a bit long to put in dts, perhaps I can put just second paragraph?
However this paragraph is still a bit confusing -
I should make it more readable, especially how 24 translates to 8 and 2 to 0, etc..

First paragraph is not needed, because bindings doc for this
interrupt controller has a good description for #interrupt-cells:
interrupt-controller/ti,pruss-intc.yaml

The 64 events, and 20 hosts have names in the TRM. Some of them
are particular, others are generic, e.g. INTR_REQ[0:15].
Creating a header with named constants seems doable.

Is it worth doing this?

Sincerely
Josua Mayer

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel