sunxi: fix sxipio func mask

[Patrick Wildt]

Hi,

in sxipio the mask is incorrect for get- and setcfg.

If bit is 1, off is (1 & 7) << 2, which is 4. That means each cfg is 4
bits wide, so the mask is 0xf and not 0x7. I cross-checked it with
NetBSD and Linux.

As far as I know it does not fix any known issue for me, it's just
something that caught my eye.

Patrick

diff --git sys/arch/armv7/sunxi/sxipio.c sys/arch/armv7/sunxi/sxipio.c
index 9a49343..07a3c76 100644
--- sys/arch/armv7/sunxi/sxipio.c
+++ sys/arch/armv7/sunxi/sxipio.c
@@ -249,7 +249,7 @@ sxipio_getcfg(int pin)

splx(s);

- return data >> off & 7;
+ return (data >> off) & 0xf;
}

void
@@ -263,7 +263,7 @@ sxipio_setcfg(int pin, int mux)
bit = pin - (port << 5);
reg = SXIPIO_CFG(port, bit >> 3);
off = (bit & 7) << 2;
- cmask = 7 << off;
+ cmask = 0xf << off;
mask = mux << off;

s = splhigh();

[Artturi Alm]

Post by Patrick Wildt
Hi,
in sxipio the mask is incorrect for get- and setcfg.
If bit is 1, off is (1 & 7) << 2, which is 4. That means each cfg is 4
bits wide, so the mask is 0xf and not 0x7. I cross-checked it with
NetBSD and Linux.
As far as I know it does not fix any known issue for me, it's just
something that caught my eye.
Patrick
diff --git sys/arch/armv7/sunxi/sxipio.c sys/arch/armv7/sunxi/sxipio.c
index 9a49343..07a3c76 100644
--- sys/arch/armv7/sunxi/sxipio.c
+++ sys/arch/armv7/sunxi/sxipio.c
@@ -249,7 +249,7 @@ sxipio_getcfg(int pin)
splx(s);
- return data >> off & 7;
+ return (data >> off) & 0xf;
}
void
@@ -263,7 +263,7 @@ sxipio_setcfg(int pin, int mux)
bit = pin - (port << 5);
reg = SXIPIO_CFG(port, bit >> 3);
off = (bit & 7) << 2;
- cmask = 7 << off;
+ cmask = 0xf << off;
mask = mux << off;
s = splhigh();

Hi,

i think you're wrong, "manuals" for both A10&A20 are rather clear about this.
last bit of each cfg is reserved, code doesn't touch the last bit on purpose.
see A10 user manual rev1.2 page 288, or A20 user manual rev1.0 page 248 for
example of one such cfg register(PA_CFG0).

-Artturi

[Patrick Wildt]

Post by Artturi Alm

Post by Patrick Wildt
Hi,
in sxipio the mask is incorrect for get- and setcfg.
If bit is 1, off is (1 & 7) << 2, which is 4. That means each cfg is 4
bits wide, so the mask is 0xf and not 0x7. I cross-checked it with
NetBSD and Linux.
As far as I know it does not fix any known issue for me, it's just
something that caught my eye.
Patrick
diff --git sys/arch/armv7/sunxi/sxipio.c sys/arch/armv7/sunxi/sxipio.c
index 9a49343..07a3c76 100644
--- sys/arch/armv7/sunxi/sxipio.c
+++ sys/arch/armv7/sunxi/sxipio.c
@@ -249,7 +249,7 @@ sxipio_getcfg(int pin)
splx(s);
- return data >> off & 7;
+ return (data >> off) & 0xf;
}
void
@@ -263,7 +263,7 @@ sxipio_setcfg(int pin, int mux)
bit = pin - (port << 5);
reg = SXIPIO_CFG(port, bit >> 3);
off = (bit & 7) << 2;
- cmask = 7 << off;
+ cmask = 0xf << off;
mask = mux << off;
s = splhigh();

Hi,
i think you're wrong, "manuals" for both A10&A20 are rather clear about this.
last bit of each cfg is reserved, code doesn't touch the last bit on purpose.
see A10 user manual rev1.2 page 288, or A20 user manual rev1.0 page 248 for
example of one such cfg register(PA_CFG0).
-Artturi

True that! I wonder if it would make sense to at least set the clear
mask to 0xf, so at least you don't write back whatever bit there
might be set or not set?

