Re: [PATCH v3 2/2] pinctrl: Add driver for Sunplus SP7021

Hi Wells,

I understand things better now!

On Fri, Dec 10, 2021 at 6:34 PM 呂芳騰 [off-list ref] wrote:

quoted

#define SSPCTL_FUNC_FLAG BIT(0)

if (func & SSPCTL_FUNC_FLAG)

Use the name that bit has in your documentation for the
define so we know what is going on.

Actually, 'if (func & 1)' is not used to test bit 0,
but test 'func' is an odd number or not.
If 'func' is even number, control-field is at bit 6 ~ 0.
Its corresponding mask-field is at bit 22 ~ 16.
If 'func' is odd number, control-field is at bit 14 ~ 8.
Its corresponding mask-field is at bit 30 ~ 24.

Aha! That makes sense. Just put in exactly that comment in
a block comment in the code so people maintaining this
can see what is going on here and why you are checking
for an odd/even number.

Control and mask fields of 'func' are arranged as shown
below:

func # | register control-field  mask-field
-------+------------------------------------
   0   | reg[0]     ( 6:0)        (22 :  6)
   1   | reg[0]     (14:8)        (30 : 24)
   2   | reg[1]     ( 6:0)        (22 :  6)
   3   | reg[1]     (14:8)        (30 : 24)

This is also nice to actually have in the code. Be generous
with comments I like that.

quoted

This layout with "mask and value" in registers needs to be explained
somewhere it looks complex. I don't understand why a machine register
contains a mask for example.

This is a hardware mechanism for protecting some important registers from
being overwritten accidentally. The corresponding mask bit should be set
first and then the control-bits or fields can be written. The design is
originally requested from car makers.

Wow that is very very interesting! So the feature is there to stop a
program that goes astray and just write random numbers all over the
memory from doing something harmful.

Insert some comments about this at the top of the file or at the first
time you use a mask.

Thanks for keeping fixing this up despite my sometimes confused
comments. Most of my comment is about making the code easy
to read by people that will maintain it, so my rule of thumb is if I
have a hard time to understand it then others may have a hard time
with it too.

Yours,
Linus Walleij

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel