* [bug report] net: airoha: Introduce ethernet support for EN7581 SoC
@ 2024-07-18 18:30 Dan Carpenter
2024-07-19 20:39 ` Lorenzo Bianconi
0 siblings, 1 reply; 3+ messages in thread
From: Dan Carpenter @ 2024-07-18 18:30 UTC (permalink / raw)
To: Lorenzo Bianconi; +Cc: linux-mediatek
[ These Smatch tests aren't published. They produce way too many false
positives for me to release them. - dan ]
Hello Lorenzo Bianconi,
Commit 23020f049327 ("net: airoha: Introduce ethernet support for
EN7581 SoC") from Jul 12, 2024 (linux-next), leads to the following
Smatch static checker warning:
drivers/net/ethernet/mediatek/airoha_eth.c:1183 airoha_fe_pse_ports_init()
warn: why is the last element skipped?
drivers/net/ethernet/mediatek/airoha_eth.c:1369 airoha_fe_init()
'0x60000 | 0x60000' has '0x60000' set on both sides
drivers/net/ethernet/mediatek/airoha_eth.c
1134 static void airoha_fe_pse_ports_init(struct airoha_eth *eth)
1135 {
1136 const u32 pse_port_num_queues[] = {
1137 [FE_PSE_PORT_CDM1] = 6,
1138 [FE_PSE_PORT_GDM1] = 6,
1139 [FE_PSE_PORT_GDM2] = 32,
1140 [FE_PSE_PORT_GDM3] = 6,
1141 [FE_PSE_PORT_PPE1] = 4,
1142 [FE_PSE_PORT_CDM2] = 6,
1143 [FE_PSE_PORT_CDM3] = 8,
^^^^^^^^^^^^^^^^^^^^^^^
1144 [FE_PSE_PORT_CDM4] = 10,
1145 [FE_PSE_PORT_PPE2] = 4,
1146 [FE_PSE_PORT_GDM4] = 2,
1147 [FE_PSE_PORT_CDM5] = 2,
1148 };
1149 int q;
1150
1151 /* hw misses PPE2 oq rsv */
1152 airoha_fe_set(eth, REG_FE_PSE_BUF_SET,
1153 PSE_RSV_PAGES * pse_port_num_queues[FE_PSE_PORT_PPE2]);
1154
1155 /* CMD1 */
1156 for (q = 0; q < pse_port_num_queues[FE_PSE_PORT_CDM1]; q++)
1157 airoha_fe_set_pse_oq_rsv(eth, FE_PSE_PORT_CDM1, q,
1158 PSE_QUEUE_RSV_PAGES);
1159 /* GMD1 */
1160 for (q = 0; q < pse_port_num_queues[FE_PSE_PORT_GDM1]; q++)
1161 airoha_fe_set_pse_oq_rsv(eth, FE_PSE_PORT_GDM1, q,
1162 PSE_QUEUE_RSV_PAGES);
1163 /* GMD2 */
1164 for (q = 6; q < pse_port_num_queues[FE_PSE_PORT_GDM2]; q++)
1165 airoha_fe_set_pse_oq_rsv(eth, FE_PSE_PORT_GDM2, q, 0);
1166 /* GMD3 */
1167 for (q = 0; q < pse_port_num_queues[FE_PSE_PORT_GDM3]; q++)
1168 airoha_fe_set_pse_oq_rsv(eth, FE_PSE_PORT_GDM3, q,
1169 PSE_QUEUE_RSV_PAGES);
1170 /* PPE1 */
1171 for (q = 0; q < pse_port_num_queues[FE_PSE_PORT_PPE1]; q++) {
1172 if (q < pse_port_num_queues[FE_PSE_PORT_PPE1])
1173 airoha_fe_set_pse_oq_rsv(eth, FE_PSE_PORT_PPE1, q,
1174 PSE_QUEUE_RSV_PAGES);
1175 else
1176 airoha_fe_set_pse_oq_rsv(eth, FE_PSE_PORT_PPE1, q, 0);
1177 }
1178 /* CDM2 */
1179 for (q = 0; q < pse_port_num_queues[FE_PSE_PORT_CDM2]; q++)
1180 airoha_fe_set_pse_oq_rsv(eth, FE_PSE_PORT_CDM2, q,
1181 PSE_QUEUE_RSV_PAGES);
1182 /* CDM3 */
--> 1183 for (q = 0; q < pse_port_num_queues[FE_PSE_PORT_CDM3] - 1; q++)
^^^^
This - 1 doesn't look intentional. It's the only time this value is used.
1184 airoha_fe_set_pse_oq_rsv(eth, FE_PSE_PORT_CDM3, q, 0);
1185 /* CDM4 */
1186 for (q = 4; q < pse_port_num_queues[FE_PSE_PORT_CDM4]; q++)
1187 airoha_fe_set_pse_oq_rsv(eth, FE_PSE_PORT_CDM4, q,
1188 PSE_QUEUE_RSV_PAGES);
1189 /* PPE2 */
1190 for (q = 0; q < pse_port_num_queues[FE_PSE_PORT_PPE2]; q++) {
1191 if (q < pse_port_num_queues[FE_PSE_PORT_PPE2] / 2)
1192 airoha_fe_set_pse_oq_rsv(eth, FE_PSE_PORT_PPE2, q,
1193 PSE_QUEUE_RSV_PAGES);
1194 else
1195 airoha_fe_set_pse_oq_rsv(eth, FE_PSE_PORT_PPE2, q, 0);
1196 }
1197 /* GMD4 */
1198 for (q = 0; q < pse_port_num_queues[FE_PSE_PORT_GDM4]; q++)
1199 airoha_fe_set_pse_oq_rsv(eth, FE_PSE_PORT_GDM4, q,
1200 PSE_QUEUE_RSV_PAGES);
1201 /* CDM5 */
1202 for (q = 0; q < pse_port_num_queues[FE_PSE_PORT_CDM5]; q++)
1203 airoha_fe_set_pse_oq_rsv(eth, FE_PSE_PORT_CDM5, q,
1204 PSE_QUEUE_RSV_PAGES);
1205 }
drivers/net/ethernet/mediatek/airoha_eth.c
1366 airoha_fe_set(eth, REG_FE_CPORT_CFG, FE_CPORT_PORT_XFC_MASK);
1367
1368 /* default aging mode for mbi unlock issue */
1369 airoha_fe_rmw(eth, REG_GDM2_CHN_RLS,
1370 MBI_RX_AGE_SEL_MASK | MBI_TX_AGE_SEL_MASK,
The RX and TX masks are identical. Possibly correct but #Suspicious.
1371 FIELD_PREP(MBI_RX_AGE_SEL_MASK, 3) |
1372 FIELD_PREP(MBI_TX_AGE_SEL_MASK, 3));
1373
1374 /* disable IFC by default */
regards,
dan carpenter
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [bug report] net: airoha: Introduce ethernet support for EN7581 SoC
2024-07-18 18:30 [bug report] net: airoha: Introduce ethernet support for EN7581 SoC Dan Carpenter
@ 2024-07-19 20:39 ` Lorenzo Bianconi
2024-07-19 22:17 ` Dan Carpenter
0 siblings, 1 reply; 3+ messages in thread
From: Lorenzo Bianconi @ 2024-07-19 20:39 UTC (permalink / raw)
To: Dan Carpenter; +Cc: linux-mediatek
[-- Attachment #1: Type: text/plain, Size: 5287 bytes --]
[...]
> drivers/net/ethernet/mediatek/airoha_eth.c
> 1134 static void airoha_fe_pse_ports_init(struct airoha_eth *eth)
> 1135 {
> 1136 const u32 pse_port_num_queues[] = {
> 1137 [FE_PSE_PORT_CDM1] = 6,
> 1138 [FE_PSE_PORT_GDM1] = 6,
> 1139 [FE_PSE_PORT_GDM2] = 32,
> 1140 [FE_PSE_PORT_GDM3] = 6,
> 1141 [FE_PSE_PORT_PPE1] = 4,
> 1142 [FE_PSE_PORT_CDM2] = 6,
> 1143 [FE_PSE_PORT_CDM3] = 8,
> ^^^^^^^^^^^^^^^^^^^^^^^
>
> 1144 [FE_PSE_PORT_CDM4] = 10,
> 1145 [FE_PSE_PORT_PPE2] = 4,
> 1146 [FE_PSE_PORT_GDM4] = 2,
> 1147 [FE_PSE_PORT_CDM5] = 2,
> 1148 };
> 1149 int q;
> 1150
> 1151 /* hw misses PPE2 oq rsv */
> 1152 airoha_fe_set(eth, REG_FE_PSE_BUF_SET,
> 1153 PSE_RSV_PAGES * pse_port_num_queues[FE_PSE_PORT_PPE2]);
> 1154
> 1155 /* CMD1 */
> 1156 for (q = 0; q < pse_port_num_queues[FE_PSE_PORT_CDM1]; q++)
> 1157 airoha_fe_set_pse_oq_rsv(eth, FE_PSE_PORT_CDM1, q,
> 1158 PSE_QUEUE_RSV_PAGES);
> 1159 /* GMD1 */
> 1160 for (q = 0; q < pse_port_num_queues[FE_PSE_PORT_GDM1]; q++)
> 1161 airoha_fe_set_pse_oq_rsv(eth, FE_PSE_PORT_GDM1, q,
> 1162 PSE_QUEUE_RSV_PAGES);
> 1163 /* GMD2 */
> 1164 for (q = 6; q < pse_port_num_queues[FE_PSE_PORT_GDM2]; q++)
> 1165 airoha_fe_set_pse_oq_rsv(eth, FE_PSE_PORT_GDM2, q, 0);
> 1166 /* GMD3 */
> 1167 for (q = 0; q < pse_port_num_queues[FE_PSE_PORT_GDM3]; q++)
> 1168 airoha_fe_set_pse_oq_rsv(eth, FE_PSE_PORT_GDM3, q,
> 1169 PSE_QUEUE_RSV_PAGES);
> 1170 /* PPE1 */
> 1171 for (q = 0; q < pse_port_num_queues[FE_PSE_PORT_PPE1]; q++) {
> 1172 if (q < pse_port_num_queues[FE_PSE_PORT_PPE1])
> 1173 airoha_fe_set_pse_oq_rsv(eth, FE_PSE_PORT_PPE1, q,
> 1174 PSE_QUEUE_RSV_PAGES);
> 1175 else
> 1176 airoha_fe_set_pse_oq_rsv(eth, FE_PSE_PORT_PPE1, q, 0);
> 1177 }
> 1178 /* CDM2 */
> 1179 for (q = 0; q < pse_port_num_queues[FE_PSE_PORT_CDM2]; q++)
> 1180 airoha_fe_set_pse_oq_rsv(eth, FE_PSE_PORT_CDM2, q,
> 1181 PSE_QUEUE_RSV_PAGES);
> 1182 /* CDM3 */
> --> 1183 for (q = 0; q < pse_port_num_queues[FE_PSE_PORT_CDM3] - 1; q++)
> ^^^^
> This - 1 doesn't look intentional. It's the only time this value is used.
I checked the vendor sdk and this is fine.
>
> 1184 airoha_fe_set_pse_oq_rsv(eth, FE_PSE_PORT_CDM3, q, 0);
> 1185 /* CDM4 */
> 1186 for (q = 4; q < pse_port_num_queues[FE_PSE_PORT_CDM4]; q++)
> 1187 airoha_fe_set_pse_oq_rsv(eth, FE_PSE_PORT_CDM4, q,
> 1188 PSE_QUEUE_RSV_PAGES);
> 1189 /* PPE2 */
> 1190 for (q = 0; q < pse_port_num_queues[FE_PSE_PORT_PPE2]; q++) {
> 1191 if (q < pse_port_num_queues[FE_PSE_PORT_PPE2] / 2)
> 1192 airoha_fe_set_pse_oq_rsv(eth, FE_PSE_PORT_PPE2, q,
> 1193 PSE_QUEUE_RSV_PAGES);
> 1194 else
> 1195 airoha_fe_set_pse_oq_rsv(eth, FE_PSE_PORT_PPE2, q, 0);
> 1196 }
> 1197 /* GMD4 */
> 1198 for (q = 0; q < pse_port_num_queues[FE_PSE_PORT_GDM4]; q++)
> 1199 airoha_fe_set_pse_oq_rsv(eth, FE_PSE_PORT_GDM4, q,
> 1200 PSE_QUEUE_RSV_PAGES);
> 1201 /* CDM5 */
> 1202 for (q = 0; q < pse_port_num_queues[FE_PSE_PORT_CDM5]; q++)
> 1203 airoha_fe_set_pse_oq_rsv(eth, FE_PSE_PORT_CDM5, q,
> 1204 PSE_QUEUE_RSV_PAGES);
> 1205 }
>
> drivers/net/ethernet/mediatek/airoha_eth.c
> 1366 airoha_fe_set(eth, REG_FE_CPORT_CFG, FE_CPORT_PORT_XFC_MASK);
> 1367
> 1368 /* default aging mode for mbi unlock issue */
> 1369 airoha_fe_rmw(eth, REG_GDM2_CHN_RLS,
> 1370 MBI_RX_AGE_SEL_MASK | MBI_TX_AGE_SEL_MASK,
>
> The RX and TX masks are identical. Possibly correct but #Suspicious.
>
> 1371 FIELD_PREP(MBI_RX_AGE_SEL_MASK, 3) |
> 1372 FIELD_PREP(MBI_TX_AGE_SEL_MASK, 3));
I posted a fix for it.
Regards,
Lorenzo
> 1373
> 1374 /* disable IFC by default */
>
>
> regards,
> dan carpenter
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [bug report] net: airoha: Introduce ethernet support for EN7581 SoC
2024-07-19 20:39 ` Lorenzo Bianconi
@ 2024-07-19 22:17 ` Dan Carpenter
0 siblings, 0 replies; 3+ messages in thread
From: Dan Carpenter @ 2024-07-19 22:17 UTC (permalink / raw)
To: Lorenzo Bianconi; +Cc: linux-mediatek
On Fri, Jul 19, 2024 at 10:39:58PM +0200, Lorenzo Bianconi wrote:
> [...]
> > drivers/net/ethernet/mediatek/airoha_eth.c
> > 1134 static void airoha_fe_pse_ports_init(struct airoha_eth *eth)
> > 1135 {
> > 1136 const u32 pse_port_num_queues[] = {
> > 1137 [FE_PSE_PORT_CDM1] = 6,
> > 1138 [FE_PSE_PORT_GDM1] = 6,
> > 1139 [FE_PSE_PORT_GDM2] = 32,
> > 1140 [FE_PSE_PORT_GDM3] = 6,
> > 1141 [FE_PSE_PORT_PPE1] = 4,
> > 1142 [FE_PSE_PORT_CDM2] = 6,
> > 1143 [FE_PSE_PORT_CDM3] = 8,
> > ^^^^^^^^^^^^^^^^^^^^^^^
> >
> > 1144 [FE_PSE_PORT_CDM4] = 10,
> > 1145 [FE_PSE_PORT_PPE2] = 4,
> > 1146 [FE_PSE_PORT_GDM4] = 2,
> > 1147 [FE_PSE_PORT_CDM5] = 2,
> > 1148 };
> > 1149 int q;
> > 1150
> > 1151 /* hw misses PPE2 oq rsv */
> > 1152 airoha_fe_set(eth, REG_FE_PSE_BUF_SET,
> > 1153 PSE_RSV_PAGES * pse_port_num_queues[FE_PSE_PORT_PPE2]);
> > 1154
> > 1155 /* CMD1 */
> > 1156 for (q = 0; q < pse_port_num_queues[FE_PSE_PORT_CDM1]; q++)
> > 1157 airoha_fe_set_pse_oq_rsv(eth, FE_PSE_PORT_CDM1, q,
> > 1158 PSE_QUEUE_RSV_PAGES);
> > 1159 /* GMD1 */
> > 1160 for (q = 0; q < pse_port_num_queues[FE_PSE_PORT_GDM1]; q++)
> > 1161 airoha_fe_set_pse_oq_rsv(eth, FE_PSE_PORT_GDM1, q,
> > 1162 PSE_QUEUE_RSV_PAGES);
> > 1163 /* GMD2 */
> > 1164 for (q = 6; q < pse_port_num_queues[FE_PSE_PORT_GDM2]; q++)
> > 1165 airoha_fe_set_pse_oq_rsv(eth, FE_PSE_PORT_GDM2, q, 0);
> > 1166 /* GMD3 */
> > 1167 for (q = 0; q < pse_port_num_queues[FE_PSE_PORT_GDM3]; q++)
> > 1168 airoha_fe_set_pse_oq_rsv(eth, FE_PSE_PORT_GDM3, q,
> > 1169 PSE_QUEUE_RSV_PAGES);
> > 1170 /* PPE1 */
> > 1171 for (q = 0; q < pse_port_num_queues[FE_PSE_PORT_PPE1]; q++) {
> > 1172 if (q < pse_port_num_queues[FE_PSE_PORT_PPE1])
> > 1173 airoha_fe_set_pse_oq_rsv(eth, FE_PSE_PORT_PPE1, q,
> > 1174 PSE_QUEUE_RSV_PAGES);
> > 1175 else
> > 1176 airoha_fe_set_pse_oq_rsv(eth, FE_PSE_PORT_PPE1, q, 0);
> > 1177 }
> > 1178 /* CDM2 */
> > 1179 for (q = 0; q < pse_port_num_queues[FE_PSE_PORT_CDM2]; q++)
> > 1180 airoha_fe_set_pse_oq_rsv(eth, FE_PSE_PORT_CDM2, q,
> > 1181 PSE_QUEUE_RSV_PAGES);
> > 1182 /* CDM3 */
> > --> 1183 for (q = 0; q < pse_port_num_queues[FE_PSE_PORT_CDM3] - 1; q++)
> > ^^^^
> > This - 1 doesn't look intentional. It's the only time this value is used.
>
> I checked the vendor sdk and this is fine.
>
If there are 7 queues, just change [FE_PSE_PORT_CDM3] = 7. If there are
8 queues but the last one is not used, then add a comment. /* The - 1 is
because we are reserving the last queue for secret reasons. Shhhh... */
That way people will know it's intentional.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-07-19 22:17 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-07-18 18:30 [bug report] net: airoha: Introduce ethernet support for EN7581 SoC Dan Carpenter
2024-07-19 20:39 ` Lorenzo Bianconi
2024-07-19 22:17 ` Dan Carpenter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox