Some switch may not support mib autocast feature and require the legacy
way of reading the regs directly.
Make the mib autocast feature optional and permit to declare support for
it using match_data struct in a dedicated qca8k_info_ops struct.
Signed-off-by: Christian Marangi <[email protected]>
---
drivers/net/dsa/qca/qca8k.c | 11 +++++++++--
drivers/net/dsa/qca/qca8k.h | 5 +++++
2 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/drivers/net/dsa/qca/qca8k.c b/drivers/net/dsa/qca/qca8k.c
index 212b284f9f73..9820c5942d2a 100644
--- a/drivers/net/dsa/qca/qca8k.c
+++ b/drivers/net/dsa/qca/qca8k.c
@@ -2104,8 +2104,8 @@ qca8k_get_ethtool_stats(struct dsa_switch *ds, int port,
u32 hi = 0;
int ret;
- if (priv->mgmt_master &&
- qca8k_get_ethtool_stats_eth(ds, port, data) > 0)
+ if (priv->mgmt_master && priv->info->ops.autocast_mib &&
+ priv->info->ops.autocast_mib(ds, port, data) > 0)
return;
for (i = 0; i < priv->info->mib_count; i++) {
@@ -3248,20 +3248,27 @@ static int qca8k_resume(struct device *dev)
static SIMPLE_DEV_PM_OPS(qca8k_pm_ops,
qca8k_suspend, qca8k_resume);
+static const struct qca8k_info_ops qca8xxx_ops = {
+ .autocast_mib = qca8k_get_ethtool_stats_eth,
+};
+
static const struct qca8k_match_data qca8327 = {
.id = QCA8K_ID_QCA8327,
.reduced_package = true,
.mib_count = QCA8K_QCA832X_MIB_COUNT,
+ .ops = qca8xxx_ops,
};
static const struct qca8k_match_data qca8328 = {
.id = QCA8K_ID_QCA8327,
.mib_count = QCA8K_QCA832X_MIB_COUNT,
+ .ops = qca8xxx_ops,
};
static const struct qca8k_match_data qca833x = {
.id = QCA8K_ID_QCA8337,
.mib_count = QCA8K_QCA833X_MIB_COUNT,
+ .ops = qca8xxx_ops,
};
static const struct of_device_id qca8k_of_match[] = {
diff --git a/drivers/net/dsa/qca/qca8k.h b/drivers/net/dsa/qca/qca8k.h
index 0b990b46890a..7b4a698f092a 100644
--- a/drivers/net/dsa/qca/qca8k.h
+++ b/drivers/net/dsa/qca/qca8k.h
@@ -324,10 +324,15 @@ enum qca8k_mid_cmd {
QCA8K_MIB_CAST = 3,
};
+struct qca8k_info_ops {
+ int (*autocast_mib)(struct dsa_switch *ds, int port, u64 *data);
+};
+
struct qca8k_match_data {
u8 id;
bool reduced_package;
u8 mib_count;
+ struct qca8k_info_ops ops;
};
enum {
--
2.36.1
On Sat, Jul 23, 2022 at 04:18:33PM +0200, Christian Marangi wrote:
> Some switch may not support mib autocast feature and require the legacy
> way of reading the regs directly.
> Make the mib autocast feature optional and permit to declare support for
> it using match_data struct in a dedicated qca8k_info_ops struct.
>
> Signed-off-by: Christian Marangi <[email protected]>
> ---
> drivers/net/dsa/qca/qca8k.c | 11 +++++++++--
> drivers/net/dsa/qca/qca8k.h | 5 +++++
> 2 files changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/dsa/qca/qca8k.c b/drivers/net/dsa/qca/qca8k.c
> index 212b284f9f73..9820c5942d2a 100644
> --- a/drivers/net/dsa/qca/qca8k.c
> +++ b/drivers/net/dsa/qca/qca8k.c
> @@ -2104,8 +2104,8 @@ qca8k_get_ethtool_stats(struct dsa_switch *ds, int port,
> u32 hi = 0;
> int ret;
>
> - if (priv->mgmt_master &&
> - qca8k_get_ethtool_stats_eth(ds, port, data) > 0)
> + if (priv->mgmt_master && priv->info->ops.autocast_mib &&
> + priv->info->ops.autocast_mib(ds, port, data) > 0)
> return;
>
> for (i = 0; i < priv->info->mib_count; i++) {
> @@ -3248,20 +3248,27 @@ static int qca8k_resume(struct device *dev)
> static SIMPLE_DEV_PM_OPS(qca8k_pm_ops,
> qca8k_suspend, qca8k_resume);
>
> +static const struct qca8k_info_ops qca8xxx_ops = {
> + .autocast_mib = qca8k_get_ethtool_stats_eth,
> +};
> +
> static const struct qca8k_match_data qca8327 = {
> .id = QCA8K_ID_QCA8327,
> .reduced_package = true,
> .mib_count = QCA8K_QCA832X_MIB_COUNT,
> + .ops = qca8xxx_ops,
> };
>
> static const struct qca8k_match_data qca8328 = {
> .id = QCA8K_ID_QCA8327,
> .mib_count = QCA8K_QCA832X_MIB_COUNT,
> + .ops = qca8xxx_ops,
> };
>
> static const struct qca8k_match_data qca833x = {
> .id = QCA8K_ID_QCA8337,
> .mib_count = QCA8K_QCA833X_MIB_COUNT,
> + .ops = qca8xxx_ops,
> };
>
> static const struct of_device_id qca8k_of_match[] = {
> diff --git a/drivers/net/dsa/qca/qca8k.h b/drivers/net/dsa/qca/qca8k.h
> index 0b990b46890a..7b4a698f092a 100644
> --- a/drivers/net/dsa/qca/qca8k.h
> +++ b/drivers/net/dsa/qca/qca8k.h
> @@ -324,10 +324,15 @@ enum qca8k_mid_cmd {
> QCA8K_MIB_CAST = 3,
> };
>
> +struct qca8k_info_ops {
> + int (*autocast_mib)(struct dsa_switch *ds, int port, u64 *data);
> +};
> +
> struct qca8k_match_data {
> u8 id;
> bool reduced_package;
> u8 mib_count;
> + struct qca8k_info_ops ops;
This creates a copy of the structure for each of qca8327, qca8328, etc etc,
which in turn will consume 3 times more space than necessary when new
ops are added. Could you make this "const struct qca8k_ops *ops"?
> };
>
> enum {
> --
> 2.36.1
>
On Mon, Jul 25, 2022 at 01:33:48AM +0300, Vladimir Oltean wrote:
> On Sat, Jul 23, 2022 at 04:18:33PM +0200, Christian Marangi wrote:
> > Some switch may not support mib autocast feature and require the legacy
> > way of reading the regs directly.
> > Make the mib autocast feature optional and permit to declare support for
> > it using match_data struct in a dedicated qca8k_info_ops struct.
> >
> > Signed-off-by: Christian Marangi <[email protected]>
> > ---
> > drivers/net/dsa/qca/qca8k.c | 11 +++++++++--
> > drivers/net/dsa/qca/qca8k.h | 5 +++++
> > 2 files changed, 14 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/dsa/qca/qca8k.c b/drivers/net/dsa/qca/qca8k.c
> > index 212b284f9f73..9820c5942d2a 100644
> > --- a/drivers/net/dsa/qca/qca8k.c
> > +++ b/drivers/net/dsa/qca/qca8k.c
> > @@ -2104,8 +2104,8 @@ qca8k_get_ethtool_stats(struct dsa_switch *ds, int port,
> > u32 hi = 0;
> > int ret;
> >
> > - if (priv->mgmt_master &&
> > - qca8k_get_ethtool_stats_eth(ds, port, data) > 0)
> > + if (priv->mgmt_master && priv->info->ops.autocast_mib &&
> > + priv->info->ops.autocast_mib(ds, port, data) > 0)
> > return;
> >
> > for (i = 0; i < priv->info->mib_count; i++) {
> > @@ -3248,20 +3248,27 @@ static int qca8k_resume(struct device *dev)
> > static SIMPLE_DEV_PM_OPS(qca8k_pm_ops,
> > qca8k_suspend, qca8k_resume);
> >
> > +static const struct qca8k_info_ops qca8xxx_ops = {
> > + .autocast_mib = qca8k_get_ethtool_stats_eth,
> > +};
> > +
> > static const struct qca8k_match_data qca8327 = {
> > .id = QCA8K_ID_QCA8327,
> > .reduced_package = true,
> > .mib_count = QCA8K_QCA832X_MIB_COUNT,
> > + .ops = qca8xxx_ops,
> > };
> >
> > static const struct qca8k_match_data qca8328 = {
> > .id = QCA8K_ID_QCA8327,
> > .mib_count = QCA8K_QCA832X_MIB_COUNT,
> > + .ops = qca8xxx_ops,
> > };
> >
> > static const struct qca8k_match_data qca833x = {
> > .id = QCA8K_ID_QCA8337,
> > .mib_count = QCA8K_QCA833X_MIB_COUNT,
> > + .ops = qca8xxx_ops,
> > };
> >
> > static const struct of_device_id qca8k_of_match[] = {
> > diff --git a/drivers/net/dsa/qca/qca8k.h b/drivers/net/dsa/qca/qca8k.h
> > index 0b990b46890a..7b4a698f092a 100644
> > --- a/drivers/net/dsa/qca/qca8k.h
> > +++ b/drivers/net/dsa/qca/qca8k.h
> > @@ -324,10 +324,15 @@ enum qca8k_mid_cmd {
> > QCA8K_MIB_CAST = 3,
> > };
> >
> > +struct qca8k_info_ops {
> > + int (*autocast_mib)(struct dsa_switch *ds, int port, u64 *data);
> > +};
> > +
> > struct qca8k_match_data {
> > u8 id;
> > bool reduced_package;
> > u8 mib_count;
> > + struct qca8k_info_ops ops;
>
> This creates a copy of the structure for each of qca8327, qca8328, etc etc,
> which in turn will consume 3 times more space than necessary when new
> ops are added. Could you make this "const struct qca8k_ops *ops"?
>
I already done this change in v4 as it was causing compilation error for
kernel test bot.
> > };
> >
> > enum {
> > --
> > 2.36.1
> >
>
--
Ansuel