Bits are probably write ignored / read as zero.

[Artturi Alm]

Post by Patrick Wildt

Post by Artturi Alm

Post by Patrick Wildt
Hi,
in sxipio the mask is incorrect for get- and setcfg.
If bit is 1, off is (1 & 7) << 2, which is 4. That means each cfg is 4
bits wide, so the mask is 0xf and not 0x7. I cross-checked it with
NetBSD and Linux.
As far as I know it does not fix any known issue for me, it's just
something that caught my eye.
Patrick
diff --git sys/arch/armv7/sunxi/sxipio.c sys/arch/armv7/sunxi/sxipio.c
index 9a49343..07a3c76 100644
--- sys/arch/armv7/sunxi/sxipio.c
+++ sys/arch/armv7/sunxi/sxipio.c
@@ -249,7 +249,7 @@ sxipio_getcfg(int pin)
splx(s);
- return data >> off & 7;
+ return (data >> off) & 0xf;
}
void
@@ -263,7 +263,7 @@ sxipio_setcfg(int pin, int mux)
bit = pin - (port << 5);
reg = SXIPIO_CFG(port, bit >> 3);
off = (bit & 7) << 2;
- cmask = 7 << off;
+ cmask = 0xf << off;
mask = mux << off;
s = splhigh();

Hi,
i think you're wrong, "manuals" for both A10&A20 are rather clear about this.
last bit of each cfg is reserved, code doesn't touch the last bit on purpose.
see A10 user manual rev1.2 page 288, or A20 user manual rev1.0 page 248 for
example of one such cfg register(PA_CFG0).
-Artturi

True that! I wonder if it would make sense to at least set the clear
mask to 0xf, so at least you don't write back whatever bit there
might be set or not set?

don't know, and don't even really care atm., just noticed today that
according to man pages i'm not apparently author of any of sunxi drivers:P
and besides that, i still do have my own sys/arch/arm88k rewritten out of
sys/arch/arm & sys/arch/armv7 on top of m88k using armv7 userland, which
i've been able to maintain alone(diff to it has 0 + out of arch/arm88k ;).

Post by Patrick Wildt
Bits are probably write ignored / read as zero.

likely, would be easy to verify, but if 0xf is what linux and netbsd do use,
i guess it doesn't matter either way.

-Artturi

[Jonathan Gray]

Post by Artturi Alm

Post by Patrick Wildt

Post by Artturi Alm

Post by Patrick Wildt
Hi,
in sxipio the mask is incorrect for get- and setcfg.
If bit is 1, off is (1 & 7) << 2, which is 4. That means each cfg is 4
bits wide, so the mask is 0xf and not 0x7. I cross-checked it with
NetBSD and Linux.
As far as I know it does not fix any known issue for me, it's just
something that caught my eye.
Patrick
diff --git sys/arch/armv7/sunxi/sxipio.c sys/arch/armv7/sunxi/sxipio.c
index 9a49343..07a3c76 100644
--- sys/arch/armv7/sunxi/sxipio.c
+++ sys/arch/armv7/sunxi/sxipio.c
@@ -249,7 +249,7 @@ sxipio_getcfg(int pin)
splx(s);
- return data >> off & 7;
+ return (data >> off) & 0xf;
}
void
@@ -263,7 +263,7 @@ sxipio_setcfg(int pin, int mux)
bit = pin - (port << 5);
reg = SXIPIO_CFG(port, bit >> 3);
off = (bit & 7) << 2;
- cmask = 7 << off;
+ cmask = 0xf << off;
mask = mux << off;
s = splhigh();

Hi,
i think you're wrong, "manuals" for both A10&A20 are rather clear about this.
last bit of each cfg is reserved, code doesn't touch the last bit on purpose.
see A10 user manual rev1.2 page 288, or A20 user manual rev1.0 page 248 for
example of one such cfg register(PA_CFG0).
-Artturi

True that! I wonder if it would make sense to at least set the clear
mask to 0xf, so at least you don't write back whatever bit there
might be set or not set?

don't know, and don't even really care atm., just noticed today that
according to man pages i'm not apparently author of any of sunxi drivers:P

It looks like rapha took the earliest copyright to be the author of
the driver. How about the below diff or should we just remove the
AUTHORS section?

