* Re: [PATCH v4 4/4] can: tcan4x5x: Add tcan4x5x driver to the kernel
From: Dan Murphy @ 2019-01-29 19:27 UTC (permalink / raw)
To: Wolfgang Grandegger, mkl, davem, b29396; +Cc: linux-can, netdev, linux-kernel
In-Reply-To: <037720c9-4846-eb11-1d6a-b99d647d21d6@grandegger.com>
Wolfgang
Sorry for the late reply
On 1/22/19 4:03 AM, Wolfgang Grandegger wrote:
> Hello,
>
> Am 17.01.19 um 21:06 schrieb Dan Murphy:
>> Add the TCAN4x5x SPI CAN driver. This device
>> uses the Bosch MCAN IP core along with a SPI
>> interface map. Leverage the MCAN common core
>> code to manage the MCAN IP.
>>
>> This device has a special method to indicate a
>> write/read operation on the data payload.
>>
>> Signed-off-by: Dan Murphy <dmurphy@ti.com>
>> ---
>> drivers/net/can/m_can/Kconfig | 6 +
>> drivers/net/can/m_can/tcan4x5x.c | 529 +++++++++++++++++++++++++++++++
>> 2 files changed, 535 insertions(+)
>> create mode 100644 drivers/net/can/m_can/tcan4x5x.c
>>
>> diff --git a/drivers/net/can/m_can/Kconfig b/drivers/net/can/m_can/Kconfig
>> index b1a9358b7660..b38959b3b8f1 100644
>> --- a/drivers/net/can/m_can/Kconfig
>> +++ b/drivers/net/can/m_can/Kconfig
>> @@ -15,3 +15,9 @@ config CAN_M_CAN_PLATFORM
>> tristate "Bosch M_CAN devices"
>> ---help---
>> Say Y here if you want to support for Bosch M_CAN controller.
>> +
>> +config CAN_M_CAN_TCAN4X5X
>> + depends on CAN_M_CAN
>> + tristate "TCAN4X5X M_CAN device"
>> + ---help---
>> + Say Y here if you want to support for TI M_CAN controller.
>> diff --git a/drivers/net/can/m_can/tcan4x5x.c b/drivers/net/can/m_can/tcan4x5x.c
>> new file mode 100644
>> index 000000000000..3cd6cd5052b6
>> --- /dev/null
>> +++ b/drivers/net/can/m_can/tcan4x5x.c
>> @@ -0,0 +1,529 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +// SPI to CAN driver for the Texas Instruments TCAN4x5x
>> +// Copyright (C) 2018-19 Texas Instruments Incorporated - http://www.ti.com/
>> +
>> +#include <linux/regmap.h>
>> +#include <linux/spi/spi.h>
>> +
>> +#include <linux/regulator/consumer.h>
>> +#include <linux/gpio/consumer.h>
>> +
>> +#include "m_can_platform.h"
>> +
>> +#define DEVICE_NAME "tcan4x5x"
>> +#define TCAN4X5X_EXT_CLK_DEF 40000000
>> +
>> +#define TCAN4X5X_DEV_ID0 0x00
>> +#define TCAN4X5X_DEV_ID1 0x04
>> +#define TCAN4X5X_REV 0x08
>> +#define TCAN4X5X_STATUS 0x0C
>> +#define TCAN4X5X_ERROR_STATUS 0x10
>> +#define TCAN4X5X_CONTROL 0x14
>> +
>> +#define TCAN4X5X_CONFIG 0x800
>> +#define TCAN4X5X_TS_PRESCALE 0x804
>> +#define TCAN4X5X_TEST_REG 0x808
>> +#define TCAN4X5X_INT_FLAGS 0x820
>> +#define TCAN4X5X_MCAN_INT_REG 0x824
>> +#define TCAN4X5X_INT_EN 0x830
>> +
>> +
>> +/* Interrupt bits */
>> +#define TCAN4X5X_CANBUSTERMOPEN_INT_EN BIT(30)
>> +#define TCAN4X5X_CANHCANL_INT_EN BIT(29)
>> +#define TCAN4X5X_CANHBAT_INT_EN BIT(28)
>> +#define TCAN4X5X_CANLGND_INT_EN BIT(27)
>> +#define TCAN4X5X_CANBUSOPEN_INT_EN BIT(26)
>> +#define TCAN4X5X_CANBUSGND_INT_EN BIT(25)
>> +#define TCAN4X5X_CANBUSBAT_INT_EN BIT(24)
>> +#define TCAN4X5X_UVSUP_INT_EN BIT(22)
>> +#define TCAN4X5X_UVIO_INT_EN BIT(21)
>> +#define TCAN4X5X_TSD_INT_EN BIT(19)
>> +#define TCAN4X5X_ECCERR_INT_EN BIT(16)
>> +#define TCAN4X5X_CANINT_INT_EN BIT(15)
>> +#define TCAN4X5X_LWU_INT_EN BIT(14)
>> +#define TCAN4X5X_CANSLNT_INT_EN BIT(10)
>> +#define TCAN4X5X_CANDOM_INT_EN BIT(8)
>> +#define TCAN4X5X_CANBUS_ERR_INT_EN BIT(5)
>> +#define TCAN4X5X_BUS_FAULT BIT(4)
>> +#define TCAN4X5X_MCAN_INT BIT(1)
>> +#define TCAN4X5X_ENABLE_TCAN_INT (TCAN4X5X_MCAN_INT | \
>> + TCAN4X5X_BUS_FAULT | \
>> + TCAN4X5X_CANBUS_ERR_INT_EN | \
>> + TCAN4X5X_CANINT_INT_EN)
>> +
>> +/* MCAN Interrupt bits */
>> +#define TCAN4X5X_MCAN_IR_ARA BIT(29)
>> +#define TCAN4X5X_MCAN_IR_PED BIT(28)
>> +#define TCAN4X5X_MCAN_IR_PEA BIT(27)
>> +#define TCAN4X5X_MCAN_IR_WD BIT(26)
>> +#define TCAN4X5X_MCAN_IR_BO BIT(25)
>> +#define TCAN4X5X_MCAN_IR_EW BIT(24)
>> +#define TCAN4X5X_MCAN_IR_EP BIT(23)
>> +#define TCAN4X5X_MCAN_IR_ELO BIT(22)
>> +#define TCAN4X5X_MCAN_IR_BEU BIT(21)
>> +#define TCAN4X5X_MCAN_IR_BEC BIT(20)
>> +#define TCAN4X5X_MCAN_IR_DRX BIT(19)
>> +#define TCAN4X5X_MCAN_IR_TOO BIT(18)
>> +#define TCAN4X5X_MCAN_IR_MRAF BIT(17)
>> +#define TCAN4X5X_MCAN_IR_TSW BIT(16)
>> +#define TCAN4X5X_MCAN_IR_TEFL BIT(15)
>> +#define TCAN4X5X_MCAN_IR_TEFF BIT(14)
>> +#define TCAN4X5X_MCAN_IR_TEFW BIT(13)
>> +#define TCAN4X5X_MCAN_IR_TEFN BIT(12)
>> +#define TCAN4X5X_MCAN_IR_TFE BIT(11)
>> +#define TCAN4X5X_MCAN_IR_TCF BIT(10)
>> +#define TCAN4X5X_MCAN_IR_TC BIT(9)
>> +#define TCAN4X5X_MCAN_IR_HPM BIT(8)
>> +#define TCAN4X5X_MCAN_IR_RF1L BIT(7)
>> +#define TCAN4X5X_MCAN_IR_RF1F BIT(6)
>> +#define TCAN4X5X_MCAN_IR_RF1W BIT(5)
>> +#define TCAN4X5X_MCAN_IR_RF1N BIT(4)
>> +#define TCAN4X5X_MCAN_IR_RF0L BIT(3)
>> +#define TCAN4X5X_MCAN_IR_RF0F BIT(2)
>> +#define TCAN4X5X_MCAN_IR_RF0W BIT(1)
>> +#define TCAN4X5X_MCAN_IR_RF0N BIT(0)
>
> These bits are already defined in the common header file.
>
These are TCAN specific interrupt enable bits there are not in the Bosch register set
>> +#define TCAN4X5X_ENABLE_MCAN_INT (TCAN4X5X_MCAN_IR_TC | \
>> + TCAN4X5X_MCAN_IR_RF0N | \
>> + TCAN4X5X_MCAN_IR_RF1N | \
>> + TCAN4X5X_MCAN_IR_RF0F | \
>> + TCAN4X5X_MCAN_IR_RF1F)
>> +#define TCAN4X5X_MRAM_START 0x8000
>> +#define TCAN4X5X_MCAN_OFFSET 0x1000
>> +#define TCAN4X5X_MAX_REGISTER 0x8fff
>> +
>> +#define TCAN4X5X_CLEAR_ALL_INT 0xffffffff
>> +#define TCAN4X5X_SET_ALL_INT 0xffffffff
>> +
>> +#define TCAN4X5X_WRITE_CMD (0x61 << 24)
>> +#define TCAN4X5X_READ_CMD (0x41 << 24)
>> +
>> +#define TCAN4X5X_MODE_SEL_MASK (BIT(7) | BIT(6))
>> +#define TCAN4X5X_MODE_SLEEP 0x00
>> +#define TCAN4X5X_MODE_STANDBY BIT(6)
>> +#define TCAN4X5X_MODE_NORMAL BIT(7)
>> +
>> +#define TCAN4X5X_SW_RESET BIT(2)
>> +
>> +#define TCAN4X5X_MCAN_CONFIGURED BIT(5)
>> +#define TCAN4X5X_WATCHDOG_EN BIT(3)
>> +#define TCAN4X5X_WD_60_MS_TIMER 0
>> +#define TCAN4X5X_WD_600_MS_TIMER BIT(28)
>> +#define TCAN4X5X_WD_3_S_TIMER BIT(29)
>> +#define TCAN4X5X_WD_6_S_TIMER (BIT(28) | BIT(29))
>> +
>> +struct tcan4x5x_priv {
>> + struct regmap *regmap;
>> + struct spi_device *spi;
>> + struct mutex tcan4x5x_lock; /* SPI device lock */
>> +
>> + struct m_can_classdev *mcan_dev;
>> +
>> + struct gpio_desc *reset_gpio;
>> + struct gpio_desc *interrupt_gpio;
>> + struct gpio_desc *device_wake_gpio;
>> + struct gpio_desc *device_state_gpio;
>> + struct regulator *power;
>> +
>> + /* Register based ip */
>> + int mram_start;
>> + int reg_offset;
>> +};
>> +
>> +static struct can_bittiming_const tcan4x5x_bittiming_const = {
>> + .name = DEVICE_NAME,
>> + .tseg1_min = 2,
>> + .tseg1_max = 31,
>> + .tseg2_min = 2,
>> + .tseg2_max = 16,
>> + .sjw_max = 16,
>> + .brp_min = 1,
>> + .brp_max = 32,
>> + .brp_inc = 1,
>> +};
>> +
>> +static struct can_bittiming_const tcan4x5x_data_bittiming_const = {
>> + .name = DEVICE_NAME,
>> + .tseg1_min = 1,
>> + .tseg1_max = 32,
>> + .tseg2_min = 1,
>> + .tseg2_max = 16,
>> + .sjw_max = 16,
>> + .brp_min = 1,
>> + .brp_max = 32,
>> + .brp_inc = 1,
>> +};
>> +
>> +static void tcan4x5x_check_wake(struct tcan4x5x_priv *priv)
>> +{
>> + int wake_state = 0;
>> +
>> + if (priv->device_state_gpio)
>> + wake_state = gpiod_get_value(priv->device_state_gpio);
>> +
>> + if (priv->device_wake_gpio && wake_state) {
>> + gpiod_set_value(priv->device_wake_gpio, 1);
>> + udelay(100);
>> + gpiod_set_value(priv->device_wake_gpio, 0);
>> + udelay(100);
>> + gpiod_set_value(priv->device_wake_gpio, 1);
>> + }
>> +}
>> +
>> +static int regmap_spi_gather_write(void *context, const void *reg,
>> + size_t reg_len, const void *val,
>> + size_t val_len)
>> +{
>> + struct device *dev = context;
>> + struct spi_device *spi = to_spi_device(dev);
>> + struct spi_message m;
>> + u32 addr;
>> + struct spi_transfer t[2] = {{ .tx_buf = &addr, .len = reg_len, .cs_change = 0,},
>> + { .tx_buf = val, .len = val_len, },};
>> +
>> + addr = TCAN4X5X_WRITE_CMD | (*((u16 *)reg) << 8) | val_len >> 3;
>> +
>> + spi_message_init(&m);
>> + spi_message_add_tail(&t[0], &m);
>> + spi_message_add_tail(&t[1], &m);
>> +
>> + return spi_sync(spi, &m);
>> +}
>> +
>> +static int tcan4x5x_regmap_write(void *context, const void *data, size_t count)
>> +{
>> + u16 *reg = (u16 *)(data);
>> + const u32 *val = data + 4;
>> +
>> + return regmap_spi_gather_write(context, reg, 4, val, count);
>> +}
>> +
>> +static int regmap_spi_async_write(void *context,
>> + const void *reg, size_t reg_len,
>> + const void *val, size_t val_len,
>> + struct regmap_async *a)
>> +{
>> + return -ENOTSUPP;
>> +}
>> +
>> +static struct regmap_async *regmap_spi_async_alloc(void)
>> +{
>> + return NULL;
>> +}
>> +
>> +static int tcan4x5x_regmap_read(void *context,
>> + const void *reg, size_t reg_size,
>> + void *val, size_t val_size)
>> +{
>> + struct device *dev = context;
>> + struct spi_device *spi = to_spi_device(dev);
>> +
>> + u32 addr = TCAN4X5X_READ_CMD | (*((u16 *)reg) << 8) | val_size >> 2;
>> +
>> + return spi_write_then_read(spi, &addr, reg_size, (u32 *)val, val_size);
>> +}
>> +
>> +static struct regmap_bus tcan4x5x_bus = {
>> + .write = tcan4x5x_regmap_write,
>> + .gather_write = regmap_spi_gather_write,
>> + .async_write = regmap_spi_async_write,
>> + .async_alloc = regmap_spi_async_alloc,
>> + .read = tcan4x5x_regmap_read,
>> + .read_flag_mask = 0x00,
>> + .reg_format_endian_default = REGMAP_ENDIAN_NATIVE,
>> + .val_format_endian_default = REGMAP_ENDIAN_NATIVE,
>> +};
>> +
>> +static u32 tcan4x5x_read_reg(struct m_can_classdev *m_can_class, int reg)
>> +{
>> + struct tcan4x5x_priv *priv = (struct tcan4x5x_priv *)m_can_class->device_data;
>> + u32 val;
>> +
>> + tcan4x5x_check_wake(priv);
>> +
>> + regmap_read(priv->regmap, priv->reg_offset + reg, &val);
>> +
>> + return val;
>> +}
>> +
>> +static u32 tcan4x5x_read_fifo(struct m_can_classdev *m_can_class,
>> + int addr_offset)
>> +{
>> + struct tcan4x5x_priv *priv = (struct tcan4x5x_priv *)m_can_class->device_data;
>> + u32 val;
>> +
>> + tcan4x5x_check_wake(priv);
>> +
>> + regmap_read(priv->regmap, priv->mram_start + addr_offset, &val);
>> +
>> + return val;
>> +}
>> +
>> +static int tcan4x5x_write_reg(struct m_can_classdev *m_can_class,
>> + int reg, int val)
>> +{
>> + struct tcan4x5x_priv *priv = (struct tcan4x5x_priv *)m_can_class->device_data;
>> +
>> + tcan4x5x_check_wake(priv);
>> +
>> + return regmap_write(priv->regmap, priv->reg_offset + reg, val);
>> +}
>> +
>> +static int tcan4x5x_write_fifo(struct m_can_classdev *m_can_class,
>> + int addr_offset, int val)
>> +{
>> + struct tcan4x5x_priv *priv = (struct tcan4x5x_priv *)m_can_class->device_data;
>> +
>> + tcan4x5x_check_wake(priv);
>> +
>> + return regmap_write(priv->regmap, priv->mram_start + addr_offset, val);
>> +}
>> +
>> +static int tcan4x5x_power_enable(struct regulator *reg, int enable)
>> +{
>> + if (IS_ERR_OR_NULL(reg))
>> + return 0;
>> +
>> + if (enable)
>> + return regulator_enable(reg);
>> + else
>> + return regulator_disable(reg);
>> +}
>> +
>> +static int tcan4x5x_write_tcan_reg(struct m_can_classdev *m_can_class,
>> + int reg, int val)
>> +{
>> + struct tcan4x5x_priv *priv = (struct tcan4x5x_priv *)m_can_class->device_data;
>> +
>> + tcan4x5x_check_wake(priv);
>> +
>> + return regmap_write(priv->regmap, reg, val);
>> +}
>> +
>> +static int tcan4x5x_clear_interrupts(struct m_can_classdev *class_dev)
>> +{
>> + struct tcan4x5x_priv *tcan4x5x = (struct tcan4x5x_priv *)class_dev->device_data;
>> + int ret;
>> +
>> + tcan4x5x_check_wake(tcan4x5x);
>> +
>> + ret = tcan4x5x_write_tcan_reg(class_dev, TCAN4X5X_STATUS,
>> + TCAN4X5X_CLEAR_ALL_INT);
>> + if (ret)
>> + return -EIO;
>> +
>> + ret = tcan4x5x_write_tcan_reg(class_dev, TCAN4X5X_MCAN_INT_REG,
>> + TCAN4X5X_ENABLE_MCAN_INT);
>> + if (ret)
>> + return -EIO;
>> +
>> + ret = tcan4x5x_write_tcan_reg(class_dev, TCAN4X5X_INT_FLAGS,
>> + TCAN4X5X_CLEAR_ALL_INT);
>> + if (ret)
>> + return -EIO;
>> +
>> +
>> + ret = tcan4x5x_write_tcan_reg(class_dev, TCAN4X5X_ERROR_STATUS,
>> + TCAN4X5X_CLEAR_ALL_INT);
>> + if (ret)
>> + return -EIO;
>> +
>> + return ret;
>> +}
>> +
>> +static int tcan4x5x_init(struct m_can_classdev *class_dev)
>> +{
>> + struct tcan4x5x_priv *tcan4x5x = (struct tcan4x5x_priv *)class_dev->device_data;
>> + int ret;
>> +
>> + tcan4x5x_check_wake(tcan4x5x);
>> +
>> + ret = tcan4x5x_clear_interrupts(class_dev);
>> + if (ret)
>> + return ret;
>> +
>> + ret = tcan4x5x_write_tcan_reg(class_dev, TCAN4X5X_INT_EN,
>> + TCAN4X5X_ENABLE_TCAN_INT);
>> + if (ret)
>> + return -EIO;
>> +
>> + ret = regmap_update_bits(tcan4x5x->regmap, TCAN4X5X_CONFIG,
>> + TCAN4X5X_MODE_SEL_MASK, TCAN4X5X_MODE_NORMAL);
>> + if (ret)
>> + return -EIO;
>> +
>> + /* Zero out the MCAN buffers */
>> + m_can_init_ram(class_dev);
>> +
>> + return ret;
>> +}
>> +
>> +static int tcan4x5x_parse_config(struct m_can_classdev *class_dev)
>> +{
>> + struct tcan4x5x_priv *tcan4x5x = (struct tcan4x5x_priv *)class_dev->device_data;
>> +
>> + tcan4x5x->reset_gpio = devm_gpiod_get_optional(class_dev->dev,
>> + "reset", GPIOD_OUT_LOW);
>> + if (IS_ERR(tcan4x5x->reset_gpio))
>> + tcan4x5x->reset_gpio = NULL;
>> +
>> + tcan4x5x->device_wake_gpio = devm_gpiod_get_optional(class_dev->dev,
>> + "device-wake",
>> + GPIOD_OUT_HIGH);
>> + if (IS_ERR(tcan4x5x->device_wake_gpio))
>> + tcan4x5x->device_wake_gpio = NULL;
>> +
>> + tcan4x5x->device_state_gpio = devm_gpiod_get_optional(class_dev->dev,
>> + "device-state",
>> + GPIOD_IN);
>> + if (IS_ERR(tcan4x5x->device_state_gpio))
>> + tcan4x5x->device_state_gpio = NULL;
>> +
>> + tcan4x5x->interrupt_gpio = devm_gpiod_get(class_dev->dev,
>> + "data-ready", GPIOD_IN);
>> + if (IS_ERR(tcan4x5x->interrupt_gpio)) {
>> + dev_err(class_dev->dev, "data-ready gpio not defined\n");
>> + return -EINVAL;
>> + }
>> +
>> + class_dev->net->irq = gpiod_to_irq(tcan4x5x->interrupt_gpio);
>> +
>> + tcan4x5x->power = devm_regulator_get_optional(class_dev->dev,
>> + "vsup");
>> + if (PTR_ERR(tcan4x5x->power) == -EPROBE_DEFER)
>> + return -EPROBE_DEFER;
>> +
>> + return 0;
>> +}
>> +
>> +static const struct regmap_config tcan4x5x_regmap = {
>> + .reg_bits = 32,
>> + .val_bits = 32,
>> + .cache_type = REGCACHE_NONE,
>> + .max_register = TCAN4X5X_MAX_REGISTER,
>> +};
>> +
>> +static int tcan4x5x_can_probe(struct spi_device *spi)
>> +{
>> + struct tcan4x5x_priv *priv;
>> + struct m_can_classdev *mcan_class;
>> + int freq, ret;
>> +
>> + mcan_class = m_can_core_allocate_dev(&spi->dev);
>> + priv = devm_kzalloc(&spi->dev, sizeof(*priv), GFP_KERNEL);
>> + if (!priv)
>> + return -ENOMEM;
>> +
>> + mcan_class->device_data = priv;
>> +
>> + m_can_core_get_clocks(mcan_class);
>> + if (IS_ERR(mcan_class->cclk)) {
>> + dev_err(&spi->dev, "no CAN clock source defined\n");
>> + freq = TCAN4X5X_EXT_CLK_DEF;
>> + } else {
>> + freq = clk_get_rate(mcan_class->cclk);
>> + }
>> +
>> + /* Sanity check */
>> + if (freq < 20000000 || freq > TCAN4X5X_EXT_CLK_DEF)
>> + return -ERANGE;
>> +
>> + priv->reg_offset = TCAN4X5X_MCAN_OFFSET;
>> + priv->mram_start = TCAN4X5X_MRAM_START;
>> + priv->spi = spi;
>> + priv->mcan_dev = mcan_class;
>> +
>> + mcan_class->pm_clock_support = 0;
>> + mcan_class->can.clock.freq = freq;
>> + mcan_class->dev = &spi->dev;
>> +
>> + mcan_class->device_init = &tcan4x5x_init;
>> + mcan_class->read_reg = &tcan4x5x_read_reg;
>> + mcan_class->write_reg = &tcan4x5x_write_reg;
>> + mcan_class->write_fifo = &tcan4x5x_write_fifo;
>> + mcan_class->read_fifo = &tcan4x5x_read_fifo;
>> + mcan_class->clr_dev_interrupts = &tcan4x5x_clear_interrupts;
>> + mcan_class->is_peripherial = true;
>> +
>> + mcan_class->bit_timing = &tcan4x5x_bittiming_const;
>> + mcan_class->data_timing = &tcan4x5x_data_bittiming_const;
>> +
>> + spi_set_drvdata(spi, priv);
>> +
>> + ret = tcan4x5x_parse_config(mcan_class);
>> + if (ret)
>> + goto out_clk;
>> +
>> + /* Configure the SPI bus */
>> + spi->bits_per_word = 32;
>> + ret = spi_setup(spi);
>> + if (ret)
>> + goto out_clk;
>> +
>> + priv->regmap = devm_regmap_init(&spi->dev, &tcan4x5x_bus,
>> + &spi->dev, &tcan4x5x_regmap);
>> +
>> + mutex_init(&priv->tcan4x5x_lock);
>> +
>> + tcan4x5x_power_enable(priv->power, 1);
>> +
>> + ret = m_can_core_register(mcan_class);
>> + if (ret)
>> + goto reg_err;
>> +
>> + netdev_info(mcan_class->net, "TCAN4X5X successfully initialized.\n");
>> + return 0;
>> +
>> +reg_err:
>> + tcan4x5x_power_enable(priv->power, 0);
>> +out_clk:
>> + if (!IS_ERR(mcan_class->cclk)) {
>> + clk_disable_unprepare(mcan_class->cclk);
>> + clk_disable_unprepare(mcan_class->hclk);
>> + }
>> +
>> + dev_err(&spi->dev, "Probe failed, err=%d\n", -ret);
>> + return ret;
>> +}
>> +
>> +static int tcan4x5x_can_remove(struct spi_device *spi)
>> +{
>> + struct tcan4x5x_priv *priv = spi_get_drvdata(spi);
>> +
>> + tcan4x5x_power_enable(priv->power, 0);
>> +
>> + m_can_core_unregister(priv->mcan_dev);
>> +
>> + return 0;
>> +}
>> +
>> +static const struct of_device_id tcan4x5x_of_match[] = {
>> + { .compatible = "ti,tcan4x5x", },
>> + { }
>> +};
>> +MODULE_DEVICE_TABLE(of, tcan4x5x_of_match);
>> +
>> +static const struct spi_device_id tcan4x5x_id_table[] = {
>> + {
>> + .name = "tcan4x5x",
>> + .driver_data = 0,
>> + },
>> + { }
>> +};
>> +MODULE_DEVICE_TABLE(spi, tcan4x5x_id_table);
>> +
>> +static struct spi_driver tcan4x5x_can_driver = {
>> + .driver = {
>> + .name = DEVICE_NAME,
>> + .of_match_table = tcan4x5x_of_match,
>> + .pm = NULL,
>> + },
>> + .id_table = tcan4x5x_id_table,
>> + .probe = tcan4x5x_can_probe,
>> + .remove = tcan4x5x_can_remove,
>> +};
>> +module_spi_driver(tcan4x5x_can_driver);
>> +
>> +MODULE_AUTHOR("Dan Murphy <dmurphy@ti.com>");
>> +MODULE_DESCRIPTION("Texas Instruments TCAN4x5x CAN driver");
>> +MODULE_LICENSE("GPL v2");
>
> Curious to hear about the performance of M_CAN connected via SPI. Does
> it miss or drop messages?
>
Here is some cangen data we have.
We can probably speed it up but we want to get the code functional first.
/home/root/can/ip link set can0 up type can bitrate 1000000 dbitrate 10000000 fd on
root@am335x-evm:~/can# cat /proc/net/can/stats
5681 transmitted frames (TXF)
5681 received frames (RXF)
0 matched frames (RXMF)
0 % total match ratio (RXMR)
167 frames/s total tx rate (TXR)
167 frames/s total rx rate (RXR)
0 % current match ratio (CRXMR)
0 frames/s current tx rate (CTXR)
0 frames/s current rx rate (CRXR)
0 % max match ratio (MRXMR)
224 frames/s max tx rate (MTXR)
222 frames/s max rx rate (MRXR)
> Thanks for your patience,
>
> Wolfgang.
>
--
------------------
Dan Murphy
^ permalink raw reply
* Re: tc filter insertion rate degradation
From: Vlad Buslov @ 2019-01-29 19:22 UTC (permalink / raw)
To: Dennis Zhou
Cc: Eric Dumazet, Tejun Heo, Linux Kernel Network Developers,
Yevgeny Kliteynik, Yossef Efraim, Maor Gottlieb
In-Reply-To: <20190124172126.GA66944@dennisz-mbp.dhcp.thefacebook.com>
On Thu 24 Jan 2019 at 17:21, Dennis Zhou <dennis@kernel.org> wrote:
> Hi Vlad and Eric,
>
> On Tue, Jan 22, 2019 at 09:33:10AM -0800, Eric Dumazet wrote:
>> On Mon, Jan 21, 2019 at 3:24 AM Vlad Buslov <vladbu@mellanox.com> wrote:
>> >
>> > Hi Eric,
>> >
>> > I've been investigating significant tc filter insertion rate degradation
>> > and it seems it is caused by your commit 001c96db0181 ("net: align
>> > gnet_stats_basic_cpu struct"). With this commit insertion rate is
>> > reduced from ~65k rules/sec to ~43k rules/sec when inserting 1m rules
>> > from file in tc batch mode on my machine.
>> >
>> > Tc perf profile indicates that pcpu allocator now consumes 2x CPU:
>> >
>> > 1) Before:
>> >
>> > Samples: 63K of event 'cycles:ppp', Event count (approx.): 48796480071
>> > Children Self Co Shared Object Symbol
>> > + 21.19% 3.38% tc [kernel.vmlinux] [k] pcpu_alloc
>> > + 3.45% 0.25% tc [kernel.vmlinux] [k] pcpu_alloc_area
>> >
>> > 2) After:
>> >
>> > Samples1: 92K of event 'cycles:ppp', Event count (approx.): 71446806550
>> > Children Self Co Shared Object Symbol
>> > + 44.67% 3.99% tc [kernel.vmlinux] [k] pcpu_alloc
>> > + 19.25% 0.22% tc [kernel.vmlinux] [k] pcpu_alloc_area
>> >
>> > It seems that it takes much more work for pcpu allocator to perform
>> > allocation with new stricter alignment requirements. Not sure if it is
>> > expected behavior or not in this case.
>> >
>> > Regards,
>> > Vlad
>
> Would you mind sharing a little more information with me:
> 1) output before and after a run of /sys/kernel/debug/percpu_stats
Hi Dennis,
Some of these files are quite large, so I put them to my Dropbox.
Output before:
Percpu Memory Statistics
Allocation Info:
----------------------------------------
unit_size : 262144
static_size : 139160
reserved_size : 8192
dyn_size : 28776
atom_size : 2097152
alloc_size : 2097152
Global Stats:
----------------------------------------
nr_alloc : 3343
nr_dealloc : 752
nr_cur_alloc : 2591
nr_max_alloc : 2598
nr_chunks : 3
nr_max_chunks : 3
min_alloc_size : 4
max_alloc_size : 8208
empty_pop_pages : 3
Per Chunk Stats:
----------------------------------------
Chunk: <- Reserved Chunk
nr_alloc : 5
max_alloc_size : 320
empty_pop_pages : 0
first_bit : 1002
free_bytes : 7448
contig_bytes : 7424
sum_frag : 24
max_frag : 24
cur_min_alloc : 16
cur_med_alloc : 64
cur_max_alloc : 320
Chunk: <- First Chunk
nr_alloc : 479
max_alloc_size : 8208
empty_pop_pages : 0
first_bit : 8192
free_bytes : 0
contig_bytes : 0
sum_frag : 0
max_frag : 0
cur_min_alloc : 4
cur_med_alloc : 24
cur_max_alloc : 8208
Chunk:
nr_alloc : 1925
max_alloc_size : 8208
empty_pop_pages : 0
first_bit : 63102
free_bytes : 852
contig_bytes : 12
sum_frag : 852
max_frag : 12
cur_min_alloc : 4
cur_med_alloc : 8
cur_max_alloc : 8208
Chunk:
nr_alloc : 182
max_alloc_size : 936
empty_pop_pages : 3
first_bit : 21
free_bytes : 256452
contig_bytes : 255120
sum_frag : 1332
max_frag : 368
cur_min_alloc : 8
cur_med_alloc : 20
cur_max_alloc : 320
After: https://www.dropbox.com/s/unyzhx4vgo2x30e/stats_after?dl=0
> 2) a full perf output
https://www.dropbox.com/s/isfcxca3npn5slx/perf.data?dl=0
> 3) a reproducer
$ sudo tc -b add.0
Example batch file: https://www.dropbox.com/s/ey7cbl5nwu5p0tg/add.0?dl=0
Thanks,
Vlad
^ permalink raw reply
* Re: [PATCH net] ixgbe: fix potential RX buffer starvation for AF_XDP
From: Jakub Kicinski @ 2019-01-29 19:14 UTC (permalink / raw)
To: Magnus Karlsson; +Cc: bjorn.topel, intel-wired-lan, netdev
In-Reply-To: <1548770630-16189-1-git-send-email-magnus.karlsson@intel.com>
On Tue, 29 Jan 2019 15:03:50 +0100, Magnus Karlsson wrote:
> When the RX rings are created they are also populated with buffers so
> that packets can be received. Usually these are kernel buffers, but
> for AF_XDP in zero-copy mode, these are user-space buffers and in this
> case the application might not have sent down any buffers to the
> driver at this point. And if no buffers are allocated at ring creation
> time, no packets can be received and no interupts will be generated so
> the napi poll function that allocates buffers to the rings will never
> get executed.
>
> To recitfy this, we kick the NAPI context of any queue with an
> attached AF_XDP zero-copy socket in two places in the code. Once after
> an XDP program has loaded and once after the umem is registered. This
> take care of both cases: XDP program gets loaded first then AF_XDP
> socket is created, and the reverse, AF_XDP socket is created first,
> then XDP program is loaded.
>
> Fixes: d0bcacd0a130 ("ixgbe: add AF_XDP zero-copy Rx support")
> Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
I may not understand the problem fully, but isn't it kind of normal
that if you create a ring empty you'll never receive packets? And it
should be reasonably easy to catch while writing an app from scratch
(i.e. it behaves deterministically).
Plus user space can already do the kick manually: create the ring empty,
attach prog, put packets on the ring, and kick xmit - that would work,
no?
Putting the kick in ixgbe_xdp_setup() seems a tiny bit random to me,
but perhaps I'm not seeing the rationale clearly.
^ permalink raw reply
* Re: WoL broken in r8169.c since kernel 4.19
From: Heiner Kallweit @ 2019-01-29 19:01 UTC (permalink / raw)
To: Marc Haber; +Cc: netdev@vger.kernel.org
In-Reply-To: <20190129153553.GL27062@torres.zugschlus.de>
Hi Marc,
the change to replace __rtl8169_set_wol(tp, 0) doesn't seem to be the right commit
because it was included in 4.18 already. And if you read the commit description you'll
see that it was replaced because it caused issues with certain boards. Having said that
it's not an option for us.
Still I'm struggling to see where the relevant difference between 4.18 and 4.19 is.
Especially as 4.19 and also later versions work perfectly fine here.
Can you in addition apply the following (again it may not apply cleanly) and provide
the results for 4.18 and 4.19?
And from today's run, can you provide the full dmesg output? I'd like to check
why the message was written on resume only.
Heiner
---
drivers/net/ethernet/realtek/r8169.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index bd26d3f2e..e9c37f10c 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -1414,6 +1414,8 @@ static void rtl8169_get_wol(struct net_device *dev, struct ethtool_wolinfo *wol)
wol->supported = WAKE_ANY;
wol->wolopts = tp->saved_wolopts;
rtl_unlock_work(tp);
+
+ pr_info("get_wol: 0x%08x\n", wol->wolopts);
}
static void __rtl8169_set_wol(struct rtl8169_private *tp, u32 wolopts)
@@ -1491,6 +1493,8 @@ static int rtl8169_set_wol(struct net_device *dev, struct ethtool_wolinfo *wol)
struct rtl8169_private *tp = netdev_priv(dev);
struct device *d = tp_to_dev(tp);
+ pr_info("set_wol: 0x%08x\n", wol->wolopts);
+
if (wol->wolopts & ~WAKE_ANY)
return -EINVAL;
--
2.20.1
On 29.01.2019 16:35, Marc Haber wrote:
> Hi,
>
> after having a good night's sleep over that, it's obviously a merge
> commit which cannot easily be reverted. How would I continue after
> identifying a merge commit as the culprit?
>
> On Tue, Jan 29, 2019 at 08:32:53AM +0100, Marc Haber wrote:
>> According to bisect, the first bad commit is
>> 19725496da5602b401eae389736ab00d1817e264
>>
>> commit 19725496da5602b401eae389736ab00d1817e264
>> Merge: aea5f654e6b7 9981b4fb8684
>
> git diff aea5f654e6b7..19725496da5602b401eae389736ab00d1817e264,
> filtered for r8169 looks manageable:
>
> --- a/drivers/net/ethernet/realtek/r8169.c
> +++ b/drivers/net/ethernet/realtek/r8169.c
> @@ -7396,8 +7396,7 @@ static int rtl_init_one(struct pci_dev *pdev, const struc
> return rc;
> }
>
> - /* override BIOS settings, use userspace tools to enable WOL */
> - __rtl8169_set_wol(tp, 0);
> + tp->saved_wolopts = __rtl8169_get_wol(tp);
>
> mutex_init(&tp->wk.mutex);
> u64_stats_init(&tp->rx_stats.syncp);
>
> but the other one seems unmanageably big:
>
> [18/5009]mh@fan:~/linux/git/linux (master % u=) $ git diff 9981b4fb8684..19725496da5602b401eae389736ab00d1817e264 -- drivers/net/ethernet/realtek/r8169.c | diffstat
> r8169.c | 815 ++++++++++++++++++----------------------------------------------
> 1 file changed, 234 insertions(+), 581 deletions(-)
> [19/5009]mh@fan:~/linux/git/linux (master % u=) $
>
> -------
> But, indeed, adding the call to __rtl8169_set_wol(tp, 0) fixes the issue
> for me and the machine now wakes up from StR on a magic packet without
> having to go through strange ethtool motions.
> -------
>
> Would that code change be suitable for the official kernel cod?
>
> Greetings
> Marc
>
^ permalink raw reply related
* Re: [PATCHv2 net] sctp: check and update stream->out_curr when allocating stream_out
From: Tuxdriver @ 2019-01-29 18:58 UTC (permalink / raw)
To: Marcelo Ricardo Leitner, Xin Long; +Cc: network dev, linux-sctp, davem
In-Reply-To: <20190129120531.GC10665@localhost.localdomain>
I was initially under the impression that with Kent's repost, the radixtree
(which is what I think you meant by rhashtables) updates would be merged
imminently, but that doesn't seem to be the case. I'd really like to know
what the hold up there is, as that patch seems to have been stalled for
months. I hate the notion of breaking the radixtree patch, but if it's
status is indeterminate, then, yes, we probably need to go with xins patch
for the short term, and let Kent fix it up in due course.
Neil
On January 29, 2019 1:06:33 PM Marcelo Ricardo Leitner
<marcelo.leitner@gmail.com> wrote:
> On Thu, Nov 29, 2018 at 02:42:56PM +0800, Xin Long wrote:
>> Now when using stream reconfig to add out streams, stream->out
>> will get re-allocated, and all old streams' information will
>> be copied to the new ones and the old ones will be freed.
>>
>> So without stream->out_curr updated, next time when trying to
>> send from stream->out_curr stream, a panic would be caused.
>>
>> This patch is to check and update stream->out_curr when
>> allocating stream_out.
>>
>> v1->v2:
>> - define fa_index() to get elem index from stream->out_curr.
>>
>> Fixes: 5bbbbe32a431 ("sctp: introduce stream scheduler foundations")
>> Reported-by: Ying Xu <yinxu@redhat.com>
>> Reported-by: syzbot+e33a3a138267ca119c7d@syzkaller.appspotmail.com
>> Signed-off-by: Xin Long <lucien.xin@gmail.com>
>
> We are sort of mixing things up here. We have a bug on SCTP stack that
> triggers panics. As good practices recommends, the code should be as
> generic as possible and the SCTP-only was dropped in favor of a more
> generic one, fixing rhashtables instead. Okay. But then we discovered
> rhashtables are going away and we are now waiting on a restructing
> to fix the panic. That's not good, especially because it cannot and
> should not be backported into -stable trees.
>
> That said, we should not wait for the restructuring to _implicitly_
> fix the bug. We should pursuit both fixes here:
> - Apply this patch, to fix SCTP stack and allow it to be easily
> backportable.
> - Apply the generic fix, which is the restructuring, whenever it
> actually lands.
>
> Thoughts?
>
> Thanks,
> Marcelo
Sent with AquaMail for Android
https://www.mobisystems.com/aqua-mail
^ permalink raw reply
* Re: [PATCH net] net: tls: Save iv in tls_rec for async crypto requests
From: David Miller @ 2019-01-29 18:57 UTC (permalink / raw)
To: davejwatson; +Cc: netdev
In-Reply-To: <20190129172215.cnzhluzzqpj3l6sd@davejwatson-mba.local>
From: Dave Watson <davejwatson@fb.com>
Date: Tue, 29 Jan 2019 17:21:41 +0000
> I'd like to merge TLS1.3 support to net-next, which depends on this
> commit. Can we get a net->net-next merge when convenient? Thanks
Sure thing Dave, I'll let you know when that happens.
^ permalink raw reply
* Re: [PATCH net-next] cxgb4: cxgb4_tc_u32: use struct_size() in kvzalloc()
From: David Miller @ 2019-01-29 18:57 UTC (permalink / raw)
To: gustavo; +Cc: arjun, netdev, linux-kernel
In-Reply-To: <20190129170523.GA29824@embeddedor>
From: "Gustavo A. R. Silva" <gustavo@embeddedor.com>
Date: Tue, 29 Jan 2019 11:05:23 -0600
> One of the more common cases of allocation size calculations is finding the
> size of a structure that has a zero-sized array at the end, along with memory
> for some number of elements for that array. For example:
>
> struct foo {
> int stuff;
> struct boo entry[];
> };
>
> instance = kvzalloc(sizeof(struct foo) + count * sizeof(struct boo), GFP_KERNEL);
>
> Instead of leaving these open-coded and prone to type mistakes, we can now
> use the new struct_size() helper:
>
> instance = kvzalloc(struct_size(instance, entry, count), GFP_KERNEL);
>
> This code was detected with the help of Coccinelle.
>
> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH net-next] cxgb4: clip_tbl: Use struct_size() in kvzalloc()
From: David Miller @ 2019-01-29 18:57 UTC (permalink / raw)
To: gustavo; +Cc: arjun, netdev, linux-kernel
In-Reply-To: <20190129164444.GA27988@embeddedor>
From: "Gustavo A. R. Silva" <gustavo@embeddedor.com>
Date: Tue, 29 Jan 2019 10:44:44 -0600
> One of the more common cases of allocation size calculations is finding the
> size of a structure that has a zero-sized array at the end, along with memory
> for some number of elements for that array. For example:
>
> struct foo {
> int stuff;
> struct boo entry[];
> };
>
> instance = kvzalloc(sizeof(struct foo) + count * sizeof(struct boo), GFP_KERNEL);
>
> Instead of leaving these open-coded and prone to type mistakes, we can now
> use the new struct_size() helper:
>
> instance = kvzalloc(struct_size(instance, entry, count), GFP_KERNEL);
>
> This code was detected with the help of Coccinelle.
>
> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
Applied.
^ permalink raw reply
* Re: [PATCH] mwifiex: don't print error message on coex event
From: Brian Norris @ 2019-01-29 18:52 UTC (permalink / raw)
To: Stefan Agner
Cc: amit karwar, Nishant Sarmukadam, Ganapathi Bhat, huxinming820,
Kalle Valo, davem, linux-wireless, netdev, Linux Kernel
In-Reply-To: <20190128154310.17322-1-stefan@agner.ch>
On Mon, Jan 28, 2019 at 7:43 AM Stefan Agner <stefan@agner.ch> wrote:
>
> The BT coex event is not an error condition. Don't print an error
> message in this case. The same even in sta_event.c prints a
> message using the debug level already.
For some reason, that one (and a handful of others) uses dev_dbg()
instead of mwifiex_dbg() with an appropriate mask (e.g., EVENT). But
mwifiex_dbg() seems preferable for consistency.
> Signed-off-by: Stefan Agner <stefan@agner.ch>
Reviewed-by: Brian Norris <briannorris@chromium.org>
> ---
> drivers/net/wireless/marvell/mwifiex/uap_event.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/marvell/mwifiex/uap_event.c b/drivers/net/wireless/marvell/mwifiex/uap_event.c
> index e86217a6b9ca..ca759d9c0253 100644
> --- a/drivers/net/wireless/marvell/mwifiex/uap_event.c
> +++ b/drivers/net/wireless/marvell/mwifiex/uap_event.c
> @@ -300,7 +300,7 @@ int mwifiex_process_uap_event(struct mwifiex_private *priv)
> mwifiex_11h_handle_radar_detected(priv, adapter->event_skb);
> break;
> case EVENT_BT_COEX_WLAN_PARA_CHANGE:
> - dev_err(adapter->dev, "EVENT: BT coex wlan param update\n");
> + mwifiex_dbg(adapter, EVENT, "event: BT coex wlan param update\n");
> mwifiex_bt_coex_wlan_param_update_event(priv,
> adapter->event_skb);
> break;
> --
> 2.20.1
>
^ permalink raw reply
* Re: [PATCH] r8169: Load MAC address from device tree if present
From: Heiner Kallweit @ 2019-01-29 18:51 UTC (permalink / raw)
To: Thierry Reding
Cc: David S. Miller, Realtek linux nic maintainers, netdev,
devicetree, linux-kernel
In-Reply-To: <20190129174035.GA25929@ulmo>
On 29.01.2019 18:40, Thierry Reding wrote:
> On Fri, Jan 25, 2019 at 07:34:31PM +0100, Heiner Kallweit wrote:
>> On 25.01.2019 11:18, Thierry Reding wrote:
>>> From: Thierry Reding <treding@nvidia.com>
>>>
>>> If the system was booted using a device tree and if the device tree
>>> contains a MAC address, use it instead of reading one from the EEPROM.
>>> This is useful in situations where the EEPROM isn't properly programmed
>>> or where the firmware wants to override the existing MAC address.
>>>
>> I rarely see DT-configured boards with RTL8168 network. Do you add this
>> patch because of a specific board?
>> And you state "if EEPROM isn't properly programmed": Did you come across
>> such a case?
>
> We use these Realtek chips on some of our boards that customers can
> either purchase individually and integrate into their own designs or
> they can get the module as part of a product.
>
> In order to easily allow customers to reprogram the device (they may
> want to do that if integrating the module into their own products), a
> so-called ID EEPROM is part of the module that stores various bits of
> information. The ethernet MAC address is part of that EEPROM.
>
> Typically the ID EEPROM will contain a valid MAC address if the module
> is part of a product, but if customers purchase the module individually,
> it is expected that they use a MAC address from their own pool.
>
> Typically early boot firmware will load the MAC address from the EEPROM
> and store it in the ethernet device's device tree node so that the MAC
> address programmed into the ID EEPROM will be used by the ethernet
> device at runtime.
>
Thanks for the explanation.
>> In general the patch is fine with me, I just want to understand the
>> motivation. One further comment see inline.
>> As of today we already have the option to set a MAC from userspace
>> via ethtool.
>>
>>> Signed-off-by: Thierry Reding <treding@nvidia.com>
>>> ---
>>> Based on net-next.
>>>
>>> drivers/net/ethernet/realtek/r8169.c | 35 +++++++++++++++++-----------
>>> 1 file changed, 22 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
>>> index f574b6b557f9..fd9edd643ca5 100644
>>> --- a/drivers/net/ethernet/realtek/r8169.c
>>> +++ b/drivers/net/ethernet/realtek/r8169.c
>>> @@ -6957,6 +6957,21 @@ static int rtl_alloc_irq(struct rtl8169_private *tp)
>>> return pci_alloc_irq_vectors(tp->pci_dev, 1, 1, flags);
>>> }
>>>
>> [...]
>>> @@ -7252,20 +7268,13 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
>>> u64_stats_init(&tp->rx_stats.syncp);
>>> u64_stats_init(&tp->tx_stats.syncp);
>>>
>>> - /* Get MAC address */
>>> - switch (tp->mac_version) {
>>> - u8 mac_addr[ETH_ALEN] __aligned(4);
>>> - case RTL_GIGA_MAC_VER_35 ... RTL_GIGA_MAC_VER_38:
>>> - case RTL_GIGA_MAC_VER_40 ... RTL_GIGA_MAC_VER_51:
>>> - *(u32 *)&mac_addr[0] = rtl_eri_read(tp, 0xe0, ERIAR_EXGMAC);
>>> - *(u16 *)&mac_addr[4] = rtl_eri_read(tp, 0xe4, ERIAR_EXGMAC);
>>> + /* get MAC address */
>>> + if (eth_platform_get_mac_address(&pdev->dev, mac_addr))
>>> + rtl_read_mac_address(tp, mac_addr);
>>> +
>>> + if (is_valid_ether_addr(mac_addr))
>>
>> Here array mac_addr may be uninitialized (if platform defines no MAC
>> and chip version is not covered by the switch statement).
>
> Good point. I can memset() mac_addr to make sure it is invalid, rather
> than undefined, for chip versions that are not covered.
>
An empty initializer would work as well.
> Thierry
>
Heiner
^ permalink raw reply
* Re: [PATCH 4/7] sh_eth: offload RX checksum on R8A7740
From: Geert Uytterhoeven @ 2019-01-29 18:20 UTC (permalink / raw)
To: Sergei Shtylyov; +Cc: netdev, David S. Miller, Linux-Renesas, Linux-sh list
In-Reply-To: <e43f07fd-1407-a87a-7fe5-67a522da9e19@cogentembedded.com>
Hi Sergei,
On Sun, Jan 27, 2019 at 6:41 PM Sergei Shtylyov
<sergei.shtylyov@cogentembedded.com> wrote:
> The R-Mobile A1 (R8A7740) SoC manual describes the Ether MAC's RX checksum
> offload the same way as it's implemented in the EtherAVB MAC...
>
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Thanks for your patch!
Running netperf as described in patch 2/7, perf tells me there's a reduction
for csum_partial from ca. 1.9% to 0.01%, so this feature seems to work.
Hence:
Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>
However, while effective according to perf results, using ethtool to
enable/disable
the feature prints an error message:
root@armadillo:~# ethtool -K eth0 rx on
Cannot get device udp-fragmentation-offload settings: Operation not supported
Cannot get device udp-fragmentation-offload settings: Operation not supported
root@armadillo:~# ethtool -K eth0 rx off
Cannot get device udp-fragmentation-offload settings: Operation not supported
Cannot get device udp-fragmentation-offload settings: Operation not supported
root@armadillo:~#
Do you have any clue?
Does this needs testing on R-Mobile A1 with VLAN enabled, too, or is that
independent from the underlying sh-eth hardware version?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH] ath10k: snoc: remove set but not used variable 'ar_snoc'
From: Brian Norris @ 2019-01-29 18:12 UTC (permalink / raw)
To: YueHaibing; +Cc: Kalle Valo, ath10k, netdev, linux-wireless, kernel-janitors
In-Reply-To: <4b16caf3-9a25-99d0-5b35-f27d6e5a5c65@huawei.com>
On Mon, Jan 28, 2019 at 9:53 PM YueHaibing <yuehaibing@huawei.com> wrote:
>
> ping...
For some reason, your patch shows up as Deferred in patchwork:
https://patchwork.kernel.org/patch/10589789/
So the maintainers have accidentally (?) ignored it. I'm not what the
official suggestion is for that, but you might just resend.
In any case...
> On 2018/9/6 10:29, YueHaibing wrote:
> > From: Yue Haibing <yuehaibing@huawei.com>
> >
> > Fixes gcc '-Wunused-but-set-variable' warning:
> >
> > drivers/net/wireless/ath/ath10k/snoc.c: In function 'ath10k_snoc_tx_pipe_cleanup':
> > drivers/net/wireless/ath/ath10k/snoc.c:681:22: warning:
> > variable 'ar_snoc' set but not used [-Wunused-but-set-variable]
> >
> > Signed-off-by: Yue Haibing <yuehaibing@huawei.com>
...patch looks fine to me:
Reviewed-by: Brian Norris <briannorris@chromium.org>
^ permalink raw reply
* Re: [RFC PATCH net-next 6/6 v2] net/sched: act_ct: Add tc recirc id set/del support
From: Marcelo Leitner @ 2019-01-29 18:12 UTC (permalink / raw)
To: Paul Blakey
Cc: Guy Shattah, Aaron Conole, John Hurley, Simon Horman,
Justin Pettit, Gregory Rose, Eelco Chaudron, Flavio Leitner,
Florian Westphal, Jiri Pirko, Rashid Khan, Sushil Kulkarni,
Andy Gospodarek, Roi Dayan, Yossi Kuperman, Or Gerlitz,
Rony Efraim, davem@davemloft.net, netdev
In-Reply-To: <1548748926-23822-7-git-send-email-paulb@mellanox.com>
On Tue, Jan 29, 2019 at 10:02:06AM +0200, Paul Blakey wrote:
> Set or clears (free) the skb tc recirc id extension.
> If used with OVS, OVS can clear this recirc id after it reads it.
>
> Signed-off-by: Paul Blakey <paulb@mellanox.com>
> ---
> include/net/tc_act/tc_ct.h | 2 ++
> include/uapi/linux/tc_act/tc_ct.h | 2 ++
> net/sched/act_ct.c | 18 ++++++++++++++++++
> 3 files changed, 22 insertions(+)
>
> diff --git a/include/net/tc_act/tc_ct.h b/include/net/tc_act/tc_ct.h
> index 4a16375..6ea19d8 100644
> --- a/include/net/tc_act/tc_ct.h
> +++ b/include/net/tc_act/tc_ct.h
> @@ -16,6 +16,8 @@ struct tcf_ct {
> u32 mark_mask;
> u16 zone;
> bool commit;
> + uint32_t set_recirc;
> + bool del_recirc;
> };
>
> #define to_ct(a) ((struct tcf_ct *)a)
> diff --git a/include/uapi/linux/tc_act/tc_ct.h b/include/uapi/linux/tc_act/tc_ct.h
> index 6dbd771..2279a9b 100644
> --- a/include/uapi/linux/tc_act/tc_ct.h
> +++ b/include/uapi/linux/tc_act/tc_ct.h
> @@ -14,6 +14,8 @@ struct tc_ct {
> __u32 labels_mask[4];
> __u32 mark;
> __u32 mark_mask;
> + uint32_t set_recirc;
> + bool del_recirc;
> bool commit;
> };
Have you considered adding a specific action for this? Asking because
setting recirc_id can be useful outside of ct context too: decap, set
recirc_id, reclassify.
>
> diff --git a/net/sched/act_ct.c b/net/sched/act_ct.c
> index 61155cc..7822385 100644
> --- a/net/sched/act_ct.c
> +++ b/net/sched/act_ct.c
> @@ -117,6 +117,11 @@ static int tcf_ct_act(struct sk_buff *skb, const struct tc_action *a,
> u_int8_t family;
> bool cached;
>
> + if (ca->del_recirc) {
> + skb_ext_del(skb, SKB_EXT_TC_RECIRC_ID);
> + return ca->tcf_action;
> + }
> +
> /* The conntrack module expects to be working at L3. */
> nh_ofs = skb_network_offset(skb);
> skb_pull_rcsum(skb, nh_ofs);
> @@ -243,6 +248,15 @@ static int tcf_ct_act(struct sk_buff *skb, const struct tc_action *a,
> skb_postpush_rcsum(skb, skb->data, nh_ofs);
>
> spin_unlock(&ca->tcf_lock);
> +
> + if (ca->set_recirc) {
> + u32 recirc = ca->set_recirc;
> + uint32_t *recircp = skb_ext_add(skb, SKB_EXT_TC_RECIRC_ID);
> +
> + if (recircp)
> + *recircp = recirc;
> + }
> +
> return ca->tcf_action;
>
> drop:
> @@ -305,6 +319,8 @@ static int tcf_ct_init(struct net *net, struct nlattr *nla,
> ci->net = net;
> ci->commit = parm->commit;
> ci->zone = parm->zone;
> + ci->set_recirc = parm->set_recirc;
> + ci->del_recirc = parm->del_recirc;
> #if !IS_ENABLED(CONFIG_NF_CONNTRACK_MARK)
> if (parm->mark_mask) {
> NL_SET_ERR_MSG_MOD(extack, "Mark not supported by kernel config");
> @@ -378,6 +394,8 @@ static inline int tcf_ct_dump(struct sk_buff *skb, struct tc_action *a,
> opt.mark_mask = ci->mark_mask,
> memcpy(opt.labels, ci->labels, sizeof(opt.labels));
> memcpy(opt.labels_mask, ci->labels_mask, sizeof(opt.labels_mask));
> + opt.set_recirc = ci->set_recirc;
> + opt.del_recirc = ci->del_recirc;
>
> if (nla_put(skb, TCA_CT_PARMS, sizeof(opt), &opt))
> goto nla_put_failure;
> --
> 1.8.3.1
>
^ permalink raw reply
* Re: [PATCH net] net: b44: replace dev_kfree_skb_xxx by dev_consume_skb_xxx for drop profiles
From: David Miller @ 2019-01-29 18:11 UTC (permalink / raw)
To: albin_yang; +Cc: netdev, michael.chan, yang.wei9
In-Reply-To: <1548774280-5757-1-git-send-email-albin_yang@163.com>
From: Yang Wei <albin_yang@163.com>
Date: Tue, 29 Jan 2019 23:04:40 +0800
> From: Yang Wei <yang.wei9@zte.com.cn>
>
> The skb should be freed by dev_consume_skb_any() in b44_start_xmit()
> when bounce_skb is used. The skb is be replaced by bounce_skb, so the
> original skb should be consumed(not drop).
>
> dev_consume_skb_irq() should be called in b44_tx() when skb xmit
> done. It makes drop profiles(dropwatch, perf) more friendly.
>
> Signed-off-by: Yang Wei <yang.wei9@zte.com.cn>
Applied.
^ permalink raw reply
* Re: [PATCH net] net: caif: call dev_consume_skb_any when skb xmit done
From: David Miller @ 2019-01-29 18:11 UTC (permalink / raw)
To: albin_yang; +Cc: netdev, dmitry.tarnyagin, yang.wei9
In-Reply-To: <1548775942-6476-1-git-send-email-albin_yang@163.com>
From: Yang Wei <albin_yang@163.com>
Date: Tue, 29 Jan 2019 23:32:22 +0800
> From: Yang Wei <yang.wei9@zte.com.cn>
>
> The skb shouled be consumed when xmit done, it makes drop profiles
> (dropwatch, perf) more friendly.
> dev_kfree_skb_irq()/kfree_skb() shouled be replaced by
> dev_consume_skb_any(), it makes code cleaner.
>
> Signed-off-by: Yang Wei <yang.wei9@zte.com.cn>
Applied.
^ permalink raw reply
* Re: [PATCH 01/28] net: thunderbolt: Unregister ThunderboltIP protocol handler when suspending
From: David Miller @ 2019-01-29 18:10 UTC (permalink / raw)
To: mika.westerberg
Cc: linux-kernel, michael.jamet, YehezkelShB, andreas.noever, lukas,
andriy.shevchenko, netdev
In-Reply-To: <20190129150143.12681-2-mika.westerberg@linux.intel.com>
From: Mika Westerberg <mika.westerberg@linux.intel.com>
Date: Tue, 29 Jan 2019 18:01:16 +0300
> The XDomain protocol messages may start as soon as Thunderbolt control
> channel is started. This means that if the other host starts sending
> ThunderboltIP packets early enough they will be passed to the network
> driver which then gets confused because its resume hook is not called
> yet.
>
> Fix this by unregistering the ThunderboltIP protocol handler when
> suspending and registering it back on resume.
>
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Acked-by: David S. Miller <davem@davemloft.net>
^ permalink raw reply
* Re: [RFC PATCH net-next 3/6 v2] net/sched: cls_flower: Add ematch support
From: Marcelo Leitner @ 2019-01-29 18:08 UTC (permalink / raw)
To: Paul Blakey
Cc: Guy Shattah, Aaron Conole, John Hurley, Simon Horman,
Justin Pettit, Gregory Rose, Eelco Chaudron, Flavio Leitner,
Florian Westphal, Jiri Pirko, Rashid Khan, Sushil Kulkarni,
Andy Gospodarek, Roi Dayan, Yossi Kuperman, Or Gerlitz,
Rony Efraim, davem@davemloft.net, netdev
In-Reply-To: <1548748926-23822-4-git-send-email-paulb@mellanox.com>
On Tue, Jan 29, 2019 at 10:02:03AM +0200, Paul Blakey wrote:
> TODO: handle EEXist.
>
> Signed-off-by: Paul Blakey <paulb@mellanox.com>
> ---
> include/uapi/linux/pkt_cls.h | 2 ++
> net/sched/cls_flower.c | 22 ++++++++++++++++++----
> 2 files changed, 20 insertions(+), 4 deletions(-)
>
> diff --git a/include/uapi/linux/pkt_cls.h b/include/uapi/linux/pkt_cls.h
> index 121f1ef..d848d6d 100644
> --- a/include/uapi/linux/pkt_cls.h
> +++ b/include/uapi/linux/pkt_cls.h
> @@ -506,6 +506,8 @@ enum {
> TCA_FLOWER_KEY_CT_LABELS,
> TCA_FLOWER_KEY_CT_LABELS_MASK,
>
> + TCA_FLOWER_EMATCHES,
> +
> __TCA_FLOWER_MAX,
> };
>
> diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
> index bf74a31..f11fda0 100644
> --- a/net/sched/cls_flower.c
> +++ b/net/sched/cls_flower.c
> @@ -104,6 +104,7 @@ struct cls_fl_filter {
> struct rhash_head ht_node;
> struct fl_flow_key mkey;
> struct tcf_exts exts;
> + struct tcf_ematch_tree ematches;
> struct tcf_result res;
> struct fl_flow_key key;
> struct list_head list;
> @@ -332,10 +333,14 @@ static int fl_classify(struct sk_buff *skb, const struct tcf_proto *tp,
> fl_set_masked_key(&skb_mkey, &skb_key, mask);
>
> f = fl_lookup(mask, &skb_mkey, &skb_key);
> - if (f && !tc_skip_sw(f->flags)) {
> - *res = f->res;
> - return tcf_exts_exec(skb, &f->exts, res);
> - }
> + if (!f || tc_skip_sw(f->flags))
> + continue;
> +
> + if (!tcf_em_tree_match(skb, &f->ematches, NULL))
> + continue;
Considering just the recirc_id (and not the other fields supported by
ematch), have you considered integrating recirc_id match on flow
dissector instead? It would avoid the matching in 2 steps here and
benefit from the hashing.
> +
> + *res = f->res;
> + return tcf_exts_exec(skb, &f->exts, res);
> }
> return -1;
> }
> @@ -388,6 +393,7 @@ static bool fl_mask_put(struct cls_fl_head *head, struct fl_flow_mask *mask,
> static void __fl_destroy_filter(struct cls_fl_filter *f)
> {
> tcf_exts_destroy(&f->exts);
> + tcf_em_tree_destroy(&f->ematches);
> tcf_exts_put_net(&f->exts);
> kfree(f);
> }
> @@ -523,6 +529,7 @@ static void *fl_get(struct tcf_proto *tp, u32 handle)
> static const struct nla_policy fl_policy[TCA_FLOWER_MAX + 1] = {
> [TCA_FLOWER_UNSPEC] = { .type = NLA_UNSPEC },
> [TCA_FLOWER_CLASSID] = { .type = NLA_U32 },
> + [TCA_FLOWER_EMATCHES] = { .type = NLA_NESTED },
> [TCA_FLOWER_INDEV] = { .type = NLA_STRING,
> .len = IFNAMSIZ },
> [TCA_FLOWER_KEY_ETH_DST] = { .len = ETH_ALEN },
> @@ -1348,6 +1355,10 @@ static int fl_set_parms(struct net *net, struct tcf_proto *tp,
> if (err < 0)
> return err;
>
> + err = tcf_em_tree_validate(tp, tb[TCA_FLOWER_EMATCHES], &f->ematches);
> + if (err < 0)
> + return err;
> +
> if (tb[TCA_FLOWER_CLASSID]) {
> f->res.classid = nla_get_u32(tb[TCA_FLOWER_CLASSID]);
> tcf_bind_filter(tp, &f->res, base);
> @@ -2143,6 +2154,9 @@ static int fl_dump(struct net *net, struct tcf_proto *tp, void *fh,
> nla_put_u32(skb, TCA_FLOWER_CLASSID, f->res.classid))
> goto nla_put_failure;
>
> + if (tcf_em_tree_dump(skb, &f->ematches, TCA_FLOWER_EMATCHES) < 0)
> + goto nla_put_failure;
> +
> key = &f->key;
> mask = &f->mask->key;
>
> --
> 1.8.3.1
>
^ permalink raw reply
* Re: [PATCH net] net: 8139cp: replace dev_kfree_skb_irq by dev_consume_skb_irq for drop profiles
From: David Miller @ 2019-01-29 18:07 UTC (permalink / raw)
To: albin_yang; +Cc: netdev, yang.wei9
In-Reply-To: <1548772851-4692-1-git-send-email-albin_yang@163.com>
From: Yang Wei <albin_yang@163.com>
Date: Tue, 29 Jan 2019 22:40:51 +0800
> From: Yang Wei <yang.wei9@zte.com.cn>
>
> dev_consume_skb_irq() should be called in cp_tx() when skb xmit
> done. It makes drop profiles(dropwatch, perf) more friendly.
>
> Signed-off-by: Yang Wei <yang.wei9@zte.com.cn>
Applied, thanks.
^ permalink raw reply
* Re: [RFC PATCH net-next 0/6 v2] connection tracking in tc and OVS offload
From: Marcelo Leitner @ 2019-01-29 18:07 UTC (permalink / raw)
To: Paul Blakey
Cc: Guy Shattah, Aaron Conole, John Hurley, Simon Horman,
Justin Pettit, Gregory Rose, Eelco Chaudron, Flavio Leitner,
Florian Westphal, Jiri Pirko, Rashid Khan, Sushil Kulkarni,
Andy Gospodarek, Roi Dayan, Yossi Kuperman, Or Gerlitz,
Rony Efraim, davem@davemloft.net, netdev
In-Reply-To: <1548748753-22540-1-git-send-email-paulb@mellanox.com>
On Tue, Jan 29, 2019 at 09:59:07AM +0200, Paul Blakey wrote:
> Hi,
> As you may know, we are working on connection tracking for a while, and we already
> had patches for tc that matched our connection tracking offload RFC.
>
> Marcelo already shared his tc patches for a similar action ct and flower match on ct_info state,
> and this patches are pretty close to his. We would like to share ours and see what's the difference
> so maybe we can merge the two.
>
> I think the main difference here is that we propose the usage of a new metadata that resembles
> ovs recirc id, so one can use tc recirculation in a similar way that ovs does.
This is a very interesting approach. Seems it can avoid the necessity
of offloading multiple chains at all and, with that, we don't need a
special fallback path for when chain X is not present in HW: it will
be just another miss. Such skb may have an extra meta data, but no
need to look for a special chain and so.
Two points on it for further discussion. I'll reply on the specific
patches.
>
> The plan is to support offloading of OVS rules to tc, so this recirculation id will
> be shared with and from OVS.
>
> The following is an example using the recirc id metadata (aa_rep and bb_rep are two veth devices)
>
> tc qdisc add dev bb_rep ingress
> tc qdisc add dev aa_rep ingress
> tc filter add dev aa_rep ingress prio 1 chain 0 proto ip flower match 'meta(tc_recirc mask 0xffffffff eq 0x1)' ct_state +trk+est ip_proto tcp action mirred egress redirect dev bb_rep
> tc filter add dev aa_rep ingress prio 1 chain 0 proto ip flower ct_state -trk ip_proto tcp action ct recirc 1 reclassify
> tc filter add dev aa_rep ingress prio 1 chain 0 proto ip flower match 'meta(tc_recirc mask 0xffffffff eq 0x1)' ct_state +trk+new ip_proto tcp action ct commit pipe action mirred egress redirect dev bb_rep
>
> tc filter add dev bb_rep ingress prio 1 chain 0 proto ip flower ct_state -trk ip_proto tcp action ct recirc 2 reclassify
> tc filter add dev bb_rep ingress prio 1 chain 0 proto ip flower match 'meta(tc_recirc mask 0xffffffff eq 0x2)' ct_state +trk+est ip_proto tcp action mirred egress redirect dev aa_rep
>
> of course, goto chain instead of reclassify will also work.
>
> There might be some difference in how we handle action ct and I'll follow up on that.
>
>
> Changelog:
> v1->v2:
> Missed first patch :|
> Added netdev mailing list
>
> Paul Blakey (6):
> net/sched: Introduce act_ct
> net/sched: cls_flower: add match on ct info
> net/sched: cls_flower: Add ematch support
> net: Add new tc recirc id skb extension
> net/sched: em_meta: add match on tc recirc id skb extension
> net/sched: act_ct: Add tc recirc id set/del support
>
> include/linux/skbuff.h | 1 +
> include/net/tc_act/tc_ct.h | 2 +
> include/uapi/linux/pkt_cls.h | 19 ++++
> include/uapi/linux/tc_act/tc_ct.h | 2 +
> include/uapi/linux/tc_ematch/tc_em_meta.h | 1 +
> net/core/skbuff.c | 2 +
> net/sched/act_ct.c | 18 ++++
> net/sched/cls_flower.c | 148 ++++++++++++++++++++++++++++--
> net/sched/em_meta.c | 8 ++
> 9 files changed, 194 insertions(+), 7 deletions(-)
>
> --
> 1.8.3.1
>
^ permalink raw reply
* [PATCH] can: mark expected switch fall-throughs
From: Gustavo A. R. Silva @ 2019-01-29 18:06 UTC (permalink / raw)
To: Wolfgang Grandegger, Marc Kleine-Budde, David S. Miller,
Nicolas Ferre, Alexandre Belloni, Ludovic Desroches
Cc: linux-can, netdev, linux-arm-kernel, linux-kernel,
Gustavo A. R. Silva
In preparation to enabling -Wimplicit-fallthrough, mark switch cases
where we are expecting to fall through.
This patch fixes the following warnings:
drivers/net/can/peak_canfd/peak_pciefd_main.c:668:3: warning: this statement may fall through [-Wimplicit-fallthrough=]
drivers/net/can/spi/mcp251x.c:875:7: warning: this statement may fall through [-Wimplicit-fallthrough=]
drivers/net/can/usb/peak_usb/pcan_usb.c:422:6: warning: this statement may fall through [-Wimplicit-fallthrough=]
drivers/net/can/at91_can.c:895:6: warning: this statement may fall through [-Wimplicit-fallthrough=]
drivers/net/can/at91_can.c:953:15: warning: this statement may fall through [-Wimplicit-fallthrough=]
drivers/net/can/usb/peak_usb/pcan_usb.c: In function ‘pcan_usb_decode_error’:
drivers/net/can/usb/peak_usb/pcan_usb.c:422:6: warning: this statement may fall through [-Wimplicit-fallthrough=]
if (n & PCAN_USB_ERROR_BUS_LIGHT) {
^
drivers/net/can/usb/peak_usb/pcan_usb.c:428:2: note: here
case CAN_STATE_ERROR_WARNING:
^~~~
Warning level 3 was used: -Wimplicit-fallthrough=3
This patch is part of the ongoing efforts to enabling
-Wimplicit-fallthrough.
Notice that in some cases spelling mistakes were fixed.
In other cases, the /* fall through */ comment is placed
at the bottom of the case statement, which is what GCC
is expecting to find.
Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
---
drivers/net/can/at91_can.c | 6 ++++--
drivers/net/can/peak_canfd/peak_pciefd_main.c | 2 +-
drivers/net/can/spi/mcp251x.c | 3 ++-
drivers/net/can/usb/peak_usb/pcan_usb.c | 2 +-
4 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/drivers/net/can/at91_can.c b/drivers/net/can/at91_can.c
index d98c69045b17..1718c20f9c99 100644
--- a/drivers/net/can/at91_can.c
+++ b/drivers/net/can/at91_can.c
@@ -902,7 +902,8 @@ static void at91_irq_err_state(struct net_device *dev,
CAN_ERR_CRTL_TX_WARNING :
CAN_ERR_CRTL_RX_WARNING;
}
- case CAN_STATE_ERROR_WARNING: /* fallthrough */
+ /* fall through */
+ case CAN_STATE_ERROR_WARNING:
/*
* from: ERROR_ACTIVE, ERROR_WARNING
* to : ERROR_PASSIVE, BUS_OFF
@@ -951,7 +952,8 @@ static void at91_irq_err_state(struct net_device *dev,
netdev_dbg(dev, "Error Active\n");
cf->can_id |= CAN_ERR_PROT;
cf->data[2] = CAN_ERR_PROT_ACTIVE;
- case CAN_STATE_ERROR_WARNING: /* fallthrough */
+ /* fall through */
+ case CAN_STATE_ERROR_WARNING:
reg_idr = AT91_IRQ_ERRA | AT91_IRQ_WARN | AT91_IRQ_BOFF;
reg_ier = AT91_IRQ_ERRP;
break;
diff --git a/drivers/net/can/peak_canfd/peak_pciefd_main.c b/drivers/net/can/peak_canfd/peak_pciefd_main.c
index c458d5fdc8d3..e4f4d65a76b4 100644
--- a/drivers/net/can/peak_canfd/peak_pciefd_main.c
+++ b/drivers/net/can/peak_canfd/peak_pciefd_main.c
@@ -668,7 +668,7 @@ static int pciefd_can_probe(struct pciefd_board *pciefd)
pciefd_can_writereg(priv, CANFD_CLK_SEL_80MHZ,
PCIEFD_REG_CAN_CLK_SEL);
- /* fallthough */
+ /* fall through */
case CANFD_CLK_SEL_80MHZ:
priv->ucan.can.clock.freq = 80 * 1000 * 1000;
break;
diff --git a/drivers/net/can/spi/mcp251x.c b/drivers/net/can/spi/mcp251x.c
index e90817608645..17257c73c302 100644
--- a/drivers/net/can/spi/mcp251x.c
+++ b/drivers/net/can/spi/mcp251x.c
@@ -875,7 +875,8 @@ static irqreturn_t mcp251x_can_ist(int irq, void *dev_id)
if (new_state >= CAN_STATE_ERROR_WARNING &&
new_state <= CAN_STATE_BUS_OFF)
priv->can.can_stats.error_warning++;
- case CAN_STATE_ERROR_WARNING: /* fallthrough */
+ /* fall through */
+ case CAN_STATE_ERROR_WARNING:
if (new_state >= CAN_STATE_ERROR_PASSIVE &&
new_state <= CAN_STATE_BUS_OFF)
priv->can.can_stats.error_passive++;
diff --git a/drivers/net/can/usb/peak_usb/pcan_usb.c b/drivers/net/can/usb/peak_usb/pcan_usb.c
index 13238a72a338..eca785532b6b 100644
--- a/drivers/net/can/usb/peak_usb/pcan_usb.c
+++ b/drivers/net/can/usb/peak_usb/pcan_usb.c
@@ -423,7 +423,7 @@ static int pcan_usb_decode_error(struct pcan_usb_msg_context *mc, u8 n,
new_state = CAN_STATE_ERROR_WARNING;
break;
}
- /* else: fall through */
+ /* fall through */
case CAN_STATE_ERROR_WARNING:
if (n & PCAN_USB_ERROR_BUS_HEAVY) {
--
2.20.1
^ permalink raw reply related
* [PATCH net v5 2/2] net/mlx5e: Don't overwrite pedit action when multiple pedit used
From: xiangxia.m.yue @ 2019-01-28 23:28 UTC (permalink / raw)
To: saeedm, gerlitz.or, davem; +Cc: netdev, Tonghao Zhang, Or Gerlitz
In-Reply-To: <1548718086-20924-1-git-send-email-xiangxia.m.yue@gmail.com>
From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
In some case, we may use multiple pedit actions to modify packets.
The command shown as below: the last pedit action is effective.
$ tc filter add dev netdev_rep parent ffff: protocol ip prio 1 \
flower skip_sw ip_proto icmp dst_ip 3.3.3.3 \
action pedit ex munge ip dst set 192.168.1.100 pipe \
action pedit ex munge eth src set 00:00:00:00:00:01 pipe \
action pedit ex munge eth dst set 00:00:00:00:00:02 pipe \
action csum ip pipe \
action tunnel_key set src_ip 1.1.1.100 dst_ip 1.1.1.200 dst_port 4789 id 100 \
action mirred egress redirect dev vxlan0
To fix it, we add max_mod_hdr_actions to mlx5e_tc_flow_parse_attr struction,
max_mod_hdr_actions will store the max pedit action number we support and
num_mod_hdr_actions indicates how many pedit action we used, and store all
pedit action to mod_hdr_actions.
Fixes: d79b6df6b10a ("net/mlx5e: Add parsing of TC pedit actions to HW format")
Cc: Or Gerlitz <ogerlitz@mellanox.com>
Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
Reviewed-by: Or Gerlitz <ogerlitz@mellanox.com>
---
v3: Remove the unnecessary init.
v2: Fix comment message and change tag from net-next to net.
---
drivers/net/ethernet/mellanox/mlx5/core/en_tc.c | 25 +++++++++++++++----------
1 file changed, 15 insertions(+), 10 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
index cae6c6d..a21724d0 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
@@ -128,6 +128,7 @@ struct mlx5e_tc_flow_parse_attr {
struct net_device *filter_dev;
struct mlx5_flow_spec spec;
int num_mod_hdr_actions;
+ int max_mod_hdr_actions;
void *mod_hdr_actions;
int mirred_ifindex[MLX5_MAX_FLOW_FWD_VPORTS];
};
@@ -1934,9 +1935,9 @@ struct mlx5_fields {
OFFLOAD(UDP_DPORT, 2, udp.dest, 0),
};
-/* On input attr->num_mod_hdr_actions tells how many HW actions can be parsed at
- * max from the SW pedit action. On success, it says how many HW actions were
- * actually parsed.
+/* On input attr->max_mod_hdr_actions tells how many HW actions can be parsed at
+ * max from the SW pedit action. On success, attr->num_mod_hdr_actions
+ * says how many HW actions were actually parsed.
*/
static int offload_pedit_fields(struct pedit_headers *masks,
struct pedit_headers *vals,
@@ -1960,9 +1961,11 @@ static int offload_pedit_fields(struct pedit_headers *masks,
add_vals = &vals[TCA_PEDIT_KEY_EX_CMD_ADD];
action_size = MLX5_UN_SZ_BYTES(set_action_in_add_action_in_auto);
- action = parse_attr->mod_hdr_actions;
- max_actions = parse_attr->num_mod_hdr_actions;
- nactions = 0;
+ action = parse_attr->mod_hdr_actions +
+ parse_attr->num_mod_hdr_actions * action_size;
+
+ max_actions = parse_attr->max_mod_hdr_actions;
+ nactions = parse_attr->num_mod_hdr_actions;
for (i = 0; i < ARRAY_SIZE(fields); i++) {
f = &fields[i];
@@ -2073,7 +2076,7 @@ static int alloc_mod_hdr_actions(struct mlx5e_priv *priv,
if (!parse_attr->mod_hdr_actions)
return -ENOMEM;
- parse_attr->num_mod_hdr_actions = max_actions;
+ parse_attr->max_mod_hdr_actions = max_actions;
return 0;
}
@@ -2119,9 +2122,11 @@ static int parse_tc_pedit_action(struct mlx5e_priv *priv,
goto out_err;
}
- err = alloc_mod_hdr_actions(priv, a, namespace, parse_attr);
- if (err)
- goto out_err;
+ if (!parse_attr->mod_hdr_actions) {
+ err = alloc_mod_hdr_actions(priv, a, namespace, parse_attr);
+ if (err)
+ goto out_err;
+ }
err = offload_pedit_fields(masks, vals, parse_attr, extack);
if (err < 0)
--
1.8.3.1
^ permalink raw reply related
* [PATCH net v5 1/2] net/mlx5e: Update hw flows when encap source mac changed
From: xiangxia.m.yue @ 2019-01-28 23:28 UTC (permalink / raw)
To: saeedm, gerlitz.or, davem; +Cc: netdev, Tonghao Zhang, Hadar Hen Zion
From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
When we offload tc filters to hardware, hardware flows can
be updated when mac of encap destination ip is changed.
But we ignore one case, that the mac of local encap ip can
be changed too, so we should also update them.
To fix it, add route_dev in mlx5e_encap_entry struct to save
the local encap netdevice, and when mac changed, kernel will
flush all the neighbour on the netdevice and send NETEVENT_NEIGH_UPDATE
event. The mlx5 driver will delete the flows and add them when neighbour
available again.
Fixes: 232c001398ae ("net/mlx5e: Add support to neighbour update flow")
Cc: Hadar Hen Zion <hadarh@mellanox.com>
Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
Reviewed-by: Or Gerlitz <ogerlitz@mellanox.com>
---
drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun.c | 2 ++
drivers/net/ethernet/mellanox/mlx5/core/en_rep.c | 4 ++++
drivers/net/ethernet/mellanox/mlx5/core/en_rep.h | 1 +
3 files changed, 7 insertions(+)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun.c b/drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun.c
index 046948e..114e0a2 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun.c
@@ -256,6 +256,7 @@ int mlx5e_tc_tun_create_header_ipv4(struct mlx5e_priv *priv,
e->m_neigh.family = n->ops->family;
memcpy(&e->m_neigh.dst_ip, n->primary_key, n->tbl->key_len);
e->out_dev = out_dev;
+ e->route_dev = route_dev;
/* It's important to add the neigh to the hash table before checking
* the neigh validity state. So if we'll get a notification, in case the
@@ -369,6 +370,7 @@ int mlx5e_tc_tun_create_header_ipv6(struct mlx5e_priv *priv,
e->m_neigh.family = n->ops->family;
memcpy(&e->m_neigh.dst_ip, n->primary_key, n->tbl->key_len);
e->out_dev = out_dev;
+ e->route_dev = route_dev;
/* It's importent to add the neigh to the hash table before checking
* the neigh validity state. So if we'll get a notification, in case the
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c b/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
index 96cc0c6..c4b9073 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
@@ -594,6 +594,10 @@ static void mlx5e_rep_update_flows(struct mlx5e_priv *priv,
if (neigh_connected && !(e->flags & MLX5_ENCAP_ENTRY_VALID)) {
ether_addr_copy(e->h_dest, ha);
ether_addr_copy(eth->h_dest, ha);
+ /* Update the encap source mac, in case that we delete
+ * the flows when encap source mac changed.
+ */
+ ether_addr_copy(eth->h_source, e->route_dev->dev_addr);
mlx5e_tc_encap_flows_add(priv, e);
}
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_rep.h b/drivers/net/ethernet/mellanox/mlx5/core/en_rep.h
index edd7228..36eafc8 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_rep.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_rep.h
@@ -148,6 +148,7 @@ struct mlx5e_encap_entry {
unsigned char h_dest[ETH_ALEN]; /* destination eth addr */
struct net_device *out_dev;
+ struct net_device *route_dev;
int tunnel_type;
int tunnel_hlen;
int reformat_type;
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH net-next] MAINTAINERS: update cxgb4 and cxgb3 maintainer
From: David Miller @ 2019-01-29 18:01 UTC (permalink / raw)
To: arjun; +Cc: netdev, nirranjan, indranil, dt, leedom
In-Reply-To: <20190129100842.19926-1-arjun@chelsio.com>
From: Arjun Vynipadath <arjun@chelsio.com>
Date: Tue, 29 Jan 2019 15:38:42 +0530
> Vishal Kulkarni will be the new maintainer for Chelsio cxgb3/cxgb4
> drivers.
>
> Signed-off-by: Arjun Vynipadath <arjun@chelsio.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next] cxgb4vf: Update port information in cxgb4vf_open()
From: David Miller @ 2019-01-29 17:50 UTC (permalink / raw)
To: arjun; +Cc: netdev, nirranjan, indranil, dt, leedom
In-Reply-To: <20190129095019.19304-1-arjun@chelsio.com>
From: Arjun Vynipadath <arjun@chelsio.com>
Date: Tue, 29 Jan 2019 15:20:19 +0530
> It's possible that the basic port information could have
> changed since we first read it.
>
> Signed-off-by: Arjun Vynipadath <arjun@chelsio.com>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH net v4 2/2] net/mlx5e: Don't overwrite pedit action when multiple pedit used
From: Tonghao Zhang @ 2019-01-29 17:47 UTC (permalink / raw)
To: Or Gerlitz; +Cc: Saeed Mahameed, Linux Netdev List, Or Gerlitz
In-Reply-To: <CAJ3xEMjHnoP8c17pki40vGs2S8xOSxP9Gm2RagtUxxBw_B7aLw@mail.gmail.com>
On Tue, Jan 29, 2019 at 11:44 PM Or Gerlitz <gerlitz.or@gmail.com> wrote:
>
> On Mon, Jan 28, 2019 at 6:18 PM Tonghao Zhang <xiangxia.m.yue@gmail.com> wrote:
> > On Mon, Jan 28, 2019 at 11:34 PM Or Gerlitz <gerlitz.or@gmail.com> wrote:
> > > On Mon, Jan 28, 2019 at 2:10 PM Tonghao Zhang <xiangxia.m.yue@gmail.com> wrote:
> > > > On Mon, Jan 28, 2019 at 12:40 AM Or Gerlitz <gerlitz.or@gmail.com> wrote:
> > > > >
> > > > > On Sun, Jan 27, 2019 at 1:06 PM <xiangxia.m.yue@gmail.com> wrote:
> > > > > > From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
> > > > > >
> > > > > > In some case, we may use multiple pedit actions to modify packets.
> > > > > > The command shown as below: the last pedit action is effective.
> > > > >
> > > > > > @@ -2073,7 +2076,8 @@ static int alloc_mod_hdr_actions(struct mlx5e_priv *priv,
> > > > > > if (!parse_attr->mod_hdr_actions)
> > > > > > return -ENOMEM;
> > > > > >
> > > > > > - parse_attr->num_mod_hdr_actions = max_actions;
> > > > > > + parse_attr->max_mod_hdr_actions = max_actions;
> > > > > > + parse_attr->num_mod_hdr_actions = 0;
> > > > >
> > > > > why would we want to do this zeroing? what purpose does it serve?
> > > > Because we use the num_mod_hdr_actions to store the number of actions
> > > > we have parsed,
> > > > and when we alloc it, we init it 0 as default.
> > > >
> > > > > On a probably related note, I suspect that the patch broke the caching
> > > > > we do for modify header contexts, see mlx5e_attach_mod_hdr where we
> > > > > look if a given set of modify header operations already has hw modify header
> > > > > context and we use it.
> > > > >
> > > > > To test that, put two tc rules with different matching but same set of
> > > > > modify header
> > > > > (pedit) actions and see that only one modify header context is used.
> > >
> > > > The patch does't break the cache, I think that different matching may
> > > > share the same set of pedit actions.
> > >
> > > I suspect it does break it.. at [1] we have this code for the cache lookup:
> > >
> > > num_actions = parse_attr->num_mod_hdr_actions;
> > > [..]
> > > key.actions = parse_attr->mod_hdr_actions;
> > > key.num_actions = num_actions;
> > >
> > > hash_key = hash_mod_hdr_info(&key);
> > >
> > > so we are doing the cached insertion and lookup with
> > > parse_attr->num_mod_hdr_actions
> > > which was zeroed along the way and not accounting for the full set of
> > > pedit actions, agree?
>
> > not really, before calling the mlx5e_attach_mod_hdr, we have call
> > the offload_pedit_fields that will
> > update the attr->num_mod_hdr_actions that indicate how many pedit
> > action we parsed.
>
> ok, got you, so why do we need this line
>
> parse_attr->num_mod_hdr_actions = 0;
>
> in alloc_mod_hdr_actions()? this should be zero
> by kzalloc somewhere, it got to confuse me..
yes, should be removed
> I suggest to remove this zeroing, otherwise the patch LGTM, once you fix it
>
> Reviewed-by: Or Gerlitz <ogerlitz@mellanox.com>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox