linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/1] net: dsa: rtl8366rb: change type of jam tables
@ 2021-01-25  4:56 Lorenzo Carletti
  2021-01-25  4:56 ` [PATCH 1/1] net: dsa: rtl8366rb: standardize init " Lorenzo Carletti
  0 siblings, 1 reply; 10+ messages in thread
From: Lorenzo Carletti @ 2021-01-25  4:56 UTC (permalink / raw)
  To: linus.walleij
  Cc: andrew, vivien.didelot, f.fainelli, olteanv, davem, kuba, netdev,
	linux-kernel, Lorenzo Carletti

I was trying to see if there were some Intel 8051 instructions in the
jam tables with Linus Walleij, when I noticed some oddities.
This patch's aim is to make the code more consistent and more similar
to the vendor's original source.
Link to the Realtek code the actual patch is based on:
https://svn.dd-wrt.com/browser/src/linux/universal/linux-3.2/drivers/net/ethernet/raeth/rb/rtl8366rb_api.c

Lorenzo Carletti (1):
  net: dsa: rtl8366rb: standardize init jam tables

 drivers/net/dsa/rtl8366rb.c | 264 ++++++++++++++++++------------------
 1 file changed, 131 insertions(+), 133 deletions(-)

-- 
2.17.1


^ permalink raw reply	[flat|nested] 10+ messages in thread
* [PATCH 1/1] net: dsa: rtl8366rb: standardize init jam tables
  2021-01-25  4:56 [PATCH 0/1] net: dsa: rtl8366rb: change type of jam tables Lorenzo Carletti
@ 2021-01-25  4:56 ` Lorenzo Carletti
  2021-01-26 21:08   ` Vladimir Oltean
                     ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Lorenzo Carletti @ 2021-01-25  4:56 UTC (permalink / raw)
  To: linus.walleij
  Cc: andrew, vivien.didelot, f.fainelli, olteanv, davem, kuba, netdev,
	linux-kernel, Lorenzo Carletti

In the rtl8366rb driver there are some jam tables which contain
undocumented values.
While trying to understand what these tables actually do,
I noticed a discrepancy in how one of those was treated.
Most of them were plain u16 arrays, while the ethernet one was
an u16 matrix.
By looking at the vendor's droplets of source code these tables came from,
I found out that they were all originally u16 matrixes.

This commit standardizes the jam tables, turning them all into
u16 matrixes.
This change makes it easier to understand how the jam tables are used
and also makes it possible for a single function to handle all of them,
removing some duplicated code.

Signed-off-by: Lorenzo Carletti <lorenzo.carletti98@gmail.com>
---
 drivers/net/dsa/rtl8366rb.c | 264 ++++++++++++++++++------------------
 1 file changed, 131 insertions(+), 133 deletions(-)

diff --git a/drivers/net/dsa/rtl8366rb.c b/drivers/net/dsa/rtl8366rb.c
index cfe56960f44b..673744e56713 100644
--- a/drivers/net/dsa/rtl8366rb.c
+++ b/drivers/net/dsa/rtl8366rb.c
@@ -602,107 +602,107 @@ static int rtl8366rb_set_addr(struct realtek_smi *smi)
 /* Found in a vendor driver */
 
 /* For the "version 0" early silicon, appear in most source releases */
-static const u16 rtl8366rb_init_jam_ver_0[] = {
-	0x000B, 0x0001, 0x03A6, 0x0100, 0x03A7, 0x0001, 0x02D1, 0x3FFF,
-	0x02D2, 0x3FFF, 0x02D3, 0x3FFF, 0x02D4, 0x3FFF, 0x02D5, 0x3FFF,
-	0x02D6, 0x3FFF, 0x02D7, 0x3FFF, 0x02D8, 0x3FFF, 0x022B, 0x0688,
-	0x022C, 0x0FAC, 0x03D0, 0x4688, 0x03D1, 0x01F5, 0x0000, 0x0830,
-	0x02F9, 0x0200, 0x02F7, 0x7FFF, 0x02F8, 0x03FF, 0x0080, 0x03E8,
-	0x0081, 0x00CE, 0x0082, 0x00DA, 0x0083, 0x0230, 0xBE0F, 0x2000,
-	0x0231, 0x422A, 0x0232, 0x422A, 0x0233, 0x422A, 0x0234, 0x422A,
-	0x0235, 0x422A, 0x0236, 0x422A, 0x0237, 0x422A, 0x0238, 0x422A,
-	0x0239, 0x422A, 0x023A, 0x422A, 0x023B, 0x422A, 0x023C, 0x422A,
-	0x023D, 0x422A, 0x023E, 0x422A, 0x023F, 0x422A, 0x0240, 0x422A,
-	0x0241, 0x422A, 0x0242, 0x422A, 0x0243, 0x422A, 0x0244, 0x422A,
-	0x0245, 0x422A, 0x0246, 0x422A, 0x0247, 0x422A, 0x0248, 0x422A,
-	0x0249, 0x0146, 0x024A, 0x0146, 0x024B, 0x0146, 0xBE03, 0xC961,
-	0x024D, 0x0146, 0x024E, 0x0146, 0x024F, 0x0146, 0x0250, 0x0146,
-	0xBE64, 0x0226, 0x0252, 0x0146, 0x0253, 0x0146, 0x024C, 0x0146,
-	0x0251, 0x0146, 0x0254, 0x0146, 0xBE62, 0x3FD0, 0x0084, 0x0320,
-	0x0255, 0x0146, 0x0256, 0x0146, 0x0257, 0x0146, 0x0258, 0x0146,
-	0x0259, 0x0146, 0x025A, 0x0146, 0x025B, 0x0146, 0x025C, 0x0146,
-	0x025D, 0x0146, 0x025E, 0x0146, 0x025F, 0x0146, 0x0260, 0x0146,
-	0x0261, 0xA23F, 0x0262, 0x0294, 0x0263, 0xA23F, 0x0264, 0x0294,
-	0x0265, 0xA23F, 0x0266, 0x0294, 0x0267, 0xA23F, 0x0268, 0x0294,
-	0x0269, 0xA23F, 0x026A, 0x0294, 0x026B, 0xA23F, 0x026C, 0x0294,
-	0x026D, 0xA23F, 0x026E, 0x0294, 0x026F, 0xA23F, 0x0270, 0x0294,
-	0x02F5, 0x0048, 0xBE09, 0x0E00, 0xBE1E, 0x0FA0, 0xBE14, 0x8448,
-	0xBE15, 0x1007, 0xBE4A, 0xA284, 0xC454, 0x3F0B, 0xC474, 0x3F0B,
-	0xBE48, 0x3672, 0xBE4B, 0x17A7, 0xBE4C, 0x0B15, 0xBE52, 0x0EDD,
-	0xBE49, 0x8C00, 0xBE5B, 0x785C, 0xBE5C, 0x785C, 0xBE5D, 0x785C,
-	0xBE61, 0x368A, 0xBE63, 0x9B84, 0xC456, 0xCC13, 0xC476, 0xCC13,
-	0xBE65, 0x307D, 0xBE6D, 0x0005, 0xBE6E, 0xE120, 0xBE2E, 0x7BAF,
+static const u16 rtl8366rb_init_jam_ver_0[][2] = {
+	{0x000B, 0x0001}, {0x03A6, 0x0100}, {0x03A7, 0x0001}, {0x02D1, 0x3FFF},
+	{0x02D2, 0x3FFF}, {0x02D3, 0x3FFF}, {0x02D4, 0x3FFF}, {0x02D5, 0x3FFF},
+	{0x02D6, 0x3FFF}, {0x02D7, 0x3FFF}, {0x02D8, 0x3FFF}, {0x022B, 0x0688},
+	{0x022C, 0x0FAC}, {0x03D0, 0x4688}, {0x03D1, 0x01F5}, {0x0000, 0x0830},
+	{0x02F9, 0x0200}, {0x02F7, 0x7FFF}, {0x02F8, 0x03FF}, {0x0080, 0x03E8},
+	{0x0081, 0x00CE}, {0x0082, 0x00DA}, {0x0083, 0x0230}, {0xBE0F, 0x2000},
+	{0x0231, 0x422A}, {0x0232, 0x422A}, {0x0233, 0x422A}, {0x0234, 0x422A},
+	{0x0235, 0x422A}, {0x0236, 0x422A}, {0x0237, 0x422A}, {0x0238, 0x422A},
+	{0x0239, 0x422A}, {0x023A, 0x422A}, {0x023B, 0x422A}, {0x023C, 0x422A},
+	{0x023D, 0x422A}, {0x023E, 0x422A}, {0x023F, 0x422A}, {0x0240, 0x422A},
+	{0x0241, 0x422A}, {0x0242, 0x422A}, {0x0243, 0x422A}, {0x0244, 0x422A},
+	{0x0245, 0x422A}, {0x0246, 0x422A}, {0x0247, 0x422A}, {0x0248, 0x422A},
+	{0x0249, 0x0146}, {0x024A, 0x0146}, {0x024B, 0x0146}, {0xBE03, 0xC961},
+	{0x024D, 0x0146}, {0x024E, 0x0146}, {0x024F, 0x0146}, {0x0250, 0x0146},
+	{0xBE64, 0x0226}, {0x0252, 0x0146}, {0x0253, 0x0146}, {0x024C, 0x0146},
+	{0x0251, 0x0146}, {0x0254, 0x0146}, {0xBE62, 0x3FD0}, {0x0084, 0x0320},
+	{0x0255, 0x0146}, {0x0256, 0x0146}, {0x0257, 0x0146}, {0x0258, 0x0146},
+	{0x0259, 0x0146}, {0x025A, 0x0146}, {0x025B, 0x0146}, {0x025C, 0x0146},
+	{0x025D, 0x0146}, {0x025E, 0x0146}, {0x025F, 0x0146}, {0x0260, 0x0146},
+	{0x0261, 0xA23F}, {0x0262, 0x0294}, {0x0263, 0xA23F}, {0x0264, 0x0294},
+	{0x0265, 0xA23F}, {0x0266, 0x0294}, {0x0267, 0xA23F}, {0x0268, 0x0294},
+	{0x0269, 0xA23F}, {0x026A, 0x0294}, {0x026B, 0xA23F}, {0x026C, 0x0294},
+	{0x026D, 0xA23F}, {0x026E, 0x0294}, {0x026F, 0xA23F}, {0x0270, 0x0294},
+	{0x02F5, 0x0048}, {0xBE09, 0x0E00}, {0xBE1E, 0x0FA0}, {0xBE14, 0x8448},
+	{0xBE15, 0x1007}, {0xBE4A, 0xA284}, {0xC454, 0x3F0B}, {0xC474, 0x3F0B},
+	{0xBE48, 0x3672}, {0xBE4B, 0x17A7}, {0xBE4C, 0x0B15}, {0xBE52, 0x0EDD},
+	{0xBE49, 0x8C00}, {0xBE5B, 0x785C}, {0xBE5C, 0x785C}, {0xBE5D, 0x785C},
+	{0xBE61, 0x368A}, {0xBE63, 0x9B84}, {0xC456, 0xCC13}, {0xC476, 0xCC13},
+	{0xBE65, 0x307D}, {0xBE6D, 0x0005}, {0xBE6E, 0xE120}, {0xBE2E, 0x7BAF},
 };
 
 /* This v1 init sequence is from Belkin F5D8235 U-Boot release */
-static const u16 rtl8366rb_init_jam_ver_1[] = {
-	0x0000, 0x0830, 0x0001, 0x8000, 0x0400, 0x8130, 0xBE78, 0x3C3C,
-	0x0431, 0x5432, 0xBE37, 0x0CE4, 0x02FA, 0xFFDF, 0x02FB, 0xFFE0,
-	0xC44C, 0x1585, 0xC44C, 0x1185, 0xC44C, 0x1585, 0xC46C, 0x1585,
-	0xC46C, 0x1185, 0xC46C, 0x1585, 0xC451, 0x2135, 0xC471, 0x2135,
-	0xBE10, 0x8140, 0xBE15, 0x0007, 0xBE6E, 0xE120, 0xBE69, 0xD20F,
-	0xBE6B, 0x0320, 0xBE24, 0xB000, 0xBE23, 0xFF51, 0xBE22, 0xDF20,
-	0xBE21, 0x0140, 0xBE20, 0x00BB, 0xBE24, 0xB800, 0xBE24, 0x0000,
-	0xBE24, 0x7000, 0xBE23, 0xFF51, 0xBE22, 0xDF60, 0xBE21, 0x0140,
-	0xBE20, 0x0077, 0xBE24, 0x7800, 0xBE24, 0x0000, 0xBE2E, 0x7B7A,
-	0xBE36, 0x0CE4, 0x02F5, 0x0048, 0xBE77, 0x2940, 0x000A, 0x83E0,
-	0xBE79, 0x3C3C, 0xBE00, 0x1340,
+static const u16 rtl8366rb_init_jam_ver_1[][2] = {
+	{0x0000, 0x0830}, {0x0001, 0x8000}, {0x0400, 0x8130}, {0xBE78, 0x3C3C},
+	{0x0431, 0x5432}, {0xBE37, 0x0CE4}, {0x02FA, 0xFFDF}, {0x02FB, 0xFFE0},
+	{0xC44C, 0x1585}, {0xC44C, 0x1185}, {0xC44C, 0x1585}, {0xC46C, 0x1585},
+	{0xC46C, 0x1185}, {0xC46C, 0x1585}, {0xC451, 0x2135}, {0xC471, 0x2135},
+	{0xBE10, 0x8140}, {0xBE15, 0x0007}, {0xBE6E, 0xE120}, {0xBE69, 0xD20F},
+	{0xBE6B, 0x0320}, {0xBE24, 0xB000}, {0xBE23, 0xFF51}, {0xBE22, 0xDF20},
+	{0xBE21, 0x0140}, {0xBE20, 0x00BB}, {0xBE24, 0xB800}, {0xBE24, 0x0000},
+	{0xBE24, 0x7000}, {0xBE23, 0xFF51}, {0xBE22, 0xDF60}, {0xBE21, 0x0140},
+	{0xBE20, 0x0077}, {0xBE24, 0x7800}, {0xBE24, 0x0000}, {0xBE2E, 0x7B7A},
+	{0xBE36, 0x0CE4}, {0x02F5, 0x0048}, {0xBE77, 0x2940}, {0x000A, 0x83E0},
+	{0xBE79, 0x3C3C}, {0xBE00, 0x1340},
 };
 
 /* This v2 init sequence is from Belkin F5D8235 U-Boot release */
-static const u16 rtl8366rb_init_jam_ver_2[] = {
-	0x0450, 0x0000, 0x0400, 0x8130, 0x000A, 0x83ED, 0x0431, 0x5432,
-	0xC44F, 0x6250, 0xC46F, 0x6250, 0xC456, 0x0C14, 0xC476, 0x0C14,
-	0xC44C, 0x1C85, 0xC44C, 0x1885, 0xC44C, 0x1C85, 0xC46C, 0x1C85,
-	0xC46C, 0x1885, 0xC46C, 0x1C85, 0xC44C, 0x0885, 0xC44C, 0x0881,
-	0xC44C, 0x0885, 0xC46C, 0x0885, 0xC46C, 0x0881, 0xC46C, 0x0885,
-	0xBE2E, 0x7BA7, 0xBE36, 0x1000, 0xBE37, 0x1000, 0x8000, 0x0001,
-	0xBE69, 0xD50F, 0x8000, 0x0000, 0xBE69, 0xD50F, 0xBE6E, 0x0320,
-	0xBE77, 0x2940, 0xBE78, 0x3C3C, 0xBE79, 0x3C3C, 0xBE6E, 0xE120,
-	0x8000, 0x0001, 0xBE15, 0x1007, 0x8000, 0x0000, 0xBE15, 0x1007,
-	0xBE14, 0x0448, 0xBE1E, 0x00A0, 0xBE10, 0x8160, 0xBE10, 0x8140,
-	0xBE00, 0x1340, 0x0F51, 0x0010,
+static const u16 rtl8366rb_init_jam_ver_2[][2] = {
+	{0x0450, 0x0000}, {0x0400, 0x8130}, {0x000A, 0x83ED}, {0x0431, 0x5432},
+	{0xC44F, 0x6250}, {0xC46F, 0x6250}, {0xC456, 0x0C14}, {0xC476, 0x0C14},
+	{0xC44C, 0x1C85}, {0xC44C, 0x1885}, {0xC44C, 0x1C85}, {0xC46C, 0x1C85},
+	{0xC46C, 0x1885}, {0xC46C, 0x1C85}, {0xC44C, 0x0885}, {0xC44C, 0x0881},
+	{0xC44C, 0x0885}, {0xC46C, 0x0885}, {0xC46C, 0x0881}, {0xC46C, 0x0885},
+	{0xBE2E, 0x7BA7}, {0xBE36, 0x1000}, {0xBE37, 0x1000}, {0x8000, 0x0001},
+	{0xBE69, 0xD50F}, {0x8000, 0x0000}, {0xBE69, 0xD50F}, {0xBE6E, 0x0320},
+	{0xBE77, 0x2940}, {0xBE78, 0x3C3C}, {0xBE79, 0x3C3C}, {0xBE6E, 0xE120},
+	{0x8000, 0x0001}, {0xBE15, 0x1007}, {0x8000, 0x0000}, {0xBE15, 0x1007},
+	{0xBE14, 0x0448}, {0xBE1E, 0x00A0}, {0xBE10, 0x8160}, {0xBE10, 0x8140},
+	{0xBE00, 0x1340}, {0x0F51, 0x0010},
 };
 
 /* Appears in a DDWRT code dump */
-static const u16 rtl8366rb_init_jam_ver_3[] = {
-	0x0000, 0x0830, 0x0400, 0x8130, 0x000A, 0x83ED, 0x0431, 0x5432,
-	0x0F51, 0x0017, 0x02F5, 0x0048, 0x02FA, 0xFFDF, 0x02FB, 0xFFE0,
-	0xC456, 0x0C14, 0xC476, 0x0C14, 0xC454, 0x3F8B, 0xC474, 0x3F8B,
-	0xC450, 0x2071, 0xC470, 0x2071, 0xC451, 0x226B, 0xC471, 0x226B,
-	0xC452, 0xA293, 0xC472, 0xA293, 0xC44C, 0x1585, 0xC44C, 0x1185,
-	0xC44C, 0x1585, 0xC46C, 0x1585, 0xC46C, 0x1185, 0xC46C, 0x1585,
-	0xC44C, 0x0185, 0xC44C, 0x0181, 0xC44C, 0x0185, 0xC46C, 0x0185,
-	0xC46C, 0x0181, 0xC46C, 0x0185, 0xBE24, 0xB000, 0xBE23, 0xFF51,
-	0xBE22, 0xDF20, 0xBE21, 0x0140, 0xBE20, 0x00BB, 0xBE24, 0xB800,
-	0xBE24, 0x0000, 0xBE24, 0x7000, 0xBE23, 0xFF51, 0xBE22, 0xDF60,
-	0xBE21, 0x0140, 0xBE20, 0x0077, 0xBE24, 0x7800, 0xBE24, 0x0000,
-	0xBE2E, 0x7BA7, 0xBE36, 0x1000, 0xBE37, 0x1000, 0x8000, 0x0001,
-	0xBE69, 0xD50F, 0x8000, 0x0000, 0xBE69, 0xD50F, 0xBE6B, 0x0320,
-	0xBE77, 0x2800, 0xBE78, 0x3C3C, 0xBE79, 0x3C3C, 0xBE6E, 0xE120,
-	0x8000, 0x0001, 0xBE10, 0x8140, 0x8000, 0x0000, 0xBE10, 0x8140,
-	0xBE15, 0x1007, 0xBE14, 0x0448, 0xBE1E, 0x00A0, 0xBE10, 0x8160,
-	0xBE10, 0x8140, 0xBE00, 0x1340, 0x0450, 0x0000, 0x0401, 0x0000,
+static const u16 rtl8366rb_init_jam_ver_3[][2] = {
+	{0x0000, 0x0830}, {0x0400, 0x8130}, {0x000A, 0x83ED}, {0x0431, 0x5432},
+	{0x0F51, 0x0017}, {0x02F5, 0x0048}, {0x02FA, 0xFFDF}, {0x02FB, 0xFFE0},
+	{0xC456, 0x0C14}, {0xC476, 0x0C14}, {0xC454, 0x3F8B}, {0xC474, 0x3F8B},
+	{0xC450, 0x2071}, {0xC470, 0x2071}, {0xC451, 0x226B}, {0xC471, 0x226B},
+	{0xC452, 0xA293}, {0xC472, 0xA293}, {0xC44C, 0x1585}, {0xC44C, 0x1185},
+	{0xC44C, 0x1585}, {0xC46C, 0x1585}, {0xC46C, 0x1185}, {0xC46C, 0x1585},
+	{0xC44C, 0x0185}, {0xC44C, 0x0181}, {0xC44C, 0x0185}, {0xC46C, 0x0185},
+	{0xC46C, 0x0181}, {0xC46C, 0x0185}, {0xBE24, 0xB000}, {0xBE23, 0xFF51},
+	{0xBE22, 0xDF20}, {0xBE21, 0x0140}, {0xBE20, 0x00BB}, {0xBE24, 0xB800},
+	{0xBE24, 0x0000}, {0xBE24, 0x7000}, {0xBE23, 0xFF51}, {0xBE22, 0xDF60},
+	{0xBE21, 0x0140}, {0xBE20, 0x0077}, {0xBE24, 0x7800}, {0xBE24, 0x0000},
+	{0xBE2E, 0x7BA7}, {0xBE36, 0x1000}, {0xBE37, 0x1000}, {0x8000, 0x0001},
+	{0xBE69, 0xD50F}, {0x8000, 0x0000}, {0xBE69, 0xD50F}, {0xBE6B, 0x0320},
+	{0xBE77, 0x2800}, {0xBE78, 0x3C3C}, {0xBE79, 0x3C3C}, {0xBE6E, 0xE120},
+	{0x8000, 0x0001}, {0xBE10, 0x8140}, {0x8000, 0x0000}, {0xBE10, 0x8140},
+	{0xBE15, 0x1007}, {0xBE14, 0x0448}, {0xBE1E, 0x00A0}, {0xBE10, 0x8160},
+	{0xBE10, 0x8140}, {0xBE00, 0x1340}, {0x0450, 0x0000}, {0x0401, 0x0000},
 };
 
 /* Belkin F5D8235 v1, "belkin,f5d8235-v1" */
-static const u16 rtl8366rb_init_jam_f5d8235[] = {
-	0x0242, 0x02BF, 0x0245, 0x02BF, 0x0248, 0x02BF, 0x024B, 0x02BF,
-	0x024E, 0x02BF, 0x0251, 0x02BF, 0x0254, 0x0A3F, 0x0256, 0x0A3F,
-	0x0258, 0x0A3F, 0x025A, 0x0A3F, 0x025C, 0x0A3F, 0x025E, 0x0A3F,
-	0x0263, 0x007C, 0x0100, 0x0004, 0xBE5B, 0x3500, 0x800E, 0x200F,
-	0xBE1D, 0x0F00, 0x8001, 0x5011, 0x800A, 0xA2F4, 0x800B, 0x17A3,
-	0xBE4B, 0x17A3, 0xBE41, 0x5011, 0xBE17, 0x2100, 0x8000, 0x8304,
-	0xBE40, 0x8304, 0xBE4A, 0xA2F4, 0x800C, 0xA8D5, 0x8014, 0x5500,
-	0x8015, 0x0004, 0xBE4C, 0xA8D5, 0xBE59, 0x0008, 0xBE09, 0x0E00,
-	0xBE36, 0x1036, 0xBE37, 0x1036, 0x800D, 0x00FF, 0xBE4D, 0x00FF,
+static const u16 rtl8366rb_init_jam_f5d8235[][2] = {
+	{0x0242, 0x02BF}, {0x0245, 0x02BF}, {0x0248, 0x02BF}, {0x024B, 0x02BF},
+	{0x024E, 0x02BF}, {0x0251, 0x02BF}, {0x0254, 0x0A3F}, {0x0256, 0x0A3F},
+	{0x0258, 0x0A3F}, {0x025A, 0x0A3F}, {0x025C, 0x0A3F}, {0x025E, 0x0A3F},
+	{0x0263, 0x007C}, {0x0100, 0x0004}, {0xBE5B, 0x3500}, {0x800E, 0x200F},
+	{0xBE1D, 0x0F00}, {0x8001, 0x5011}, {0x800A, 0xA2F4}, {0x800B, 0x17A3},
+	{0xBE4B, 0x17A3}, {0xBE41, 0x5011}, {0xBE17, 0x2100}, {0x8000, 0x8304},
+	{0xBE40, 0x8304}, {0xBE4A, 0xA2F4}, {0x800C, 0xA8D5}, {0x8014, 0x5500},
+	{0x8015, 0x0004}, {0xBE4C, 0xA8D5}, {0xBE59, 0x0008}, {0xBE09, 0x0E00},
+	{0xBE36, 0x1036}, {0xBE37, 0x1036}, {0x800D, 0x00FF}, {0xBE4D, 0x00FF},
 };
 
 /* DGN3500, "netgear,dgn3500", "netgear,dgn3500b" */
-static const u16 rtl8366rb_init_jam_dgn3500[] = {
-	0x0000, 0x0830, 0x0400, 0x8130, 0x000A, 0x83ED, 0x0F51, 0x0017,
-	0x02F5, 0x0048, 0x02FA, 0xFFDF, 0x02FB, 0xFFE0, 0x0450, 0x0000,
-	0x0401, 0x0000, 0x0431, 0x0960,
+static const u16 rtl8366rb_init_jam_dgn3500[][2] = {
+	{0x0000, 0x0830}, {0x0400, 0x8130}, {0x000A, 0x83ED}, {0x0F51, 0x0017},
+	{0x02F5, 0x0048}, {0x02FA, 0xFFDF}, {0x02FB, 0xFFE0}, {0x0450, 0x0000},
+	{0x0401, 0x0000}, {0x0431, 0x0960},
 };
 
 /* This jam table activates "green ethernet", which means low power mode
@@ -716,10 +716,46 @@ static const u16 rtl8366rb_green_jam[][2] = {
 	{0xBE5C, 0x785C}, {0xBE6E, 0xE120}, {0xBE79, 0x323C},
 };
 
+/* Function that jams the tables in the proper registers */
+static int rtl8366rb_jam_table(const u16 (*jam_table)[2], int jam_size,
+			       struct realtek_smi *smi, bool write_dbg)
+{
+	u32 val;
+	int ret;
+	int i;
+
+	for (i = 0; i < jam_size; i++) {
+		if ((jam_table[i][0] & 0xBE00) == 0xBE00) {
+			ret = regmap_read(smi->map,
+					  RTL8366RB_PHY_ACCESS_BUSY_REG,
+					  &val);
+			if (ret)
+				return ret;
+			if (!(val & RTL8366RB_PHY_INT_BUSY)) {
+				ret = regmap_write(smi->map,
+						RTL8366RB_PHY_ACCESS_CTRL_REG,
+						RTL8366RB_PHY_CTRL_WRITE);
+				if (ret)
+					return ret;
+			}
+		}
+		if (write_dbg)
+			dev_dbg(smi->dev, "jam %04x into register %04x\n",
+				jam_table[i][1],
+				jam_table[i][0]);
+		ret = regmap_write(smi->map,
+				   jam_table[i][0],
+				   jam_table[i][1]);
+		if (ret)
+			return ret;
+	}
+	return 0;
+}
+
 static int rtl8366rb_setup(struct dsa_switch *ds)
 {
 	struct realtek_smi *smi = ds->priv;
-	const u16 *jam_table;
+	const u16 (*jam_table)[2];
 	struct rtl8366rb *rb;
 	u32 chip_ver = 0;
 	u32 chip_id = 0;
@@ -788,54 +824,16 @@ static int rtl8366rb_setup(struct dsa_switch *ds)
 		jam_size = ARRAY_SIZE(rtl8366rb_init_jam_dgn3500);
 	}
 
-	i = 0;
-	while (i < jam_size) {
-		if ((jam_table[i] & 0xBE00) == 0xBE00) {
-			ret = regmap_read(smi->map,
-					  RTL8366RB_PHY_ACCESS_BUSY_REG,
-					  &val);
-			if (ret)
-				return ret;
-			if (!(val & RTL8366RB_PHY_INT_BUSY)) {
-				ret = regmap_write(smi->map,
-						RTL8366RB_PHY_ACCESS_CTRL_REG,
-						RTL8366RB_PHY_CTRL_WRITE);
-				if (ret)
-					return ret;
-			}
-		}
-		dev_dbg(smi->dev, "jam %04x into register %04x\n",
-			jam_table[i + 1],
-			jam_table[i]);
-		ret = regmap_write(smi->map,
-				   jam_table[i],
-				   jam_table[i + 1]);
-		if (ret)
-			return ret;
-		i += 2;
-	}
+	ret = rtl8366rb_jam_table(jam_table, jam_size, smi, true);
+	if (ret)
+		return ret;
 
 	/* Set up the "green ethernet" feature */
-	i = 0;
-	while (i < ARRAY_SIZE(rtl8366rb_green_jam)) {
-		ret = regmap_read(smi->map, RTL8366RB_PHY_ACCESS_BUSY_REG,
-				  &val);
-		if (ret)
-			return ret;
-		if (!(val & RTL8366RB_PHY_INT_BUSY)) {
-			ret = regmap_write(smi->map,
-					   RTL8366RB_PHY_ACCESS_CTRL_REG,
-					   RTL8366RB_PHY_CTRL_WRITE);
-			if (ret)
-				return ret;
-			ret = regmap_write(smi->map,
-					   rtl8366rb_green_jam[i][0],
-					   rtl8366rb_green_jam[i][1]);
-			if (ret)
-				return ret;
-			i++;
-		}
-	}
+	ret = rtl8366rb_jam_table(rtl8366rb_green_jam,
+				  ARRAY_SIZE(rtl8366rb_green_jam), smi, false);
+	if (ret)
+		return ret;
+
 	ret = regmap_write(smi->map,
 			   RTL8366RB_GREEN_FEATURE_REG,
 			   (chip_ver == 1) ? 0x0007 : 0x0003);
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread
* Re: [PATCH 1/1] net: dsa: rtl8366rb: standardize init jam tables
  2021-01-25  4:56 ` [PATCH 1/1] net: dsa: rtl8366rb: standardize init " Lorenzo Carletti
