linux-can.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* pull-request: can 2020-11-27
@ 2020-11-27 10:02 Marc Kleine-Budde
  2020-11-27 10:02 ` [net 1/6] can: gs_usb: fix endianess problem with candleLight firmware Marc Kleine-Budde
                   ` (7 more replies)
  0 siblings, 8 replies; 10+ messages in thread
From: Marc Kleine-Budde @ 2020-11-27 10:02 UTC (permalink / raw)
  To: netdev; +Cc: davem, kuba, linux-can, kernel

Hello Jakub, hello David,

here's a pull request of 6 patches for net/master.

The first patch is by me and target the gs_usb driver and fixes the endianess
problem with candleLight firmware.

Another patch by me for the mcp251xfd driver add sanity checking to bail out if
no IRQ is configured.

The next three patches target the m_can driver. A patch by me removes the
hardcoded IRQF_TRIGGER_FALLING from the request_threaded_irq() as this clashes
with the trigger level specified in the DT. Further a patch by me fixes the
nominal bitiming tseg2 min value for modern m_can cores. Pankaj Sharma's patch
add support for cores version 3.3.x.

The last patch by Oliver Hartkopp is for af_can and converts a WARN() into a
pr_warn(), which is triggered by the syzkaller. It was able to create a
situation where the closing of a socket runs simultaneously to the notifier
call chain for removing the CAN network device in use.

regards,
Marc

---

The following changes since commit cbf3d60329c4e11edcecac0c8fc6767b0f05e3a7:

  ch_ktls: lock is not freed (2020-11-25 17:44:42 -0800)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can.git tags/linux-can-fixes-for-5.10-20201127

for you to fetch changes up to d73ff9b7c4eacaba0fd956d14882bcae970f8307:

  can: af_can: can_rx_unregister(): remove WARN() statement from list operation sanity check (2020-11-27 10:49:28 +0100)

----------------------------------------------------------------
linux-can-fixes-for-5.10-20201127

----------------------------------------------------------------
Marc Kleine-Budde (4):
      can: gs_usb: fix endianess problem with candleLight firmware
      can: mcp251xfd: mcp251xfd_probe(): bail out if no IRQ was given
      can: m_can: m_can_open(): remove IRQF_TRIGGER_FALLING from request_threaded_irq()'s flags
      can: m_can: fix nominal bitiming tseg2 min for version >= 3.1

Oliver Hartkopp (1):
      can: af_can: can_rx_unregister(): remove WARN() statement from list operation sanity check

Pankaj Sharma (1):
      can: m_can: m_can_dev_setup(): add support for bosch mcan version 3.3.0

 drivers/net/can/m_can/m_can.c                  |   6 +-
 drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c |   4 +
 drivers/net/can/usb/gs_usb.c                   | 131 +++++++++++++------------
 net/can/af_can.c                               |   7 +-
 4 files changed, 83 insertions(+), 65 deletions(-)



^ permalink raw reply	[flat|nested] 10+ messages in thread

* [net 1/6] can: gs_usb: fix endianess problem with candleLight firmware
  2020-11-27 10:02 pull-request: can 2020-11-27 Marc Kleine-Budde
@ 2020-11-27 10:02 ` Marc Kleine-Budde
  2020-11-27 10:02 ` [net 2/6] can: mcp251xfd: mcp251xfd_probe(): bail out if no IRQ was given Marc Kleine-Budde
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 10+ messages in thread
From: Marc Kleine-Budde @ 2020-11-27 10:02 UTC (permalink / raw)
  To: netdev
  Cc: davem, kuba, linux-can, kernel, Marc Kleine-Budde,
	Maximilian Schneider, Hubert Denkmair, Michael Rausch,
	Oleksij Rempel

The firmware on the original USB2CAN by Geschwister Schneider Technologie
Entwicklungs- und Vertriebs UG exchanges all data between the host and the
device in host byte order. This is done with the struct
gs_host_config::byte_order member, which is sent first to indicate the desired
byte order.

The widely used open source firmware candleLight doesn't support this feature
and exchanges the data in little endian byte order. This breaks if a device
with candleLight firmware is used on big endianess systems.

To fix this problem, all u32 (but not the struct gs_host_frame::echo_id, which
is a transparent cookie) are converted to __le32.

Cc: Maximilian Schneider <max@schneidersoft.net>
Cc: Hubert Denkmair <hubert@denkmair.de>
Reported-by: Michael Rausch <mr@netadair.de>
Link: https://lore.kernel.org/r/b58aace7-61f3-6df7-c6df-69fee2c66906@netadair.de
Tested-by: Oleksij Rempel <o.rempel@pengutronix.de>
Fixes: d08e973a77d1 ("can: gs_usb: Added support for the GS_USB CAN devices")
Link: https://lore.kernel.org/r/20201120103818.3386964-1-mkl@pengutronix.de
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/usb/gs_usb.c | 131 +++++++++++++++++++----------------
 1 file changed, 70 insertions(+), 61 deletions(-)

