Message ID | 20240121134017.374992-1-vaishnav.a@ti.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | arm64: dts: ti: k3-am62p-mcu/wakeup: Disable MCU and wakeup R5FSS nodes | expand |
Hello Vaishnav, On 21/01/24 19:10, Vaishnav Achath wrote: > K3 Remoteproc R5 driver requires reserved memory carveouts and > mailbox configuration to instantiate the cores successfully. > Since this is a board level dependency, keep the R5 subsytem > disabled at SoC dtsi, otherwise it results in probe errors like > below during AM62P SK boot: > > r5fss@79000000: reserved memory init failed, ret = -22 > r5fss@79000000: k3_r5_cluster_rproc_init failed, ret = -22 > r5fss@78000000: reserved memory init failed, ret = -22 > r5fss@78000000: k3_r5_cluster_rproc_init failed, ret = -22 > > Fixes: b5080c7c1f7e ("arm64: dts: ti: k3-am62p: Add nodes for more IPs") > > Signed-off-by: Vaishnav Achath <vaishnav.a@ti.com> Reviewed-by: Jayesh Choudhary <j-choudhary@ti.com> > --- > arch/arm64/boot/dts/ti/k3-am62p-mcu.dtsi | 2 ++ > arch/arm64/boot/dts/ti/k3-am62p-wakeup.dtsi | 1 + > 2 files changed, 3 insertions(+) > > diff --git a/arch/arm64/boot/dts/ti/k3-am62p-mcu.dtsi b/arch/arm64/boot/dts/ti/k3-am62p-mcu.dtsi > index c4b0b91d70cf..14eb9ba836d3 100644 > --- a/arch/arm64/boot/dts/ti/k3-am62p-mcu.dtsi > +++ b/arch/arm64/boot/dts/ti/k3-am62p-mcu.dtsi > @@ -187,6 +187,8 @@ mcu_r5fss0: r5fss@79000000 { > ranges = <0x79000000 0x00 0x79000000 0x8000>, > <0x79020000 0x00 0x79020000 0x8000>; > power-domains = <&k3_pds 7 TI_SCI_PD_EXCLUSIVE>; > + status = "disabled"; > + > mcu_r5fss0_core0: r5f@79000000 { > compatible = "ti,am62-r5f"; > reg = <0x79000000 0x00008000>, > diff --git a/arch/arm64/boot/dts/ti/k3-am62p-wakeup.dtsi b/arch/arm64/boot/dts/ti/k3-am62p-wakeup.dtsi > index 19f42b39394e..10a7059b2d9b 100644 > --- a/arch/arm64/boot/dts/ti/k3-am62p-wakeup.dtsi > +++ b/arch/arm64/boot/dts/ti/k3-am62p-wakeup.dtsi > @@ -78,6 +78,7 @@ wkup_r5fss0: r5fss@78000000 { > ranges = <0x78000000 0x00 0x78000000 0x8000>, > <0x78100000 0x00 0x78100000 0x8000>; > power-domains = <&k3_pds 119 TI_SCI_PD_EXCLUSIVE>; > + status = "disabled"; > > wkup_r5fss0_core0: r5f@78000000 { > compatible = "ti,am62-r5f"; Thanks.
On 11:26-20240124, Jayesh Choudhary wrote: > Hello Vaishnav, > > On 21/01/24 19:10, Vaishnav Achath wrote: > > K3 Remoteproc R5 driver requires reserved memory carveouts and > > mailbox configuration to instantiate the cores successfully. > > Since this is a board level dependency, keep the R5 subsytem > > disabled at SoC dtsi, otherwise it results in probe errors like > > below during AM62P SK boot: > > > > r5fss@79000000: reserved memory init failed, ret = -22 > > r5fss@79000000: k3_r5_cluster_rproc_init failed, ret = -22 > > r5fss@78000000: reserved memory init failed, ret = -22 > > r5fss@78000000: k3_r5_cluster_rproc_init failed, ret = -22 > > > > Fixes: b5080c7c1f7e ("arm64: dts: ti: k3-am62p: Add nodes for more IPs") > > > > Signed-off-by: Vaishnav Achath <vaishnav.a@ti.com> > > Reviewed-by: Jayesh Choudhary <j-choudhary@ti.com> > > > --- > > arch/arm64/boot/dts/ti/k3-am62p-mcu.dtsi | 2 ++ > > arch/arm64/boot/dts/ti/k3-am62p-wakeup.dtsi | 1 + > > 2 files changed, 3 insertions(+) > > > > diff --git a/arch/arm64/boot/dts/ti/k3-am62p-mcu.dtsi b/arch/arm64/boot/dts/ti/k3-am62p-mcu.dtsi > > index c4b0b91d70cf..14eb9ba836d3 100644 > > --- a/arch/arm64/boot/dts/ti/k3-am62p-mcu.dtsi > > +++ b/arch/arm64/boot/dts/ti/k3-am62p-mcu.dtsi > > @@ -187,6 +187,8 @@ mcu_r5fss0: r5fss@79000000 { > > ranges = <0x79000000 0x00 0x79000000 0x8000>, > > <0x79020000 0x00 0x79020000 0x8000>; > > power-domains = <&k3_pds 7 TI_SCI_PD_EXCLUSIVE>; > > + status = "disabled"; > > + > > mcu_r5fss0_core0: r5f@79000000 { > > compatible = "ti,am62-r5f"; > > reg = <0x79000000 0x00008000>, > > diff --git a/arch/arm64/boot/dts/ti/k3-am62p-wakeup.dtsi b/arch/arm64/boot/dts/ti/k3-am62p-wakeup.dtsi > > index 19f42b39394e..10a7059b2d9b 100644 > > --- a/arch/arm64/boot/dts/ti/k3-am62p-wakeup.dtsi > > +++ b/arch/arm64/boot/dts/ti/k3-am62p-wakeup.dtsi > > @@ -78,6 +78,7 @@ wkup_r5fss0: r5fss@78000000 { > > ranges = <0x78000000 0x00 0x78000000 0x8000>, > > <0x78100000 0x00 0x78100000 0x8000>; > > power-domains = <&k3_pds 119 TI_SCI_PD_EXCLUSIVE>; > > + status = "disabled"; Is there a reason for difference in white space addition? > > wkup_r5fss0_core0: r5f@78000000 { > > compatible = "ti,am62-r5f"; > > Thanks.
Hi Nishanth, On 24/01/24 22:51, Nishanth Menon wrote: > On 11:26-20240124, Jayesh Choudhary wrote: >> Hello Vaishnav, >> >> On 21/01/24 19:10, Vaishnav Achath wrote: >>> K3 Remoteproc R5 driver requires reserved memory carveouts and >>> mailbox configuration to instantiate the cores successfully. >>> Since this is a board level dependency, keep the R5 subsytem >>> disabled at SoC dtsi, otherwise it results in probe errors like >>> below during AM62P SK boot: >>> >>> r5fss@79000000: reserved memory init failed, ret = -22 >>> r5fss@79000000: k3_r5_cluster_rproc_init failed, ret = -22 >>> r5fss@78000000: reserved memory init failed, ret = -22 >>> r5fss@78000000: k3_r5_cluster_rproc_init failed, ret = -22 >>> >>> Fixes: b5080c7c1f7e ("arm64: dts: ti: k3-am62p: Add nodes for more IPs") >>> >>> Signed-off-by: Vaishnav Achath <vaishnav.a@ti.com> >> >> Reviewed-by: Jayesh Choudhary <j-choudhary@ti.com> >> >>> --- >>> arch/arm64/boot/dts/ti/k3-am62p-mcu.dtsi | 2 ++ >>> arch/arm64/boot/dts/ti/k3-am62p-wakeup.dtsi | 1 + >>> 2 files changed, 3 insertions(+) >>> >>> diff --git a/arch/arm64/boot/dts/ti/k3-am62p-mcu.dtsi b/arch/arm64/boot/dts/ti/k3-am62p-mcu.dtsi >>> index c4b0b91d70cf..14eb9ba836d3 100644 >>> --- a/arch/arm64/boot/dts/ti/k3-am62p-mcu.dtsi >>> +++ b/arch/arm64/boot/dts/ti/k3-am62p-mcu.dtsi >>> @@ -187,6 +187,8 @@ mcu_r5fss0: r5fss@79000000 { >>> ranges = <0x79000000 0x00 0x79000000 0x8000>, >>> <0x79020000 0x00 0x79020000 0x8000>; >>> power-domains = <&k3_pds 7 TI_SCI_PD_EXCLUSIVE>; >>> + status = "disabled"; >>> + >>> mcu_r5fss0_core0: r5f@79000000 { >>> compatible = "ti,am62-r5f"; >>> reg = <0x79000000 0x00008000>, >>> diff --git a/arch/arm64/boot/dts/ti/k3-am62p-wakeup.dtsi b/arch/arm64/boot/dts/ti/k3-am62p-wakeup.dtsi >>> index 19f42b39394e..10a7059b2d9b 100644 >>> --- a/arch/arm64/boot/dts/ti/k3-am62p-wakeup.dtsi >>> +++ b/arch/arm64/boot/dts/ti/k3-am62p-wakeup.dtsi >>> @@ -78,6 +78,7 @@ wkup_r5fss0: r5fss@78000000 { >>> ranges = <0x78000000 0x00 0x78000000 0x8000>, >>> <0x78100000 0x00 0x78100000 0x8000>; >>> power-domains = <&k3_pds 119 TI_SCI_PD_EXCLUSIVE>; >>> + status = "disabled"; > > Is there a reason for difference in white space addition? > For mcu_r5fss0_core0 child node there was no blank line as per the recommended coding style : https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/dts-coding-style.rst#n124 So I added a newline there and wkup_r5fss0 already had it correct, since the change was trivial it was not mentioned in commit message. Thanks and Regards, Vaishnav >>> wkup_r5fss0_core0: r5f@78000000 { >>> compatible = "ti,am62-r5f"; >> >> Thanks. >
On 11:17-20240125, Vaishnav Achath wrote: > Hi Nishanth, > > On 24/01/24 22:51, Nishanth Menon wrote: > > On 11:26-20240124, Jayesh Choudhary wrote: > > > Hello Vaishnav, > > > > > > On 21/01/24 19:10, Vaishnav Achath wrote: > > > > K3 Remoteproc R5 driver requires reserved memory carveouts and > > > > mailbox configuration to instantiate the cores successfully. > > > > Since this is a board level dependency, keep the R5 subsytem > > > > disabled at SoC dtsi, otherwise it results in probe errors like > > > > below during AM62P SK boot: > > > > > > > > r5fss@79000000: reserved memory init failed, ret = -22 > > > > r5fss@79000000: k3_r5_cluster_rproc_init failed, ret = -22 > > > > r5fss@78000000: reserved memory init failed, ret = -22 > > > > r5fss@78000000: k3_r5_cluster_rproc_init failed, ret = -22 > > > > > > > > Fixes: b5080c7c1f7e ("arm64: dts: ti: k3-am62p: Add nodes for more IPs") > > > > > > > > Signed-off-by: Vaishnav Achath <vaishnav.a@ti.com> > > > > > > Reviewed-by: Jayesh Choudhary <j-choudhary@ti.com> > > > > > > > --- > > > > arch/arm64/boot/dts/ti/k3-am62p-mcu.dtsi | 2 ++ > > > > arch/arm64/boot/dts/ti/k3-am62p-wakeup.dtsi | 1 + > > > > 2 files changed, 3 insertions(+) > > > > > > > > diff --git a/arch/arm64/boot/dts/ti/k3-am62p-mcu.dtsi b/arch/arm64/boot/dts/ti/k3-am62p-mcu.dtsi > > > > index c4b0b91d70cf..14eb9ba836d3 100644 > > > > --- a/arch/arm64/boot/dts/ti/k3-am62p-mcu.dtsi > > > > +++ b/arch/arm64/boot/dts/ti/k3-am62p-mcu.dtsi > > > > @@ -187,6 +187,8 @@ mcu_r5fss0: r5fss@79000000 { > > > > ranges = <0x79000000 0x00 0x79000000 0x8000>, > > > > <0x79020000 0x00 0x79020000 0x8000>; > > > > power-domains = <&k3_pds 7 TI_SCI_PD_EXCLUSIVE>; > > > > + status = "disabled"; > > > > + ^^ Look here. > > > > mcu_r5fss0_core0: r5f@79000000 { > > > > compatible = "ti,am62-r5f"; > > > > reg = <0x79000000 0x00008000>, > > > > diff --git a/arch/arm64/boot/dts/ti/k3-am62p-wakeup.dtsi b/arch/arm64/boot/dts/ti/k3-am62p-wakeup.dtsi > > > > index 19f42b39394e..10a7059b2d9b 100644 > > > > --- a/arch/arm64/boot/dts/ti/k3-am62p-wakeup.dtsi > > > > +++ b/arch/arm64/boot/dts/ti/k3-am62p-wakeup.dtsi > > > > @@ -78,6 +78,7 @@ wkup_r5fss0: r5fss@78000000 { > > > > ranges = <0x78000000 0x00 0x78000000 0x8000>, > > > > <0x78100000 0x00 0x78100000 0x8000>; > > > > power-domains = <&k3_pds 119 TI_SCI_PD_EXCLUSIVE>; > > > > + status = "disabled"; > > ^^ no white space here. > > Is there a reason for difference in white space addition? > > > > For mcu_r5fss0_core0 child node there was no blank line as per the > recommended coding style : https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/dts-coding-style.rst#n124 > > So I added a newline there and wkup_r5fss0 already had it correct, since the > change was trivial it was not mentioned in commit message. Sigh, please add a EoL here to keep the look consistent between mcu and wakeup dtsis. there is no need to state in commit message.
On 11:17-20240125, Vaishnav Achath wrote: > Hi Nishanth, > > On 24/01/24 22:51, Nishanth Menon wrote: > > On 11:26-20240124, Jayesh Choudhary wrote: > > > Hello Vaishnav, > > > > > > On 21/01/24 19:10, Vaishnav Achath wrote: > > > > K3 Remoteproc R5 driver requires reserved memory carveouts and > > > > mailbox configuration to instantiate the cores successfully. > > > > Since this is a board level dependency, keep the R5 subsytem > > > > disabled at SoC dtsi, otherwise it results in probe errors like > > > > below during AM62P SK boot: > > > > > > > > r5fss@79000000: reserved memory init failed, ret = -22 > > > > r5fss@79000000: k3_r5_cluster_rproc_init failed, ret = -22 > > > > r5fss@78000000: reserved memory init failed, ret = -22 > > > > r5fss@78000000: k3_r5_cluster_rproc_init failed, ret = -22 > > > > > > > > Fixes: b5080c7c1f7e ("arm64: dts: ti: k3-am62p: Add nodes for more IPs") > > > > > > > > Signed-off-by: Vaishnav Achath <vaishnav.a@ti.com> > > > > > > Reviewed-by: Jayesh Choudhary <j-choudhary@ti.com> > > > > > > > --- > > > > arch/arm64/boot/dts/ti/k3-am62p-mcu.dtsi | 2 ++ > > > > arch/arm64/boot/dts/ti/k3-am62p-wakeup.dtsi | 1 + > > > > 2 files changed, 3 insertions(+) > > > > > > > > diff --git a/arch/arm64/boot/dts/ti/k3-am62p-mcu.dtsi b/arch/arm64/boot/dts/ti/k3-am62p-mcu.dtsi > > > > index c4b0b91d70cf..14eb9ba836d3 100644 > > > > --- a/arch/arm64/boot/dts/ti/k3-am62p-mcu.dtsi > > > > +++ b/arch/arm64/boot/dts/ti/k3-am62p-mcu.dtsi > > > > @@ -187,6 +187,8 @@ mcu_r5fss0: r5fss@79000000 { > > > > ranges = <0x79000000 0x00 0x79000000 0x8000>, > > > > <0x79020000 0x00 0x79020000 0x8000>; > > > > power-domains = <&k3_pds 7 TI_SCI_PD_EXCLUSIVE>; > > > > + status = "disabled"; > > > > + > > > > mcu_r5fss0_core0: r5f@79000000 { > > > > compatible = "ti,am62-r5f"; > > > > reg = <0x79000000 0x00008000>, > > > > diff --git a/arch/arm64/boot/dts/ti/k3-am62p-wakeup.dtsi b/arch/arm64/boot/dts/ti/k3-am62p-wakeup.dtsi > > > > index 19f42b39394e..10a7059b2d9b 100644 > > > > --- a/arch/arm64/boot/dts/ti/k3-am62p-wakeup.dtsi > > > > +++ b/arch/arm64/boot/dts/ti/k3-am62p-wakeup.dtsi > > > > @@ -78,6 +78,7 @@ wkup_r5fss0: r5fss@78000000 { > > > > ranges = <0x78000000 0x00 0x78000000 0x8000>, > > > > <0x78100000 0x00 0x78100000 0x8000>; > > > > power-domains = <&k3_pds 119 TI_SCI_PD_EXCLUSIVE>; > > > > + status = "disabled"; > > > > Is there a reason for difference in white space addition? > > > > For mcu_r5fss0_core0 child node there was no blank line as per the > recommended coding style : https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/dts-coding-style.rst#n124 > > So I added a newline there and wkup_r5fss0 already had it correct, since the > change was trivial it was not mentioned in commit message. > For some reason i was misled to think that the EoL spacing was messed up. Looking at https://lore.kernel.org/all/20240121134017.374992-1-vaishnav.a@ti.com/ i realise it is being fixed properly in the patch. Oops.. my bad. Reviewed-by: Nishanth Menon <nm@ti.com>
Am 21.01.24 um 14:40 schrieb Vaishnav Achath: > K3 Remoteproc R5 driver requires reserved memory carveouts and > mailbox configuration to instantiate the cores successfully. > Since this is a board level dependency, keep the R5 subsytem > disabled at SoC dtsi, otherwise it results in probe errors like > below during AM62P SK boot: > > r5fss@79000000: reserved memory init failed, ret = -22 > r5fss@79000000: k3_r5_cluster_rproc_init failed, ret = -22 > r5fss@78000000: reserved memory init failed, ret = -22 > r5fss@78000000: k3_r5_cluster_rproc_init failed, ret = -22 Shouldn't we have a similar patch for the am64 R5 cores? > > Fixes: b5080c7c1f7e ("arm64: dts: ti: k3-am62p: Add nodes for more IPs") > > Signed-off-by: Vaishnav Achath <vaishnav.a@ti.com> > --- > arch/arm64/boot/dts/ti/k3-am62p-mcu.dtsi | 2 ++ > arch/arm64/boot/dts/ti/k3-am62p-wakeup.dtsi | 1 + > 2 files changed, 3 insertions(+) > > diff --git a/arch/arm64/boot/dts/ti/k3-am62p-mcu.dtsi b/arch/arm64/boot/dts/ti/k3-am62p-mcu.dtsi > index c4b0b91d70cf..14eb9ba836d3 100644 > --- a/arch/arm64/boot/dts/ti/k3-am62p-mcu.dtsi > +++ b/arch/arm64/boot/dts/ti/k3-am62p-mcu.dtsi > @@ -187,6 +187,8 @@ mcu_r5fss0: r5fss@79000000 { > ranges = <0x79000000 0x00 0x79000000 0x8000>, > <0x79020000 0x00 0x79020000 0x8000>; > power-domains = <&k3_pds 7 TI_SCI_PD_EXCLUSIVE>; > + status = "disabled"; > + > mcu_r5fss0_core0: r5f@79000000 { > compatible = "ti,am62-r5f"; > reg = <0x79000000 0x00008000>, > diff --git a/arch/arm64/boot/dts/ti/k3-am62p-wakeup.dtsi b/arch/arm64/boot/dts/ti/k3-am62p-wakeup.dtsi > index 19f42b39394e..10a7059b2d9b 100644 > --- a/arch/arm64/boot/dts/ti/k3-am62p-wakeup.dtsi > +++ b/arch/arm64/boot/dts/ti/k3-am62p-wakeup.dtsi > @@ -78,6 +78,7 @@ wkup_r5fss0: r5fss@78000000 { > ranges = <0x78000000 0x00 0x78000000 0x8000>, > <0x78100000 0x00 0x78100000 0x8000>; > power-domains = <&k3_pds 119 TI_SCI_PD_EXCLUSIVE>; > + status = "disabled"; > > wkup_r5fss0_core0: r5f@78000000 { > compatible = "ti,am62-r5f";
Hi Vaishnav Achath, On Sun, 21 Jan 2024 19:10:17 +0530, Vaishnav Achath wrote: > K3 Remoteproc R5 driver requires reserved memory carveouts and > mailbox configuration to instantiate the cores successfully. > Since this is a board level dependency, keep the R5 subsytem > disabled at SoC dtsi, otherwise it results in probe errors like > below during AM62P SK boot: > > r5fss@79000000: reserved memory init failed, ret = -22 > r5fss@79000000: k3_r5_cluster_rproc_init failed, ret = -22 > r5fss@78000000: reserved memory init failed, ret = -22 > r5fss@78000000: k3_r5_cluster_rproc_init failed, ret = -22 > > [...] I have applied the following to branch ti-k3-dts-next on [1]. Thank you! [1/1] arm64: dts: ti: k3-am62p-mcu/wakeup: Disable MCU and wakeup R5FSS nodes commit: dfc90e5f1a0fe0f8124521bc1911e38aa6cd9118 All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent up the chain during the next merge window (or sooner if it is a relevant bug fix), however if problems are discovered then the patch may be dropped or reverted. You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed. If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced. Please add any relevant lists and maintainers to the CCs when replying to this mail. [1] https://git.kernel.org/pub/scm/linux/kernel/git/ti/linux.git -- Vignesh
diff --git a/arch/arm64/boot/dts/ti/k3-am62p-mcu.dtsi b/arch/arm64/boot/dts/ti/k3-am62p-mcu.dtsi index c4b0b91d70cf..14eb9ba836d3 100644 --- a/arch/arm64/boot/dts/ti/k3-am62p-mcu.dtsi +++ b/arch/arm64/boot/dts/ti/k3-am62p-mcu.dtsi @@ -187,6 +187,8 @@ mcu_r5fss0: r5fss@79000000 { ranges = <0x79000000 0x00 0x79000000 0x8000>, <0x79020000 0x00 0x79020000 0x8000>; power-domains = <&k3_pds 7 TI_SCI_PD_EXCLUSIVE>; + status = "disabled"; + mcu_r5fss0_core0: r5f@79000000 { compatible = "ti,am62-r5f"; reg = <0x79000000 0x00008000>, diff --git a/arch/arm64/boot/dts/ti/k3-am62p-wakeup.dtsi b/arch/arm64/boot/dts/ti/k3-am62p-wakeup.dtsi index 19f42b39394e..10a7059b2d9b 100644 --- a/arch/arm64/boot/dts/ti/k3-am62p-wakeup.dtsi +++ b/arch/arm64/boot/dts/ti/k3-am62p-wakeup.dtsi @@ -78,6 +78,7 @@ wkup_r5fss0: r5fss@78000000 { ranges = <0x78000000 0x00 0x78000000 0x8000>, <0x78100000 0x00 0x78100000 0x8000>; power-domains = <&k3_pds 119 TI_SCI_PD_EXCLUSIVE>; + status = "disabled"; wkup_r5fss0_core0: r5f@78000000 { compatible = "ti,am62-r5f";
K3 Remoteproc R5 driver requires reserved memory carveouts and mailbox configuration to instantiate the cores successfully. Since this is a board level dependency, keep the R5 subsytem disabled at SoC dtsi, otherwise it results in probe errors like below during AM62P SK boot: r5fss@79000000: reserved memory init failed, ret = -22 r5fss@79000000: k3_r5_cluster_rproc_init failed, ret = -22 r5fss@78000000: reserved memory init failed, ret = -22 r5fss@78000000: k3_r5_cluster_rproc_init failed, ret = -22 Fixes: b5080c7c1f7e ("arm64: dts: ti: k3-am62p: Add nodes for more IPs") Signed-off-by: Vaishnav Achath <vaishnav.a@ti.com> --- arch/arm64/boot/dts/ti/k3-am62p-mcu.dtsi | 2 ++ arch/arm64/boot/dts/ti/k3-am62p-wakeup.dtsi | 1 + 2 files changed, 3 insertions(+)