@ 2021-01-26 21:08   ` Vladimir Oltean
  2021-01-26 21:21     ` Vladimir Oltean
                       ` (2 more replies)
  2021-01-26 21:41   ` Linus Walleij
  2021-01-28 16:07   ` Andy Shevchenko
  2 siblings, 3 replies; 10+ messages in thread
From: Vladimir Oltean @ 2021-01-26 21:08 UTC (permalink / raw)
  To: Lorenzo Carletti
  Cc: linus.walleij, andrew, vivien.didelot, f.fainelli, davem, kuba,
	netdev, linux-kernel

Hi Lorenzo,

On Mon, Jan 25, 2021 at 05:56:31AM +0100, Lorenzo Carletti wrote:
> In the rtl8366rb driver there are some jam tables which contain
> undocumented values.
> While trying to understand what these tables actually do,
> I noticed a discrepancy in how one of those was treated.

And did you manage to find out what these tables actually do?

> Most of them were plain u16 arrays, while the ethernet one was
> an u16 matrix.
> By looking at the vendor's droplets of source code these tables came from,
> I found out that they were all originally u16 matrixes.
> 
> This commit standardizes the jam tables, turning them all into
> u16 matrixes.

Why? What difference does it make?

> This change makes it easier to understand how the jam tables are used

No it doesn't?

> and also makes it possible for a single function to handle all of them,
> removing some duplicated code.

On which RTL8366RB chip revisions did you test for regressions?

On another note, the patch doesn't apply cleanly to net-next/master.

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH 1/1] net: dsa: rtl8366rb: standardize init jam tables
  2021-01-26 21:08   ` Vladimir Oltean
