Hi Andrew,
Andrew Lunn <and...@lunn.ch> writes:
> #define PORT_TAG_REGMAP_0123 0x18
> #define PORT_TAG_REGMAP_4567 0x19
> +#define PORT_PRIO_MAP_TABLE 0x18 /* 6390 */
> +#define PORT_PRIO_MAP_TABLE_UPDATE BIT(15)
> +#define PORT_PRIO_MAP_TABLE_INGRESS_PCP (0x0 << 12)
> +#define PORT_PRIO_MAP_TABLE_EGRESS_GREEN_PCP (0x1 << 12)
> +#define PORT_PRIO_MAP_TABLE_EGRESS_YELLOW_PCP (0x2 << 12)
> +#define PORT_PRIO_MAP_TABLE_EGRESS_AVB_PCP (0x3 << 12)
> +#define PORT_PRIO_MAP_TABLE_EGRESS_GREEN_DSCP (0x5 << 12)
> +#define PORT_PRIO_MAP_TABLE_EGRESS_YELLOW_DSCP (0x6 << 12)
> +#define PORT_PRIO_MAP_TABLE_EGRESS_AVB_DSCP (0x7 << 12)
0x17 is the "IP Priority Mapping Table" register, so I'd define 0x18 as
PORT_IEEE_PRIO_MAP_TABLE to avoid later confusion.
>
> #define GLOBAL_STATUS 0x00
> #define GLOBAL_STATUS_PPU_STATE BIT(15) /* 6351 and 6171 */
> @@ -813,6 +822,7 @@ struct mv88e6xxx_ops {
> void (*stats_get_strings)(struct mv88e6xxx_chip *chip, uint8_t *data);
> void (*stats_get_stats)(struct mv88e6xxx_chip *chip, int port,
> uint64_t *data);
> + int (*tag_remap)(struct mv88e6xxx_chip *chip, int port);
I would've prefered an op like .tag_remap(*chip, port, prio, new) and a
wrapper in chip.c which loops over priority 0-7, but that would make the
implementation unnecessarily complex, so let's keep it as is for now ;)
> };
>
> #define STATS_TYPE_PORT BIT(0)
> diff --git a/drivers/net/dsa/mv88e6xxx/port.c
> b/drivers/net/dsa/mv88e6xxx/port.c
> index af4772d86086..b7fab70f6cd7 100644
> --- a/drivers/net/dsa/mv88e6xxx/port.c
> +++ b/drivers/net/dsa/mv88e6xxx/port.c
> @@ -496,3 +496,60 @@ int mv88e6xxx_port_set_8021q_mode(struct mv88e6xxx_chip
> *chip, int port,
>
> return 0;
> }
Please add an ordered comment:
/* Offset 0x18: Port IEEE Priority Remapping Registers [0-3]
* Offset 0x19: Port IEEE Priority Remapping Registers [4-7]
*/
> +
> +int mv88e6095_tag_remap(struct mv88e6xxx_chip *chip, int port)
> +{
> + int err;
> +
> + /* Tag Remap: use an identity 802.1p prio -> switch prio
> + * mapping.
> + */
> + err = mv88e6xxx_port_write(chip, port, PORT_TAG_REGMAP_0123, 0x3210);
> + if (err)
> + return err;
> +
> + /* Tag Remap 2: use an identity 802.1p prio -> switch
> + * prio mapping.
> + */
A single comment like this before the 2 writes will be enough:
/* Use a direct priority mapping for all IEEE tagged frames */
> + return mv88e6xxx_port_write(chip, port, PORT_TAG_REGMAP_4567, 0x7654);
> +}
Functions of port.c implementing Port Registers features must be
prefixed mv88e6xxx_port_* (6xxx can be a model in case of conflict).
mv88e6xxx_port_tag_remap() seems fine.
> +
> +int mv88e6390_tag_remap(struct mv88e6xxx_chip *chip, int port)
> +{
> + int err, reg, i;
> +
> + for (i = 0; i <= 7; i++) {
> + reg = i | (i << 4) |
The pointer offset is 9, not 4.
> + PORT_PRIO_MAP_TABLE_INGRESS_PCP |
> + PORT_PRIO_MAP_TABLE_UPDATE;
> + err = mv88e6xxx_port_write(chip, port, PORT_PRIO_MAP_TABLE,
> + reg);
> + if (err)
> + return err;
> +
> + reg = i | PORT_PRIO_MAP_TABLE_EGRESS_GREEN_PCP |
> + PORT_PRIO_MAP_TABLE_UPDATE;
> + err = mv88e6xxx_port_write(chip, port, PORT_PRIO_MAP_TABLE,
> + reg);
> + if (err)
> + return err;
> +
> + reg = i |
> + PORT_PRIO_MAP_TABLE_EGRESS_YELLOW_PCP |
> + PORT_PRIO_MAP_TABLE_UPDATE;
> + err = mv88e6xxx_port_write(chip, port, PORT_PRIO_MAP_TABLE,
> + reg);
> + if (err)
> + return err;
> +
> + reg = i |
> + PORT_PRIO_MAP_TABLE_EGRESS_AVB_PCP |
> + PORT_PRIO_MAP_TABLE_UPDATE;
> + err = mv88e6xxx_port_write(chip, port, PORT_PRIO_MAP_TABLE,
> + reg);
> + if (err)
> + return err;
> + }
> +
> + return 0;
> +}
Please add a static helper first to write the table, e.g.
/* Offset 0x18: Port IEEE Priority Mapping Table (88E6190) */
static int mv88e6xxx_port_ieeepmt_write(struct mv88e6xxx_chip *chip,
int port, u8 table,
u8 pointer, u16 data)
And then provide
int mv88e6xxx_port_ieeepmt_tag_remap(...)
Thanks,
Vivien