Index: a1xintc.4
===================================================================
RCS file: /cvs/src/share/man/man4/man4.armv7/a1xintc.4,v
retrieving revision 1.1
diff -u -p -r1.1 a1xintc.4
--- a1xintc.4 22 Sep 2014 14:02:38 -0000 1.1
+++ a1xintc.4 1 Feb 2016 22:53:02 -0000
@@ -36,4 +36,4 @@ device driver first appeared in
The
.Nm
driver was written by
-.An Dale Rahn Aq Mt ***@dalerahn.com .
+.An Artturi Alm .
Index: sxiccmu.4
===================================================================
RCS file: /cvs/src/share/man/man4/man4.armv7/sxiccmu.4,v
retrieving revision 1.1
diff -u -p -r1.1 sxiccmu.4
--- sxiccmu.4 22 Sep 2014 14:02:38 -0000 1.1
+++ sxiccmu.4 1 Feb 2016 22:55:02 -0000
@@ -36,4 +36,4 @@ device driver first appeared in
The
.Nm
driver was written by
-.An Dale Rahn Aq Mt ***@dalerahn.com .
+.An Artturi Alm .
Index: sxidog.4
===================================================================
RCS file: /cvs/src/share/man/man4/man4.armv7/sxidog.4,v
retrieving revision 1.1
diff -u -p -r1.1 sxidog.4
--- sxidog.4 22 Sep 2014 14:02:38 -0000 1.1
+++ sxidog.4 1 Feb 2016 22:56:14 -0000
@@ -36,4 +36,4 @@ device driver first appeared in
The
.Nm
driver was written by
-.An Dale Rahn Aq Mt ***@dalerahn.com .
+.An Artturi Alm .
Index: sxie.4
===================================================================
RCS file: /cvs/src/share/man/man4/man4.armv7/sxie.4,v
retrieving revision 1.1
diff -u -p -r1.1 sxie.4
--- sxie.4 22 Sep 2014 14:02:38 -0000 1.1
+++ sxie.4 1 Feb 2016 22:56:52 -0000
@@ -43,4 +43,4 @@ device driver first appeared in
The
.Nm
driver was written by
-.An Patrick Wildt Aq Mt ***@blueri.se .
+.An Artturi Alm .
Index: sxipio.4
===================================================================
RCS file: /cvs/src/share/man/man4/man4.armv7/sxipio.4,v
retrieving revision 1.1
diff -u -p -r1.1 sxipio.4
--- sxipio.4 22 Sep 2014 14:02:38 -0000 1.1
+++ sxipio.4 1 Feb 2016 22:59:11 -0000
@@ -46,4 +46,4 @@ driver first appeared in
The
.Nm
driver was written by
-.An Miodrag Vallat .
+.An Artturi Alm .
Index: sxitimer.4
===================================================================
RCS file: /cvs/src/share/man/man4/man4.armv7/sxitimer.4,v
retrieving revision 1.1
diff -u -p -r1.1 sxitimer.4
--- sxitimer.4 22 Sep 2014 14:02:38 -0000 1.1
+++ sxitimer.4 1 Feb 2016 22:57:46 -0000
@@ -36,4 +36,4 @@ device driver first appeared in
The
.Nm
driver was written by
-.An Dale Rahn Aq Mt ***@dalerahn.com .
+.An Artturi Alm .

[Artturi Alm]

Post by Jonathan Gray

Post by Artturi Alm

Post by Patrick Wildt

Post by Artturi Alm

Post by Patrick Wildt
Hi,
in sxipio the mask is incorrect for get- and setcfg.
If bit is 1, off is (1 & 7) << 2, which is 4. That means each cfg is 4
bits wide, so the mask is 0xf and not 0x7. I cross-checked it with
NetBSD and Linux.
As far as I know it does not fix any known issue for me, it's just
something that caught my eye.
Patrick
diff --git sys/arch/armv7/sunxi/sxipio.c sys/arch/armv7/sunxi/sxipio.c
index 9a49343..07a3c76 100644
--- sys/arch/armv7/sunxi/sxipio.c
+++ sys/arch/armv7/sunxi/sxipio.c
@@ -249,7 +249,7 @@ sxipio_getcfg(int pin)
splx(s);
- return data >> off & 7;
+ return (data >> off) & 0xf;
}
void
@@ -263,7 +263,7 @@ sxipio_setcfg(int pin, int mux)
bit = pin - (port << 5);
reg = SXIPIO_CFG(port, bit >> 3);
off = (bit & 7) << 2;
- cmask = 7 << off;
+ cmask = 0xf << off;
mask = mux << off;
s = splhigh();