@ 2021-01-26 21:21     ` Vladimir Oltean
  2021-01-26 21:38     ` Vladimir Oltean
  2021-01-26 21:40     ` Linus Walleij
  2 siblings, 0 replies; 10+ messages in thread
From: Vladimir Oltean @ 2021-01-26 21:21 UTC (permalink / raw)
  To: Lorenzo Carletti
  Cc: linus.walleij, andrew, vivien.didelot, f.fainelli, davem, kuba,
	netdev, linux-kernel

On Tue, Jan 26, 2021 at 11:08:37PM +0200, Vladimir Oltean wrote:
> On another note, the patch doesn't apply cleanly to net-next/master.

Sorry, it does, I should learn how to apply a patch some day.

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH 1/1] net: dsa: rtl8366rb: standardize init jam tables
  2021-01-26 21:08   ` Vladimir Oltean
  2021-01-26 21:21     ` Vladimir Oltean
@ 2021-01-26 21:38     ` Vladimir Oltean
       [not found]       ` <CABRCJOSzm6s3hv17KFXMZigJjuBEidLLAM8+dqrGk9xTE=FkcQ@mail.gmail.com>
  2021-01-26 21:40     ` Linus Walleij
  2 siblings, 1 reply; 10+ messages in thread
From: Vladimir Oltean @ 2021-01-26 21:38 UTC (permalink / raw)
  To: Lorenzo Carletti
  Cc: linus.walleij, andrew, vivien.didelot, f.fainelli, davem, kuba,
	netdev, linux-kernel

