[PATCH net-next 06/15] net: sparx5: add constants to match data

Daniel Machon posted 15 patches 1 month, 3 weeks ago
There is a newer version of this series
[PATCH net-next 06/15] net: sparx5: add constants to match data
Posted by Daniel Machon 1 month, 3 weeks ago
Add new struct sparx5_consts, containing all the chip constants that are
known to be different for Sparx5 and lan969x. Also add a macro to access
the constants.

Signed-off-by: Daniel Machon <daniel.machon@microchip.com>
Reviewed-by: Steen Hegelund <Steen.Hegelund@microchip.com>
---
 .../net/ethernet/microchip/sparx5/sparx5_main.c    | 21 ++++++++++++++++++++
 .../net/ethernet/microchip/sparx5/sparx5_main.h    | 23 ++++++++++++++++++++++
 2 files changed, 44 insertions(+)

diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_main.c b/drivers/net/ethernet/microchip/sparx5/sparx5_main.c
index 9a8d2e8c02a5..5f3690a59ac1 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_main.c
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_main.c
@@ -953,11 +953,32 @@ static const struct sparx5_regs sparx5_regs = {
 	.fsize = sparx5_fsize,
 };
 
+static const struct sparx5_consts sparx5_consts = {
+	.n_ports             = 65,
+	.n_ports_all         = 70,
+	.n_hsch_l1_elems     = 64,
+	.n_hsch_queues       = 8,
+	.n_lb_groups         = 10,
+	.n_pgids             = 2113, /* (2048 + n_ports) */
+	.n_sio_clks          = 3,
+	.n_own_upsids        = 3,
+	.n_auto_cals         = 7,
+	.n_filters           = 1024,
+	.n_gates             = 1024,
+	.n_sdlbs             = 4096,
+	.n_dsm_cal_taxis     = 8,
+	.buf_size            = 4194280,
+	.qres_max_prio_idx   = 630,
+	.qres_max_colour_idx = 638,
+	.tod_pin             = 4,
+};
+
 static const struct sparx5_match_data sparx5_desc = {
 	.iomap = sparx5_main_iomap,
 	.iomap_size = ARRAY_SIZE(sparx5_main_iomap),
 	.ioranges = 3,
 	.regs = &sparx5_regs,
+	.consts = &sparx5_consts,
 };
 
 static const struct of_device_id mchp_sparx5_match[] = {
diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_main.h b/drivers/net/ethernet/microchip/sparx5/sparx5_main.h
index 738b86999fd8..91f5a3be829e 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_main.h
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_main.h
@@ -51,6 +51,8 @@ enum sparx5_vlan_port_type {
 	SPX5_VLAN_PORT_TYPE_S_CUSTOM /* S-port using custom type */
 };
 
+#define SPX5_CONST(const) sparx5->data->consts->const
+
 #define SPX5_PORTS             65
 #define SPX5_PORTS_ALL         70 /* Total number of ports */
 
@@ -238,6 +240,26 @@ struct sparx5_regs {
 	const unsigned int *fsize;
 };
 
+struct sparx5_consts {
+	u32 n_ports;             /* Number of front ports */
+	u32 n_ports_all;         /* Number of front ports + internal ports */
+	u32 n_hsch_l1_elems;     /* Number of HSCH layer 1 elements */
+	u32 n_hsch_queues;       /* Number of HSCH queues */
+	u32 n_lb_groups;         /* Number of leacky bucket groupd */
+	u32 n_pgids;             /* Number of PGID's */
+	u32 n_sio_clks;          /* Number of serial IO clocks */
+	u32 n_own_upsids;        /* Number of own UPSID's */
+	u32 n_auto_cals;         /* Number of auto calendars */
+	u32 n_filters;           /* Number of PSFP filters */
+	u32 n_gates;             /* Number of PSFP gates */
+	u32 n_sdlbs;             /* Number of service dual leaky buckets */
+	u32 n_dsm_cal_taxis;     /* Number of DSM calendar taxis */
+	u32 buf_size;            /* Amount of QLIM watermark memory */
+	u32 qres_max_prio_idx;   /* Maximum QRES prio index */
+	u32 qres_max_colour_idx; /* Maximum QRES colour index */
+	u32 tod_pin;             /* PTP TOD pin */
+};
+
 struct sparx5_main_io_resource {
 	enum sparx5_target id;
 	phys_addr_t offset;
@@ -246,6 +268,7 @@ struct sparx5_main_io_resource {
 
 struct sparx5_match_data {
 	const struct sparx5_regs *regs;
+	const struct sparx5_consts *consts;
 	const struct sparx5_main_io_resource *iomap;
 	int ioranges;
 	int iomap_size;

-- 
2.34.1
Re: [PATCH net-next 06/15] net: sparx5: add constants to match data
Posted by Jakub Kicinski 1 month, 3 weeks ago
On Tue, 1 Oct 2024 15:50:36 +0200 Daniel Machon wrote:
> +#define SPX5_CONST(const) sparx5->data->consts->const

This is way too ugly too live.
Please type the code out, there's no prize for having low LoC count.
Re: [PATCH net-next 06/15] net: sparx5: add constants to match data
Posted by Daniel Machon 1 month, 3 weeks ago
> > +#define SPX5_CONST(const) sparx5->data->consts->const
> 
> This is way too ugly too live.
> Please type the code out, there's no prize for having low LoC count.
>

Hi Jakub,

By "type the code out" - are you saying that we should not be using a macro
for accessing the const at all? and rather just:

    struct sparx5_consts *consts = sparx5->data->consts;
    consts->some_var

or pass in the sparx5 pointer to the macro too, which was the concert that
Jacob raised.

Thanks.

/Daniel
Re: [PATCH net-next 06/15] net: sparx5: add constants to match data
Posted by Jakub Kicinski 1 month, 3 weeks ago
On Wed, 2 Oct 2024 13:31:32 +0000 Daniel Machon wrote:
> By "type the code out" - are you saying that we should not be using a macro
> for accessing the const at all? and rather just:
> 
>     struct sparx5_consts *consts = sparx5->data->consts;
>     consts->some_var

This.

> or pass in the sparx5 pointer to the macro too, which was the concert that
> Jacob raised.

The implicit arguments are part of the ugliness, and should also go
away. But in this case the entire macro should go.
Re: [PATCH net-next 06/15] net: sparx5: add constants to match data
Posted by Daniel Machon 1 month, 3 weeks ago
> > By "type the code out" - are you saying that we should not be using a macro
> > for accessing the const at all? and rather just:
> >
> >     struct sparx5_consts *consts = sparx5->data->consts;
> >     consts->some_var
> 
> This.
> 
> > or pass in the sparx5 pointer to the macro too, which was the concert that
> > Jacob raised.
> 
> The implicit arguments are part of the ugliness, and should also go
> away. But in this case the entire macro should go.

Ack.

Will get rid of the macro in v2.

/Daniel
Re: [PATCH net-next 06/15] net: sparx5: add constants to match data
Posted by Jacob Keller 1 month, 3 weeks ago

On 10/1/2024 6:50 AM, Daniel Machon wrote:
> Add new struct sparx5_consts, containing all the chip constants that are
> known to be different for Sparx5 and lan969x. Also add a macro to access
> the constants.
> 
> Signed-off-by: Daniel Machon <daniel.machon@microchip.com>
> Reviewed-by: Steen Hegelund <Steen.Hegelund@microchip.com>
> ---
>  .../net/ethernet/microchip/sparx5/sparx5_main.c    | 21 ++++++++++++++++++++
>  .../net/ethernet/microchip/sparx5/sparx5_main.h    | 23 ++++++++++++++++++++++
>  2 files changed, 44 insertions(+)
> 
> diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_main.c b/drivers/net/ethernet/microchip/sparx5/sparx5_main.c
> index 9a8d2e8c02a5..5f3690a59ac1 100644
> --- a/drivers/net/ethernet/microchip/sparx5/sparx5_main.c
> +++ b/drivers/net/ethernet/microchip/sparx5/sparx5_main.c
> @@ -953,11 +953,32 @@ static const struct sparx5_regs sparx5_regs = {
>  	.fsize = sparx5_fsize,
>  };
>  
> +static const struct sparx5_consts sparx5_consts = {
> +	.n_ports             = 65,
> +	.n_ports_all         = 70,
> +	.n_hsch_l1_elems     = 64,
> +	.n_hsch_queues       = 8,
> +	.n_lb_groups         = 10,
> +	.n_pgids             = 2113, /* (2048 + n_ports) */
> +	.n_sio_clks          = 3,
> +	.n_own_upsids        = 3,
> +	.n_auto_cals         = 7,
> +	.n_filters           = 1024,
> +	.n_gates             = 1024,
> +	.n_sdlbs             = 4096,
> +	.n_dsm_cal_taxis     = 8,
> +	.buf_size            = 4194280,
> +	.qres_max_prio_idx   = 630,
> +	.qres_max_colour_idx = 638,
> +	.tod_pin             = 4,
> +};
> +
>  static const struct sparx5_match_data sparx5_desc = {
>  	.iomap = sparx5_main_iomap,
>  	.iomap_size = ARRAY_SIZE(sparx5_main_iomap),
>  	.ioranges = 3,
>  	.regs = &sparx5_regs,
> +	.consts = &sparx5_consts,
>  };
>  
>  static const struct of_device_id mchp_sparx5_match[] = {
> diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_main.h b/drivers/net/ethernet/microchip/sparx5/sparx5_main.h
> index 738b86999fd8..91f5a3be829e 100644
> --- a/drivers/net/ethernet/microchip/sparx5/sparx5_main.h
> +++ b/drivers/net/ethernet/microchip/sparx5/sparx5_main.h
> @@ -51,6 +51,8 @@ enum sparx5_vlan_port_type {
>  	SPX5_VLAN_PORT_TYPE_S_CUSTOM /* S-port using custom type */
>  };
>  
> +#define SPX5_CONST(const) sparx5->data->consts->const
> +

I'm not a fan of implicit dependency here. Whats the reason for having
it done this way vs passing something in as a parameter here?