From: Atish Patra <Atish.Patra@wdc.com> To: "hch@infradead.org" <hch@infradead.org> Cc: "linux-riscv@lists.infradead.org" <linux-riscv@lists.infradead.org>, "alankao@andestech.com" <alankao@andestech.com>, "gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>, "gary@garyguo.net" <gary@garyguo.net>, "rppt@linux.ibm.com" <rppt@linux.ibm.com>, "paul.walmsley@sifive.com" <paul.walmsley@sifive.com>, "aou@eecs.berkeley.edu" <aou@eecs.berkeley.edu>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, "tglx@linutronix.de" <tglx@linutronix.de>, "anup@brainfault.org" <anup@brainfault.org>, "alexios.zavras@intel.com" <alexios.zavras@intel.com>, "palmer@sifive.com" <palmer@sifive.com> Subject: Re: [RFC PATCH 1/2] RISC-V: Mark existing SBI as legacy SBI. Date: Tue, 27 Aug 2019 20:37:27 +0000 [thread overview] Message-ID: <ac3cfe4502090354a7c49fae277adb757ad900d5.camel@wdc.com> (raw) In-Reply-To: <20190827140304.GA21855@infradead.org> On Tue, 2019-08-27 at 07:03 -0700, Christoph Hellwig wrote: > > +#define SBI_EXT_LEGACY_SET_TIMER 0x0 > > +#define SBI_EXT_LEGACY_CONSOLE_PUTCHAR 0x1 > > +#define SBI_EXT_LEGACY_CONSOLE_GETCHAR 0x2 > > +#define SBI_EXT_LEGACY_CLEAR_IPI 0x3 > > +#define SBI_EXT_LEGACY_SEND_IPI 0x4 > > +#define SBI_EXT_LEGACY_REMOTE_FENCE_I 0x5 > > +#define SBI_EXT_LEGACY_REMOTE_SFENCE_VMA 0x6 > > +#define SBI_EXT_LEGACY_REMOTE_SFENCE_VMA_ASID 0x7 > > +#define SBI_EXT_LEGACY_SHUTDOWN 0x8 > > As Mike said legacy is a bit of a weird name. I think this should > be SBI01_* or so. And pleae align the numeric values and maybe use > an enum. > Will do. > > + > > +#define SBI_CALL_LEGACY(which, arg0, arg1, arg2, arg3) > > ({ \ > > register uintptr_t a0 asm ("a0") = (uintptr_t)(arg0); \ > > register uintptr_t a1 asm ("a1") = (uintptr_t)(arg1); \ > > register uintptr_t a2 asm ("a2") = (uintptr_t)(arg2); \ > > Instead of the weird inline assembly with forced register allocation, > why not move this to pure assembly? AFAICs this is the whole > assembly > code we'd need: > > ENTRY(sbi01_call) > ecall > END(sbi01_call) > That would split the implementation between C file & assembly file for no good reason. How about moving everything in sbi.c and just write everything inline assembly there. > > /* Lazy implementations until SBI is finalized */ > > -#define SBI_CALL_0(which) SBI_CALL(which, 0, 0, 0, 0) > > -#define SBI_CALL_1(which, arg0) SBI_CALL(which, arg0, 0, 0, 0) > > -#define SBI_CALL_2(which, arg0, arg1) SBI_CALL(which, arg0, arg1, > > 0, 0) > > -#define SBI_CALL_3(which, arg0, arg1, arg2) \ > > - SBI_CALL(which, arg0, arg1, arg2, 0) > > -#define SBI_CALL_4(which, arg0, arg1, arg2, arg3) \ > > - SBI_CALL(which, arg0, arg1, arg2, arg3) > > +#define SBI_CALL_LEGACY_0(which) SBI_CALL_LEGACY(which, 0, 0, 0, > > 0) > > +#define SBI_CALL_LEGACY_1(which, arg0) SBI_CALL_LEGACY(which, > > arg0, 0, 0, 0) > > +#define SBI_CALL_LEGACY_2(which, arg0, arg1) \ > > + SBI_CALL_LEGACY(which, arg0, arg1, 0, 0) > > +#define SBI_CALL_LEGACY_3(which, arg0, arg1, arg2) \ > > + SBI_CALL_LEGACY(which, arg0, arg1, arg2, 0) > > +#define SBI_CALL_LEGACY_4(which, arg0, arg1, arg2, arg3) \ > > + SBI_CALL_LEGACY(which, arg0, arg1, arg2, arg3) > > When you touch this anyway I'd suggest you kill these rather > pointless wrappers as well as the comment above them. Sure. I did not want make bigger chagnes in 1st RFC patches. But looking at the comments, I feel it was not a good decission. I will get rid of these wrappers and move necessary ones to sbi.c -- Regards, Atish
WARNING: multiple messages have this Message-ID (diff)
From: Atish Patra <Atish.Patra@wdc.com> To: "hch@infradead.org" <hch@infradead.org> Cc: "aou@eecs.berkeley.edu" <aou@eecs.berkeley.edu>, "alankao@andestech.com" <alankao@andestech.com>, "gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>, "anup@brainfault.org" <anup@brainfault.org>, "palmer@sifive.com" <palmer@sifive.com>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, "rppt@linux.ibm.com" <rppt@linux.ibm.com>, "alexios.zavras@intel.com" <alexios.zavras@intel.com>, "gary@garyguo.net" <gary@garyguo.net>, "paul.walmsley@sifive.com" <paul.walmsley@sifive.com>, "linux-riscv@lists.infradead.org" <linux-riscv@lists.infradead.org>, "tglx@linutronix.de" <tglx@linutronix.de> Subject: Re: [RFC PATCH 1/2] RISC-V: Mark existing SBI as legacy SBI. Date: Tue, 27 Aug 2019 20:37:27 +0000 [thread overview] Message-ID: <ac3cfe4502090354a7c49fae277adb757ad900d5.camel@wdc.com> (raw) In-Reply-To: <20190827140304.GA21855@infradead.org> On Tue, 2019-08-27 at 07:03 -0700, Christoph Hellwig wrote: > > +#define SBI_EXT_LEGACY_SET_TIMER 0x0 > > +#define SBI_EXT_LEGACY_CONSOLE_PUTCHAR 0x1 > > +#define SBI_EXT_LEGACY_CONSOLE_GETCHAR 0x2 > > +#define SBI_EXT_LEGACY_CLEAR_IPI 0x3 > > +#define SBI_EXT_LEGACY_SEND_IPI 0x4 > > +#define SBI_EXT_LEGACY_REMOTE_FENCE_I 0x5 > > +#define SBI_EXT_LEGACY_REMOTE_SFENCE_VMA 0x6 > > +#define SBI_EXT_LEGACY_REMOTE_SFENCE_VMA_ASID 0x7 > > +#define SBI_EXT_LEGACY_SHUTDOWN 0x8 > > As Mike said legacy is a bit of a weird name. I think this should > be SBI01_* or so. And pleae align the numeric values and maybe use > an enum. > Will do. > > + > > +#define SBI_CALL_LEGACY(which, arg0, arg1, arg2, arg3) > > ({ \ > > register uintptr_t a0 asm ("a0") = (uintptr_t)(arg0); \ > > register uintptr_t a1 asm ("a1") = (uintptr_t)(arg1); \ > > register uintptr_t a2 asm ("a2") = (uintptr_t)(arg2); \ > > Instead of the weird inline assembly with forced register allocation, > why not move this to pure assembly? AFAICs this is the whole > assembly > code we'd need: > > ENTRY(sbi01_call) > ecall > END(sbi01_call) > That would split the implementation between C file & assembly file for no good reason. How about moving everything in sbi.c and just write everything inline assembly there. > > /* Lazy implementations until SBI is finalized */ > > -#define SBI_CALL_0(which) SBI_CALL(which, 0, 0, 0, 0) > > -#define SBI_CALL_1(which, arg0) SBI_CALL(which, arg0, 0, 0, 0) > > -#define SBI_CALL_2(which, arg0, arg1) SBI_CALL(which, arg0, arg1, > > 0, 0) > > -#define SBI_CALL_3(which, arg0, arg1, arg2) \ > > - SBI_CALL(which, arg0, arg1, arg2, 0) > > -#define SBI_CALL_4(which, arg0, arg1, arg2, arg3) \ > > - SBI_CALL(which, arg0, arg1, arg2, arg3) > > +#define SBI_CALL_LEGACY_0(which) SBI_CALL_LEGACY(which, 0, 0, 0, > > 0) > > +#define SBI_CALL_LEGACY_1(which, arg0) SBI_CALL_LEGACY(which, > > arg0, 0, 0, 0) > > +#define SBI_CALL_LEGACY_2(which, arg0, arg1) \ > > + SBI_CALL_LEGACY(which, arg0, arg1, 0, 0) > > +#define SBI_CALL_LEGACY_3(which, arg0, arg1, arg2) \ > > + SBI_CALL_LEGACY(which, arg0, arg1, arg2, 0) > > +#define SBI_CALL_LEGACY_4(which, arg0, arg1, arg2, arg3) \ > > + SBI_CALL_LEGACY(which, arg0, arg1, arg2, arg3) > > When you touch this anyway I'd suggest you kill these rather > pointless wrappers as well as the comment above them. Sure. I did not want make bigger chagnes in 1st RFC patches. But looking at the comments, I feel it was not a good decission. I will get rid of these wrappers and move necessary ones to sbi.c -- Regards, Atish _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv
next prev parent reply other threads:[~2019-08-27 20:37 UTC|newest] Thread overview: 54+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-08-26 23:32 [RFC PATCH 0/2] Add support for SBI version to 0.2 Atish Patra 2019-08-26 23:32 ` Atish Patra 2019-08-26 23:32 ` [RFC PATCH 1/2] RISC-V: Mark existing SBI as legacy SBI Atish Patra 2019-08-26 23:32 ` Atish Patra 2019-08-27 7:51 ` Mike Rapoport 2019-08-27 7:51 ` Mike Rapoport 2019-08-27 8:28 ` Anup Patel 2019-08-27 8:28 ` Anup Patel 2019-08-27 8:37 ` Mike Rapoport 2019-08-27 8:37 ` Mike Rapoport 2019-08-28 21:37 ` Palmer Dabbelt 2019-08-28 21:37 ` Palmer Dabbelt 2019-08-27 20:34 ` Atish Patra 2019-08-27 20:34 ` Atish Patra 2019-08-27 14:03 ` Christoph Hellwig 2019-08-27 14:03 ` Christoph Hellwig 2019-08-27 14:04 ` Christoph Hellwig 2019-08-27 14:04 ` Christoph Hellwig 2019-08-27 20:37 ` Atish Patra [this message] 2019-08-27 20:37 ` Atish Patra 2019-08-29 10:56 ` hch 2019-08-29 10:56 ` hch 2019-08-26 23:32 ` [RFC PATCH 2/2] RISC-V: Add basic support for SBI v0.2 Atish Patra 2019-08-26 23:32 ` Atish Patra 2019-08-27 7:58 ` Mike Rapoport 2019-08-27 7:58 ` Mike Rapoport 2019-08-27 8:23 ` Anup Patel 2019-08-27 8:23 ` Anup Patel 2019-08-27 8:39 ` Mike Rapoport 2019-08-27 8:39 ` Mike Rapoport 2019-08-27 9:28 ` Anup Patel 2019-08-27 9:28 ` Anup Patel 2019-08-27 20:30 ` Atish Patra 2019-08-27 20:30 ` Atish Patra 2019-08-27 9:36 ` Anup Patel 2019-08-27 9:36 ` Anup Patel 2019-08-27 20:43 ` Atish Patra 2019-08-27 20:43 ` Atish Patra 2019-08-27 14:11 ` Christoph Hellwig 2019-08-27 14:11 ` Christoph Hellwig 2019-08-27 14:46 ` [RFC PATCH 0/2] Add support for SBI version to 0.2 Christoph Hellwig 2019-08-27 14:46 ` Christoph Hellwig 2019-08-27 22:19 ` Atish Patra 2019-08-27 22:19 ` Atish Patra 2019-08-29 10:59 ` hch 2019-08-29 10:59 ` hch 2019-08-30 23:13 ` Atish Patra 2019-08-30 23:13 ` Atish Patra 2019-09-03 7:38 ` hch 2019-09-03 7:38 ` hch [not found] ` <CANs6eMmcbtJ5KTU00LpfTtXszsdi1Jem_5j6GWO+8Yo3JnvTqg@mail.gmail.com> 2019-09-16 6:54 ` hch 2019-09-16 6:54 ` hch 2019-09-16 16:12 ` Palmer Dabbelt 2019-09-16 16:12 ` Palmer Dabbelt
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=ac3cfe4502090354a7c49fae277adb757ad900d5.camel@wdc.com \ --to=atish.patra@wdc.com \ --cc=alankao@andestech.com \ --cc=alexios.zavras@intel.com \ --cc=anup@brainfault.org \ --cc=aou@eecs.berkeley.edu \ --cc=gary@garyguo.net \ --cc=gregkh@linuxfoundation.org \ --cc=hch@infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-riscv@lists.infradead.org \ --cc=palmer@sifive.com \ --cc=paul.walmsley@sifive.com \ --cc=rppt@linux.ibm.com \ --cc=tglx@linutronix.de \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.