On Tue, Jan 26, 2021 at 11:08:37PM +0200, Vladimir Oltean wrote:
> > and also makes it possible for a single function to handle all of them,
> > removing some duplicated code.

But at least I'll give you that, it is pretty straightforward refactoring.
The register jamming routine for green Ethernet did not check for the
address to start with 0xBE00 before checking RTL8366RB_PHY_ACCESS_BUSY_REG,
because apparently that's pointless, since all register addresses start
with 0xBE00 for the green Ethernet register jamming table. So no
functional changes should be introduced by the patch, and no harm in
checking for those high address bits in order to reuse a common function.

Reviewed-by: Vladimir Oltean <olteanv@gmail.com>

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH 1/1] net: dsa: rtl8366rb: standardize init jam tables
  2021-01-26 21:08   ` Vladimir Oltean
  2021-01-26 21:21     ` Vladimir Oltean
  2021-01-26 21:38     ` Vladimir Oltean
@ 2021-01-26 21:40     ` Linus Walleij
  2 siblings, 0 replies; 10+ messages in thread
From: Linus Walleij @ 2021-01-26 21:40 UTC (permalink / raw)
  To: Vladimir Oltean
  Cc: Lorenzo Carletti, Andrew Lunn, Vivien Didelot, Florian Fainelli,
	David S. Miller, Jakub Kicinski, netdev, linux-kernel

