| Message ID | 20231113005503.2423-5-jszhang@kernel.org (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | riscv: sophgo: add reset support for cv1800b | expand |
| Context | Check | Description |
|---|---|---|
| conchuod/vmtest-for-next-PR | fail | PR summary |
| conchuod/patch-4-test-1 | fail | .github/scripts/patches/build_rv32_defconfig.sh |
| conchuod/patch-4-test-2 | fail | .github/scripts/patches/build_rv64_clang_allmodconfig.sh |
| conchuod/patch-4-test-3 | fail | .github/scripts/patches/build_rv64_gcc_allmodconfig.sh |
| conchuod/patch-4-test-4 | success | .github/scripts/patches/build_rv64_nommu_k210_defconfig.sh |
| conchuod/patch-4-test-5 | success | .github/scripts/patches/build_rv64_nommu_virt_defconfig.sh |
| conchuod/patch-4-test-6 | success | .github/scripts/patches/checkpatch.sh |
| conchuod/patch-4-test-7 | fail | .github/scripts/patches/dtb_warn_rv64.sh |
| conchuod/patch-4-test-8 | success | .github/scripts/patches/header_inline.sh |
| conchuod/patch-4-test-9 | success | .github/scripts/patches/kdoc.sh |
| conchuod/patch-4-test-10 | success | .github/scripts/patches/module_param.sh |
| conchuod/patch-4-test-11 | success | .github/scripts/patches/verify_fixes.sh |
| conchuod/patch-4-test-12 | success | .github/scripts/patches/verify_signedoff.sh |
Hi Jisheng, On 2023-11-12 6:55 PM, Jisheng Zhang wrote: > Although, the resets are deasserted by default. Add them for > completeness. > > Signed-off-by: Jisheng Zhang <jszhang@kernel.org> > --- > arch/riscv/boot/dts/sophgo/cv1800b.dtsi | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/arch/riscv/boot/dts/sophgo/cv1800b.dtsi b/arch/riscv/boot/dts/sophgo/cv1800b.dtsi > index 4032419486be..e04df04a91c0 100644 > --- a/arch/riscv/boot/dts/sophgo/cv1800b.dtsi > +++ b/arch/riscv/boot/dts/sophgo/cv1800b.dtsi > @@ -4,6 +4,7 @@ > */ > > #include <dt-bindings/interrupt-controller/irq.h> > +#include <dt-bindings/reset/sophgo,cv1800b-reset.h> > > / { > compatible = "sophgo,cv1800b"; > @@ -65,6 +66,7 @@ uart0: serial@4140000 { > reg = <0x04140000 0x100>; > interrupts = <44 IRQ_TYPE_LEVEL_HIGH>; > clocks = <&osc>; > + resets = <&rst RST_UART0>; Since it's not obvious: this breaks devicetree forward compatibility. An existing kernel will fail the devm_reset_control_get_optional_exclusive() in 8250_dw.c because it has no driver for the reset controller. This may not be a concern yet, since the devicetree is still "in development". But it is something to keep in mind for the future. To avoid this sort of problem, it's best to fully model the clocks/resets/other dependencies as early as possible, and not rely on the firmware setting anything up. Regards, Samuel > reg-shift = <2>; > reg-io-width = <4>; > status = "disabled"; > @@ -75,6 +77,7 @@ uart1: serial@4150000 { > reg = <0x04150000 0x100>; > interrupts = <45 IRQ_TYPE_LEVEL_HIGH>; > clocks = <&osc>; > + resets = <&rst RST_UART1>; > reg-shift = <2>; > reg-io-width = <4>; > status = "disabled"; > @@ -85,6 +88,7 @@ uart2: serial@4160000 { > reg = <0x04160000 0x100>; > interrupts = <46 IRQ_TYPE_LEVEL_HIGH>; > clocks = <&osc>; > + resets = <&rst RST_UART2>; > reg-shift = <2>; > reg-io-width = <4>; > status = "disabled"; > @@ -95,6 +99,7 @@ uart3: serial@4170000 { > reg = <0x04170000 0x100>; > interrupts = <47 IRQ_TYPE_LEVEL_HIGH>; > clocks = <&osc>; > + resets = <&rst RST_UART3>; > reg-shift = <2>; > reg-io-width = <4>; > status = "disabled"; > @@ -105,6 +110,7 @@ uart4: serial@41c0000 { > reg = <0x041c0000 0x100>; > interrupts = <48 IRQ_TYPE_LEVEL_HIGH>; > clocks = <&osc>; > + resets = <&rst RST_UART4>; > reg-shift = <2>; > reg-io-width = <4>; > status = "disabled";
Hi Jisheng,
kernel test robot noticed the following build errors:
[auto build test ERROR on next-20231110]
[also build test ERROR on linus/master v6.7-rc1]
[cannot apply to robh/for-next krzk/for-next krzk-dt/for-next pza/reset/next pza/imx-drm/next v6.6 v6.6-rc7 v6.6-rc6]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Jisheng-Zhang/dt-bindings-reset-Add-binding-for-Sophgo-CV1800B-reset-controller/20231113-091129
base: next-20231110
patch link: https://lore.kernel.org/r/20231113005503.2423-5-jszhang%40kernel.org
patch subject: [PATCH 4/4] riscv: dts: sophgo: add reset phandle to all uart nodes
config: riscv-randconfig-001-20231113 (https://download.01.org/0day-ci/archive/20231113/202311131220.lnq9Gdut-lkp@intel.com/config)
compiler: riscv64-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231113/202311131220.lnq9Gdut-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202311131220.lnq9Gdut-lkp@intel.com/
All errors (new ones prefixed by >>):
In file included from arch/riscv/boot/dts/sophgo/cv1800b.dtsi:7,
from arch/riscv/boot/dts/sophgo/cv1800b-milkv-duo.dts:8:
>> scripts/dtc/include-prefixes/dt-bindings/reset/sophgo,cv1800b-reset.h:7: error: unterminated #ifndef
7 | #ifndef _DT_BINDINGS_CV1800B_RESET_H
|
vim +7 scripts/dtc/include-prefixes/dt-bindings/reset/sophgo,cv1800b-reset.h
On Sun, Nov 12, 2023 at 09:04:55PM -0500, Samuel Holland wrote: > Hi Jisheng, > > On 2023-11-12 6:55 PM, Jisheng Zhang wrote: > > Although, the resets are deasserted by default. Add them for > > completeness. > > > > Signed-off-by: Jisheng Zhang <jszhang@kernel.org> > > --- > > arch/riscv/boot/dts/sophgo/cv1800b.dtsi | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/arch/riscv/boot/dts/sophgo/cv1800b.dtsi b/arch/riscv/boot/dts/sophgo/cv1800b.dtsi > > index 4032419486be..e04df04a91c0 100644 > > --- a/arch/riscv/boot/dts/sophgo/cv1800b.dtsi > > +++ b/arch/riscv/boot/dts/sophgo/cv1800b.dtsi > > @@ -4,6 +4,7 @@ > > */ > > > > #include <dt-bindings/interrupt-controller/irq.h> > > +#include <dt-bindings/reset/sophgo,cv1800b-reset.h> > > > > / { > > compatible = "sophgo,cv1800b"; > > @@ -65,6 +66,7 @@ uart0: serial@4140000 { > > reg = <0x04140000 0x100>; > > interrupts = <44 IRQ_TYPE_LEVEL_HIGH>; > > clocks = <&osc>; > > + resets = <&rst RST_UART0>; > > Since it's not obvious: this breaks devicetree forward compatibility. An > existing kernel will fail the devm_reset_control_get_optional_exclusive() in > 8250_dw.c because it has no driver for the reset controller. > > This may not be a concern yet, since the devicetree is still "in development". > But it is something to keep in mind for the future. To avoid this sort of > problem, it's best to fully model the clocks/resets/other dependencies as early > as possible, and not rely on the firmware setting anything up. Thank you. This may be discussed before, "DT backward compatibility is a must while forward compatibility is optional"? maybe I'm wrong. And Indeed, it's better if we can have forward compatibility, will take care this in future. > > Regards, > Samuel > > > reg-shift = <2>; > > reg-io-width = <4>; > > status = "disabled"; > > @@ -75,6 +77,7 @@ uart1: serial@4150000 { > > reg = <0x04150000 0x100>; > > interrupts = <45 IRQ_TYPE_LEVEL_HIGH>; > > clocks = <&osc>; > > + resets = <&rst RST_UART1>; > > reg-shift = <2>; > > reg-io-width = <4>; > > status = "disabled"; > > @@ -85,6 +88,7 @@ uart2: serial@4160000 { > > reg = <0x04160000 0x100>; > > interrupts = <46 IRQ_TYPE_LEVEL_HIGH>; > > clocks = <&osc>; > > + resets = <&rst RST_UART2>; > > reg-shift = <2>; > > reg-io-width = <4>; > > status = "disabled"; > > @@ -95,6 +99,7 @@ uart3: serial@4170000 { > > reg = <0x04170000 0x100>; > > interrupts = <47 IRQ_TYPE_LEVEL_HIGH>; > > clocks = <&osc>; > > + resets = <&rst RST_UART3>; > > reg-shift = <2>; > > reg-io-width = <4>; > > status = "disabled"; > > @@ -105,6 +110,7 @@ uart4: serial@41c0000 { > > reg = <0x041c0000 0x100>; > > interrupts = <48 IRQ_TYPE_LEVEL_HIGH>; > > clocks = <&osc>; > > + resets = <&rst RST_UART4>; > > reg-shift = <2>; > > reg-io-width = <4>; > > status = "disabled"; >
diff --git a/arch/riscv/boot/dts/sophgo/cv1800b.dtsi b/arch/riscv/boot/dts/sophgo/cv1800b.dtsi index 4032419486be..e04df04a91c0 100644 --- a/arch/riscv/boot/dts/sophgo/cv1800b.dtsi +++ b/arch/riscv/boot/dts/sophgo/cv1800b.dtsi @@ -4,6 +4,7 @@ */ #include <dt-bindings/interrupt-controller/irq.h> +#include <dt-bindings/reset/sophgo,cv1800b-reset.h> / { compatible = "sophgo,cv1800b"; @@ -65,6 +66,7 @@ uart0: serial@4140000 { reg = <0x04140000 0x100>; interrupts = <44 IRQ_TYPE_LEVEL_HIGH>; clocks = <&osc>; + resets = <&rst RST_UART0>; reg-shift = <2>; reg-io-width = <4>; status = "disabled"; @@ -75,6 +77,7 @@ uart1: serial@4150000 { reg = <0x04150000 0x100>; interrupts = <45 IRQ_TYPE_LEVEL_HIGH>; clocks = <&osc>; + resets = <&rst RST_UART1>; reg-shift = <2>; reg-io-width = <4>; status = "disabled"; @@ -85,6 +88,7 @@ uart2: serial@4160000 { reg = <0x04160000 0x100>; interrupts = <46 IRQ_TYPE_LEVEL_HIGH>; clocks = <&osc>; + resets = <&rst RST_UART2>; reg-shift = <2>; reg-io-width = <4>; status = "disabled"; @@ -95,6 +99,7 @@ uart3: serial@4170000 { reg = <0x04170000 0x100>; interrupts = <47 IRQ_TYPE_LEVEL_HIGH>; clocks = <&osc>; + resets = <&rst RST_UART3>; reg-shift = <2>; reg-io-width = <4>; status = "disabled"; @@ -105,6 +110,7 @@ uart4: serial@41c0000 { reg = <0x041c0000 0x100>; interrupts = <48 IRQ_TYPE_LEVEL_HIGH>; clocks = <&osc>; + resets = <&rst RST_UART4>; reg-shift = <2>; reg-io-width = <4>; status = "disabled";
Although, the resets are deasserted by default. Add them for completeness. Signed-off-by: Jisheng Zhang <jszhang@kernel.org> --- arch/riscv/boot/dts/sophgo/cv1800b.dtsi | 6 ++++++ 1 file changed, 6 insertions(+)