diff --git a/drivers/net/can/usb/gs_usb.c b/drivers/net/can/usb/gs_usb.c
index 3005157059ca..018ca3b057a3 100644
--- a/drivers/net/can/usb/gs_usb.c
+++ b/drivers/net/can/usb/gs_usb.c
@@ -63,21 +63,27 @@ enum gs_can_identify_mode {
 };
 
 /* data types passed between host and device */
+
+/* The firmware on the original USB2CAN by Geschwister Schneider
+ * Technologie Entwicklungs- und Vertriebs UG exchanges all data
+ * between the host and the device in host byte order. This is done
+ * with the struct gs_host_config::byte_order member, which is sent
+ * first to indicate the desired byte order.
+ *
+ * The widely used open source firmware candleLight doesn't support
+ * this feature and exchanges the data in little endian byte order.
+ */
 struct gs_host_config {
-	u32 byte_order;
+	__le32 byte_order;
 } __packed;
-/* All data exchanged between host and device is exchanged in host byte order,
- * thanks to the struct gs_host_config byte_order member, which is sent first
- * to indicate the desired byte order.
- */
 
 struct gs_device_config {
 	u8 reserved1;
 	u8 reserved2;
 	u8 reserved3;
 	u8 icount;
-	u32 sw_version;
-	u32 hw_version;
+	__le32 sw_version;
+	__le32 hw_version;
 } __packed;
 
 #define GS_CAN_MODE_NORMAL               0
