Linux-mediatek Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Lorenzo Bianconi <lorenzo@kernel.org>
To: Dan Carpenter <dan.carpenter@linaro.org>
Cc: linux-mediatek@lists.infradead.org
Subject: Re: [bug report] net: airoha: Introduce ethernet support for EN7581 SoC
Date: Fri, 19 Jul 2024 22:39:58 +0200	[thread overview]
Message-ID: <ZprPHlFgp1G2iTkM@lore-desk> (raw)
In-Reply-To: <395b364b-8a37-40ce-ac24-4f0bd7eb0e2a@stanley.mountain>

[-- 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 --]

  reply	other threads:[~2024-07-19 20:40 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-07-18 18:30 [bug report] net: airoha: Introduce ethernet support for EN7581 SoC Dan Carpenter
2024-07-19 20:39 ` Lorenzo Bianconi [this message]
2024-07-19 22:17   ` Dan Carpenter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZprPHlFgp1G2iTkM@lore-desk \
    --to=lorenzo@kernel.org \
    --cc=dan.carpenter@linaro.org \
    --cc=linux-mediatek@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox