Hi Paul, On Tue, 2018-09-25 at 17:45 +0000, Paul Burton wrote: > Hi Yasha, > > On Thu, Sep 20, 2018 at 08:03:06PM +0300, Yasha Cherikovsky wrote: > > MIPSR6 doesn't support unaligned access instructions (lwl, lwr, > > swl, swr). > > The MIPS tree has some special cases to avoid these instructions, > > and currently the code is testing for CONFIG_CPU_MIPSR6. > > > > Declare a new Kconfig variable: > > CONFIG_CPU_HAS_NO_UNALIGNED_LOAD_STORE, > > and make CONFIG_CPU_MIPSR6 select it. > > And switch all the special cases to test for the new variable. > > > > Also, the new variable selects CONFIG_GENERIC_CSUM, to use > > generic C checksum code (to avoid special assembly code that uses > > the unsupported instructions). > > Thanks for your patch :) > > I think it would be cleaner to invert this logic & instead have the > Kconfig entry indicate when kernel's build target *does* support the > [ls]w[lr] instructions. > > It would be good for the name to be clear that these instructions are > what it's about too - "unaligned load store" is a little too vague > for > my liking. For example one could easily misconstrue it to mean > something > akin to the inverse of CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS, > whereas > in the MIPSr6 case many CPUs actually handle unaligned accesses in > hardware when using the regular load/store instructions. They don't > have > the [ls]w[lr] instructions, but they don't need them because they > handle > unaligned accesses more naturally without needing us to be explicit > about them. > > How about we: > > - Add a Kconfig option CONFIG_CPU_SUPPORTS_LOAD_STORE_LR, and > select > it for all existing pre-r6 targets (probably from CONFIG_CPU_*). > > - Change CONFIG_GENERIC_CSUM to default y if > !CONFIG_CPU_SUPPORTS_LOAD_STORE_LR, and drop the selects of it. > > That would avoid the double-negative ("if we don't not support this") > that the #ifndef's currently represent. It would also mean any future > architecture/ISA targets beyond MIPSr6 automatically avoid the > instructions. > > Thanks, > Paul Thanks for your feedback, I'll start preparing v2. Looking in arch/mips/Kconfig, some CPU options start with CPU_SUPPORTS_ and some with CPU_HAS_. Which perfix should we use here? Thanks, Yasha