From 14ea4a470494528c7e88da5c4116c24eb027059f Mon Sep 17 00:00:00 2001 From: Jarkko Nikula Date: Thu, 12 May 2022 15:41:43 +0300 Subject: [PATCH 1/2] Revert "can: m_can: pci: use custom bit timings for Elkhart Lake" This reverts commit 0e8ffdf3b86dfd44b651f91b12fcae76c25c453b. Commit 0e8ffdf3b86d ("can: m_can: pci: use custom bit timings for Elkhart Lake") broke the test case using bitrate switching. | ip link set can0 up type can bitrate 500000 dbitrate 4000000 fd on | ip link set can1 up type can bitrate 500000 dbitrate 4000000 fd on | candump can0 & | cangen can1 -I 0x800 -L 64 -e -fb \ | -D 11223344deadbeef55667788feedf00daabbccdd44332211 -n 1 -v -v Above commit does everything correctly according to the datasheet. However datasheet wasn't correct. I got confirmation from hardware engineers that the actual CAN hardware on Intel Elkhart Lake is based on M_CAN version v3.2.0. Datasheet was mirroring values from an another specification which was based on earlier M_CAN version leading to wrong bit timings. Therefore revert the commit and switch back to common bit timings. Fixes: ea4c1787685d ("can: m_can: pci: use custom bit timings for Elkhart Lake") Link: https://lore.kernel.org/all/20220512124144.536850-1-jarkko.nikula@linux.intel.com Signed-off-by: Jarkko Nikula Reported-by: Chee Hou Ong Reported-by: Aman Kumar Reported-by: Pallavi Kumari Cc: # v5.16+ Signed-off-by: Marc Kleine-Budde --- drivers/net/can/m_can/m_can_pci.c | 48 +++---------------------------- 1 file changed, 4 insertions(+), 44 deletions(-) diff --git a/drivers/net/can/m_can/m_can_pci.c b/drivers/net/can/m_can/m_can_pci.c index b56a54d6c5a9..8f184a852a0a 100644 --- a/drivers/net/can/m_can/m_can_pci.c +++ b/drivers/net/can/m_can/m_can_pci.c @@ -18,14 +18,9 @@ #define M_CAN_PCI_MMIO_BAR 0 +#define M_CAN_CLOCK_FREQ_EHL 200000000 #define CTL_CSR_INT_CTL_OFFSET 0x508 -struct m_can_pci_config { - const struct can_bittiming_const *bit_timing; - const struct can_bittiming_const *data_timing; - unsigned int clock_freq; -}; - struct m_can_pci_priv { struct m_can_classdev cdev; @@ -89,40 +84,9 @@ static struct m_can_ops m_can_pci_ops = { .read_fifo = iomap_read_fifo, }; -static const struct can_bittiming_const m_can_bittiming_const_ehl = { - .name = KBUILD_MODNAME, - .tseg1_min = 2, /* Time segment 1 = prop_seg + phase_seg1 */ - .tseg1_max = 64, - .tseg2_min = 1, /* Time segment 2 = phase_seg2 */ - .tseg2_max = 128, - .sjw_max = 128, - .brp_min = 1, - .brp_max = 512, - .brp_inc = 1, -}; - -static const struct can_bittiming_const m_can_data_bittiming_const_ehl = { - .name = KBUILD_MODNAME, - .tseg1_min = 2, /* Time segment 1 = prop_seg + phase_seg1 */ - .tseg1_max = 16, - .tseg2_min = 1, /* Time segment 2 = phase_seg2 */ - .tseg2_max = 8, - .sjw_max = 4, - .brp_min = 1, - .brp_max = 32, - .brp_inc = 1, -}; - -static const struct m_can_pci_config m_can_pci_ehl = { - .bit_timing = &m_can_bittiming_const_ehl, - .data_timing = &m_can_data_bittiming_const_ehl, - .clock_freq = 200000000, -}; - static int m_can_pci_probe(struct pci_dev *pci, const struct pci_device_id *id) { struct device *dev = &pci->dev; - const struct m_can_pci_config *cfg; struct m_can_classdev *mcan_class; struct m_can_pci_priv *priv; void __iomem *base; @@ -150,8 +114,6 @@ static int m_can_pci_probe(struct pci_dev *pci, const struct pci_device_id *id) if (!mcan_class) return -ENOMEM; - cfg = (const struct m_can_pci_config *)id->driver_data; - priv = cdev_to_priv(mcan_class); priv->base = base; @@ -163,9 +125,7 @@ static int m_can_pci_probe(struct pci_dev *pci, const struct pci_device_id *id) mcan_class->dev = &pci->dev; mcan_class->net->irq = pci_irq_vector(pci, 0); mcan_class->pm_clock_support = 1; - mcan_class->bit_timing = cfg->bit_timing; - mcan_class->data_timing = cfg->data_timing; - mcan_class->can.clock.freq = cfg->clock_freq; + mcan_class->can.clock.freq = id->driver_data; mcan_class->ops = &m_can_pci_ops; pci_set_drvdata(pci, mcan_class); @@ -218,8 +178,8 @@ static SIMPLE_DEV_PM_OPS(m_can_pci_pm_ops, m_can_pci_suspend, m_can_pci_resume); static const struct pci_device_id m_can_pci_id_table[] = { - { PCI_VDEVICE(INTEL, 0x4bc1), (kernel_ulong_t)&m_can_pci_ehl, }, - { PCI_VDEVICE(INTEL, 0x4bc2), (kernel_ulong_t)&m_can_pci_ehl, }, + { PCI_VDEVICE(INTEL, 0x4bc1), M_CAN_CLOCK_FREQ_EHL, }, + { PCI_VDEVICE(INTEL, 0x4bc2), M_CAN_CLOCK_FREQ_EHL, }, { } /* Terminating Entry */ }; MODULE_DEVICE_TABLE(pci, m_can_pci_id_table); From d6da7881020f9b37edb80ada12ce9b50b9232dc1 Mon Sep 17 00:00:00 2001 From: Jarkko Nikula Date: Thu, 12 May 2022 15:41:44 +0300 Subject: [PATCH 2/2] can: m_can: remove support for custom bit timing, take #2 Now when Intel Elkhart Lake uses again common bit timing and there are no other users for custom bit timing, we can bring back the changes done by the commit 0ddd83fbebbc ("can: m_can: remove support for custom bit timing"). This effectively reverts commit ea768b2ffec6 ("Revert "can: m_can: remove support for custom bit timing"") while taking into account commit ea22ba40debe ("can: m_can: make custom bittiming fields const") and commit 7d4a101c0bd3 ("can: dev: add sanity check in can_set_static_ctrlmode()"). Link: https://lore.kernel.org/all/20220512124144.536850-2-jarkko.nikula@linux.intel.com Signed-off-by: Jarkko Nikula Signed-off-by: Marc Kleine-Budde --- drivers/net/can/m_can/m_can.c | 24 ++++++------------------ drivers/net/can/m_can/m_can.h | 3 --- 2 files changed, 6 insertions(+), 21 deletions(-) diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c index b3b5bc1c803b..088bb1bcf1ef 100644 --- a/drivers/net/can/m_can/m_can.c +++ b/drivers/net/can/m_can/m_can.c @@ -1495,34 +1495,22 @@ static int m_can_dev_setup(struct m_can_classdev *cdev) err = can_set_static_ctrlmode(dev, CAN_CTRLMODE_FD_NON_ISO); if (err) return err; - cdev->can.bittiming_const = cdev->bit_timing ? - cdev->bit_timing : &m_can_bittiming_const_30X; - - cdev->can.data_bittiming_const = cdev->data_timing ? - cdev->data_timing : - &m_can_data_bittiming_const_30X; + cdev->can.bittiming_const = &m_can_bittiming_const_30X; + cdev->can.data_bittiming_const = &m_can_data_bittiming_const_30X; break; case 31: /* CAN_CTRLMODE_FD_NON_ISO is fixed with M_CAN IP v3.1.x */ err = can_set_static_ctrlmode(dev, CAN_CTRLMODE_FD_NON_ISO); if (err) return err; - cdev->can.bittiming_const = cdev->bit_timing ? - cdev->bit_timing : &m_can_bittiming_const_31X; - - cdev->can.data_bittiming_const = cdev->data_timing ? - cdev->data_timing : - &m_can_data_bittiming_const_31X; + cdev->can.bittiming_const = &m_can_bittiming_const_31X; + cdev->can.data_bittiming_const = &m_can_data_bittiming_const_31X; break; case 32: case 33: /* Support both MCAN version v3.2.x and v3.3.0 */ - cdev->can.bittiming_const = cdev->bit_timing ? - cdev->bit_timing : &m_can_bittiming_const_31X; - - cdev->can.data_bittiming_const = cdev->data_timing ? - cdev->data_timing : - &m_can_data_bittiming_const_31X; + cdev->can.bittiming_const = &m_can_bittiming_const_31X; + cdev->can.data_bittiming_const = &m_can_data_bittiming_const_31X; cdev->can.ctrlmode_supported |= (m_can_niso_supported(cdev) ? diff --git a/drivers/net/can/m_can/m_can.h b/drivers/net/can/m_can/m_can.h index 2c5d40997168..d18b515e6ccc 100644 --- a/drivers/net/can/m_can/m_can.h +++ b/drivers/net/can/m_can/m_can.h @@ -85,9 +85,6 @@ struct m_can_classdev { struct sk_buff *tx_skb; struct phy *transceiver; - const struct can_bittiming_const *bit_timing; - const struct can_bittiming_const *data_timing; - struct m_can_ops *ops; int version;