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

From: Simon Horman
Date: Sat Apr 01 2023 - 11:06:44 EST


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)