On Thu, Nov 14, 2024 at 02:47:07PM +0200, Roger Quadros wrote: > > > 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. Yes, very nice. > >> 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 >
