Re: [PATCH net-next v4 5/9] net: dsa: microchip: add support for different DCB app configurations

[<Arun.Ramadoss@microchip.com>]

Hi Oleksij,

> 
>  void ksz_port_stp_state_set(struct dsa_switch *ds, int port, u8
> state)
> @@ -3943,6 +3948,11 @@ static const struct dsa_switch_ops
> ksz_switch_ops = {
>         .port_setup_tc          = ksz_setup_tc,
>         .get_mac_eee            = ksz_get_mac_eee,
>         .set_mac_eee            = ksz_set_mac_eee,
> +       .port_get_default_prio  = ksz_port_get_default_prio,
> +       .port_set_default_prio  = ksz_port_set_default_prio,
> +       .port_get_dscp_prio     = ksz_port_get_dscp_prio,
> +       .port_set_apptrust      = ksz_port_set_apptrust,
> +       .port_get_apptrust      = ksz_port_get_apptrust,

nitpick: port_set_apptrust and port_get_apptrust can be swapped to have
ommon flow of get and set.
>  };
> 
> diff --git a/drivers/net/dsa/microchip/ksz_dcb.c
> b/drivers/net/dsa/microchip/ksz_dcb.c
> new file mode 100644
> index 0000000000000..46491c6dd6f60
> --- /dev/null
> +++ b/drivers/net/dsa/microchip/ksz_dcb.c
> @@ -0,0 +1,544 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright (c) 2024 Pengutronix, Oleksij Rempel <
> kernel@pengutronix.de>
> +
> +#include <linux/dsa/ksz_common.h>
> +#include <net/dsa.h>
> +#include <net/dscp.h>
> +#include <net/ieee8021q.h>
> +
> 
> +
> +static void ksz_get_defult_port_prio_reg(struct ksz_device *dev, int

s/defult/default

> *reg,
> +                                        u8 *mask, int *shift)
> +{
> +       if (is_ksz8(dev)) {
> +               *reg = KSZ8_REG_PORT_1_CTRL_0;
> +               *mask = KSZ8_PORT_BASED_PRIO_M;
> +               *shift = __bf_shf(KSZ8_PORT_BASED_PRIO_M);
> +       } else {
> +               *reg = KSZ9477_REG_PORT_MRI_MAC_CTRL;
> +               *mask = KSZ9477_PORT_BASED_PRIO_M;
> +               *shift = __bf_shf(KSZ9477_PORT_BASED_PRIO_M);
> +       }
> +}
> +
> +
> +int ksz_port_get_default_prio(struct dsa_switch *ds, int port)
> +{
> +       struct ksz_device *dev = ds->priv;
> +       int ret, reg, shift;
> +       u8 data, mask;
> +
> +       ksz_get_defult_port_prio_reg(dev, &reg, &mask, &shift);
> +
> +       ret = ksz_pread8(dev, port, reg, &data);
> +       if (ret)
> +               return ret;
> +
> +       return (data & mask) >> shift;

I assume we can use retrun FIELD_GET(mask, data), since mask is GENMASK
format. 

> 
> +int ksz_port_set_default_prio(struct dsa_switch *ds, int port, u8
> prio)
> prio)
> +{
> +       struct ksz_device *dev = ds->priv;
> +       int reg, shift;
> +       u8 mask;
> +
> +       if (prio >= dev->info->num_tx_queues)
> +               return -EINVAL;
> +
> +       ksz_get_defult_port_prio_reg(dev, &reg, &mask, &shift);
> +
> +       return ksz_prmw8(dev, port, reg, mask, (prio << shift) &
> mask);

FIELD_PREP(mask, prio)

> +}
> +
> +/**
> + * ksz_get_dscp_prio_reg - Retrieves the DSCP-to-priority-mapping
> register
> + * @dev: Pointer to the KSZ switch device structure
> + * @reg: Pointer to the register address to be set
> + * @per_reg: Pointer to the number of DSCP values per register
> + * @mask: Pointer to the mask to be set
> + *
> + * This function retrieves the DSCP to priority mapping register,
> the number of
> + * DSCP values per register, and the mask to be set.
> + */
> +static void ksz_get_dscp_prio_reg(struct ksz_device *dev, int *reg,
> +                                 int *per_reg, u8 *mask)
> +{
> +       if (ksz_is_ksz87xx(dev)) {
> +               *reg = KSZ8765_REG_TOS_DSCP_CTRL;
> +               *per_reg = 4;
> +               *mask = GENMASK(1, 0);
> +       } else if (ksz_is_ksz88x3(dev)) {
> +               *reg = KSZ88X3_REG_TOS_DSCP_CTRL;
> +               *per_reg = 4;
> +               *mask = GENMASK(1, 0);
> +       } else {
> +               *reg = KSZ9477_REG_DIFFSERV_PRIO_MAP;
> +               *per_reg = 2;
> +               *mask = GENMASK(2, 0);
> +       }
> +}

nitpick: to have common flow, you can group all the get register
function in top of file. 

> 
> +static int ksz_port_set_apptrust_validate(struct ksz_device *dev,
> int port,
> +                                         const u8 *sel, int nsel)
> +{
> +       int i, j, found;
> +       int j_prev = 0;
> +
> +       /* Iterate through the requested selectors */
> +       for (i = 0; i < nsel; i++) {
> +               found = 0;
> +
> +               /* Check if the current selector is supported by the
> hardware */
> +               for (j = 0; j < sizeof(ksz_supported_apptrust); j++)
> {
> +                       if (sel[i] != ksz_supported_apptrust[j])
> +                               continue;
> +
> +                       found = 1;
> +
> +                       /* Ensure that no higher priority selector
> (lower index)
> +                        * precedes a lower priority one
> +                        */
> +                       if (i > 0 && j <= j_prev)
> +                               goto invalid;
> +
> +                       j_prev = j;
> +                       break;
> +               }
> +
> +               if (!found)
> +                       goto invalid;

different label other than invalid can be used like err_

> +       }
> +
> +       return 0;
> +
> +invalid:
> +       ksz_apptrust_error(dev);
> +
> +       return -EINVAL;
> +}
> +
> +/**
> + * ksz_get_apptrus_map_and_reg - Retrieves the apptrust map and
> register
> + * @dev: Pointer to the KSZ switch device structure
> + * @map: Pointer to the apptrust map to be set
> + * @reg: Pointer to the register address to be set
> + * @mask: Pointer to the mask to be set
> + *
> + * This function retrieves the apptrust map and register address for
> the
> + * apptrust configuration.
> + */
> +static void ksz_get_apptrus_map_and_reg(struct ksz_device *dev,

s/apptrus_/apptrust_

> +                                       const struct ksz_apptrust_map
> **map,
> +                                       int *reg, u8 *mask)
> +{
> +       if (is_ksz8(dev)) {
> +               *map = ksz8_apptrust_map_to_bit;
> +               *reg = KSZ8_REG_PORT_1_CTRL_0;
> +               *mask = KSZ8_PORT_DIFFSERV_ENABLE |
> KSZ8_PORT_802_1P_ENABLE;
> +       } else {
> +               *map = ksz9477_apptrust_map_to_bit;
> +               *reg = KSZ9477_REG_PORT_MRI_PRIO_CTRL;
> +               *mask = KSZ9477_PORT_802_1P_PRIO_ENABLE |
> +                       KSZ9477_PORT_DIFFSERV_PRIO_ENABLE;
> +       }
> +}
> +
> +
>