Message ID | 20230507182304.2934-1-jszhang@kernel.org |
---|---|
Headers | show |
Series | Add Sipeed Lichee Pi 4A RISC-V board support | expand |
On Mon, May 08, 2023 at 02:52:29PM +0800, Guo Ren wrote: > On Mon, May 8, 2023 at 2:34 AM Jisheng Zhang <jszhang@kernel.org> wrote: > > > > The T-HEAD's C910 PLIC still needs the delegation bit settingto allow > > access from S-mode, but it doesn't need the edge quirk. > > > > Signed-off-by: Jisheng Zhang <jszhang@kernel.org> > > --- > > .../bindings/interrupt-controller/sifive,plic-1.0.0.yaml | 4 ++++ > > drivers/irqchip/irq-sifive-plic.c | 1 + > > 2 files changed, 5 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml > > index f75736a061af..64b43a3c3748 100644 > > --- a/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml > > +++ b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml > > @@ -62,6 +62,10 @@ properties: > > - starfive,jh7110-plic > > - canaan,k210-plic > > - const: sifive,plic-1.0.0 > > + - items: > > + - enum: > > + - thead,light-plic > > + - const: thead,c910-plic > > - items: > > - enum: > > - allwinner,sun20i-d1-plic > > diff --git a/drivers/irqchip/irq-sifive-plic.c b/drivers/irqchip/irq-sifive-plic.c > > index e1484905b7bd..71afa2a584d9 100644 > > --- a/drivers/irqchip/irq-sifive-plic.c > > +++ b/drivers/irqchip/irq-sifive-plic.c > > @@ -569,6 +569,7 @@ static int __init plic_init(struct device_node *node, > > } > > > > IRQCHIP_DECLARE(sifive_plic, "sifive,plic-1.0.0", plic_init); > > +IRQCHIP_DECLARE(thead_c910_plic, "thead,c910-plic", plic_init); > > IRQCHIP_DECLARE(riscv_plic0, "riscv,plic0", plic_init); /* for legacy systems */ > > > > static int __init plic_edge_init(struct device_node *node, > > -- > > 2.40.0 > > > opensbi needs thead,c900-plic, and we could put multi compatible name > in the dts. So, it's no need here. Thanks, I misunderstood the PLIC edge quirk. This patch isn't needed any more.
On Sun, May 07, 2023 at 10:21:26PM +0100, Conor Dooley wrote: > Hey Jisheng, Hi Conor, > > On Mon, May 08, 2023 at 02:23:04AM +0800, Jisheng Zhang wrote: > > I would like to temporarily maintain the T-HEAD RISC-V SoC support. > > What does "temporarily" mean? I got a Lichee Pi 4A board, and want to mainline its support. Sending the new dts patches needs to touch MAINTAINERS entry, so I added it. But I expected an experienced people from T-HEAD, with many kernel contribuitions in the past, will take the maintainership finally, for example, Ren Guo. He knew this SoC better than me.
On Sun, May 07, 2023 at 10:35:12PM +0100, Conor Dooley wrote: > Hey Jisheng, > > On Mon, May 08, 2023 at 02:23:02AM +0800, Jisheng Zhang wrote: > > > + c910_0: cpu@0 { > > + compatible = "thead,c910", "riscv"; > > + device_type = "cpu"; > > + riscv,isa = "rv64imafdc"; > > Does this support more than "rv64imafdc"? > I assume there's some _xtheadfoo extensions that it does support, > although I am not sure how we are proceeding with those - Heiko might > have a more nuanced take. > > > + reset: reset-sample { > > + compatible = "thead,reset-sample"; > > What is a "reset-sample"? This node is only for opensbi. The compatible string is already in opensbi. Do we also need to add dt-binding for it in linux? > > > + entry-reg = <0xff 0xff019050>; > > + entry-cnt = <4>; > > + control-reg = <0xff 0xff015004>; > > + control-val = <0x1c>; > > + csr-copy = <0x7f3 0x7c0 0x7c1 0x7c2 0x7c3 0x7c5 0x7cc>; > > + }; > > + > > + plic: interrupt-controller@ffd8000000 { > > + compatible = "thead,c910-plic"; > > + reg = <0xff 0xd8000000 0x0 0x01000000>; > > + interrupts-extended = <&cpu0_intc 11>, <&cpu0_intc 9>, > > + <&cpu1_intc 11>, <&cpu1_intc 9>, > > + <&cpu2_intc 11>, <&cpu2_intc 9>, > > + <&cpu3_intc 11>, <&cpu3_intc 9>; > > + interrupt-controller; > > + #interrupt-cells = <1>; > > + riscv,ndev = <240>; > > + }; > > + > > + clint: timer@ffdc000000 { > > + compatible = "thead,c900-clint"; > > "c900"? That a typo or intentional. Hard to tell since this compatible > is undocumented ;) Per my understanding, this node is only for opensbi too. Add will add dt-binding in v2. > > > + reg = <0xff 0xdc000000 0x0 0x00010000>; > > + interrupts-extended = <&cpu0_intc 3>, <&cpu0_intc 7>, > > + <&cpu1_intc 3>, <&cpu1_intc 7>, > > + <&cpu2_intc 3>, <&cpu2_intc 7>, > > + <&cpu3_intc 3>, <&cpu3_intc 7>; > > + }; > > + > > + uart0: serial@ffe7014000 { > > + compatible = "snps,dw-apb-uart"; > > + reg = <0xff 0xe7014000 0x0 0x4000>; > > + interrupts = <36>; > > + clocks = <&uart_sclk>; > > + clock-names = "baudclk"; > > dtbs_check complains about this clock name. > > + > > + dmac0: dmac@ffefc00000 { > > dma-controller@ > > As I mentioned in the other patch, please clean up the dtbs_check > complaints for v2. > Thanks for the reminding.
On Tue, May 09, 2023 at 12:17:52AM +0800, Jisheng Zhang wrote: > On Sun, May 07, 2023 at 10:21:26PM +0100, Conor Dooley wrote: > > Hey Jisheng, > > Hi Conor, > > > > > On Mon, May 08, 2023 at 02:23:04AM +0800, Jisheng Zhang wrote: > > > I would like to temporarily maintain the T-HEAD RISC-V SoC support. > > > > What does "temporarily" mean? > > I got a Lichee Pi 4A board, and want to mainline its support. Sending > the new dts patches needs to touch MAINTAINERS entry, so I added it. > But I expected an experienced people from T-HEAD, with many > kernel contribuitions in the past, will take the maintainership > finally, for example, Ren Guo. He knew this SoC better than me. I see. I don't mind applying the patches for the platform for now, it's not too much more on top of the other vendors that I am doing as things are still low enough volume, as long as someone is willing to review them as they come in. Cheers, Conor.
On Sun, May 21, 2023 at 11:37:58PM +0800, Guo Ren wrote: > On Tue, May 9, 2023 at 12:44 AM Conor Dooley <conor@kernel.org> wrote: > > > > On Tue, May 09, 2023 at 12:26:10AM +0800, Jisheng Zhang wrote: > > > On Sun, May 07, 2023 at 10:35:12PM +0100, Conor Dooley wrote: > > > > On Mon, May 08, 2023 at 02:23:02AM +0800, Jisheng Zhang wrote: > > > > > > > > > + c910_0: cpu@0 { > > > > > + compatible = "thead,c910", "riscv"; > > > > > + device_type = "cpu"; > > > > > + riscv,isa = "rv64imafdc"; > > > > > > > > Does this support more than "rv64imafdc"? > > > > I assume there's some _xtheadfoo extensions that it does support, > > > > although I am not sure how we are proceeding with those - Heiko might > > > > have a more nuanced take. > > > > > > > > > + reset: reset-sample { > > > > > + compatible = "thead,reset-sample"; > > > > > > > > What is a "reset-sample"? > > > > > > This node is only for opensbi. The compatible string is already in > > > opensbi. Do we also need to add dt-binding for it in linux? > > > > If it's to be included in the kernel's dts, then yes, you do need a > > dt-binding. If you remove it, then you don't :) > > > > That said, "thead,reset-sample" is a strangely named compatible, so if > > you do keep it it may end up needing a rename! > How about compatible = "thead,reset-th1520" ? "vendor,soc-function" is more typical, but "reset" is usually used for reset controllers of which this isn't as far as I can tell. I commented on the v2, hoping that you might actually know what the IP block' full/proper name is: https://lore.kernel.org/all/20230518-driving-secluding-793b3192776e@spud/ Do you? Cheers, Conor.