On Tue, Jan 26, 2021 at 10:08 PM Vladimir Oltean <olteanv@gmail.com> wrote:
> On Mon, Jan 25, 2021 at 05:56:31AM +0100, Lorenzo Carletti wrote:

> > In the rtl8366rb driver there are some jam tables which contain
> > undocumented values.
> > While trying to understand what these tables actually do,
> > I noticed a discrepancy in how one of those was treated.
>
> And did you manage to find out what these tables actually do?

I think Lorenzo mentioned that he found some settings in there,
I don't know if it was anything substantial though?

I put Lorenzon on track to investigate the driver, we thought
it could be an 8051 CPU so that some of the arrays could
be decoded into 8051 instructions, but so far we didn't get
anywhere with it.

The background was some mumble on the internet on
8051 in RTL8366 switches:
https://news.ycombinator.com/item?id=21040488
https://web.archive.org/web/20190922094616if_/https://twitter.com/whitequark/status/1175701730819895296

> > Most of them were plain u16 arrays, while the ethernet one was
> > an u16 matrix.
> > By looking at the vendor's droplets of source code these tables came from,
> > I found out that they were all originally u16 matrixes.
> >
> > This commit standardizes the jam tables, turning them all into
> > u16 matrixes.
>
> Why? What difference does it make?

