On 14/11/2024 14:02, Guillaume Nault wrote: > On Thu, Nov 14, 2024 at 12:12:47PM +0200, Roger Quadros wrote: >> On 14/11/2024 11:41, Roger Quadros wrote: >>> On 14/11/2024 02:16, Guillaume Nault wrote: >>>> So what about following the IETF mapping found in section 4.3? >>>> https://datatracker.ietf.org/doc/html/rfc8325#section-4.3 >>> >>> Thanks for this tip. >>> I will update this patch to have the default DSCP to UP mapping as per >>> above link and map all unused DSCP to UP 0. >> >> How does the below code look in this regard? > > Looks generally good to me. A few comments inline though. > >> static void am65_cpsw_port_enable_dscp_map(struct am65_cpsw_port *slave) >> { >> int dscp, pri; >> u32 val; >> >> /* Default DSCP to User Priority mapping as per: >> * https://datatracker.ietf.org/doc/html/rfc8325#section-4.3 > > Maybe also add a link to > https://datatracker.ietf.org/doc/html/rfc8622#section-11 > which defines the LE PHB (Low Effort) and updates RFC 8325 accordingly. > >> */ >> for (dscp = 0; dscp <= AM65_CPSW_DSCP_MAX; dscp++) { >> switch (dscp) { >> case 56: /* CS7 */ >> case 48: /* CS6 */ >> pri = 7; >> break; >> case 46: /* EF */ >> case 44: /* VA */ >> pri = 6; >> break; >> case 40: /* CS5 */ >> pri = 5; >> break; >> case 32: /* CS4 */ >> case 34: /* AF41 */ >> case 36: /* AF42 */ >> case 38: /* AF43 */ >> case 24: /* CS3 */ >> case 26: /* AF31 */ >> case 28: /* AF32 */ >> case 30: /* AF33 */ > > Until case 32 (CS4) you've kept the order of RFC 8325, table 1. > It'd make life easier for reviewers if you could keep this order > here. That is, moving CS4 after AF43 and CS3 after AF33. > >> pri = 4; >> break; >> case 17: /* AF21 */ > > AF21 is 18, not 17. > >> case 20: /* AF22 */ >> case 22: /* AF23 */ >> pri = 3; >> break; >> case 8: /* CS1 */ > > Let's be complete and add the case for LE (RFC 8622), which also > maps to 1. All comments are valid. I will fix and send v4 for this series. > >> pri = 1; >> break; For sake of completeness I will mention CS2, AF11, AF12, AF13 here that can fallback to default case. >> default: >> pri = 0; >> break; >> } >> >> am65_cpsw_port_set_dscp_map(slave, dscp, pri); >> } >> >> /* enable port IPV4 and IPV6 DSCP for this port */ >> val = readl(slave->port_base + AM65_CPSW_PORTN_REG_CTL); >> val |= AM65_CPSW_PN_REG_CTL_DSCP_IPV4_EN | >> AM65_CPSW_PN_REG_CTL_DSCP_IPV6_EN; >> writel(val, slave->port_base + AM65_CPSW_PORTN_REG_CTL); >> } >> >>> -- cheers, -roger
