* [PATCH net 1/2] Revert "can: m_can: pci: use custom bit timings for Elkhart Lake" [not found] <20220514185742.407230-1-mkl@pengutronix.de> @ 2022-05-14 18:57 ` Marc Kleine-Budde 2022-05-16 9:10 ` patchwork-bot+netdevbpf 0 siblings, 1 reply; 6+ messages in thread From: Marc Kleine-Budde @ 2022-05-14 18:57 UTC (permalink / raw) To: netdev Cc: davem, kuba, linux-can, kernel, Jarkko Nikula, Chee Hou Ong, Aman Kumar, Pallavi Kumari, stable, Marc Kleine-Budde From: Jarkko Nikula <jarkko.nikula@linux.intel.com> 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 <jarkko.nikula@linux.intel.com> Reported-by: Chee Hou Ong <chee.houx.ong@intel.com> Reported-by: Aman Kumar <aman.kumar@intel.com> Reported-by: Pallavi Kumari <kumari.pallavi@intel.com> Cc: <stable@vger.kernel.org> # v5.16+ Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de> --- 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); base-commit: f3f19f939c11925dadd3f4776f99f8c278a7017b -- 2.35.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH net 1/2] Revert "can: m_can: pci: use custom bit timings for Elkhart Lake" 2022-05-14 18:57 ` [PATCH net 1/2] Revert "can: m_can: pci: use custom bit timings for Elkhart Lake" Marc Kleine-Budde @ 2022-05-16 9:10 ` patchwork-bot+netdevbpf 0 siblings, 0 replies; 6+ messages in thread From: patchwork-bot+netdevbpf @ 2022-05-16 9:10 UTC (permalink / raw) To: Marc Kleine-Budde Cc: netdev, davem, kuba, linux-can, kernel, jarkko.nikula, chee.houx.ong, aman.kumar, kumari.pallavi, stable Hello: This series was applied to netdev/net.git (master) by Marc Kleine-Budde <mkl@pengutronix.de>: On Sat, 14 May 2022 20:57:41 +0200 you wrote: > From: Jarkko Nikula <jarkko.nikula@linux.intel.com> > > This reverts commit 0e8ffdf3b86dfd44b651f91b12fcae76c25c453b. > > Commit 0e8ffdf3b86d ("can: m_can: pci: use custom bit timings for > Elkhart Lake") broke the test case using bitrate switching. > > [...] Here is the summary with links: - [net,1/2] Revert "can: m_can: pci: use custom bit timings for Elkhart Lake" https://git.kernel.org/netdev/net/c/14ea4a470494 - [net,2/2] can: m_can: remove support for custom bit timing, take #2 https://git.kernel.org/netdev/net/c/d6da7881020f 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] 6+ messages in thread
[parent not found: <20220513130819.386012-1-mkl@pengutronix.de>]
* [PATCH net 1/2] Revert "can: m_can: pci: use custom bit timings for Elkhart Lake" [not found] <20220513130819.386012-1-mkl@pengutronix.de> @ 2022-05-13 13:08 ` Marc Kleine-Budde 2022-05-13 17:21 ` Jakub Kicinski 0 siblings, 1 reply; 6+ messages in thread From: Marc Kleine-Budde @ 2022-05-13 13:08 UTC (permalink / raw) To: netdev Cc: davem, kuba, linux-can, kernel, Jarkko Nikula, Chee Hou Ong, Aman Kumar, Pallavi Kumari, stable, Marc Kleine-Budde From: Jarkko Nikula <jarkko.nikula@linux.intel.com> 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: 0e8ffdf3b86d ("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 <jarkko.nikula@linux.intel.com> Reported-by: Chee Hou Ong <chee.houx.ong@intel.com> Reported-by: Aman Kumar <aman.kumar@intel.com> Reported-by: Pallavi Kumari <kumari.pallavi@intel.com> Cc: <stable@vger.kernel.org> # v5.16+ Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de> --- 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); base-commit: f3f19f939c11925dadd3f4776f99f8c278a7017b -- 2.35.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH net 1/2] Revert "can: m_can: pci: use custom bit timings for Elkhart Lake" 2022-05-13 13:08 ` Marc Kleine-Budde @ 2022-05-13 17:21 ` Jakub Kicinski 2022-05-14 19:00 ` Marc Kleine-Budde 2022-05-18 12:47 ` Jarkko Nikula 0 siblings, 2 replies; 6+ messages in thread From: Jakub Kicinski @ 2022-05-13 17:21 UTC (permalink / raw) To: Marc Kleine-Budde Cc: netdev, davem, linux-can, kernel, Jarkko Nikula, Chee Hou Ong, Aman Kumar, Pallavi Kumari, stable On Fri, 13 May 2022 15:08:18 +0200 Marc Kleine-Budde wrote: > From: Jarkko Nikula <jarkko.nikula@linux.intel.com> > > 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: 0e8ffdf3b86d ("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 <jarkko.nikula@linux.intel.com> > Reported-by: Chee Hou Ong <chee.houx.ong@intel.com> > Reported-by: Aman Kumar <aman.kumar@intel.com> > Reported-by: Pallavi Kumari <kumari.pallavi@intel.com> > Cc: <stable@vger.kernel.org> # v5.16+ > Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de> nit: the hash in the fixes tag should be: Fixes: ea4c1787685d ("can: m_can: pci: use custom bit timings for Elkhart Lake") Do you want to respin or is the can tree non-rebasable? ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net 1/2] Revert "can: m_can: pci: use custom bit timings for Elkhart Lake" 2022-05-13 17:21 ` Jakub Kicinski @ 2022-05-14 19:00 ` Marc Kleine-Budde 2022-05-18 12:47 ` Jarkko Nikula 1 sibling, 0 replies; 6+ messages in thread From: Marc Kleine-Budde @ 2022-05-14 19:00 UTC (permalink / raw) To: Jakub Kicinski Cc: netdev, davem, linux-can, kernel, Jarkko Nikula, Chee Hou Ong, Aman Kumar, Pallavi Kumari, stable [-- Attachment #1: Type: text/plain, Size: 2367 bytes --] On 13.05.2022 10:21:45, Jakub Kicinski wrote: > On Fri, 13 May 2022 15:08:18 +0200 Marc Kleine-Budde wrote: > > From: Jarkko Nikula <jarkko.nikula@linux.intel.com> > > > > 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: 0e8ffdf3b86d ("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 <jarkko.nikula@linux.intel.com> > > Reported-by: Chee Hou Ong <chee.houx.ong@intel.com> > > Reported-by: Aman Kumar <aman.kumar@intel.com> > > Reported-by: Pallavi Kumari <kumari.pallavi@intel.com> > > Cc: <stable@vger.kernel.org> # v5.16+ > > Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de> > > nit: the hash in the fixes tag should be: Doh! > Fixes: ea4c1787685d ("can: m_can: pci: use custom bit timings for Elkhart Lake") > > Do you want to respin or is the can tree non-rebasable? No - it's non-rebasable as soon as merged to net(-next) :) Here's a new pull request with a adjusted Fixes tag. | https://lore.kernel.org/all/20220514185742.407230-1-mkl@pengutronix.de regards, Marc -- Pengutronix e.K. | Marc Kleine-Budde | Embedded Linux | https://www.pengutronix.de | Vertretung West/Dortmund | Phone: +49-231-2826-924 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net 1/2] Revert "can: m_can: pci: use custom bit timings for Elkhart Lake" 2022-05-13 17:21 ` Jakub Kicinski 2022-05-14 19:00 ` Marc Kleine-Budde @ 2022-05-18 12:47 ` Jarkko Nikula 1 sibling, 0 replies; 6+ messages in thread From: Jarkko Nikula @ 2022-05-18 12:47 UTC (permalink / raw) To: Jakub Kicinski, Marc Kleine-Budde Cc: netdev, davem, linux-can, kernel, Chee Hou Ong, Aman Kumar, Pallavi Kumari, stable Hi Sorry the late response. I was offline a few days. On 5/13/22 20:21, Jakub Kicinski wrote: >> Fixes: 0e8ffdf3b86d ("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 <jarkko.nikula@linux.intel.com> >> Reported-by: Chee Hou Ong <chee.houx.ong@intel.com> >> Reported-by: Aman Kumar <aman.kumar@intel.com> >> Reported-by: Pallavi Kumari <kumari.pallavi@intel.com> >> Cc: <stable@vger.kernel.org> # v5.16+ >> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de> > > nit: the hash in the fixes tag should be: > > Fixes: ea4c1787685d ("can: m_can: pci: use custom bit timings for Elkhart Lake") > > Do you want to respin or is the can tree non-rebasable? Grr... Looks like I took accidentally linux-stable commit Id. Obviously too hurry to vacation :-| Thanks for fixing it up Marc! ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-05-18 12:48 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20220514185742.407230-1-mkl@pengutronix.de>
2022-05-14 18:57 ` [PATCH net 1/2] Revert "can: m_can: pci: use custom bit timings for Elkhart Lake" Marc Kleine-Budde
2022-05-16 9:10 ` patchwork-bot+netdevbpf
[not found] <20220513130819.386012-1-mkl@pengutronix.de>
2022-05-13 13:08 ` Marc Kleine-Budde
2022-05-13 17:21 ` Jakub Kicinski
2022-05-14 19:00 ` Marc Kleine-Budde
2022-05-18 12:47 ` Jarkko Nikula
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).