I think it's nice that the format is the same on all tables.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH 1/1] net: dsa: rtl8366rb: standardize init jam tables
  2021-01-25  4:56 ` [PATCH 1/1] net: dsa: rtl8366rb: standardize init " Lorenzo Carletti
  2021-01-26 21:08   ` Vladimir Oltean
@ 2021-01-26 21:41   ` Linus Walleij
  2021-01-28 16:07   ` Andy Shevchenko
  2 siblings, 0 replies; 10+ messages in thread
From: Linus Walleij @ 2021-01-26 21:41 UTC (permalink / raw)
  To: Lorenzo Carletti
  Cc: Andrew Lunn, Vivien Didelot, Florian Fainelli, Vladimir Oltean,
	David S. Miller, Jakub Kicinski, netdev, linux-kernel

On Mon, Jan 25, 2021 at 5:56 AM Lorenzo Carletti
<lorenzo.carletti98@gmail.com> wrote:

> In the rtl8366rb driver there are some jam tables which contain
> undocumented values.
> While trying to understand what these tables actually do,
> I noticed a discrepancy in how one of those was treated.
> Most of them were plain u16 arrays, while the ethernet one was
> an u16 matrix.
> By looking at the vendor's droplets of source code these tables came from,
> I found out that they were all originally u16 matrixes.
>
> This commit standardizes the jam tables, turning them all into
> u16 matrixes.
> This change makes it easier to understand how the jam tables are used
> and also makes it possible for a single function to handle all of them,
> removing some duplicated code.
>
> Signed-off-by: Lorenzo Carletti <lorenzo.carletti98@gmail.com>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH 1/1] net: dsa: rtl8366rb: standardize init jam tables
       [not found]       ` <CABRCJOSzm6s3hv17KFXMZigJjuBEidLLAM8+dqrGk9xTE=FkcQ@mail.gmail.com>
@ 2021-01-26 22:28         ` Vladimir Oltean
  2021-01-26 22:37           ` Vladimir Oltean
  0 siblings, 1 reply; 10+ messages in thread
From: Vladimir Oltean @ 2021-01-26 22:28 UTC (permalink / raw)
  To: Lorenzo Carletti
  Cc: Linus Walleij, andrew, vivien.didelot, f.fainelli, davem, kuba,
	netdev, linux-kernel

On Tue, Jan 26, 2021 at 11:15:33PM +0100, Lorenzo Carletti wrote:
> > And did you manage to find out what these tables actually do?
>
> I was unable to do so. I was looking for Intel 8051 instructions in them:
> I created a small piece of code that generates an hypotetical
> registers space in which the tables are then jammed, but I didn't
> find anything.
> It's clear that some of the values of the tables are configuration
> parameters for stuff like the bandwidth, but that's the extent of what
> I was able to understand... So not that much.
>
> > Why? What difference does it make?
>
> So, allow me to explain. The kernel jams every "i + 1" value in the array
> tables into the registers at " i", and then increments "i" by 2.
> These can be seen as [n][2] matrixes, just like the ethernet one.
> Having the arrays converted to matrixes can help visualize which
> value is jammed where, or at least that's how I feel like it is.
> I know it's not a big change...

Got it, thanks. It is better, in fact, once you get over that whole
0xBE00 thing...

> > On which RTL8366RB chip revisions did you test for regressions?
>
> I don't have any of the chips to test this. What I agreed on with
> Linus Walleji was to send the patch after making sure everything
> compiled properly and checkpatch was happy with what I produced.
> Once the patch was sent, he said he'd test it.
> I ran some simulations, but that's pretty much it. I know those
> are not enough, so I'm waiting as well.

It is probably a safe change as it is, if you ran some simulations and
the same values at the same register addresses are jammed before and
after, it should be fine. The code looks okay.

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH 1/1] net: dsa: rtl8366rb: standardize init jam tables
  2021-01-26 22:28         ` Vladimir Oltean
