linux-kernel - Re: [PATCH] arm64: dts: exynos: gs101: define all PERIC USI nodes

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Fri, 22 Mar 2024 13:39:46 +0000
From: Tudor Ambarus <tudor.ambarus@...aro.org>
To: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
 peter.griffin@...aro.org, robh+dt@...nel.org,
 krzysztof.kozlowski+dt@...aro.org, conor+dt@...nel.org
Cc: alim.akhtar@...sung.com, linux-arm-kernel@...ts.infradead.org,
 linux-samsung-soc@...r.kernel.org, devicetree@...r.kernel.org,
 linux-kernel@...r.kernel.org, andre.draszik@...aro.org,
 willmcvicker@...gle.com, kernel-team@...roid.com
Subject: Re: [PATCH] arm64: dts: exynos: gs101: define all PERIC USI nodes


Hi, Krzysztof!

On 3/8/24 16:45, Krzysztof Kozlowski wrote:
> On 07/03/2024 14:59, Tudor Ambarus wrote:
>> Universal Serial Interface (USI) supports three types of serial
>> interface such as UART, SPI and I2C. Each protocol works independently.
>> USI can be configured to work as one of these protocols. Define all the
>> USI nodes from the PERIC blocks (USI0-14), in all their possible
>> configurations. These blocks have the TX/RX FIFO depth of 64 bytes.
>>
>> Signed-off-by: Tudor Ambarus <tudor.ambarus@...aro.org>
>> ---
>> Please note that:
>> - google,gs101-spi compatible was queued through the SPI tree:
>>   https://lore.kernel.org/linux-arm-kernel/170742731537.2266792.3851016361229293846.b4-ty@kernel.org/
>> - SPI dma properties were marked as not requiered. Queued through the
>>   SPI tree:
>>   https://lore.kernel.org/linux-spi/170967132774.228925.1759895846287455970.b4-ty@kernel.org/
>>
>>  arch/arm64/boot/dts/exynos/google/gs101.dtsi | 782 +++++++++++++++++++
>>  1 file changed, 782 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/exynos/google/gs101.dtsi b/arch/arm64/boot/dts/exynos/google/gs101.dtsi
>> index ee65ed9d2cfc..d7ecfbc7e440 100644
>> --- a/arch/arm64/boot/dts/exynos/google/gs101.dtsi
>> +++ b/arch/arm64/boot/dts/exynos/google/gs101.dtsi
>> @@ -373,6 +373,398 @@ pinctrl_peric0: pinctrl@...40000 {

cut

>> +
>> +			hsi2c_1: i2c@...00000 {
>> +				compatible = "google,gs101-hsi2c",
>> +					     "samsung,exynosautov9-hsi2c";
>> +				reg = <0x10900000 0xc0>;
>> +				interrupts = <GIC_SPI 635 IRQ_TYPE_LEVEL_HIGH 0>;
> 
> I wonder why we use four cells in GIC...
> 
>> +				#address-cells = <1>;
>> +				#size-cells = <0>;

I'd like to respin this patch. Any preference on coding style for
#address-cells and #size-cells? I guess they shall be above ranges
property if present.

>> +				pinctrl-names = "default";
>> +				pinctrl-0 = <&hsi2c1_bus>;
> 
> Please reverse two lines, first pinctrl-0 then pinctrl-names. I know we

Ok.

> did not follow this convention till now, but at least new code can be
> correct. Also clocks should be before pinctrl, so we keep some sort of
> alphabetical order.

Ok.

I guess the order shall be:

1. compatible
2. reg
3. #address-cells (if applicable)
   #size-cells (if applicable)
4. ranges (if applicable)
5. Standard/common properties ordered alphabetically (ex. clocks,
   interrupts, pinctrl)
6. vendor-specific properties
7. status (if applicable)

Please let me know if you have other preference. Thanks!
ta