Hi,
i think you're wrong, "manuals" for both A10&A20 are rather clear about this.
last bit of each cfg is reserved, code doesn't touch the last bit on purpose.
see A10 user manual rev1.2 page 288, or A20 user manual rev1.0 page 248 for
example of one such cfg register(PA_CFG0).
-Artturi

True that! I wonder if it would make sense to at least set the clear
mask to 0xf, so at least you don't write back whatever bit there
might be set or not set?

don't know, and don't even really care atm., just noticed today that
according to man pages i'm not apparently author of any of sunxi drivers:P

It looks like rapha took the earliest copyright to be the author of
the driver. How about the below diff or should we just remove the
AUTHORS section?

likely so, i left those copyrights of previously (likely) more than once
copy&paste'd drivers as they were, and ofc in most cases used the existing
code using necessary MI interfaces.
i would have sent the diff below if it did matter to me, personally i feel
that AUTHORS section provides no value given copyright in actual sources.

-Artturi

Post by Jonathan Gray
Index: a1xintc.4
===================================================================
RCS file: /cvs/src/share/man/man4/man4.armv7/a1xintc.4,v
retrieving revision 1.1
diff -u -p -r1.1 a1xintc.4
--- a1xintc.4 22 Sep 2014 14:02:38 -0000 1.1
+++ a1xintc.4 1 Feb 2016 22:53:02 -0000
@@ -36,4 +36,4 @@ device driver first appeared in
The
.Nm
driver was written by
+.An Artturi Alm .
Index: sxiccmu.4
===================================================================
RCS file: /cvs/src/share/man/man4/man4.armv7/sxiccmu.4,v
retrieving revision 1.1
diff -u -p -r1.1 sxiccmu.4
--- sxiccmu.4 22 Sep 2014 14:02:38 -0000 1.1
+++ sxiccmu.4 1 Feb 2016 22:55:02 -0000
@@ -36,4 +36,4 @@ device driver first appeared in
The
.Nm
driver was written by
+.An Artturi Alm .
Index: sxidog.4
===================================================================
RCS file: /cvs/src/share/man/man4/man4.armv7/sxidog.4,v
retrieving revision 1.1
diff -u -p -r1.1 sxidog.4
--- sxidog.4 22 Sep 2014 14:02:38 -0000 1.1
+++ sxidog.4 1 Feb 2016 22:56:14 -0000
@@ -36,4 +36,4 @@ device driver first appeared in
The
.Nm
driver was written by
+.An Artturi Alm .
Index: sxie.4
===================================================================
RCS file: /cvs/src/share/man/man4/man4.armv7/sxie.4,v
retrieving revision 1.1
diff -u -p -r1.1 sxie.4
--- sxie.4 22 Sep 2014 14:02:38 -0000 1.1
+++ sxie.4 1 Feb 2016 22:56:52 -0000
@@ -43,4 +43,4 @@ device driver first appeared in
The
.Nm
driver was written by
+.An Artturi Alm .
Index: sxipio.4
===================================================================
RCS file: /cvs/src/share/man/man4/man4.armv7/sxipio.4,v
retrieving revision 1.1
diff -u -p -r1.1 sxipio.4
--- sxipio.4 22 Sep 2014 14:02:38 -0000 1.1
+++ sxipio.4 1 Feb 2016 22:59:11 -0000
@@ -46,4 +46,4 @@ driver first appeared in
The
.Nm
driver was written by
-.An Miodrag Vallat .
+.An Artturi Alm .
Index: sxitimer.4
===================================================================
RCS file: /cvs/src/share/man/man4/man4.armv7/sxitimer.4,v
retrieving revision 1.1
diff -u -p -r1.1 sxitimer.4
--- sxitimer.4 22 Sep 2014 14:02:38 -0000 1.1
+++ sxitimer.4 1 Feb 2016 22:57:46 -0000
@@ -36,4 +36,4 @@ device driver first appeared in
The
.Nm
driver was written by
+.An Artturi Alm .