Thread (15 messages) 15 messages, 4 authors, 2022-06-15

Re: [PATCH 2/2] uio:powerpc:mpc85xx: l2-cache-sram uio driver implementation

From: Wenhu Wang <hidden>
Date: 2022-06-14 07:54:23
Also in: lkml

quoted
quoted
quoted
+
+struct mpc85xx_l2ctlr {
+     u32     ctl;            /* 0x000 - L2 control */
What is the endian of these u32 values?  You map them directly to
memory, so they must be specified some way, right?  Please make it
obvious what they are.
Surely, the values should be u32 here, modified in v2
The controller info could be found in
"QorIQ™ P2020 Integrated Processor Reference Manual"
"Chapter 6 L2 Look-Aside Cache/SRAM"
See: http://m4udit.dinauz.org/P2020RM_rev0.pdf
That's not the answer to my question :)

These are big-endian, right?  Please mark them as such and access them
properly with the correct functions.
Yes, they are big-edian.
Does it work to add comments(about order and access functions) for the structure ahead of it?
And appending like "_be", "_access_be" or "_big_endian"? (struct mpc85xx_l2ctlr_be {……};
Tabs in Linux are always 8 spaces wide.
I will re-confirm the v2 of identation.
quoted
I looked at that patch.

I don't think you can just drop the #ifdef in function
__access_remote_vm() in mm/memory.c

You have to replace it with something like:

     if (!IS_ENABLED(CONFIG_HAVE_IOREMAP_PROT))
         break;

Another thing in that patch:

By making generic_access_phys() a static inline, it means that everytime
you refer to the address of that function in a vm_operations_struct
struct, the compiler has to provide an outlined instance of the
function. It means you'll likely have several instances of a
generic_access_phys().

What you could do instead is to add the following at the start of
generic_access_phys() in mm/memory.c :

       if (!IS_ENABLED(CONFIG_HAVE_IOREMAP_PROT))
               return 0;
It is really a better chmoce, thanks for the advice.
Multiple instances exist as you mentioned, the block returns 0 with no-op
instance which makes no difference with the function return value.

I will update the patch after a re-confirming.

Thanks,
Wenhu
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help