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

[Tudor Ambarus <tudor.ambarus@linaro.org>]

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@linaro.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@10840000 {

cut

>> +
>> +			hsi2c_1: i2c@10900000 {
>> +				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