On Thu, Jul 09, 2026 at 11:40:44AM +0530, Nikhil Kumar Singh wrote: > Hi Mukesh, > > Ideally, checkpatch should take care of this, I see no checkpatch errors. > > ``` > ./scripts/checkpatch.pl > v2-0001-powerpc-64s-Clarify-copy_and_flush-cache-sync-loo.patch > total: 0 errors, 0 warnings, 26 lines checked > > v2-0001-powerpc-64s-Clarify-copy_and_flush-cache-sync-loo.patch has no > obvious style problems and is ready for submission. > ``` > > Thanks > ~ Nikhil > I am saying it looks nicer and easy to read for everyone if they are aligned. If you think it's not readable with the 80 char limit, put the whole comment before the function start. That will also makes it a little nicer. Right now it's getting wrapped around and it's difficult to read.
Regards, Mukesh > On 07/07/26 12:48, Nikhil Kumar Singh wrote: > > Hi Mukesh, > > > > Thanks for reviewing the patch and for the suggestion. > > I'd prefer to keep the current alignment as I find it a bit more > > readable. > > > > Thanks, > > ~ Nikhil > > > > On 07/07/26 11:08, Mukesh Kumar Chaurasiya wrote: > > > On Mon, Jul 06, 2026 at 01:57:08PM +0530, Nikhil Kumar Singh wrote: > > > > The value loaded into r0 in copy_and_flush() represents the number of > > > > 8-byte words processed between cache synchronization operations. > > > > > > > > The existing comment refers to cache line size, which can make > > > > it appear > > > > that the value is a cache line size in bytes rather than a loop count. > > > > Clarify the comment to explain that the loop processes 8 words > > > > (64 bytes) > > > > per cache synchronization iteration, and that increasing the > > > > value would > > > > skip cache maintenance for intermediate cache lines. > > > > > > > > This is a comment-only change with no functional impact. > > > > > > > > Signed-off-by: Nikhil Kumar Singh <[email protected]> > > > > --- > > > > arch/powerpc/kernel/head_64.S | 20 ++++++++++++-------- > > > > 1 file changed, 12 insertions(+), 8 deletions(-) > > > > > > > > diff --git a/arch/powerpc/kernel/head_64.S > > > > b/arch/powerpc/kernel/head_64.S > > > > index 63432a33ec49..a54f6f979173 100644 > > > > --- a/arch/powerpc/kernel/head_64.S > > > > +++ b/arch/powerpc/kernel/head_64.S > > > > @@ -713,14 +713,18 @@ p_end: .8byte _end - copy_to_here > > > > _GLOBAL(copy_and_flush) > > > > addi r5,r5,-8 > > > > addi r6,r6,-8 > > > > -4: li r0,8 /* Use the smallest common */ > > > > - /* denominator cache line */ > > > > - /* size. This results in */ > > > > - /* extra cache line flushes */ > > > > - /* but operation is correct. */ > > > > - /* Can't get cache line size */ > > > > - /* from NACA as it is being */ > > > > - /* moved too. */ > > > > +4: li r0,8 /* r0 is the number of 8-byte words */ > > > > + /* to copy per cache sync iteration. */ > > > > + /* 8 words * 8 bytes = 64 bytes. 64B is */ > > > > + /* the current default cache line size. */ > > > > + /* This is a loop count, not a byte */ > > > > + /* count. Increasing it may skip */ > > > > + /* dcbst/icbi for lines in between and */ > > > > + /* leave stale instructions in icache. */ > > > > + /* This results in extra cache line */ > > > > + /* flushes but operation is correct. */ > > > > + /* Can't get cache line size from NACA */ > > > > + /* as it is being moved too. */ > > > The comment looks fine. But the alignement of the comment is still > > > different from earlier. Keep the alignment same as earlier, it looks a > > > little nicer. > > > > > > Regards, > > > Mukesh > > > > mtctr r0 /* put # words/line in ctr */ > > > > 3: addi r6,r6,8 /* copy a cache line */ > > > > -- > > > > 2.43.5 > > > >
