[RESEND,v1,2/2] riscv: dts: starfive: Add spi node for JH7110 SoC
Commit Message
Add spi node for JH7110 SoC.
Co-developed-by: Xingyu Wu <xingyu.wu@starfivetech.com>
Signed-off-by: William Qiu <william.qiu@starfivetech.com>
---
.../jh7110-starfive-visionfive-2.dtsi | 52 ++++++++++
arch/riscv/boot/dts/starfive/jh7110.dtsi | 98 +++++++++++++++++++
2 files changed, 150 insertions(+)
--
2.34.1
Comments
On 04/07/2023 11:22, William Qiu wrote:
> Add spi node for JH7110 SoC.
>
> Co-developed-by: Xingyu Wu <xingyu.wu@starfivetech.com>
Missing SoB.
> Signed-off-by: William Qiu <william.qiu@starfivetech.com>
> ---
> .../jh7110-starfive-visionfive-2.dtsi | 52 ++++++++++
> arch/riscv/boot/dts/starfive/jh7110.dtsi | 98 +++++++++++++++++++
> 2 files changed, 150 insertions(+)
>
> diff --git a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
> index 2a6d81609284..a066d2e399c4 100644
> --- a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
> +++ b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
> @@ -126,6 +126,20 @@ &i2c6 {
> status = "okay";
> };
>
> +&spi0 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&spi0_pins>;
> + status = "okay";
> +
> + spi_dev0: spi@0 {
> + compatible = "st,m25p80";
> + pl022,com-mode = <1>;
> + spi-max-frequency = <10000000>;
> + reg = <0>;
reg is always following compatible, not somewhere deep in properties.
> + status = "okay";
okay is by default
> + };
> +};
Best regards,
Krzysztof
On Tue, Jul 04, 2023 at 11:39:29AM +0200, Krzysztof Kozlowski wrote:
> On 04/07/2023 11:22, William Qiu wrote:
> > Add spi node for JH7110 SoC.
> >
> > Co-developed-by: Xingyu Wu <xingyu.wu@starfivetech.com>
>
> Missing SoB.
It's fine not to have a signoff for the codeveloper of codeveloped
patches, see case (a) for the DCO.
On 04/07/2023 14:26, Mark Brown wrote:
> On Tue, Jul 04, 2023 at 11:39:29AM +0200, Krzysztof Kozlowski wrote:
>> On 04/07/2023 11:22, William Qiu wrote:
>>> Add spi node for JH7110 SoC.
>>>
>>> Co-developed-by: Xingyu Wu <xingyu.wu@starfivetech.com>
>>
>> Missing SoB.
>
> It's fine not to have a signoff for the codeveloper of codeveloped
> patches, see case (a) for the DCO.
Also see:
"every Co-developed-by: must be immediately
followed by a Signed-off-by: of the associated co-author."
https://elixir.bootlin.com/linux/v6.4/source/Documentation/process/submitting-patches.rst#L467
Best regards,
Krzysztof
On Tue, Jul 04, 2023 at 02:27:57PM +0200, Krzysztof Kozlowski wrote:
> Also see:
> "every Co-developed-by: must be immediately
> followed by a Signed-off-by: of the associated co-author."
> https://elixir.bootlin.com/linux/v6.4/source/Documentation/process/submitting-patches.rst#L467
Oh, that seems unhelpful especially with it not lining up with the DCO.
On 04/07/2023 15:16, Mark Brown wrote:
> On Tue, Jul 04, 2023 at 02:27:57PM +0200, Krzysztof Kozlowski wrote:
>
>> Also see:
>> "every Co-developed-by: must be immediately
>> followed by a Signed-off-by: of the associated co-author."
>
>> https://elixir.bootlin.com/linux/v6.4/source/Documentation/process/submitting-patches.rst#L467
>
> Oh, that seems unhelpful especially with it not lining up with the DCO.
I assume the intention was here that if I attribute some co-author with
Co-developed-by, then I know that author, therefore I expect author to
explicitly participate in DCO chain.
Otherwise, just drop the Co-developed-by.
Best regards,
Krzysztof
On Tue, Jul 04, 2023 at 03:21:30PM +0200, Krzysztof Kozlowski wrote:
> On 04/07/2023 15:16, Mark Brown wrote:
> > On Tue, Jul 04, 2023 at 02:27:57PM +0200, Krzysztof Kozlowski wrote:
> >> Also see:
> >> "every Co-developed-by: must be immediately
> >> followed by a Signed-off-by: of the associated co-author."
> >> https://elixir.bootlin.com/linux/v6.4/source/Documentation/process/submitting-patches.rst#L467
> > Oh, that seems unhelpful especially with it not lining up with the DCO.
> I assume the intention was here that if I attribute some co-author with
> Co-developed-by, then I know that author, therefore I expect author to
> explicitly participate in DCO chain.
Why? They're not the one sending the patch out, nor are they relying on
someone else having certified anything.
> Otherwise, just drop the Co-developed-by.
It seems separately useful.
On Tue, Jul 04, 2023 at 03:13:17PM +0100, Mark Brown wrote:
> On Tue, Jul 04, 2023 at 03:21:30PM +0200, Krzysztof Kozlowski wrote:
> > On 04/07/2023 15:16, Mark Brown wrote:
> > > On Tue, Jul 04, 2023 at 02:27:57PM +0200, Krzysztof Kozlowski wrote:
>
> > >> Also see:
> > >> "every Co-developed-by: must be immediately
> > >> followed by a Signed-off-by: of the associated co-author."
>
> > >> https://elixir.bootlin.com/linux/v6.4/source/Documentation/process/submitting-patches.rst#L467
>
> > > Oh, that seems unhelpful especially with it not lining up with the DCO.
>
> > I assume the intention was here that if I attribute some co-author with
> > Co-developed-by, then I know that author, therefore I expect author to
> > explicitly participate in DCO chain.
>
> Why? They're not the one sending the patch out, nor are they relying on
> someone else having certified anything.
It's probably safe to say that StarFive owns the contributions anyway,
so I doubt adding really makes a difference here.
> > Otherwise, just drop the Co-developed-by.
>
> It seems separately useful.
Yup, I'd rather have the people there if I ever have to run `git blame`
on whatever commit this becomes.
On 2023/7/4 17:39, Krzysztof Kozlowski wrote:
> On 04/07/2023 11:22, William Qiu wrote:
>> Add spi node for JH7110 SoC.
>>
>> Co-developed-by: Xingyu Wu <xingyu.wu@starfivetech.com>
>
> Missing SoB.
>
It looks like that drop it is the best solution.
>> Signed-off-by: William Qiu <william.qiu@starfivetech.com>
>> ---
>> .../jh7110-starfive-visionfive-2.dtsi | 52 ++++++++++
>> arch/riscv/boot/dts/starfive/jh7110.dtsi | 98 +++++++++++++++++++
>> 2 files changed, 150 insertions(+)
>>
>> diff --git a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
>> index 2a6d81609284..a066d2e399c4 100644
>> --- a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
>> +++ b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
>> @@ -126,6 +126,20 @@ &i2c6 {
>> status = "okay";
>> };
>>
>> +&spi0 {
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&spi0_pins>;
>> + status = "okay";
>> +
>> + spi_dev0: spi@0 {
>> + compatible = "st,m25p80";
>> + pl022,com-mode = <1>;
>> + spi-max-frequency = <10000000>;
>> + reg = <0>;
>
> reg is always following compatible, not somewhere deep in properties.
>
Will update.
>> + status = "okay";
>
> okay is by default
>
Will drop.
>> + };
>> +};
>
>
> Best regards,
> Krzysztof
>
Thanks for taking time to review this patch series.
@@ -126,6 +126,20 @@ &i2c6 {
status = "okay";
};
+&spi0 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&spi0_pins>;
+ status = "okay";
+
+ spi_dev0: spi@0 {
+ compatible = "st,m25p80";
+ pl022,com-mode = <1>;
+ spi-max-frequency = <10000000>;
+ reg = <0>;
+ status = "okay";
+ };
+};
+
&sysgpio {
i2c0_pins: i2c0-0 {
i2c-pins {
@@ -183,6 +197,44 @@ GPOEN_SYS_I2C6_DATA,
};
};
+ spi0_pins: spi0-0 {
+ mosi-pins {
+ pinmux = <GPIOMUX(52, GPOUT_SYS_SPI0_TXD,
+ GPOEN_ENABLE,
+ GPI_NONE)>;
+ bias-disable;
+ input-disable;
+ input-schmitt-disable;
+ };
+
+ miso-pins {
+ pinmux = <GPIOMUX(53, GPOUT_LOW,
+ GPOEN_DISABLE,
+ GPI_SYS_SPI0_RXD)>;
+ bias-pull-up;
+ input-enable;
+ input-schmitt-enable;
+ };
+
+ sck-pins {
+ pinmux = <GPIOMUX(48, GPOUT_SYS_SPI0_CLK,
+ GPOEN_ENABLE,
+ GPI_SYS_SPI0_CLK)>;
+ bias-disable;
+ input-disable;
+ input-schmitt-disable;
+ };
+
+ ss-pins {
+ pinmux = <GPIOMUX(48, GPOUT_SYS_SPI0_FSS,
+ GPOEN_ENABLE,
+ GPI_SYS_SPI0_FSS)>;
+ bias-disable;
+ input-disable;
+ input-schmitt-disable;
+ };
+ };
+
uart0_pins: uart0-0 {
tx-pins {
pinmux = <GPIOMUX(5, GPOUT_SYS_UART0_TX,
@@ -353,6 +353,48 @@ i2c2: i2c@10050000 {
status = "disabled";
};
+ spi0: spi@10060000 {
+ compatible = "arm,pl022", "arm,primecell";
+ reg = <0x0 0x10060000 0x0 0x10000>;
+ clocks = <&syscrg JH7110_SYSCLK_SPI0_APB>;
+ clock-names = "apb_pclk";
+ resets = <&syscrg JH7110_SYSRST_SPI0_APB>;
+ interrupts = <38>;
+ arm,primecell-periphid = <0x00041022>;
+ num-cs = <1>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ status = "disabled";
+ };
+
+ spi1: spi@10070000 {
+ compatible = "arm,pl022", "arm,primecell";
+ reg = <0x0 0x10070000 0x0 0x10000>;
+ clocks = <&syscrg JH7110_SYSCLK_SPI1_APB>;
+ clock-names = "apb_pclk";
+ resets = <&syscrg JH7110_SYSRST_SPI1_APB>;
+ interrupts = <39>;
+ arm,primecell-periphid = <0x00041022>;
+ num-cs = <1>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ status = "disabled";
+ };
+
+ spi2: spi@10080000 {
+ compatible = "arm,pl022", "arm,primecell";
+ reg = <0x0 0x10080000 0x0 0x10000>;
+ clocks = <&syscrg JH7110_SYSCLK_SPI2_APB>;
+ clock-names = "apb_pclk";
+ resets = <&syscrg JH7110_SYSRST_SPI2_APB>;
+ interrupts = <40>;
+ arm,primecell-periphid = <0x00041022>;
+ num-cs = <1>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ status = "disabled";
+ };
+
uart3: serial@12000000 {
compatible = "snps,dw-apb-uart";
reg = <0x0 0x12000000 0x0 0x10000>;
@@ -440,6 +482,62 @@ i2c6: i2c@12060000 {
status = "disabled";
};
+ spi3: spi@12070000 {
+ compatible = "arm,pl022", "arm,primecell";
+ reg = <0x0 0x12070000 0x0 0x10000>;
+ clocks = <&syscrg JH7110_SYSCLK_SPI3_APB>;
+ clock-names = "apb_pclk";
+ resets = <&syscrg JH7110_SYSRST_SPI3_APB>;
+ interrupts = <52>;
+ arm,primecell-periphid = <0x00041022>;
+ num-cs = <1>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ status = "disabled";
+ };
+
+ spi4: spi@12080000 {
+ compatible = "arm,pl022", "arm,primecell";
+ reg = <0x0 0x12080000 0x0 0x10000>;
+ clocks = <&syscrg JH7110_SYSCLK_SPI4_APB>;
+ clock-names = "apb_pclk";
+ resets = <&syscrg JH7110_SYSRST_SPI4_APB>;
+ interrupts = <53>;
+ arm,primecell-periphid = <0x00041022>;
+ num-cs = <1>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ status = "disabled";
+ };
+
+ spi5: spi@12090000 {
+ compatible = "arm,pl022", "arm,primecell";
+ reg = <0x0 0x12090000 0x0 0x10000>;
+ clocks = <&syscrg JH7110_SYSCLK_SPI5_APB>;
+ clock-names = "apb_pclk";
+ resets = <&syscrg JH7110_SYSRST_SPI5_APB>;
+ interrupts = <54>;
+ arm,primecell-periphid = <0x00041022>;
+ num-cs = <1>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ status = "disabled";
+ };
+
+ spi6: spi@120a0000 {
+ compatible = "arm,pl022", "arm,primecell";
+ reg = <0x0 0x120A0000 0x0 0x10000>;
+ clocks = <&syscrg JH7110_SYSCLK_SPI6_APB>;
+ clock-names = "apb_pclk";
+ resets = <&syscrg JH7110_SYSRST_SPI6_APB>;
+ interrupts = <55>;
+ arm,primecell-periphid = <0x00041022>;
+ num-cs = <1>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ status = "disabled";
+ };
+
syscrg: clock-controller@13020000 {
compatible = "starfive,jh7110-syscrg";
reg = <0x0 0x13020000 0x0 0x10000>;