@ 2021-01-26 22:37           ` Vladimir Oltean
  0 siblings, 0 replies; 10+ messages in thread
From: Vladimir Oltean @ 2021-01-26 22:37 UTC (permalink / raw)
  To: Lorenzo Carletti
  Cc: Linus Walleij, andrew, vivien.didelot, f.fainelli, davem, kuba,
	netdev, linux-kernel

On Wed, Jan 27, 2021 at 12:28:05AM +0200, Vladimir Oltean wrote:
> > So, allow me to explain. The kernel jams every "i + 1" value in the array
> > tables into the registers at " i", and then increments "i" by 2.
> > These can be seen as [n][2] matrixes, just like the ethernet one.
> > Having the arrays converted to matrixes can help visualize which
> > value is jammed where, or at least that's how I feel like it is.
> > I know it's not a big change...
> 
> Got it, thanks. It is better, in fact, once you get over that whole
> 0xBE00 thing...

If you really want beautiful code, I guess you could create a structure
with two fields:

struct rtl8366rb_jam_table_entry {
	u16 addr;
	u16 val;
};

and then convert those ugly looking matrix definitions:
u16 (*jam_table)[2]
with:
struct rtl8366rb_jam_table_entry *jam_table

and this:
		ret = regmap_write(smi->map,
				   jam_table[i][0],
				   jam_table[i][1]);
with this:
		ret = regmap_write(smi->map,
				   jam_table[i].addr,
				   jam_table[i].val);

The memory footprint would be exactly the same, and the struct
initializers would look exactly the same as your current array
declarations.

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH 1/1] net: dsa: rtl8366rb: standardize init jam tables
  2021-01-25  4:56 ` [PATCH 1/1] net: dsa: rtl8366rb: standardize init " Lorenzo Carletti
  2021-01-26 21:08   ` Vladimir Oltean
  2021-01-26 21:41   ` Linus Walleij