@@ -87,26 +93,26 @@ struct gs_device_config {
 #define GS_CAN_MODE_ONE_SHOT             BIT(3)
 
 struct gs_device_mode {
-	u32 mode;
-	u32 flags;
+	__le32 mode;
+	__le32 flags;
 } __packed;
 
 struct gs_device_state {
-	u32 state;
-	u32 rxerr;
-	u32 txerr;
+	__le32 state;
+	__le32 rxerr;
+	__le32 txerr;
 } __packed;
 
 struct gs_device_bittiming {
-	u32 prop_seg;
-	u32 phase_seg1;
-	u32 phase_seg2;
-	u32 sjw;
-	u32 brp;
+	__le32 prop_seg;
+	__le32 phase_seg1;
+	__le32 phase_seg2;
+	__le32 sjw;
+	__le32 brp;
 } __packed;
 
 struct gs_identify_mode {
-	u32 mode;
+	__le32 mode;
 } __packed;
 
 #define GS_CAN_FEATURE_LISTEN_ONLY      BIT(0)
@@ -117,23 +123,23 @@ struct gs_identify_mode {
 #define GS_CAN_FEATURE_IDENTIFY         BIT(5)
 
 struct gs_device_bt_const {
-	u32 feature;
-	u32 fclk_can;
-	u32 tseg1_min;
-	u32 tseg1_max;
-	u32 tseg2_min;
-	u32 tseg2_max;
-	u32 sjw_max;
-	u32 brp_min;
-	u32 brp_max;
-	u32 brp_inc;
+	__le32 feature;
+	__le32 fclk_can;
+	__le32 tseg1_min;
+	__le32 tseg1_max;
+	__le32 tseg2_min;
+	__le32 tseg2_max;
+	__le32 sjw_max;
+	__le32 brp_min;
+	__le32 brp_max;
+	__le32 brp_inc;
 } __packed;
 
 #define GS_CAN_FLAG_OVERFLOW 1
 
 struct gs_host_frame {
 	u32 echo_id;
-	u32 can_id;
+	__le32 can_id;
 
 	u8 can_dlc;
 	u8 channel;
@@ -329,13 +335,13 @@ static void gs_usb_receive_bulk_callback(struct urb *urb)
 		if (!skb)
 			return;
 
-		cf->can_id = hf->can_id;
+		cf->can_id = le32_to_cpu(hf->can_id);
 
 		cf->can_dlc = get_can_dlc(hf->can_dlc);
 		memcpy(cf->data, hf->data, 8);
 
 		/* ERROR frames tell us information about the controller */
-		if (hf->can_id & CAN_ERR_FLAG)
+		if (le32_to_cpu(hf->can_id) & CAN_ERR_FLAG)
 			gs_update_state(dev, cf);
 
 		netdev->stats.rx_packets++;
@@ -418,11 +424,11 @@ static int gs_usb_set_bittiming(struct net_device *netdev)
 	if (!dbt)
 		return -ENOMEM;
 
-	dbt->prop_seg = bt->prop_seg;
-	dbt->phase_seg1 = bt->phase_seg1;
-	dbt->phase_seg2 = bt->phase_seg2;
-	dbt->sjw = bt->sjw;
-	dbt->brp = bt->brp;
+	dbt->prop_seg = cpu_to_le32(bt->prop_seg);
+	dbt->phase_seg1 = cpu_to_le32(bt->phase_seg1);
+	dbt->phase_seg2 = cpu_to_le32(bt->phase_seg2);
+	dbt->sjw = cpu_to_le32(bt->sjw);
+	dbt->brp = cpu_to_le32(bt->brp);
 
 	/* request bit timings */
 	rc = usb_control_msg(interface_to_usbdev(intf),
@@ -503,7 +509,7 @@ static netdev_tx_t gs_can_start_xmit(struct sk_buff *skb,
 
 	cf = (struct can_frame *)skb->data;
 
-	hf->can_id = cf->can_id;
+	hf->can_id = cpu_to_le32(cf->can_id);
 	hf->can_dlc = cf->can_dlc;
 	memcpy(hf->data, cf->data, cf->can_dlc);
 
@@ -573,6 +579,7 @@ static int gs_can_open(struct net_device *netdev)
 	int rc, i;
 	struct gs_device_mode *dm;
 	u32 ctrlmode;
+	u32 flags = 0;
 
 	rc = open_candev(netdev);
 	if (rc)
@@ -640,24 +647,24 @@ static int gs_can_open(struct net_device *netdev)
 
 	/* flags */
 	ctrlmode = dev->can.ctrlmode;
-	dm->flags = 0;
 
 	if (ctrlmode & CAN_CTRLMODE_LOOPBACK)
-		dm->flags |= GS_CAN_MODE_LOOP_BACK;
+		flags |= GS_CAN_MODE_LOOP_BACK;
 	else if (ctrlmode & CAN_CTRLMODE_LISTENONLY)
-		dm->flags |= GS_CAN_MODE_LISTEN_ONLY;
+		flags |= GS_CAN_MODE_LISTEN_ONLY;
 
 	/* Controller is not allowed to retry TX
 	 * this mode is unavailable on atmels uc3c hardware
 	 */
 	if (ctrlmode & CAN_CTRLMODE_ONE_SHOT)
-		dm->flags |= GS_CAN_MODE_ONE_SHOT;
+		flags |= GS_CAN_MODE_ONE_SHOT;
 
 	if (ctrlmode & CAN_CTRLMODE_3_SAMPLES)
-		dm->flags |= GS_CAN_MODE_TRIPLE_SAMPLE;
+		flags |= GS_CAN_MODE_TRIPLE_SAMPLE;
 
 	/* finally start device */
-	dm->mode = GS_CAN_MODE_START;
+	dm->mode = cpu_to_le32(GS_CAN_MODE_START);
+	dm->flags = cpu_to_le32(flags);
 	rc = usb_control_msg(interface_to_usbdev(dev->iface),
 			     usb_sndctrlpipe(interface_to_usbdev(dev->iface), 0),
 			     GS_USB_BREQ_MODE,
@@ -737,9 +744,9 @@ static int gs_usb_set_identify(struct net_device *netdev, bool do_identify)
 		return -ENOMEM;
 
 	if (do_identify)
-		imode->mode = GS_CAN_IDENTIFY_ON;
+		imode->mode = cpu_to_le32(GS_CAN_IDENTIFY_ON);
 	else
-		imode->mode = GS_CAN_IDENTIFY_OFF;
+		imode->mode = cpu_to_le32(GS_CAN_IDENTIFY_OFF);
 
 	rc = usb_control_msg(interface_to_usbdev(dev->iface),
 			     usb_sndctrlpipe(interface_to_usbdev(dev->iface),
@@ -790,6 +797,7 @@ static struct gs_can *gs_make_candev(unsigned int channel,
 	struct net_device *netdev;
 	int rc;
 	struct gs_device_bt_const *bt_const;
+	u32 feature;
 
 	bt_const = kmalloc(sizeof(*bt_const), GFP_KERNEL);
 	if (!bt_const)
@@ -830,14 +838,14 @@ static struct gs_can *gs_make_candev(unsigned int channel,
 
 	/* dev setup */
 	strcpy(dev->bt_const.name, "gs_usb");
-	dev->bt_const.tseg1_min = bt_const->tseg1_min;
-	dev->bt_const.tseg1_max = bt_const->tseg1_max;
-	dev->bt_const.tseg2_min = bt_const->tseg2_min;
-	dev->bt_const.tseg2_max = bt_const->tseg2_max;
-	dev->bt_const.sjw_max = bt_const->sjw_max;
-	dev->bt_const.brp_min = bt_const->brp_min;
-	dev->bt_const.brp_max = bt_const->brp_max;
-	dev->bt_const.brp_inc = bt_const->brp_inc;
+	dev->bt_const.tseg1_min = le32_to_cpu(bt_const->tseg1_min);
+	dev->bt_const.tseg1_max = le32_to_cpu(bt_const->tseg1_max);
+	dev->bt_const.tseg2_min = le32_to_cpu(bt_const->tseg2_min);
+	dev->bt_const.tseg2_max = le32_to_cpu(bt_const->tseg2_max);
+	dev->bt_const.sjw_max = le32_to_cpu(bt_const->sjw_max);
+	dev->bt_const.brp_min = le32_to_cpu(bt_const->brp_min);
+	dev->bt_const.brp_max = le32_to_cpu(bt_const->brp_max);
+	dev->bt_const.brp_inc = le32_to_cpu(bt_const->brp_inc);
 
 	dev->udev = interface_to_usbdev(intf);
 	dev->iface = intf;
@@ -854,28 +862,29 @@ static struct gs_can *gs_make_candev(unsigned int channel,
 
 	/* can setup */
 	dev->can.state = CAN_STATE_STOPPED;
-	dev->can.clock.freq = bt_const->fclk_can;
+	dev->can.clock.freq = le32_to_cpu(bt_const->fclk_can);
 	dev->can.bittiming_const = &dev->bt_const;
 	dev->can.do_set_bittiming = gs_usb_set_bittiming;
 
 	dev->can.ctrlmode_supported = 0;
 
-	if (bt_const->feature & GS_CAN_FEATURE_LISTEN_ONLY)
+	feature = le32_to_cpu(bt_const->feature);
+	if (feature & GS_CAN_FEATURE_LISTEN_ONLY)
 		dev->can.ctrlmode_supported |= CAN_CTRLMODE_LISTENONLY;
 
-	if (bt_const->feature & GS_CAN_FEATURE_LOOP_BACK)
+	if (feature & GS_CAN_FEATURE_LOOP_BACK)
 		dev->can.ctrlmode_supported |= CAN_CTRLMODE_LOOPBACK;
 
-	if (bt_const->feature & GS_CAN_FEATURE_TRIPLE_SAMPLE)
+	if (feature & GS_CAN_FEATURE_TRIPLE_SAMPLE)
 		dev->can.ctrlmode_supported |= CAN_CTRLMODE_3_SAMPLES;
 
-	if (bt_const->feature & GS_CAN_FEATURE_ONE_SHOT)
+	if (feature & GS_CAN_FEATURE_ONE_SHOT)
 		dev->can.ctrlmode_supported |= CAN_CTRLMODE_ONE_SHOT;
 
 	SET_NETDEV_DEV(netdev, &intf->dev);
 
-	if (dconf->sw_version > 1)
-		if (bt_const->feature & GS_CAN_FEATURE_IDENTIFY)
+	if (le32_to_cpu(dconf->sw_version) > 1)
+		if (feature & GS_CAN_FEATURE_IDENTIFY)
 			netdev->ethtool_ops = &gs_usb_ethtool_ops;
 
 	kfree(bt_const);
@@ -910,7 +919,7 @@ static int gs_usb_probe(struct usb_interface *intf,
 	if (!hconf)
 		return -ENOMEM;
 
-	hconf->byte_order = 0x0000beef;
+	hconf->byte_order = cpu_to_le32(0x0000beef);
 
 	/* send host config */
 	rc = usb_control_msg(interface_to_usbdev(intf),

base-commit: cbf3d60329c4e11edcecac0c8fc6767b0f05e3a7
-- 
2.29.2



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [net 2/6] can: mcp251xfd: mcp251xfd_probe(): bail out if no IRQ was given
  2020-11-27 10:02 pull-request: can 2020-11-27 Marc Kleine-Budde
  2020-11-27 10:02 ` [net 1/6] can: gs_usb: fix endianess problem with candleLight firmware Marc Kleine-Budde
@ 2020-11-27 10:02 ` Marc Kleine-Budde
  2020-11-27 19:15   ` Jakub Kicinski
  2020-11-27 10:02 ` [net 3/6] can: m_can: m_can_open(): remove IRQF_TRIGGER_FALLING from request_threaded_irq()'s flags Marc Kleine-Budde
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 10+ messages in thread
From: Marc Kleine-Budde @ 2020-11-27 10:02 UTC (permalink / raw)
  To: netdev; +Cc: davem, kuba, linux-can, kernel, Marc Kleine-Budde, Niels Petter

This patch add a check to the mcp251xfd_probe() function to bail out and give
the user a proper error message if no IRQ is specified. Otherwise the driver
will probe just fine but ifup will fail with a meaningless "RTNETLINK answers:
Invalid argument" error message.

Link: https://lore.kernel.org/r/20201123113522.3820052-1-mkl@pengutronix.de
Reported-by: Niels Petter <petter@ka-long.de>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c b/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c
index 9c215f7c5f81..8a39be076e14 100644
--- a/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c
+++ b/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c
@@ -2738,6 +2738,10 @@ static int mcp251xfd_probe(struct spi_device *spi)
 	u32 freq;
 	int err;
 
+	if (!spi->irq)
+		return dev_err_probe(&spi->dev, -ENXIO,
+				     "No IRQ specified (maybe node \"interrupts-extended\" in DT missing)!\n");
+
 	rx_int = devm_gpiod_get_optional(&spi->dev, "microchip,rx-int",
 					 GPIOD_IN);
 	if (PTR_ERR(rx_int) == -EPROBE_DEFER)
-- 
2.29.2



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [net 3/6] can: m_can: m_can_open(): remove IRQF_TRIGGER_FALLING from request_threaded_irq()'s flags
  2020-11-27 10:02 pull-request: can 2020-11-27 Marc Kleine-Budde
  2020-11-27 10:02 ` [net 1/6] can: gs_usb: fix endianess problem with candleLight firmware Marc Kleine-Budde
  2020-11-27 10:02 ` [net 2/6] can: mcp251xfd: mcp251xfd_probe(): bail out if no IRQ was given Marc Kleine-Budde
@ 2020-11-27 10:02 ` Marc Kleine-Budde
  2020-11-27 10:02 ` [net 4/6] can: m_can: fix nominal bitiming tseg2 min for version >= 3.1 Marc Kleine-Budde
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 10+ messages in thread
From: Marc Kleine-Budde @ 2020-11-27 10:02 UTC (permalink / raw)
  To: netdev
  Cc: davem, kuba, linux-can, kernel, Marc Kleine-Budde, Dan Murphy,
	Sriram Dash, Pankaj Sharma

The threaded IRQ handler is used for the tcan4x5x driver only. The IRQ pin of
the tcan4x5x controller is active low, so better not use IRQF_TRIGGER_FALLING
when requesting the IRQ. As this can result in missing interrupts.

Further, if the device tree specified the interrupt as "IRQ_TYPE_LEVEL_LOW",
unloading and reloading of the driver results in the following error during
ifup:

| irq: type mismatch, failed to map hwirq-31 for gpio@20a8000!
| tcan4x5x spi1.1: m_can device registered (irq=0, version=32)
| tcan4x5x spi1.1 can2: TCAN4X5X successfully initialized.
| tcan4x5x spi1.1 can2: failed to request interrupt

This patch fixes the problem by removing the IRQF_TRIGGER_FALLING from the
request_threaded_irq().

Fixes: f524f829b75a ("can: m_can: Create a m_can platform framework")
Cc: Dan Murphy <dmurphy@ti.com>
Cc: Sriram Dash <sriram.dash@samsung.com>
Cc: Pankaj Sharma <pankj.sharma@samsung.com>
Link: https://lore.kernel.org/r/20201127093548.509253-1-mkl@pengutronix.de
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/m_can/m_can.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
index f3fc37e96b08..c62439bdee28 100644
--- a/drivers/net/can/m_can/m_can.c
+++ b/drivers/net/can/m_can/m_can.c
@@ -1653,7 +1653,7 @@ static int m_can_open(struct net_device *dev)
 		INIT_WORK(&cdev->tx_work, m_can_tx_work_queue);
 
 		err = request_threaded_irq(dev->irq, NULL, m_can_isr,
-					   IRQF_ONESHOT | IRQF_TRIGGER_FALLING,
+					   IRQF_ONESHOT,
 					   dev->name, dev);
 	} else {
 		err = request_irq(dev->irq, m_can_isr, IRQF_SHARED, dev->name,
-- 
2.29.2



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [net 4/6] can: m_can: fix nominal bitiming tseg2 min for version >= 3.1
  2020-11-27 10:02 pull-request: can 2020-11-27 Marc Kleine-Budde
                   ` (2 preceding siblings ...)
  2020-11-27 10:02 ` [net 3/6] can: m_can: m_can_open(): remove IRQF_TRIGGER_FALLING from request_threaded_irq()'s flags Marc Kleine-Budde
@ 2020-11-27 10:02 ` Marc Kleine-Budde
  2020-11-27 10:03 ` [net 5/6] can: m_can: m_can_dev_setup(): add support for bosch mcan version 3.3.0 Marc Kleine-Budde
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 10+ messages in thread
From: Marc Kleine-Budde @ 2020-11-27 10:02 UTC (permalink / raw)
  To: netdev
  Cc: davem, kuba, linux-can, kernel, Marc Kleine-Budde, Dan Murphy,
	Mario Huettel, Sriram Dash

At lest the revision 3.3.0 of the bosch m_can IP core specifies that valid
register values for "Nominal Time segment after sample point (NTSEG2)" are from
1 to 127. As the hardware uses a value of one more than the programmed value,
mean tseg2_min is 2.

This patch fixes the tseg2_min value accordingly.

Cc: Dan Murphy <dmurphy@ti.com>
Cc: Mario Huettel <mario.huettel@gmx.net>
Acked-by: Sriram Dash <sriram.dash@samsung.com>
Link: https://lore.kernel.org/r/20201124190751.3972238-1-mkl@pengutronix.de
Fixes: b03cfc5bb0e1 ("can: m_can: Enable M_CAN version dependent initialization")
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/m_can/m_can.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
index c62439bdee28..d4030abad935 100644
--- a/drivers/net/can/m_can/m_can.c
+++ b/drivers/net/can/m_can/m_can.c
@@ -1033,7 +1033,7 @@ static const struct can_bittiming_const m_can_bittiming_const_31X = {
 	.name = KBUILD_MODNAME,
 	.tseg1_min = 2,		/* Time segment 1 = prop_seg + phase_seg1 */
 	.tseg1_max = 256,
-	.tseg2_min = 1,		/* Time segment 2 = phase_seg2 */
+	.tseg2_min = 2,		/* Time segment 2 = phase_seg2 */
 	.tseg2_max = 128,
 	.sjw_max = 128,
 	.brp_min = 1,
-- 
2.29.2



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [net 5/6] can: m_can: m_can_dev_setup(): add support for bosch mcan version 3.3.0
  2020-11-27 10:02 pull-request: can 2020-11-27 Marc Kleine-Budde
                   ` (3 preceding siblings ...)
  2020-11-27 10:02 ` [net 4/6] can: m_can: fix nominal bitiming tseg2 min for version >= 3.1 Marc Kleine-Budde
@ 2020-11-27 10:03 ` Marc Kleine-Budde
  2020-11-27 10:03 ` [net 6/6] can: af_can: can_rx_unregister(): remove WARN() statement from list operation sanity check Marc Kleine-Budde
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 10+ messages in thread
From: Marc Kleine-Budde @ 2020-11-27 10:03 UTC (permalink / raw)
  To: netdev; +Cc: davem, kuba, linux-can, kernel, Pankaj Sharma, Marc Kleine-Budde

From: Pankaj Sharma <pankj.sharma@samsung.com>

Add support for mcan bit timing and control mode according to bosch mcan IP
version 3.3.0. The mcan version read from the Core Release field of CREL
register would be 33. Accordingly the properties are to be set for mcan v3.3.0

Signed-off-by: Pankaj Sharma <pankj.sharma@samsung.com>
Link: https://lore.kernel.org/r/1606366302-5520-1-git-send-email-pankj.sharma@samsung.com
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/m_can/m_can.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
index d4030abad935..61a93b192037 100644
--- a/drivers/net/can/m_can/m_can.c
+++ b/drivers/net/can/m_can/m_can.c
@@ -1385,6 +1385,8 @@ static int m_can_dev_setup(struct m_can_classdev *m_can_dev)
 						&m_can_data_bittiming_const_31X;
 		break;
 	case 32:
+	case 33:
+		/* Support both MCAN version v3.2.x and v3.3.0 */
 		m_can_dev->can.bittiming_const = m_can_dev->bit_timing ?
 			m_can_dev->bit_timing : &m_can_bittiming_const_31X;
 
-- 
2.29.2



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [net 6/6] can: af_can: can_rx_unregister(): remove WARN() statement from list operation sanity check
  2020-11-27 10:02 pull-request: can 2020-11-27 Marc Kleine-Budde
                   ` (4 preceding siblings ...)
  2020-11-27 10:03 ` [net 5/6] can: m_can: m_can_dev_setup(): add support for bosch mcan version 3.3.0 Marc Kleine-Budde
@ 2020-11-27 10:03 ` Marc Kleine-Budde
  2020-11-27 19:16 ` pull-request: can 2020-11-27 Jakub Kicinski
  2020-11-27 19:20 ` patchwork-bot+netdevbpf
  7 siblings, 0 replies; 10+ messages in thread
From: Marc Kleine-Budde @ 2020-11-27 10:03 UTC (permalink / raw)
  To: netdev
  Cc: davem, kuba, linux-can, kernel, Oliver Hartkopp,
	syzbot+381d06e0c8eaacb8706f, syzbot+d0ddd88c9a7432f041e6,
	syzbot+76d62d3b8162883c7d11, Marc Kleine-Budde

From: Oliver Hartkopp <socketcan@hartkopp.net>

To detect potential bugs in CAN protocol implementations (double removal of
receiver entries) a WARN() statement has been used if no matching list item was
found for removal.

The fault injection issued by syzkaller was able to create a situation where
the closing of a socket runs simultaneously to the notifier call chain for
removing the CAN network device in use.

This case is very unlikely in real life but it doesn't break anything.
Therefore we just replace the WARN() statement with pr_warn() to preserve the
notification for the CAN protocol development.

Reported-by: syzbot+381d06e0c8eaacb8706f@syzkaller.appspotmail.com
Reported-by: syzbot+d0ddd88c9a7432f041e6@syzkaller.appspotmail.com
Reported-by: syzbot+76d62d3b8162883c7d11@syzkaller.appspotmail.com
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
Link: https://lore.kernel.org/r/20201126192140.14350-1-socketcan@hartkopp.net
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 net/can/af_can.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/net/can/af_can.c b/net/can/af_can.c
index 5d124c155904..4c343b43067f 100644
--- a/net/can/af_can.c
+++ b/net/can/af_can.c
@@ -541,10 +541,13 @@ void can_rx_unregister(struct net *net, struct net_device *dev, canid_t can_id,
 
 	/* Check for bugs in CAN protocol implementations using af_can.c:
 	 * 'rcv' will be NULL if no matching list item was found for removal.
+	 * As this case may potentially happen when closing a socket while
+	 * the notifier for removing the CAN netdev is running we just print
+	 * a warning here.
 	 */
 	if (!rcv) {
-		WARN(1, "BUG: receive list entry not found for dev %s, id %03X, mask %03X\n",
-		     DNAME(dev), can_id, mask);
+		pr_warn("can: receive list entry not found for dev %s, id %03X, mask %03X\n",
+			DNAME(dev), can_id, mask);
 		goto out;
 	}
 
-- 
2.29.2



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [net 2/6] can: mcp251xfd: mcp251xfd_probe(): bail out if no IRQ was given
  2020-11-27 10:02 ` [net 2/6] can: mcp251xfd: mcp251xfd_probe(): bail out if no IRQ was given Marc Kleine-Budde
@ 2020-11-27 19:15   ` Jakub Kicinski
  0 siblings, 0 replies; 10+ messages in thread
From: Jakub Kicinski @ 2020-11-27 19:15 UTC (permalink / raw)
  To: Marc Kleine-Budde; +Cc: netdev, davem, linux-can, kernel, Niels Petter

On Fri, 27 Nov 2020 11:02:57 +0100 Marc Kleine-Budde wrote:
> This patch add a check to the mcp251xfd_probe() function to bail out and give
> the user a proper error message if no IRQ is specified. Otherwise the driver
> will probe just fine but ifup will fail with a meaningless "RTNETLINK answers:
> Invalid argument" error message.
> 
> Link: https://lore.kernel.org/r/20201123113522.3820052-1-mkl@pengutronix.de
> Reported-by: Niels Petter <petter@ka-long.de>
> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
> ---
>  drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c b/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c
> index 9c215f7c5f81..8a39be076e14 100644
> --- a/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c
> +++ b/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c
> @@ -2738,6 +2738,10 @@ static int mcp251xfd_probe(struct spi_device *spi)
>  	u32 freq;
>  	int err;
>  
> +	if (!spi->irq)
> +		return dev_err_probe(&spi->dev, -ENXIO,
> +				     "No IRQ specified (maybe node \"interrupts-extended\" 

FWIW this looks like an abuse of dev_err_probe() to me. What's the point
of calling it with a constant err which is not EPROBE_DEFER?

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: pull-request: can 2020-11-27
  2020-11-27 10:02 pull-request: can 2020-11-27 Marc Kleine-Budde
                   ` (5 preceding siblings ...)
  2020-11-27 10:03 ` [net 6/6] can: af_can: can_rx_unregister(): remove WARN() statement from list operation sanity check Marc Kleine-Budde
@ 2020-11-27 19:16 ` Jakub Kicinski
  2020-11-27 19:20 ` patchwork-bot+netdevbpf
  7 siblings, 0 replies; 10+ messages in thread
From: Jakub Kicinski @ 2020-11-27 19:16 UTC (permalink / raw)
  To: Marc Kleine-Budde; +Cc: netdev, davem, linux-can, kernel

On Fri, 27 Nov 2020 11:02:55 +0100 Marc Kleine-Budde wrote:
> The first patch is by me and target the gs_usb driver and fixes the endianess
> problem with candleLight firmware.
> 
> Another patch by me for the mcp251xfd driver add sanity checking to bail out if
> no IRQ is configured.
> 
> The next three patches target the m_can driver. A patch by me removes the
> hardcoded IRQF_TRIGGER_FALLING from the request_threaded_irq() as this clashes
> with the trigger level specified in the DT. Further a patch by me fixes the
> nominal bitiming tseg2 min value for modern m_can cores. Pankaj Sharma's patch
> add support for cores version 3.3.x.
> 
> The last patch by Oliver Hartkopp is for af_can and converts a WARN() into a
> pr_warn(), which is triggered by the syzkaller. It was able to create a
> situation where the closing of a socket runs simultaneously to the notifier
> call chain for removing the CAN network device in use.

Pulled, thanks!

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: pull-request: can 2020-11-27
  2020-11-27 10:02 pull-request: can 2020-11-27 Marc Kleine-Budde
                   ` (6 preceding siblings ...)
  2020-11-27 19:16 ` pull-request: can 2020-11-27 Jakub Kicinski
@ 2020-11-27 19:20 ` patchwork-bot+netdevbpf
  7 siblings, 0 replies; 10+ messages in thread
From: patchwork-bot+netdevbpf @ 2020-11-27 19:20 UTC (permalink / raw)
  To: Marc Kleine-Budde; +Cc: netdev, davem, kuba, linux-can, kernel

Hello:

This pull request was applied to netdev/net.git (refs/heads/master):

On Fri, 27 Nov 2020 11:02:55 +0100 you wrote:
> Hello Jakub, hello David,
> 
> here's a pull request of 6 patches for net/master.
> 
> The first patch is by me and target the gs_usb driver and fixes the endianess
> problem with candleLight firmware.
> 
> [...]

Here is the summary with links:
  - pull-request: can 2020-11-27
    https://git.kernel.org/netdev/net/c/d0742c49cab5
  - [net,2/6] can: mcp251xfd: mcp251xfd_probe(): bail out if no IRQ was given
    https://git.kernel.org/netdev/net/c/1a1c436bad34
  - [net,3/6] can: m_can: m_can_open(): remove IRQF_TRIGGER_FALLING from request_threaded_irq()'s flags
    https://git.kernel.org/netdev/net/c/865f5b671b48
  - [net,4/6] can: m_can: fix nominal bitiming tseg2 min for version >= 3.1
    https://git.kernel.org/netdev/net/c/e3409e419253
  - [net,5/6] can: m_can: m_can_dev_setup(): add support for bosch mcan version 3.3.0
    https://git.kernel.org/netdev/net/c/5c7d55bded77
  - [net,6/6] can: af_can: can_rx_unregister(): remove WARN() statement from list operation sanity check
    https://git.kernel.org/netdev/net/c/d73ff9b7c4ea

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2020-11-28 22:13 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-27 10:02 pull-request: can 2020-11-27 Marc Kleine-Budde
2020-11-27 10:02 ` [net 1/6] can: gs_usb: fix endianess problem with candleLight firmware Marc Kleine-Budde
2020-11-27 10:02 ` [net 2/6] can: mcp251xfd: mcp251xfd_probe(): bail out if no IRQ was given Marc Kleine-Budde
2020-11-27 19:15   ` Jakub Kicinski
2020-11-27 10:02 ` [net 3/6] can: m_can: m_can_open(): remove IRQF_TRIGGER_FALLING from request_threaded_irq()'s flags Marc Kleine-Budde
2020-11-27 10:02 ` [net 4/6] can: m_can: fix nominal bitiming tseg2 min for version >= 3.1 Marc Kleine-Budde
2020-11-27 10:03 ` [net 5/6] can: m_can: m_can_dev_setup(): add support for bosch mcan version 3.3.0 Marc Kleine-Budde
2020-11-27 10:03 ` [net 6/6] can: af_can: can_rx_unregister(): remove WARN() statement from list operation sanity check Marc Kleine-Budde
2020-11-27 19:16 ` pull-request: can 2020-11-27 Jakub Kicinski
2020-11-27 19:20 ` patchwork-bot+netdevbpf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).