linux-can.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net 0/2] pull-request: can 2022-05-14
@ 2022-05-14 18:57 Marc Kleine-Budde
  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-14 18:57 ` [PATCH net 2/2] can: m_can: remove support for custom bit timing, take #2 Marc Kleine-Budde
  0 siblings, 2 replies; 5+ messages in thread
From: Marc Kleine-Budde @ 2022-05-14 18:57 UTC (permalink / raw)
  To: netdev; +Cc: davem, kuba, linux-can, kernel

Hello Jakub, hello David,

this is a pull request of 2 patches for net/master.

Changes to linux-can-fixes-for-5.18-20220513:
- adjusted Fixes: Tag on "Revert "can: m_can: pci: use custom bit timings for Elkhart Lake""
  (Thanks Jakub)

Both patches are by Jarkko Nikula, target the m_can PCI driver
bindings, and fix usage of wrong bit timing constants for the Elkhart
Lake platform.

regards,
Marc

---

The following changes since commit f3f19f939c11925dadd3f4776f99f8c278a7017b:

  Merge tag 'net-5.18-rc7' of git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net (2022-05-12 11:51:45 -0700)

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.18-20220514

for you to fetch changes up to d6da7881020f9b37edb80ada12ce9b50b9232dc1:

  can: m_can: remove support for custom bit timing, take #2 (2022-05-14 20:53:01 +0200)

----------------------------------------------------------------
linux-can-fixes-for-5.18-20220514

----------------------------------------------------------------
Jarkko Nikula (2):
      Revert "can: m_can: pci: use custom bit timings for Elkhart Lake"
      can: m_can: remove support for custom bit timing, take #2

 drivers/net/can/m_can/m_can.c     | 24 +++++---------------
 drivers/net/can/m_can/m_can.h     |  3 ---
 drivers/net/can/m_can/m_can_pci.c | 48 ++++-----------------------------------
 3 files changed, 10 insertions(+), 65 deletions(-)



^ permalink raw reply	[flat|nested] 5+ messages in thread
* [PATCH net 1/2] Revert "can: m_can: pci: use custom bit timings for Elkhart Lake"
  2022-05-14 18:57 [PATCH net 0/2] pull-request: can 2022-05-14 Marc Kleine-Budde
@ 2022-05-14 18:57 ` Marc Kleine-Budde
  2022-05-16  9:10   ` patchwork-bot+netdevbpf
  2022-05-14 18:57 ` [PATCH net 2/2] can: m_can: remove support for custom bit timing, take #2 Marc Kleine-Budde
  1 sibling, 1 reply; 5+ 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] 5+ messages in thread
* [PATCH net 2/2] can: m_can: remove support for custom bit timing, take #2
  2022-05-14 18:57 [PATCH net 0/2] pull-request: can 2022-05-14 Marc Kleine-Budde
  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-14 18:57 ` Marc Kleine-Budde
  1 sibling, 0 replies; 5+ 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, Marc Kleine-Budde

From: Jarkko Nikula <jarkko.nikula@linux.intel.com>

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 <jarkko.nikula@linux.intel.com>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 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;
-- 
2.35.1



^ permalink raw reply related	[flat|nested] 5+ 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; 5+ 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] 5+ messages in thread
* [PATCH net 2/2] can: m_can: remove support for custom bit timing, take #2
  2022-05-13 13:08 [PATCH net 0/2] pull-request: can 2022-05-13 Marc Kleine-Budde
@ 2022-05-13 13:08 ` Marc Kleine-Budde
  0 siblings, 0 replies; 5+ 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, Marc Kleine-Budde

From: Jarkko Nikula <jarkko.nikula@linux.intel.com>

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 <jarkko.nikula@linux.intel.com>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 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;
-- 
2.35.1



^ permalink raw reply related	[flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-05-16  9:10 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-14 18:57 [PATCH net 0/2] pull-request: can 2022-05-14 Marc Kleine-Budde
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
2022-05-14 18:57 ` [PATCH net 2/2] can: m_can: remove support for custom bit timing, take #2 Marc Kleine-Budde
  -- strict thread matches above, loose matches on Subject: below --
2022-05-13 13:08 [PATCH net 0/2] pull-request: can 2022-05-13 Marc Kleine-Budde
2022-05-13 13:08 ` [PATCH net 2/2] can: m_can: remove support for custom bit timing, take #2 Marc Kleine-Budde

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).