Re: [PATCH net-next v3 08/12] net: stmmac: Pass stmmac_priv in some callbacks

[Simon Horman]

On Fri, Mar 31, 2023 at 04:45:45PM -0500, Andrew Halaney wrote:
> Passing stmmac_priv to some of the callbacks allows hwif implementations
> to grab some data that platforms can customize. Adjust the callbacks
> accordingly in preparation of such a platform customization.
> 
> Signed-off-by: Andrew Halaney <ahalaney@xxxxxxxxxx>

...

>  #define stmmac_reset(__priv, __args...) \
> @@ -223,59 +240,59 @@ struct stmmac_dma_ops {
>  #define stmmac_dma_init(__priv, __args...) \
>  	stmmac_do_void_callback(__priv, dma, init, __args)
>  #define stmmac_init_chan(__priv, __args...) \
> -	stmmac_do_void_callback(__priv, dma, init_chan, __args)
> +	stmmac_do_void_callback(__priv, dma, init_chan, __priv, __args)

Hi Andrew,

Rather than maintaining these macros can we just get rid of them?
I'd be surprised if things aren't nicer with functions in their place [1].

f.e., we now have (__priv, ..., __priv, ...) due to a generalisation
      that seems to take a lot more than it gives.

[1] https://lore.kernel.org/linux-arm-kernel/ZBst1SzcIS4j+t46@xxxxxxxxxxxx/

>  #define stmmac_init_rx_chan(__priv, __args...) \
> -	stmmac_do_void_callback(__priv, dma, init_rx_chan, __args)
> +	stmmac_do_void_callback(__priv, dma, init_rx_chan, __priv, __args)
>  #define stmmac_init_tx_chan(__priv, __args...) \
> -	stmmac_do_void_callback(__priv, dma, init_tx_chan, __args)
> +	stmmac_do_void_callback(__priv, dma, init_tx_chan, __priv, __args)
>  #define stmmac_axi(__priv, __args...) \
>  	stmmac_do_void_callback(__priv, dma, axi, __args)
>  #define stmmac_dump_dma_regs(__priv, __args...) \
> -	stmmac_do_void_callback(__priv, dma, dump_regs, __args)
> +	stmmac_do_void_callback(__priv, dma, dump_regs, __priv, __args)
>  #define stmmac_dma_rx_mode(__priv, __args...) \
> -	stmmac_do_void_callback(__priv, dma, dma_rx_mode, __args)
> +	stmmac_do_void_callback(__priv, dma, dma_rx_mode, __priv, __args)
>  #define stmmac_dma_tx_mode(__priv, __args...) \
> -	stmmac_do_void_callback(__priv, dma, dma_tx_mode, __args)
> +	stmmac_do_void_callback(__priv, dma, dma_tx_mode, __priv, __args)
>  #define stmmac_dma_diagnostic_fr(__priv, __args...) \
>  	stmmac_do_void_callback(__priv, dma, dma_diagnostic_fr, __args)
>  #define stmmac_enable_dma_transmission(__priv, __args...) \
>  	stmmac_do_void_callback(__priv, dma, enable_dma_transmission, __args)
>  #define stmmac_enable_dma_irq(__priv, __args...) \
> -	stmmac_do_void_callback(__priv, dma, enable_dma_irq, __args)
> +	stmmac_do_void_callback(__priv, dma, enable_dma_irq, __priv, __args)
>  #define stmmac_disable_dma_irq(__priv, __args...) \
> -	stmmac_do_void_callback(__priv, dma, disable_dma_irq, __args)
> +	stmmac_do_void_callback(__priv, dma, disable_dma_irq, __priv, __args)
>  #define stmmac_start_tx(__priv, __args...) \
> -	stmmac_do_void_callback(__priv, dma, start_tx, __args)
> +	stmmac_do_void_callback(__priv, dma, start_tx, __priv, __args)
>  #define stmmac_stop_tx(__priv, __args...) \
> -	stmmac_do_void_callback(__priv, dma, stop_tx, __args)
> +	stmmac_do_void_callback(__priv, dma, stop_tx, __priv, __args)
>  #define stmmac_start_rx(__priv, __args...) \
> -	stmmac_do_void_callback(__priv, dma, start_rx, __args)
> +	stmmac_do_void_callback(__priv, dma, start_rx, __priv, __args)
>  #define stmmac_stop_rx(__priv, __args...) \
> -	stmmac_do_void_callback(__priv, dma, stop_rx, __args)
> +	stmmac_do_void_callback(__priv, dma, stop_rx, __priv, __args)
>  #define stmmac_dma_interrupt_status(__priv, __args...) \
> -	stmmac_do_callback(__priv, dma, dma_interrupt, __args)
> +	stmmac_do_callback(__priv, dma, dma_interrupt, __priv, __args)
>  #define stmmac_get_hw_feature(__priv, __args...) \
>  	stmmac_do_callback(__priv, dma, get_hw_feature, __args)
>  #define stmmac_rx_watchdog(__priv, __args...) \
> -	stmmac_do_void_callback(__priv, dma, rx_watchdog, __args)
> +	stmmac_do_void_callback(__priv, dma, rx_watchdog, __priv, __args)
>  #define stmmac_set_tx_ring_len(__priv, __args...) \
> -	stmmac_do_void_callback(__priv, dma, set_tx_ring_len, __args)
> +	stmmac_do_void_callback(__priv, dma, set_tx_ring_len, __priv, __args)
>  #define stmmac_set_rx_ring_len(__priv, __args...) \
> -	stmmac_do_void_callback(__priv, dma, set_rx_ring_len, __args)
> +	stmmac_do_void_callback(__priv, dma, set_rx_ring_len, __priv, __args)
>  #define stmmac_set_rx_tail_ptr(__priv, __args...) \
> -	stmmac_do_void_callback(__priv, dma, set_rx_tail_ptr, __args)
> +	stmmac_do_void_callback(__priv, dma, set_rx_tail_ptr, __priv, __args)
>  #define stmmac_set_tx_tail_ptr(__priv, __args...) \
> -	stmmac_do_void_callback(__priv, dma, set_tx_tail_ptr, __args)
> +	stmmac_do_void_callback(__priv, dma, set_tx_tail_ptr, __priv, __args)
>  #define stmmac_enable_tso(__priv, __args...) \
> -	stmmac_do_void_callback(__priv, dma, enable_tso, __args)
> +	stmmac_do_void_callback(__priv, dma, enable_tso, __priv, __args)
>  #define stmmac_dma_qmode(__priv, __args...) \
> -	stmmac_do_void_callback(__priv, dma, qmode, __args)
> +	stmmac_do_void_callback(__priv, dma, qmode, __priv, __args)
>  #define stmmac_set_dma_bfsize(__priv, __args...) \
> -	stmmac_do_void_callback(__priv, dma, set_bfsize, __args)
> +	stmmac_do_void_callback(__priv, dma, set_bfsize, __priv, __args)
>  #define stmmac_enable_sph(__priv, __args...) \
> -	stmmac_do_void_callback(__priv, dma, enable_sph, __args)
> +	stmmac_do_void_callback(__priv, dma, enable_sph, __priv, __args)
>  #define stmmac_enable_tbs(__priv, __args...) \
> -	stmmac_do_callback(__priv, dma, enable_tbs, __args)
> +	stmmac_do_callback(__priv, dma, enable_tbs, __priv, __args)
>  
>  struct mac_device_info;
>  struct net_device;
> @@ -307,21 +324,23 @@ struct stmmac_ops {
>  	/* Program TX Algorithms */
>  	void (*prog_mtl_tx_algorithms)(struct mac_device_info *hw, u32 tx_alg);
>  	/* Set MTL TX queues weight */
> -	void (*set_mtl_tx_queue_weight)(struct mac_device_info *hw,
> +	void (*set_mtl_tx_queue_weight)(struct stmmac_priv *priv,
> +					struct mac_device_info *hw,
>  					u32 weight, u32 queue);
>  	/* RX MTL queue to RX dma mapping */
>  	void (*map_mtl_to_dma)(struct mac_device_info *hw, u32 queue, u32 chan);
>  	/* Configure AV Algorithm */
> -	void (*config_cbs)(struct mac_device_info *hw, u32 send_slope,
> -			   u32 idle_slope, u32 high_credit, u32 low_credit,
> -			   u32 queue);
> +	void (*config_cbs)(struct stmmac_priv *priv, struct mac_device_info *hw,
> +			   u32 send_slope, u32 idle_slope, u32 high_credit,
> +			   u32 low_credit, u32 queue);
>  	/* Dump MAC registers */
>  	void (*dump_regs)(struct mac_device_info *hw, u32 *reg_space);
>  	/* Handle extra events on specific interrupts hw dependent */
>  	int (*host_irq_status)(struct mac_device_info *hw,
>  			       struct stmmac_extra_stats *x);
>  	/* Handle MTL interrupts */
> -	int (*host_mtl_irq_status)(struct mac_device_info *hw, u32 chan);
> +	int (*host_mtl_irq_status)(struct stmmac_priv *priv,
> +				   struct mac_device_info *hw, u32 chan);
>  	/* Multicast filter setting */
>  	void (*set_filter)(struct mac_device_info *hw, struct net_device *dev);
>  	/* Flow control setting */
> @@ -341,8 +360,9 @@ struct stmmac_ops {
>  	void (*set_eee_lpi_entry_timer)(struct mac_device_info *hw, int et);
>  	void (*set_eee_timer)(struct mac_device_info *hw, int ls, int tw);
>  	void (*set_eee_pls)(struct mac_device_info *hw, int link);
> -	void (*debug)(void __iomem *ioaddr, struct stmmac_extra_stats *x,
> -		      u32 rx_queues, u32 tx_queues);
> +	void (*debug)(struct stmmac_priv *priv, void __iomem *ioaddr,
> +		      struct stmmac_extra_stats *x, u32 rx_queues,
> +		      u32 tx_queues);
>  	/* PCS calls */
>  	void (*pcs_ctrl_ane)(void __iomem *ioaddr, bool ane, bool srgmi_ral,
>  			     bool loopback);

...

> @@ -422,17 +442,17 @@ struct stmmac_ops {
>  #define stmmac_prog_mtl_tx_algorithms(__priv, __args...) \
>  	stmmac_do_void_callback(__priv, mac, prog_mtl_tx_algorithms, __args)
>  #define stmmac_set_mtl_tx_queue_weight(__priv, __args...) \
> -	stmmac_do_void_callback(__priv, mac, set_mtl_tx_queue_weight, __args)
> +	stmmac_do_void_callback(__priv, mac, set_mtl_tx_queue_weight, __priv, __args)
>  #define stmmac_map_mtl_to_dma(__priv, __args...) \
>  	stmmac_do_void_callback(__priv, mac, map_mtl_to_dma, __args)
>  #define stmmac_config_cbs(__priv, __args...) \
> -	stmmac_do_void_callback(__priv, mac, config_cbs, __args)
> +	stmmac_do_void_callback(__priv, mac, config_cbs, __priv, __args)
>  #define stmmac_dump_mac_regs(__priv, __args...) \
>  	stmmac_do_void_callback(__priv, mac, dump_regs, __args)
>  #define stmmac_host_irq_status(__priv, __args...) \
>  	stmmac_do_callback(__priv, mac, host_irq_status, __args)
>  #define stmmac_host_mtl_irq_status(__priv, __args...) \
> -	stmmac_do_callback(__priv, mac, host_mtl_irq_status, __args)
> +	stmmac_do_callback(__priv, mac, host_mtl_irq_status, __priv, __args)
>  #define stmmac_set_filter(__priv, __args...) \
>  	stmmac_do_void_callback(__priv, mac, set_filter, __args)
>  #define stmmac_flow_ctrl(__priv, __args...) \
> @@ -454,11 +474,11 @@ struct stmmac_ops {
>  #define stmmac_set_eee_pls(__priv, __args...) \
>  	stmmac_do_void_callback(__priv, mac, set_eee_pls, __args)
>  #define stmmac_mac_debug(__priv, __args...) \
> -	stmmac_do_void_callback(__priv, mac, debug, __args)
> +	stmmac_do_void_callback(__priv, mac, debug, __priv, __args)
>  #define stmmac_pcs_ctrl_ane(__priv, __args...) \
>  	stmmac_do_void_callback(__priv, mac, pcs_ctrl_ane, __args)
>  #define stmmac_pcs_rane(__priv, __args...) \
> -	stmmac_do_void_callback(__priv, mac, pcs_rane, __args)
> +	stmmac_do_void_callback(__priv, mac, pcs_rane, __priv, __args)
>  #define stmmac_pcs_get_adv_lp(__priv, __args...) \
>  	stmmac_do_void_callback(__priv, mac, pcs_get_adv_lp, __args)
>  #define stmmac_safety_feat_config(__priv, __args...) \
> @@ -506,8 +526,6 @@ struct stmmac_ops {
>  #define stmmac_fpe_irq_status(__priv, __args...) \
>  	stmmac_do_callback(__priv, mac, fpe_irq_status, __args)