Message-ID: <1a98fe0a-fd6f-91ed-ff72-efecdf377334@ti.com>
Date:   Mon, 27 Feb 2023 14:05:57 +0530
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.7.1
Subject: Re: [PATCH v11 6/8] arm64: dts: ti: k3-j721s2: Add support for OSPI
 Flashes
Content-Language: en-US
To:     Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>, <nm@ti.com>,
        <afd@ti.com>, <vigneshr@ti.com>, <kristo@kernel.org>,
        <robh+dt@kernel.org>, <krzysztof.kozlowski+dt@linaro.org>,
        <s-vadapalli@ti.com>, <vaishnav.a@ti.com>
CC:     <linux-arm-kernel@lists.infradead.org>,
        <devicetree@vger.kernel.org>, <linux-kernel@vger.kernel.org>
References: <20230224102438.6541-1-r-gunasekaran@ti.com>
 <20230224102438.6541-7-r-gunasekaran@ti.com>
 <e20f5d80-e53a-559d-9a21-fcf77c1cf968@linaro.org>
From:   Ravi Gunasekaran <r-gunasekaran@ti.com>
In-Reply-To: <e20f5d80-e53a-559d-9a21-fcf77c1cf968@linaro.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
Precedence: bulk


On 24/02/23 4:41 pm, Krzysztof Kozlowski wrote:
> On 24/02/2023 11:24, Ravi Gunasekaran wrote:
>> From: Aswath Govindraju <a-govindraju@ti.com>
>>
>> J721S2 has an OSPI NOR flash on its SOM connected the OSPI0 instance and a
>> QSPI NOR flash on the common processor board connected to the OSPI1
>> instance. Add support for the same
>>
>> Reviewed-by: Vaishnav Achath <vaishnav.a@ti.com>
>> Signed-off-by: Aswath Govindraju <a-govindraju@ti.com>
>> Signed-off-by: Matt Ranostay <mranostay@ti.com>
>> Signed-off-by: Ravi Gunasekaran <r-gunasekaran@ti.com>
>> ---
>> Changes from v10:
>> * Removed Link tag from commit message
>>

[...]

>>
>>  .../dts/ti/k3-j721s2-common-proc-board.dts    | 39 +++++++++++++++++
>>  arch/arm64/boot/dts/ti/k3-j721s2-som-p0.dtsi  | 43 +++++++++++++++++++
>>  2 files changed, 82 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/ti/k3-j721s2-common-proc-board.dts b/arch/arm64/boot/dts/ti/k3-j721s2-common-proc-board.dts
>> index fa38940fe6cd..76b420379645 100644
>> --- a/arch/arm64/boot/dts/ti/k3-j721s2-common-proc-board.dts
>> +++ b/arch/arm64/boot/dts/ti/k3-j721s2-common-proc-board.dts
>> @@ -206,6 +206,20 @@
>>  			J721S2_WKUP_IOPAD(0x0c8, PIN_INPUT, 7) /* (C28) WKUP_GPIO0_2 */
>>  		>;
>>  	};
>> +
>> +	mcu_fss0_ospi1_pins_default: mcu-fss0-ospi1-pins-default {
>> +		pinctrl-single,pins = <
>> +			J721S2_WKUP_IOPAD(0x040, PIN_OUTPUT, 0) /* (A19) MCU_OSPI1_CLK */
>> +			J721S2_WKUP_IOPAD(0x05c, PIN_OUTPUT, 0) /* (D20) MCU_OSPI1_CSn0 */
>> +			J721S2_WKUP_IOPAD(0x060, PIN_OUTPUT, 0) /* (C21) MCU_OSPI1_CSn1 */
>> +			J721S2_WKUP_IOPAD(0x04c, PIN_INPUT, 0) /* (D21) MCU_OSPI1_D0 */
>> +			J721S2_WKUP_IOPAD(0x050, PIN_INPUT, 0) /* (G20) MCU_OSPI1_D1 */
>> +			J721S2_WKUP_IOPAD(0x054, PIN_INPUT, 0) /* (C20) MCU_OSPI1_D2 */
>> +			J721S2_WKUP_IOPAD(0x058, PIN_INPUT, 0) /* (A20) MCU_OSPI1_D3 */
>> +			J721S2_WKUP_IOPAD(0x048, PIN_INPUT, 0) /* (B19) MCU_OSPI1_DQS */
>> +			J721S2_WKUP_IOPAD(0x044, PIN_INPUT, 0) /* (B20) MCU_OSPI1_LBCLKO */
>> +		>;
>> +	};
>>  };
>>  
>>  &main_gpio2 {
>> @@ -347,6 +361,31 @@
>>  	maximum-speed = "high-speed";
>>  };
>>  
>> +&fss {
>> +	status = "okay";
> 
> Where is the pinmux usage you said is required for the bus?

Will remove the comment in [3/8] in the next series.

> 
>> +};
>> +
>> +&ospi1 {
>> +	status = "okay";
>> +	pinctrl-names = "default";
>> +	pinctrl-0 = <&mcu_fss0_ospi1_pins_default>;
>> +
>> +	flash@0{
>> +		compatible = "jedec,spi-nor";
>> +		reg = <0x0>;
>> +		spi-tx-bus-width = <1>;
>> +		spi-rx-bus-width = <4>;
>> +		spi-max-frequency = <40000000>;
>> +		cdns,tshsl-ns = <60>;
>> +		cdns,tsd2d-ns = <60>;
>> +		cdns,tchsh-ns = <60>;
>> +		cdns,tslch-ns = <60>;
>> +		cdns,read-delay = <2>;
>> +		#address-cells = <1>;
>> +		#size-cells = <1>;
> 
> Are you sure these are correct? Aren't they marked as deprecated?

Right. I found that "address-cells" and "size-cells" are deprecated. 
I will remove these in the next series.

Thank you for reviewing the patch.

> 
> 
> 
> Best regards,
> Krzysztof
> 

-- 
Regards,
Ravi