@ 2021-01-28 16:07   ` Andy Shevchenko
  2 siblings, 0 replies; 10+ messages in thread
From: Andy Shevchenko @ 2021-01-28 16:07 UTC (permalink / raw)
  To: Lorenzo Carletti
  Cc: Linus Walleij, Andrew Lunn, vivien.didelot, Florian Fainelli,
	Vladimir Oltean, David S. Miller, Jakub Kicinski, netdev,
	Linux Kernel Mailing List

On Mon, Jan 25, 2021 at 7:00 AM Lorenzo Carletti
<lorenzo.carletti98@gmail.com> wrote:
>
> In the rtl8366rb driver there are some jam tables which contain
> undocumented values.
> While trying to understand what these tables actually do,
> I noticed a discrepancy in how one of those was treated.
> Most of them were plain u16 arrays, while the ethernet one was
> an u16 matrix.
> By looking at the vendor's droplets of source code these tables came from,
> I found out that they were all originally u16 matrixes.
>
> This commit standardizes the jam tables, turning them all into
> u16 matrixes.
> This change makes it easier to understand how the jam tables are used
> and also makes it possible for a single function to handle all of them,
> removing some duplicated code.

...

Since further replies removed code, I reply here for everybody with an
example of my thoughts against this cryptic data.

If you look into the below, for example, you may notice a few things.
 - it pokes different address regions (sounds like data section, text
section, etc.)
 - it has different meaning for some addresses (0xBE prefix)

+static const u16 rtl8366rb_init_jam_f5d8235[][2] = {
+       {0x0242, 0x02BF}, {0x0245, 0x02BF}, {0x0248, 0x02BF}, {0x024B, 0x02BF},
+       {0x024E, 0x02BF}, {0x0251, 0x02BF}, {0x0254, 0x0A3F}, {0x0256, 0x0A3F},
+       {0x0258, 0x0A3F}, {0x025A, 0x0A3F}, {0x025C, 0x0A3F}, {0x025E, 0x0A3F},

Sounds like we program some buffer lengths / limits (0x2c0, 0xa40 if
it rings any bell to anybody).

+       {0x0263, 0x007C}, {0x0100, 0x0004}, {0xBE5B, 0x3500}, {0x800E, 0x200F},

BE5B seems like "execute the routine at 0x3500 address".
Thus I think shuffling those pairs before 0xbe shouldn't give any
difference (but I have no hw to try).

+       {0xBE1D, 0x0F00}, {0x8001, 0x5011}, {0x800A, 0xA2F4}, {0x800B, 0x17A3},
+       {0xBE4B, 0x17A3}, {0xBE41, 0x5011}, {0xBE17, 0x2100}, {0x8000, 0x8304},
+       {0xBE40, 0x8304}, {0xBE4A, 0xA2F4}, {0x800C, 0xA8D5}, {0x8014, 0x5500},
+       {0x8015, 0x0004}, {0xBE4C, 0xA8D5}, {0xBE59, 0x0008}, {0xBE09, 0x0E00},
+       {0xBE36, 0x1036}, {0xBE37, 0x1036}, {0x800D, 0x00FF}, {0xBE4D, 0x00FF},

0x80 addresses are some kind of magic, like interrupt vector returns
or so. You may notice some 0xBE commands against the addresses that
are put into the 0x8000 address region.

 };

Overall it seems you have to discover a full firmware image to make
any assumptions about CPU ISA used there and address mapping.

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 10+ messages in thread
end of thread, other threads:[~2021-01-28 16:08 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-25  4:56 [PATCH 0/1] net: dsa: rtl8366rb: change type of jam tables Lorenzo Carletti
2021-01-25  4:56 ` [PATCH 1/1] net: dsa: rtl8366rb: standardize init " Lorenzo Carletti
2021-01-26 21:08   ` Vladimir Oltean
2021-01-26 21:21     ` Vladimir Oltean
2021-01-26 21:38     ` Vladimir Oltean
     [not found]       ` <CABRCJOSzm6s3hv17KFXMZigJjuBEidLLAM8+dqrGk9xTE=FkcQ@mail.gmail.com>
2021-01-26 22:28         ` Vladimir Oltean
2021-01-26 22:37           ` Vladimir Oltean
2021-01-26 21:40     ` Linus Walleij
2021-01-26 21:41   ` Linus Walleij
2021-01-28 16:07   ` Andy Shevchenko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).