Add the possibility to set coalescing parameters with ethtool.
rx-frames-irq and rx-usecs-irq can only be set and unset together as the
implemented mechanism would not work otherwise. rx-frames-irq can't be
greater than the RX FIFO size.
Also all values can only be changed if the chip is not active.
Signed-off-by: Markus Schneider-Pargmann <[email protected]>
Reviewed-by: Simon Horman <[email protected]>
---
drivers/net/can/m_can/m_can.c | 46 +++++++++++++++++++++++++++++++++++
1 file changed, 46 insertions(+)
diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
index 781f287e7ce4..5238a5967971 100644
--- a/drivers/net/can/m_can/m_can.c
+++ b/drivers/net/can/m_can/m_can.c
@@ -1944,8 +1944,54 @@ static const struct net_device_ops m_can_netdev_ops = {
.ndo_change_mtu = can_change_mtu,
};
+static int m_can_get_coalesce(struct net_device *dev,
+ struct ethtool_coalesce *ec,
+ struct kernel_ethtool_coalesce *kec,
+ struct netlink_ext_ack *ext_ack)
+{
+ struct m_can_classdev *cdev = netdev_priv(dev);
+
+ ec->rx_max_coalesced_frames_irq = cdev->rx_max_coalesced_frames_irq;
+ ec->rx_coalesce_usecs_irq = cdev->rx_coalesce_usecs_irq;
+
+ return 0;
+}
+
+static int m_can_set_coalesce(struct net_device *dev,
+ struct ethtool_coalesce *ec,
+ struct kernel_ethtool_coalesce *kec,
+ struct netlink_ext_ack *ext_ack)
+{
+ struct m_can_classdev *cdev = netdev_priv(dev);
+
+ if (cdev->can.state != CAN_STATE_STOPPED) {
+ netdev_err(dev, "Device is in use, please shut it down first\n");
+ return -EBUSY;
+ }
+
+ if (ec->rx_max_coalesced_frames_irq > cdev->mcfg[MRAM_RXF0].num) {
+ netdev_err(dev, "rx-frames-irq %u greater than the RX FIFO %u\n",
+ ec->rx_max_coalesced_frames_irq,
+ cdev->mcfg[MRAM_RXF0].num);
+ return -EINVAL;
+ }
+ if (ec->rx_max_coalesced_frames_irq == 0 != ec->rx_coalesce_usecs_irq == 0) {
+ netdev_err(dev, "rx-frames-irq and rx-usecs-irq can only be set together\n");
+ return -EINVAL;
+ }
+
+ cdev->rx_max_coalesced_frames_irq = ec->rx_max_coalesced_frames_irq;
+ cdev->rx_coalesce_usecs_irq = ec->rx_coalesce_usecs_irq;
+
+ return 0;
+}
+
static const struct ethtool_ops m_can_ethtool_ops = {
+ .supported_coalesce_params = ETHTOOL_COALESCE_RX_USECS_IRQ |
+ ETHTOOL_COALESCE_RX_MAX_FRAMES_IRQ,
.get_ts_info = ethtool_op_get_ts_info,
+ .get_coalesce = m_can_get_coalesce,
+ .set_coalesce = m_can_set_coalesce,
};
static int register_m_can_dev(struct net_device *dev)
--
2.40.1
On Wed, Jun 21, 2023 at 11:23:42AM +0200, Markus Schneider-Pargmann wrote:
...
> +static int m_can_set_coalesce(struct net_device *dev,
> + struct ethtool_coalesce *ec,
> + struct kernel_ethtool_coalesce *kec,
> + struct netlink_ext_ack *ext_ack)
> +{
> + struct m_can_classdev *cdev = netdev_priv(dev);
> +
> + if (cdev->can.state != CAN_STATE_STOPPED) {
> + netdev_err(dev, "Device is in use, please shut it down first\n");
> + return -EBUSY;
> + }
> +
> + if (ec->rx_max_coalesced_frames_irq > cdev->mcfg[MRAM_RXF0].num) {
> + netdev_err(dev, "rx-frames-irq %u greater than the RX FIFO %u\n",
> + ec->rx_max_coalesced_frames_irq,
> + cdev->mcfg[MRAM_RXF0].num);
> + return -EINVAL;
> + }
> + if (ec->rx_max_coalesced_frames_irq == 0 != ec->rx_coalesce_usecs_irq == 0) {
Hi Markus,
For a W=1 build GCC 12.3.0 suggests, rather forcefully, that it would like
some more parentheses here.
drivers/net/can/m_can/m_can.c: In function 'm_can_set_coalesce':
drivers/net/can/m_can/m_can.c:1978:45: warning: suggest parentheses around comparison in operand of '!=' [-Wparentheses]
1978 | if (ec->rx_max_coalesced_frames_irq == 0 != ec->rx_coalesce_usecs_irq == 0) {
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~
drivers/net/can/m_can/m_can.c:1978:50: warning: suggest parentheses around comparison in operand of '==' [-Wparentheses]
1978 | if (ec->rx_max_coalesced_frames_irq == 0 != ec->rx_coalesce_usecs_irq == 0) {
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~
> + netdev_err(dev, "rx-frames-irq and rx-usecs-irq can only be set together\n");
> + return -EINVAL;
> + }
> +
> + cdev->rx_max_coalesced_frames_irq = ec->rx_max_coalesced_frames_irq;
> + cdev->rx_coalesce_usecs_irq = ec->rx_coalesce_usecs_irq;
> +
> + return 0;
> +}
...
Hi Markus,
kernel test robot noticed the following build warnings:
[auto build test WARNING on ac9a78681b921877518763ba0e89202254349d1b]
url: https://github.com/intel-lab-lkp/linux/commits/Markus-Schneider-Pargmann/can-m_can-Write-transmit-header-and-data-in-one-transaction/20230621-173848
base: ac9a78681b921877518763ba0e89202254349d1b
patch link: https://lore.kernel.org/r/20230621092350.3130866-5-msp%40baylibre.com
patch subject: [PATCH v4 04/12] can: m_can: Add rx coalescing ethtool support
config: sparc-allyesconfig (https://download.01.org/0day-ci/archive/20230622/[email protected]/config)
compiler: sparc64-linux-gcc (GCC) 12.3.0
reproduce: (https://download.01.org/0day-ci/archive/20230622/[email protected]/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All warnings (new ones prefixed by >>):
drivers/net/can/m_can/m_can.c: In function 'm_can_tx_handler':
drivers/net/can/m_can/m_can.c:1697:27: warning: unused variable 'fifo_header' [-Wunused-variable]
1697 | struct id_and_dlc fifo_header;
| ^~~~~~~~~~~
drivers/net/can/m_can/m_can.c: In function 'm_can_set_coalesce':
>> drivers/net/can/m_can/m_can.c:1978:45: warning: suggest parentheses around comparison in operand of '!=' [-Wparentheses]
1978 | if (ec->rx_max_coalesced_frames_irq == 0 != ec->rx_coalesce_usecs_irq == 0) {
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~
drivers/net/can/m_can/m_can.c:1978:50: warning: suggest parentheses around comparison in operand of '==' [-Wparentheses]
1978 | if (ec->rx_max_coalesced_frames_irq == 0 != ec->rx_coalesce_usecs_irq == 0) {
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~
vim +1978 drivers/net/can/m_can/m_can.c
1959
1960 static int m_can_set_coalesce(struct net_device *dev,
1961 struct ethtool_coalesce *ec,
1962 struct kernel_ethtool_coalesce *kec,
1963 struct netlink_ext_ack *ext_ack)
1964 {
1965 struct m_can_classdev *cdev = netdev_priv(dev);
1966
1967 if (cdev->can.state != CAN_STATE_STOPPED) {
1968 netdev_err(dev, "Device is in use, please shut it down first\n");
1969 return -EBUSY;
1970 }
1971
1972 if (ec->rx_max_coalesced_frames_irq > cdev->mcfg[MRAM_RXF0].num) {
1973 netdev_err(dev, "rx-frames-irq %u greater than the RX FIFO %u\n",
1974 ec->rx_max_coalesced_frames_irq,
1975 cdev->mcfg[MRAM_RXF0].num);
1976 return -EINVAL;
1977 }
> 1978 if (ec->rx_max_coalesced_frames_irq == 0 != ec->rx_coalesce_usecs_irq == 0) {
1979 netdev_err(dev, "rx-frames-irq and rx-usecs-irq can only be set together\n");
1980 return -EINVAL;
1981 }
1982
1983 cdev->rx_max_coalesced_frames_irq = ec->rx_max_coalesced_frames_irq;
1984 cdev->rx_coalesce_usecs_irq = ec->rx_coalesce_usecs_irq;
1985
1986 return 0;
1987 }
1988
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Hi Simon,
On Wed, Jun 21, 2023 at 04:22:12PM +0200, Simon Horman wrote:
> On Wed, Jun 21, 2023 at 11:23:42AM +0200, Markus Schneider-Pargmann wrote:
>
> ...
>
> > +static int m_can_set_coalesce(struct net_device *dev,
> > + struct ethtool_coalesce *ec,
> > + struct kernel_ethtool_coalesce *kec,
> > + struct netlink_ext_ack *ext_ack)
> > +{
> > + struct m_can_classdev *cdev = netdev_priv(dev);
> > +
> > + if (cdev->can.state != CAN_STATE_STOPPED) {
> > + netdev_err(dev, "Device is in use, please shut it down first\n");
> > + return -EBUSY;
> > + }
> > +
> > + if (ec->rx_max_coalesced_frames_irq > cdev->mcfg[MRAM_RXF0].num) {
> > + netdev_err(dev, "rx-frames-irq %u greater than the RX FIFO %u\n",
> > + ec->rx_max_coalesced_frames_irq,
> > + cdev->mcfg[MRAM_RXF0].num);
> > + return -EINVAL;
> > + }
> > + if (ec->rx_max_coalesced_frames_irq == 0 != ec->rx_coalesce_usecs_irq == 0) {
>
> Hi Markus,
>
> For a W=1 build GCC 12.3.0 suggests, rather forcefully, that it would like
> some more parentheses here.
>
> drivers/net/can/m_can/m_can.c: In function 'm_can_set_coalesce':
> drivers/net/can/m_can/m_can.c:1978:45: warning: suggest parentheses around comparison in operand of '!=' [-Wparentheses]
> 1978 | if (ec->rx_max_coalesced_frames_irq == 0 != ec->rx_coalesce_usecs_irq == 0) {
> | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~
> drivers/net/can/m_can/m_can.c:1978:50: warning: suggest parentheses around comparison in operand of '==' [-Wparentheses]
> 1978 | if (ec->rx_max_coalesced_frames_irq == 0 != ec->rx_coalesce_usecs_irq == 0) {
> | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~
Thanks, yes I just changed it because checkpatch doesn't like it the
other way. I am going to change it back. Also I am wondering why clang
doesn't complain at this point.
Best,
Markus
>
> > + netdev_err(dev, "rx-frames-irq and rx-usecs-irq can only be set together\n");
> > + return -EINVAL;
> > + }
> > +
> > + cdev->rx_max_coalesced_frames_irq = ec->rx_max_coalesced_frames_irq;
> > + cdev->rx_coalesce_usecs_irq = ec->rx_coalesce_usecs_irq;
> > +
> > + return 0;
> > +}
>
> ...