[PATCH v4 0/6] can: mcp251xfd: add gpio functionality

[PATCH v4 0/6] can: mcp251xfd: add gpio functionality

[Viken Dadhaniya]

Hi all,

The mcp251xfd allows two pins to be configured as GPIOs. This series
adds support for this feature.

The GPIO functionality is controlled with the IOCON register which has
an erratum.

Patch 1 from https://lore.kernel.org/linux-can/20240429-mcp251xfd-runtime_pm-v1-3-c26a93a66544@pengutronix.de/
Patch 2 refactor of no-crc functions to prepare workaround for non-crc writes
Patch 3 is the fix/workaround for the aforementioned erratum
Patch 4 only configure pin1 for rx-int
Patch 5 adds the gpio support
Patch 6 updates dt-binding

As per Marc's comment on below patch, we aim to get this series into
linux-next since the functionality is essential for CAN on the RB3 Gen2
board. As progress has stalled, Take this series forward with minor code
adjustments. Include a Tested-by tag to reflect validation performed on the
target hardware.

https://lore.kernel.org/all/20240806-industrious-augmented-crane-44239a-mkl@pengutronix.de/
---
Changes in v4:
- Moved GPIO register initialization into mcp251xfd_register after enabling
  runtime PM to avoid GPIO request failures when using the gpio-hog
  property to set default GPIO state.
- Added Tested-by and Signed-off-by tags.
- Dropped the 1st and 2nd patches from the v3 series as they have already been merged.
- Link to v3: https://lore.kernel.org/linux-can/20240522-mcp251xfd-gpio-feature-v3-0-8829970269c5@ew.tq-group.com/

Changes in v3:
- Implement workaround for non-crc writes
- Configure only Pin1 for rx-int feature
- moved errata check to .gather_write callback function
- Added MCP251XFD_REG_IOCON_*() macros
- Added Marcs suggestions
- Collect Krzysztofs Acked-By
- Link to v2: https://lore.kernel.org/r/20240506-mcp251xfd-gpio-feature-v2-0-615b16fa8789@ew.tq-group.com

Changes in v2:
- picked Marcs patches from https://lore.kernel.org/linux-can/20240429-mcp251xfd-runtime_pm-v1-0-c26a93a66544@pengutronix.de/
- Drop regcache
- Add pm_runtime in mcp251xfd_gpio_request/mcp251xfd_gpio_free
- Implement mcp251xfd_gpio_get_multiple/mcp251xfd_gpio_set_multiple
- Move check for rx_int/gpio conflict to mcp251xfd_gpio_request
- Link to v1: https://lore.kernel.org/r/20240417-mcp251xfd-gpio-feature-v1-0-bc0c61fd0c80@ew.tq-group.com

---
Gregor Herburger (5):
  can: mcp251xfd: utilize gather_write function for all non-CRC writes
  can: mcp251xfd: add workaround for errata 5
  can: mcp251xfd: only configure PIN1 when rx_int is set
  can: mcp251xfd: add gpio functionality
  dt-bindings: can: mcp251xfd: add gpio-controller property

Marc Kleine-Budde (1):
  can: mcp251xfd: move chip sleep mode into runtime pm

 .../bindings/net/can/microchip,mcp251xfd.yaml |   5 +
 .../net/can/spi/mcp251xfd/mcp251xfd-core.c    | 292 +++++++++++++++---
 .../net/can/spi/mcp251xfd/mcp251xfd-regmap.c  | 114 +++++--
 drivers/net/can/spi/mcp251xfd/mcp251xfd.h     |  10 +
 4 files changed, 355 insertions(+), 66 deletions(-)

-- 
2.34.1

[PATCH v4 1/6] can: mcp251xfd: move chip sleep mode into runtime pm

[Viken Dadhaniya]

From: Marc Kleine-Budde <mkl@pengutronix.de>

This is a preparation patch to add GPIO support.

Up to now, the Vdd regulator and the clocks have been managed by
Runtime-PM (on systems without CONFIG_PM these remain permanently
switched on).

During the mcp251xfd_open() callback the mcp251xfd is powered,
soft-reset and configured. In mcp251xfd_stop() the chip is shut down
again. To support the on-chip GPIOs, the chip must be supplied with
power while GPIOs are being requested, even if the networking
interface is down.

To support this, move the functions mcp251xfd_chip_softreset() and
mcp251xfd_chip_clock_init() from mcp251xfd_chip_start() to
mcp251xfd_runtime_resume(). Instead of setting the controller to sleep
mode in mcp251xfd_chip_stop(), bring it into configuration mode. This
way it doesn't take part in bus activity and doesn't enter sleep mode.

Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
Signed-off-by: Gregor Herburger <gregor.herburger@ew.tq-group.com>
Tested-by: Viken Dadhaniya <viken.dadhaniya@oss.qualcomm.com>
Signed-off-by: Viken Dadhaniya <viken.dadhaniya@oss.qualcomm.com>
---
 .../net/can/spi/mcp251xfd/mcp251xfd-core.c    | 91 ++++++++++++-------
 1 file changed, 57 insertions(+), 34 deletions(-)

diff --git a/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c b/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c
index 7450ea42c1ea..f9eabb1810cf 100644
--- a/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c
+++ b/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c
@@ -767,21 +767,13 @@ static void mcp251xfd_chip_stop(struct mcp251xfd_priv *priv,
 	mcp251xfd_chip_interrupts_disable(priv);
 	mcp251xfd_chip_rx_int_disable(priv);
 	mcp251xfd_timestamp_stop(priv);
-	mcp251xfd_chip_sleep(priv);
+	mcp251xfd_chip_set_mode(priv, MCP251XFD_REG_CON_MODE_CONFIG);
 }
 
 static int mcp251xfd_chip_start(struct mcp251xfd_priv *priv)
 {
 	int err;
 
-	err = mcp251xfd_chip_softreset(priv);
-	if (err)
-		goto out_chip_stop;
-
-	err = mcp251xfd_chip_clock_init(priv);
-	if (err)
-		goto out_chip_stop;
-
 	err = mcp251xfd_chip_timestamp_init(priv);
 	if (err)
 		goto out_chip_stop;
@@ -1625,8 +1617,11 @@ static int mcp251xfd_open(struct net_device *ndev)
 		return err;
 
 	err = pm_runtime_resume_and_get(ndev->dev.parent);
-	if (err)
+	if (err) {
+		if (err == -ETIMEDOUT || err == -ENODEV)
+			pm_runtime_set_suspended(ndev->dev.parent);
 		goto out_close_candev;
+	}
 
 	err = mcp251xfd_ring_alloc(priv);
 	if (err)
@@ -1907,53 +1902,53 @@ static int mcp251xfd_register(struct mcp251xfd_priv *priv)
 	struct net_device *ndev = priv->ndev;
 	int err;
 
+	mcp251xfd_register_quirks(priv);
+
 	err = mcp251xfd_clks_and_vdd_enable(priv);
 	if (err)
 		return err;
 
-	pm_runtime_get_noresume(ndev->dev.parent);
-	err = pm_runtime_set_active(ndev->dev.parent);
-	if (err)
-		goto out_runtime_put_noidle;
-	pm_runtime_enable(ndev->dev.parent);
-
-	mcp251xfd_register_quirks(priv);
-
 	err = mcp251xfd_chip_softreset(priv);
 	if (err == -ENODEV)
-		goto out_runtime_disable;
+		goto out_clks_and_vdd_disable;
 	if (err)
 		goto out_chip_sleep;
 
 	err = mcp251xfd_chip_clock_init(priv);
 	if (err == -ENODEV)
-		goto out_runtime_disable;
+		goto out_clks_and_vdd_disable;
 	if (err)
 		goto out_chip_sleep;
 
+	pm_runtime_get_noresume(ndev->dev.parent);
+	err = pm_runtime_set_active(ndev->dev.parent);
+	if (err)
+		goto out_runtime_put_noidle;
+	pm_runtime_enable(ndev->dev.parent);
+
 	err = mcp251xfd_register_chip_detect(priv);
 	if (err)
-		goto out_chip_sleep;
+		goto out_runtime_disable;
 
 	err = mcp251xfd_register_check_rx_int(priv);
 	if (err)
-		goto out_chip_sleep;
+		goto out_runtime_disable;
 
 	mcp251xfd_ethtool_init(priv);
 
 	err = register_candev(ndev);
 	if (err)
-		goto out_chip_sleep;
+		goto out_runtime_disable;
 
 	err = mcp251xfd_register_done(priv);
 	if (err)
 		goto out_unregister_candev;
 
-	/* Put controller into sleep mode and let pm_runtime_put()
-	 * disable the clocks and vdd. If CONFIG_PM is not enabled,
-	 * the clocks and vdd will stay powered.
+	/* Put controller into Config mode and let pm_runtime_put()
+	 * put in sleep mode, disable the clocks and vdd. If CONFIG_PM
+	 * is not enabled, the clocks and vdd will stay powered.
 	 */
-	err = mcp251xfd_chip_sleep(priv);
+	err = mcp251xfd_chip_set_mode(priv, MCP251XFD_REG_CON_MODE_CONFIG);
 	if (err)
 		goto out_unregister_candev;
 
@@ -1963,12 +1958,13 @@ static int mcp251xfd_register(struct mcp251xfd_priv *priv)
 
 out_unregister_candev:
 	unregister_candev(ndev);
-out_chip_sleep:
-	mcp251xfd_chip_sleep(priv);
 out_runtime_disable:
 	pm_runtime_disable(ndev->dev.parent);
 out_runtime_put_noidle:
 	pm_runtime_put_noidle(ndev->dev.parent);
+out_chip_sleep:
+	mcp251xfd_chip_sleep(priv);
+out_clks_and_vdd_disable:
 	mcp251xfd_clks_and_vdd_disable(priv);
 
 	return err;
@@ -1980,10 +1976,12 @@ static inline void mcp251xfd_unregister(struct mcp251xfd_priv *priv)
 
 	unregister_candev(ndev);
 
-	if (pm_runtime_enabled(ndev->dev.parent))
+	if (pm_runtime_enabled(ndev->dev.parent)) {
 		pm_runtime_disable(ndev->dev.parent);
-	else
+	} else {
+		mcp251xfd_chip_sleep(priv);
 		mcp251xfd_clks_and_vdd_disable(priv);
+	}
 }
 
 static const struct of_device_id mcp251xfd_of_match[] = {
@@ -2206,16 +2204,41 @@ static void mcp251xfd_remove(struct spi_device *spi)
 
 static int __maybe_unused mcp251xfd_runtime_suspend(struct device *device)
 {
-	const struct mcp251xfd_priv *priv = dev_get_drvdata(device);
+	struct mcp251xfd_priv *priv = dev_get_drvdata(device);
 
+	mcp251xfd_chip_sleep(priv);
 	return mcp251xfd_clks_and_vdd_disable(priv);
 }
 
 static int __maybe_unused mcp251xfd_runtime_resume(struct device *device)
 {
-	const struct mcp251xfd_priv *priv = dev_get_drvdata(device);
+	struct mcp251xfd_priv *priv = dev_get_drvdata(device);
+	int err;
+
+	err = mcp251xfd_clks_and_vdd_enable(priv);
+	if (err)
+		return err;
 
-	return mcp251xfd_clks_and_vdd_enable(priv);
+	err = mcp251xfd_chip_softreset(priv);
+	if (err == -ENODEV)
+		goto out_clks_and_vdd_disable;
+	if (err)
+		goto out_chip_sleep;
+
+	err = mcp251xfd_chip_clock_init(priv);
+	if (err == -ENODEV)
+		goto out_clks_and_vdd_disable;
+	if (err)
+		goto out_chip_sleep;
+
+	return 0;
+
+out_chip_sleep:
+	mcp251xfd_chip_sleep(priv);
+out_clks_and_vdd_disable:
+	mcp251xfd_clks_and_vdd_disable(priv);
+
+	return err;
 }
 
 static const struct dev_pm_ops mcp251xfd_pm_ops = {
-- 
2.34.1

[PATCH v4 2/6] can: mcp251xfd: utilize gather_write function for all non-CRC writes

[Viken Dadhaniya]

From: Gregor Herburger <gregor.herburger@ew.tq-group.com>

This is a preparation patch to add errata workaround for non crc writes.

Currently for non-crc writes to the chip can go through the
.gather_write, .write or the reg_update_bits callback.

To allow the addition of the errata fix at a single location use
mcp251xfd_regmap_nocrc_gather_write for all non-CRC write instructions,
similar to the crc regmap.

Signed-off-by: Gregor Herburger <gregor.herburger@ew.tq-group.com>
Tested-by: Viken Dadhaniya <viken.dadhaniya@oss.qualcomm.com>
Signed-off-by: Viken Dadhaniya <viken.dadhaniya@oss.qualcomm.com>
---
 .../net/can/spi/mcp251xfd/mcp251xfd-regmap.c  | 25 ++++++++++---------
 1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/drivers/net/can/spi/mcp251xfd/mcp251xfd-regmap.c b/drivers/net/can/spi/mcp251xfd/mcp251xfd-regmap.c
index 8c5be8d1c519..e61cbd209955 100644
--- a/drivers/net/can/spi/mcp251xfd/mcp251xfd-regmap.c
+++ b/drivers/net/can/spi/mcp251xfd/mcp251xfd-regmap.c
@@ -12,14 +12,6 @@
 
 static const struct regmap_config mcp251xfd_regmap_crc;
 
-static int
-mcp251xfd_regmap_nocrc_write(void *context, const void *data, size_t count)
-{
-	struct spi_device *spi = context;
-
-	return spi_write(spi, data, count);
-}
-
 static int
 mcp251xfd_regmap_nocrc_gather_write(void *context,
 				    const void *reg, size_t reg_len,
@@ -47,6 +39,15 @@ mcp251xfd_regmap_nocrc_gather_write(void *context,
 	return spi_sync_transfer(spi, xfer, ARRAY_SIZE(xfer));
 }
 
+static int
+mcp251xfd_regmap_nocrc_write(void *context, const void *data, size_t count)
+{
+	const size_t data_offset = sizeof(__be16);
+
+	return mcp251xfd_regmap_nocrc_gather_write(context, data, data_offset,
+						   data + data_offset, count - data_offset);
+}
+
 static inline bool
 mcp251xfd_update_bits_read_reg(const struct mcp251xfd_priv *priv,
 			       unsigned int reg)
@@ -64,6 +65,7 @@ mcp251xfd_update_bits_read_reg(const struct mcp251xfd_priv *priv,
 	case MCP251XFD_REG_CON:
 	case MCP251XFD_REG_OSC:
 	case MCP251XFD_REG_ECCCON:
+	case MCP251XFD_REG_IOCON:
 		return true;
 	default:
 		mcp251xfd_for_each_rx_ring(priv, ring, n) {
@@ -139,10 +141,9 @@ mcp251xfd_regmap_nocrc_update_bits(void *context, unsigned int reg,
 	tmp_le32 = orig_le32 & ~mask_le32;
 	tmp_le32 |= val_le32 & mask_le32;
 
-	mcp251xfd_spi_cmd_write_nocrc(&buf_tx->cmd, reg + first_byte);
-	memcpy(buf_tx->data, &tmp_le32, len);
-
-	return spi_write(spi, buf_tx, sizeof(buf_tx->cmd) + len);
+	reg += first_byte;
+	mcp251xfd_spi_cmd_write_nocrc(&buf_tx->cmd, reg);
+	return mcp251xfd_regmap_nocrc_gather_write(context, &buf_tx->cmd, 2, &tmp_le32, len);
 }
 
 static int
-- 
2.34.1

[PATCH v4 3/6] can: mcp251xfd: add workaround for errata 5

[Viken Dadhaniya]

From: Gregor Herburger <gregor.herburger@ew.tq-group.com>

According to Errata DS80000789E 5 writing IOCON register using one SPI
write command clears LAT0/LAT1.

Errata Fix/Work Around suggests to write registers with single byte write
instructions. However, it seems that every write to the second byte
causes the overwrite of LAT0/LAT1.

Never write byte 2 of IOCON register to avoid clearing of LAT0/LAT1.

Signed-off-by: Gregor Herburger <gregor.herburger@ew.tq-group.com>
Tested-by: Viken Dadhaniya <viken.dadhaniya@oss.qualcomm.com>
Signed-off-by: Viken Dadhaniya <viken.dadhaniya@oss.qualcomm.com>
---
 .../net/can/spi/mcp251xfd/mcp251xfd-regmap.c  | 89 +++++++++++++++++--
 1 file changed, 83 insertions(+), 6 deletions(-)

diff --git a/drivers/net/can/spi/mcp251xfd/mcp251xfd-regmap.c b/drivers/net/can/spi/mcp251xfd/mcp251xfd-regmap.c
index e61cbd209955..bc24a837bcd0 100644
--- a/drivers/net/can/spi/mcp251xfd/mcp251xfd-regmap.c
+++ b/drivers/net/can/spi/mcp251xfd/mcp251xfd-regmap.c
@@ -13,9 +13,9 @@
 static const struct regmap_config mcp251xfd_regmap_crc;
 
 static int
-mcp251xfd_regmap_nocrc_gather_write(void *context,
-				    const void *reg, size_t reg_len,
-				    const void *val, size_t val_len)
+_mcp251xfd_regmap_nocrc_gather_write(void *context,
+				     const void *reg, size_t reg_len,
+				     const void *val, size_t val_len)
 {
 	struct spi_device *spi = context;
 	struct mcp251xfd_priv *priv = spi_get_drvdata(spi);
@@ -39,6 +39,45 @@ mcp251xfd_regmap_nocrc_gather_write(void *context,
 	return spi_sync_transfer(spi, xfer, ARRAY_SIZE(xfer));
 }
 
+static int
+mcp251xfd_regmap_nocrc_gather_write(void *context,
+				    const void *reg_p, size_t reg_len,
+				    const void *val, size_t val_len)
+{
+	const u16 byte_exclude = MCP251XFD_REG_IOCON +
+				 mcp251xfd_first_byte_set(MCP251XFD_REG_IOCON_GPIO_MASK);
+	u16 reg = be16_to_cpu(*(u16 *)reg_p) & MCP251XFD_SPI_ADDRESS_MASK;
+	int ret;
+
+	/* Never write to bits 16..23 of IOCON register to avoid clearing of LAT0/LAT1
+	 *
+	 * According to MCP2518FD Errata DS80000789E 5 writing IOCON register using one
+	 * SPI write command clears LAT0/LAT1.
+	 *
+	 * Errata Fix/Work Around suggests to write registers with single byte
+	 * write instructions. However, it seems that the byte at 0xe06(IOCON[23:16])
+	 * is for read-only access and writing to it causes the clearing of LAT0/LAT1.
+	 */
+	if (reg <= byte_exclude && reg + val_len > byte_exclude) {
+		size_t len = byte_exclude - reg;
+
+		/* Write up to 0xe05 */
+		ret = _mcp251xfd_regmap_nocrc_gather_write(context, reg_p, reg_len, val, len);
+		if (ret)
+			return ret;
+
+		/* Write from 0xe07 on */
+		reg += len + 1;
+		reg = cpu_to_be16(MCP251XFD_SPI_INSTRUCTION_WRITE | reg);
+		return _mcp251xfd_regmap_nocrc_gather_write(context, &reg, reg_len,
+							    val + len + 1,
+							    val_len - len - 1);
+	}
+
+	return _mcp251xfd_regmap_nocrc_gather_write(context, reg_p, reg_len,
+						  val, val_len);
+}
+
 static int
 mcp251xfd_regmap_nocrc_write(void *context, const void *data, size_t count)
 {
@@ -197,9 +236,9 @@ mcp251xfd_regmap_nocrc_read(void *context,
 }
 
 static int
-mcp251xfd_regmap_crc_gather_write(void *context,
-				  const void *reg_p, size_t reg_len,
-				  const void *val, size_t val_len)
+_mcp251xfd_regmap_crc_gather_write(void *context,
+				   const void *reg_p, size_t reg_len,
+				   const void *val, size_t val_len)
 {
 	struct spi_device *spi = context;
 	struct mcp251xfd_priv *priv = spi_get_drvdata(spi);
@@ -230,6 +269,44 @@ mcp251xfd_regmap_crc_gather_write(void *context,
 	return spi_sync_transfer(spi, xfer, ARRAY_SIZE(xfer));
 }
 
+static int
+mcp251xfd_regmap_crc_gather_write(void *context,
+				  const void *reg_p, size_t reg_len,
+				  const void *val, size_t val_len)
+{
+	const u16 byte_exclude = MCP251XFD_REG_IOCON +
+				 mcp251xfd_first_byte_set(MCP251XFD_REG_IOCON_GPIO_MASK);
+	u16 reg = *(u16 *)reg_p;
+	int ret;
+
+	/* Never write to bits 16..23 of IOCON register to avoid clearing of LAT0/LAT1
+	 *
+	 * According to MCP2518FD Errata DS80000789E 5 writing IOCON register using one
+	 * SPI write command clears LAT0/LAT1.
+	 *
+	 * Errata Fix/Work Around suggests to write registers with single byte
+	 * write instructions. However, it seems that the byte at 0xe06(IOCON[23:16])
+	 * is for read-only access and writing to it causes the clearing of LAT0/LAT1.
+	 */
+	if (reg <= byte_exclude  && reg + val_len > byte_exclude) {
+		size_t len = byte_exclude - reg;
+
+		/* Write up to 0xe05 */
+		ret = _mcp251xfd_regmap_crc_gather_write(context, &reg, reg_len, val, len);
+		if (ret)
+			return ret;
+
+		/* Write from 0xe07 on */
+		reg += len + 1;
+		return _mcp251xfd_regmap_crc_gather_write(context, &reg, reg_len,
+							  val + len + 1,
+							  val_len - len - 1);
+	}
+
+	return _mcp251xfd_regmap_crc_gather_write(context, reg_p, reg_len,
+						  val, val_len);
+}
+
 static int
 mcp251xfd_regmap_crc_write(void *context,
 			   const void *data, size_t count)
-- 
2.34.1

[PATCH v4 4/6] can: mcp251xfd: only configure PIN1 when rx_int is set

[Viken Dadhaniya]

From: Gregor Herburger <gregor.herburger@ew.tq-group.com>

When rx_int is used th mcp251xfd_chip_rx_int_enable and
mcp251xfd_chip_rx_int_disable function configure both PIN0 and PIN1. To
prepare the support of the GPIOS only configure PIN1 with
regmap_update_bits.

This way PIN0 can be used as GPIO while PIN1 is used as rx_int
interrupt.

Signed-off-by: Gregor Herburger <gregor.herburger@ew.tq-group.com>
Tested-by: Viken Dadhaniya <viken.dadhaniya@oss.qualcomm.com>
Signed-off-by: Viken Dadhaniya <viken.dadhaniya@oss.qualcomm.com>
---
 .../net/can/spi/mcp251xfd/mcp251xfd-core.c    | 22 +++++++------------
 drivers/net/can/spi/mcp251xfd/mcp251xfd.h     |  6 +++++
 2 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c b/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c
index f9eabb1810cf..ea41f04ae1a6 100644
--- a/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c
+++ b/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c
@@ -608,23 +608,21 @@ static int mcp251xfd_set_bittiming(const struct mcp251xfd_priv *priv)
 
 static int mcp251xfd_chip_rx_int_enable(const struct mcp251xfd_priv *priv)
 {
-	u32 val;
+	u32 val, mask;
 
 	if (!priv->rx_int)
 		return 0;
 
-	/* Configure GPIOs:
-	 * - PIN0: GPIO Input
-	 * - PIN1: GPIO Input/RX Interrupt
+	/* Configure PIN1 as RX Interrupt:
 	 *
 	 * PIN1 must be Input, otherwise there is a glitch on the
 	 * rx-INT line. It happens between setting the PIN as output
 	 * (in the first byte of the SPI transfer) and configuring the
 	 * PIN as interrupt (in the last byte of the SPI transfer).
 	 */
-	val = MCP251XFD_REG_IOCON_PM0 | MCP251XFD_REG_IOCON_TRIS1 |
-		MCP251XFD_REG_IOCON_TRIS0;
-	return regmap_write(priv->map_reg, MCP251XFD_REG_IOCON, val);
+	val = MCP251XFD_REG_IOCON_TRIS(1);
+	mask = MCP251XFD_REG_IOCON_TRIS(1) | MCP251XFD_REG_IOCON_PM(1);
+	return regmap_update_bits(priv->map_reg, MCP251XFD_REG_IOCON, mask, val);
 }
 
 static int mcp251xfd_chip_rx_int_disable(const struct mcp251xfd_priv *priv)
@@ -634,13 +632,9 @@ static int mcp251xfd_chip_rx_int_disable(const struct mcp251xfd_priv *priv)
 	if (!priv->rx_int)
 		return 0;
 
-	/* Configure GPIOs:
-	 * - PIN0: GPIO Input
-	 * - PIN1: GPIO Input
-	 */
-	val = MCP251XFD_REG_IOCON_PM1 | MCP251XFD_REG_IOCON_PM0 |
-		MCP251XFD_REG_IOCON_TRIS1 | MCP251XFD_REG_IOCON_TRIS0;
-	return regmap_write(priv->map_reg, MCP251XFD_REG_IOCON, val);
+	/* Configure PIN1 as GPIO Input */
+	val = MCP251XFD_REG_IOCON_PM(1) | MCP251XFD_REG_IOCON_TRIS(1);
+	return regmap_update_bits(priv->map_reg, MCP251XFD_REG_IOCON, val, val);
 }
 
 static int mcp251xfd_chip_ecc_init(struct mcp251xfd_priv *priv)
diff --git a/drivers/net/can/spi/mcp251xfd/mcp251xfd.h b/drivers/net/can/spi/mcp251xfd/mcp251xfd.h
index dcbbd2b2fae8..bd28510a6583 100644
--- a/drivers/net/can/spi/mcp251xfd/mcp251xfd.h
+++ b/drivers/net/can/spi/mcp251xfd/mcp251xfd.h
@@ -335,13 +335,19 @@
 #define MCP251XFD_REG_IOCON_TXCANOD BIT(28)
 #define MCP251XFD_REG_IOCON_PM1 BIT(25)
 #define MCP251XFD_REG_IOCON_PM0 BIT(24)
+#define MCP251XFD_REG_IOCON_PM(n) (MCP251XFD_REG_IOCON_PM0 << (n))
 #define MCP251XFD_REG_IOCON_GPIO1 BIT(17)
 #define MCP251XFD_REG_IOCON_GPIO0 BIT(16)
+#define MCP251XFD_REG_IOCON_GPIO(n) (MCP251XFD_REG_IOCON_GPIO0 << (n))
+#define MCP251XFD_REG_IOCON_GPIO_MASK GENMASK(17, 16)
 #define MCP251XFD_REG_IOCON_LAT1 BIT(9)
 #define MCP251XFD_REG_IOCON_LAT0 BIT(8)
+#define MCP251XFD_REG_IOCON_LAT(n) (MCP251XFD_REG_IOCON_LAT0 << (n))
+#define MCP251XFD_REG_IOCON_LAT_MASK GENMASK(9, 8)
 #define MCP251XFD_REG_IOCON_XSTBYEN BIT(6)
 #define MCP251XFD_REG_IOCON_TRIS1 BIT(1)
 #define MCP251XFD_REG_IOCON_TRIS0 BIT(0)
+#define MCP251XFD_REG_IOCON_TRIS(n) (MCP251XFD_REG_IOCON_TRIS0 << (n))
 
 #define MCP251XFD_REG_CRC 0xe08
 #define MCP251XFD_REG_CRC_FERRIE BIT(25)
-- 
2.34.1

[PATCH v4 5/6] can: mcp251xfd: add gpio functionality

[Viken Dadhaniya]

From: Gregor Herburger <gregor.herburger@ew.tq-group.com>

The mcp251xfd devices allow two pins to be configured as gpio. Add this
functionality to driver.

Signed-off-by: Gregor Herburger <gregor.herburger@ew.tq-group.com>
Tested-by: Viken Dadhaniya <viken.dadhaniya@oss.qualcomm.com>
Signed-off-by: Viken Dadhaniya <viken.dadhaniya@oss.qualcomm.com>
---
 .../net/can/spi/mcp251xfd/mcp251xfd-core.c    | 179 ++++++++++++++++++
 drivers/net/can/spi/mcp251xfd/mcp251xfd.h     |   4 +
 2 files changed, 183 insertions(+)

diff --git a/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c b/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c
index ea41f04ae1a6..8c253091f498 100644
--- a/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c
+++ b/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c
@@ -16,6 +16,7 @@
 #include <linux/bitfield.h>
 #include <linux/clk.h>
 #include <linux/device.h>
+#include <linux/gpio/driver.h>
 #include <linux/mod_devicetable.h>
 #include <linux/module.h>
 #include <linux/pm_runtime.h>
@@ -1797,6 +1798,178 @@ static int mcp251xfd_register_check_rx_int(struct mcp251xfd_priv *priv)
 	return 0;
 }
 
+#ifdef CONFIG_GPIOLIB
+static const char * const mcp251xfd_gpio_names[] = { "GPIO0", "GPIO1" };
+
+static int mcp251xfd_gpio_request(struct gpio_chip *chip, unsigned int offset)
+{
+	struct mcp251xfd_priv *priv = gpiochip_get_data(chip);
+	u32 pin_mask = MCP251XFD_REG_IOCON_PM(offset);
+	int ret;
+
+	if (priv->rx_int && offset == 1) {
+		netdev_err(priv->ndev, "Can't use GPIO 1 with RX-INT!\n");
+		return -EINVAL;
+	}
+
+	ret = pm_runtime_resume_and_get(priv->ndev->dev.parent);
+	if (ret)
+		return ret;
+
+	return regmap_update_bits(priv->map_reg, MCP251XFD_REG_IOCON,
+				  pin_mask, pin_mask);
+}
+
+static void mcp251xfd_gpio_free(struct gpio_chip *chip, unsigned int offset)
+{
+	struct mcp251xfd_priv *priv = gpiochip_get_data(chip);
+
+	pm_runtime_put(priv->ndev->dev.parent);
+}
+
+static int mcp251xfd_gpio_get_direction(struct gpio_chip *chip,
+					unsigned int offset)
+{
+	struct mcp251xfd_priv *priv = gpiochip_get_data(chip);
+	u32 mask = MCP251XFD_REG_IOCON_TRIS(offset);
+	u32 val;
+	int ret;
+
+	ret = regmap_read(priv->map_reg, MCP251XFD_REG_IOCON, &val);
+	if (ret)
+		return ret;
+
+	if (mask & val)
+		return GPIO_LINE_DIRECTION_IN;
+
+	return GPIO_LINE_DIRECTION_OUT;
+}
+
+static int mcp251xfd_gpio_get(struct gpio_chip *chip, unsigned int offset)
+{
+	struct mcp251xfd_priv *priv = gpiochip_get_data(chip);
+	u32 mask = MCP251XFD_REG_IOCON_GPIO(offset);
+	u32 val;
+	int ret;
+
+	ret = regmap_read(priv->map_reg, MCP251XFD_REG_IOCON, &val);
+	if (ret)
+		return ret;
+
+	return !!(mask & val);
+}
+
+static int mcp251xfd_gpio_get_multiple(struct gpio_chip *chip, unsigned long *mask,
+				       unsigned long *bit)
+{
+	struct mcp251xfd_priv *priv = gpiochip_get_data(chip);
+	u32 val;
+	int ret;
+
+	ret = regmap_read(priv->map_reg, MCP251XFD_REG_IOCON, &val);
+	if (ret)
+		return ret;
+
+	*bit = FIELD_GET(MCP251XFD_REG_IOCON_GPIO_MASK, val) & *mask;
+
+	return 0;
+}
+
+static int mcp251xfd_gpio_direction_output(struct gpio_chip *chip,
+					   unsigned int offset, int value)
+{
+	struct mcp251xfd_priv *priv = gpiochip_get_data(chip);
+	u32 dir_mask = MCP251XFD_REG_IOCON_TRIS(offset);
+	u32 val_mask = MCP251XFD_REG_IOCON_LAT(offset);
+	u32 val;
+
+	if (value)
+		val = val_mask;
+	else
+		val = 0;
+
+	return regmap_update_bits(priv->map_reg, MCP251XFD_REG_IOCON,
+				  dir_mask | val_mask, val);
+}
+
+static int mcp251xfd_gpio_direction_input(struct gpio_chip *chip,
+					  unsigned int offset)
+{
+	struct mcp251xfd_priv *priv = gpiochip_get_data(chip);
+	u32 dir_mask = MCP251XFD_REG_IOCON_TRIS(offset);
+
+	return regmap_update_bits(priv->map_reg, MCP251XFD_REG_IOCON,
+				  dir_mask, dir_mask);
+}
+
+static void mcp251xfd_gpio_set(struct gpio_chip *chip, unsigned int offset,
+			       int value)
+{
+	struct mcp251xfd_priv *priv = gpiochip_get_data(chip);
+	u32 val_mask = MCP251XFD_REG_IOCON_LAT(offset);
+	u32 val;
+	int ret;
+
+	if (value)
+		val = val_mask;
+	else
+		val = 0;
+
+	ret = regmap_update_bits(priv->map_reg, MCP251XFD_REG_IOCON,
+				 val_mask, val);
+	if (ret)
+		dev_err(&priv->spi->dev, "Failed to set GPIO %u: %d\n",
+			offset, ret);
+}
+
+static void mcp251xfd_gpio_set_multiple(struct gpio_chip *chip, unsigned long *mask,
+					unsigned long *bits)
+{
+	struct mcp251xfd_priv *priv = gpiochip_get_data(chip);
+	u32 val;
+	int ret;
+
+	val = FIELD_PREP(MCP251XFD_REG_IOCON_LAT_MASK, *bits);
+
+	ret = regmap_update_bits(priv->map_reg, MCP251XFD_REG_IOCON,
+				 MCP251XFD_REG_IOCON_LAT_MASK, val);
+	if (ret)
+		dev_err(&priv->spi->dev, "Failed to set GPIOs %d\n", ret);
+}
+
+static int mcp251fdx_gpio_setup(struct mcp251xfd_priv *priv)
+{
+	struct gpio_chip *gc = &priv->gc;
+
+	if (!device_property_present(&priv->spi->dev, "gpio-controller"))
+		return 0;
+
+	gc->label = dev_name(&priv->spi->dev);
+	gc->parent = &priv->spi->dev;
+	gc->owner = THIS_MODULE;
+	gc->request = mcp251xfd_gpio_request;
+	gc->free = mcp251xfd_gpio_free;
+	gc->get_direction = mcp251xfd_gpio_get_direction;
+	gc->direction_output = mcp251xfd_gpio_direction_output;
+	gc->direction_input = mcp251xfd_gpio_direction_input;
+	gc->get = mcp251xfd_gpio_get;
+	gc->get_multiple = mcp251xfd_gpio_get_multiple;
+	gc->set = mcp251xfd_gpio_set;
+	gc->set_multiple = mcp251xfd_gpio_set_multiple;
+	gc->base = -1;
+	gc->can_sleep = true;
+	gc->ngpio = ARRAY_SIZE(mcp251xfd_gpio_names);
+	gc->names = mcp251xfd_gpio_names;
+
+	return devm_gpiochip_add_data(&priv->spi->dev, gc, priv);
+}
+#else
+static inline int mcp251fdx_gpio_setup(struct mcp251xfd_priv *priv)
+{
+	return 0;
+}
+#endif
+
 static int
 mcp251xfd_register_get_dev_id(const struct mcp251xfd_priv *priv, u32 *dev_id,
 			      u32 *effective_speed_hz_slow,
@@ -1930,6 +2103,12 @@ static int mcp251xfd_register(struct mcp251xfd_priv *priv)
 
 	mcp251xfd_ethtool_init(priv);
 
+	err = mcp251fdx_gpio_setup(priv);
+	if (err) {
+		dev_err_probe(&priv->spi->dev, err, "Failed to register gpio-controller.\n");
+		goto out_runtime_disable;
+	}
+
 	err = register_candev(ndev);
 	if (err)
 		goto out_runtime_disable;
diff --git a/drivers/net/can/spi/mcp251xfd/mcp251xfd.h b/drivers/net/can/spi/mcp251xfd/mcp251xfd.h
index bd28510a6583..fd9e005708e4 100644
--- a/drivers/net/can/spi/mcp251xfd/mcp251xfd.h
+++ b/drivers/net/can/spi/mcp251xfd/mcp251xfd.h
@@ -15,6 +15,7 @@
 #include <linux/can/dev.h>
 #include <linux/can/rx-offload.h>
 #include <linux/gpio/consumer.h>
+#include <linux/gpio/driver.h>
 #include <linux/kernel.h>
 #include <linux/netdevice.h>
 #include <linux/regmap.h>
@@ -676,6 +677,9 @@ struct mcp251xfd_priv {
 
 	struct mcp251xfd_devtype_data devtype_data;
 	struct can_berr_counter bec;
+#ifdef CONFIG_GPIOLIB
+	struct gpio_chip gc;
+#endif
 };
 
 #define MCP251XFD_IS(_model) \
-- 
2.34.1

[PATCH v4 6/6] dt-bindings: can: mcp251xfd: add gpio-controller property

[Viken Dadhaniya]

From: Gregor Herburger <gregor.herburger@ew.tq-group.com>

The mcp251xfd has two pins that can be used as gpio. Add gpio-controller
property to binding description.

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Gregor Herburger <gregor.herburger@ew.tq-group.com>
Signed-off-by: Viken Dadhaniya <viken.dadhaniya@oss.qualcomm.com>
---
 .../devicetree/bindings/net/can/microchip,mcp251xfd.yaml     | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/can/microchip,mcp251xfd.yaml b/Documentation/devicetree/bindings/net/can/microchip,mcp251xfd.yaml
index c155c9c6db39..2d13638ebc6a 100644
--- a/Documentation/devicetree/bindings/net/can/microchip,mcp251xfd.yaml
+++ b/Documentation/devicetree/bindings/net/can/microchip,mcp251xfd.yaml
@@ -49,6 +49,11 @@ properties:
       Must be half or less of "clocks" frequency.
     maximum: 20000000
 
+  gpio-controller: true
+
+  "#gpio-cells":
+    const: 2
+
 required:
   - compatible
   - reg
-- 
2.34.1

Re: [PATCH v4 5/6] can: mcp251xfd: add gpio functionality

[Bartosz Golaszewski]

On Thu, 18 Sep 2025 08:49:02 +0200, Viken Dadhaniya
<viken.dadhaniya@oss.qualcomm.com> said:
> From: Gregor Herburger <gregor.herburger@ew.tq-group.com>
>
> The mcp251xfd devices allow two pins to be configured as gpio. Add this
> functionality to driver.
>
> Signed-off-by: Gregor Herburger <gregor.herburger@ew.tq-group.com>
> Tested-by: Viken Dadhaniya <viken.dadhaniya@oss.qualcomm.com>
> Signed-off-by: Viken Dadhaniya <viken.dadhaniya@oss.qualcomm.com>
> ---
>  .../net/can/spi/mcp251xfd/mcp251xfd-core.c    | 179 ++++++++++++++++++
>  drivers/net/can/spi/mcp251xfd/mcp251xfd.h     |   4 +
>  2 files changed, 183 insertions(+)
>
> diff --git a/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c b/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c
> index ea41f04ae1a6..8c253091f498 100644
> --- a/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c
> +++ b/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c
> @@ -16,6 +16,7 @@
>  #include <linux/bitfield.h>
>  #include <linux/clk.h>
>  #include <linux/device.h>
> +#include <linux/gpio/driver.h>
>  #include <linux/mod_devicetable.h>
>  #include <linux/module.h>
>  #include <linux/pm_runtime.h>
> @@ -1797,6 +1798,178 @@ static int mcp251xfd_register_check_rx_int(struct mcp251xfd_priv *priv)
>  	return 0;
>  }
>
> +#ifdef CONFIG_GPIOLIB

Any reason why you don't just depend on GPIOLIB in Kconfig? There's no
reason to make it optional if the device always has the GPIO pins.

> +static const char * const mcp251xfd_gpio_names[] = { "GPIO0", "GPIO1" };
> +
> +static int mcp251xfd_gpio_request(struct gpio_chip *chip, unsigned int offset)
> +{
> +	struct mcp251xfd_priv *priv = gpiochip_get_data(chip);
> +	u32 pin_mask = MCP251XFD_REG_IOCON_PM(offset);
> +	int ret;
> +
> +	if (priv->rx_int && offset == 1) {
> +		netdev_err(priv->ndev, "Can't use GPIO 1 with RX-INT!\n");
> +		return -EINVAL;
> +	}
> +
> +	ret = pm_runtime_resume_and_get(priv->ndev->dev.parent);
> +	if (ret)
> +		return ret;
> +
> +	return regmap_update_bits(priv->map_reg, MCP251XFD_REG_IOCON,
> +				  pin_mask, pin_mask);
> +}
> +
> +static void mcp251xfd_gpio_free(struct gpio_chip *chip, unsigned int offset)
> +{
> +	struct mcp251xfd_priv *priv = gpiochip_get_data(chip);
> +
> +	pm_runtime_put(priv->ndev->dev.parent);
> +}
> +
> +static int mcp251xfd_gpio_get_direction(struct gpio_chip *chip,
> +					unsigned int offset)
> +{
> +	struct mcp251xfd_priv *priv = gpiochip_get_data(chip);
> +	u32 mask = MCP251XFD_REG_IOCON_TRIS(offset);
> +	u32 val;
> +	int ret;
> +
> +	ret = regmap_read(priv->map_reg, MCP251XFD_REG_IOCON, &val);
> +	if (ret)
> +		return ret;
> +
> +	if (mask & val)
> +		return GPIO_LINE_DIRECTION_IN;
> +
> +	return GPIO_LINE_DIRECTION_OUT;
> +}
> +
> +static int mcp251xfd_gpio_get(struct gpio_chip *chip, unsigned int offset)
> +{
> +	struct mcp251xfd_priv *priv = gpiochip_get_data(chip);
> +	u32 mask = MCP251XFD_REG_IOCON_GPIO(offset);
> +	u32 val;
> +	int ret;
> +
> +	ret = regmap_read(priv->map_reg, MCP251XFD_REG_IOCON, &val);
> +	if (ret)
> +		return ret;
> +
> +	return !!(mask & val);
> +}
> +
> +static int mcp251xfd_gpio_get_multiple(struct gpio_chip *chip, unsigned long *mask,
> +				       unsigned long *bit)
> +{
> +	struct mcp251xfd_priv *priv = gpiochip_get_data(chip);
> +	u32 val;
> +	int ret;
> +
> +	ret = regmap_read(priv->map_reg, MCP251XFD_REG_IOCON, &val);
> +	if (ret)
> +		return ret;
> +
> +	*bit = FIELD_GET(MCP251XFD_REG_IOCON_GPIO_MASK, val) & *mask;
> +
> +	return 0;
> +}
> +
> +static int mcp251xfd_gpio_direction_output(struct gpio_chip *chip,
> +					   unsigned int offset, int value)
> +{
> +	struct mcp251xfd_priv *priv = gpiochip_get_data(chip);
> +	u32 dir_mask = MCP251XFD_REG_IOCON_TRIS(offset);
> +	u32 val_mask = MCP251XFD_REG_IOCON_LAT(offset);
> +	u32 val;
> +
> +	if (value)
> +		val = val_mask;
> +	else
> +		val = 0;
> +
> +	return regmap_update_bits(priv->map_reg, MCP251XFD_REG_IOCON,
> +				  dir_mask | val_mask, val);
> +}
> +
> +static int mcp251xfd_gpio_direction_input(struct gpio_chip *chip,
> +					  unsigned int offset)
> +{
> +	struct mcp251xfd_priv *priv = gpiochip_get_data(chip);
> +	u32 dir_mask = MCP251XFD_REG_IOCON_TRIS(offset);
> +
> +	return regmap_update_bits(priv->map_reg, MCP251XFD_REG_IOCON,
> +				  dir_mask, dir_mask);
> +}
> +
> +static void mcp251xfd_gpio_set(struct gpio_chip *chip, unsigned int offset,
> +			       int value)

You must be rebased on pre v6.17 code, this will not compile with current
mainline.

> +{
> +	struct mcp251xfd_priv *priv = gpiochip_get_data(chip);
> +	u32 val_mask = MCP251XFD_REG_IOCON_LAT(offset);
> +	u32 val;
> +	int ret;
> +
> +	if (value)
> +		val = val_mask;
> +	else
> +		val = 0;
> +
> +	ret = regmap_update_bits(priv->map_reg, MCP251XFD_REG_IOCON,
> +				 val_mask, val);
> +	if (ret)
> +		dev_err(&priv->spi->dev, "Failed to set GPIO %u: %d\n",
> +			offset, ret);
> +}
> +
> +static void mcp251xfd_gpio_set_multiple(struct gpio_chip *chip, unsigned long *mask,
> +					unsigned long *bits)
> +{

Same here, the setter callbacks now return int.

> +	struct mcp251xfd_priv *priv = gpiochip_get_data(chip);
> +	u32 val;
> +	int ret;
> +
> +	val = FIELD_PREP(MCP251XFD_REG_IOCON_LAT_MASK, *bits);
> +
> +	ret = regmap_update_bits(priv->map_reg, MCP251XFD_REG_IOCON,
> +				 MCP251XFD_REG_IOCON_LAT_MASK, val);
> +	if (ret)
> +		dev_err(&priv->spi->dev, "Failed to set GPIOs %d\n", ret);
> +}
> +
> +static int mcp251fdx_gpio_setup(struct mcp251xfd_priv *priv)
> +{
> +	struct gpio_chip *gc = &priv->gc;
> +
> +	if (!device_property_present(&priv->spi->dev, "gpio-controller"))
> +		return 0;
> +
> +	gc->label = dev_name(&priv->spi->dev);
> +	gc->parent = &priv->spi->dev;
> +	gc->owner = THIS_MODULE;
> +	gc->request = mcp251xfd_gpio_request;
> +	gc->free = mcp251xfd_gpio_free;
> +	gc->get_direction = mcp251xfd_gpio_get_direction;
> +	gc->direction_output = mcp251xfd_gpio_direction_output;
> +	gc->direction_input = mcp251xfd_gpio_direction_input;
> +	gc->get = mcp251xfd_gpio_get;
> +	gc->get_multiple = mcp251xfd_gpio_get_multiple;
> +	gc->set = mcp251xfd_gpio_set;
> +	gc->set_multiple = mcp251xfd_gpio_set_multiple;
> +	gc->base = -1;
> +	gc->can_sleep = true;
> +	gc->ngpio = ARRAY_SIZE(mcp251xfd_gpio_names);
> +	gc->names = mcp251xfd_gpio_names;
> +
> +	return devm_gpiochip_add_data(&priv->spi->dev, gc, priv);
> +}
> +#else
> +static inline int mcp251fdx_gpio_setup(struct mcp251xfd_priv *priv)
> +{
> +	return 0;
> +}
> +#endif
> +
>  static int
>  mcp251xfd_register_get_dev_id(const struct mcp251xfd_priv *priv, u32 *dev_id,
>  			      u32 *effective_speed_hz_slow,
> @@ -1930,6 +2103,12 @@ static int mcp251xfd_register(struct mcp251xfd_priv *priv)
>
>  	mcp251xfd_ethtool_init(priv);
>
> +	err = mcp251fdx_gpio_setup(priv);
> +	if (err) {
> +		dev_err_probe(&priv->spi->dev, err, "Failed to register gpio-controller.\n");
> +		goto out_runtime_disable;
> +	}
> +
>  	err = register_candev(ndev);
>  	if (err)
>  		goto out_runtime_disable;
> diff --git a/drivers/net/can/spi/mcp251xfd/mcp251xfd.h b/drivers/net/can/spi/mcp251xfd/mcp251xfd.h
> index bd28510a6583..fd9e005708e4 100644
> --- a/drivers/net/can/spi/mcp251xfd/mcp251xfd.h
> +++ b/drivers/net/can/spi/mcp251xfd/mcp251xfd.h
> @@ -15,6 +15,7 @@
>  #include <linux/can/dev.h>
>  #include <linux/can/rx-offload.h>
>  #include <linux/gpio/consumer.h>
> +#include <linux/gpio/driver.h>
>  #include <linux/kernel.h>
>  #include <linux/netdevice.h>
>  #include <linux/regmap.h>
> @@ -676,6 +677,9 @@ struct mcp251xfd_priv {
>
>  	struct mcp251xfd_devtype_data devtype_data;
>  	struct can_berr_counter bec;
> +#ifdef CONFIG_GPIOLIB
> +	struct gpio_chip gc;
> +#endif
>  };
>
>  #define MCP251XFD_IS(_model) \
> --
> 2.34.1
>
>

Bart

Re: [PATCH v4 5/6] can: mcp251xfd: add gpio functionality

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 1582 bytes --]

On 18.09.2025 05:46:44, Bartosz Golaszewski wrote:
> > diff --git a/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c b/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c
> > index ea41f04ae1a6..8c253091f498 100644
> > --- a/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c
> > +++ b/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c
> > @@ -16,6 +16,7 @@
> >  #include <linux/bitfield.h>
> >  #include <linux/clk.h>
> >  #include <linux/device.h>
> > +#include <linux/gpio/driver.h>
> >  #include <linux/mod_devicetable.h>
> >  #include <linux/module.h>
> >  #include <linux/pm_runtime.h>
> > @@ -1797,6 +1798,178 @@ static int mcp251xfd_register_check_rx_int(struct mcp251xfd_priv *priv)
> >  	return 0;
> >  }
> >
> > +#ifdef CONFIG_GPIOLIB
> 
> Any reason why you don't just depend on GPIOLIB in Kconfig? There's no
> reason to make it optional if the device always has the GPIO pins.

I don't mind having the ifdef. But it's up to you.

[...]

> > +static void mcp251xfd_gpio_set(struct gpio_chip *chip, unsigned int offset,
> > +			       int value)
> 
> You must be rebased on pre v6.17 code, this will not compile with current
> mainline.

You mean "post" v6.17? Best rebase to latest net-next/main, which
already contains the new signatures for the GPIO callbacks.

regards,
Marc

-- 
Pengutronix e.K.                 | Marc Kleine-Budde          |
Embedded Linux                   | https://www.pengutronix.de |
Vertretung Nürnberg              | Phone: +49-5121-206917-129 |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-9   |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v4 4/6] can: mcp251xfd: only configure PIN1 when rx_int is set

[kernel test robot]

Hi Viken,

kernel test robot noticed the following build warnings:

[auto build test WARNING on mkl-can-next/testing]
[also build test WARNING on linus/master v6.17-rc6 next-20250919]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Viken-Dadhaniya/can-mcp251xfd-move-chip-sleep-mode-into-runtime-pm/20250918-145404
base:   https://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can-next.git testing
patch link:    https://lore.kernel.org/r/20250918064903.241372-5-viken.dadhaniya%40oss.qualcomm.com
patch subject: [PATCH v4 4/6] can: mcp251xfd: only configure PIN1 when rx_int is set
config: x86_64-randconfig-123-20250920 (https://download.01.org/0day-ci/archive/20250921/202509210053.BtsqeDKp-lkp@intel.com/config)
compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250921/202509210053.BtsqeDKp-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202509210053.BtsqeDKp-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
>> drivers/net/can/spi/mcp251xfd/mcp251xfd-regmap.c:49:19: sparse: sparse: cast to restricted __be16
>> drivers/net/can/spi/mcp251xfd/mcp251xfd-regmap.c:71:21: sparse: sparse: incorrect type in assignment (different base types) @@     expected unsigned short [assigned] [usertype] reg @@     got restricted __be16 [usertype] @@
   drivers/net/can/spi/mcp251xfd/mcp251xfd-regmap.c:71:21: sparse:     expected unsigned short [assigned] [usertype] reg
   drivers/net/can/spi/mcp251xfd/mcp251xfd-regmap.c:71:21: sparse:     got restricted __be16 [usertype]

vim +49 drivers/net/can/spi/mcp251xfd/mcp251xfd-regmap.c

875347fe575636 drivers/net/can/spi/mcp25xxfd/mcp25xxfd-regmap.c Marc Kleine-Budde 2020-09-18  41  
182c647943b5c0 drivers/net/can/spi/mcp251xfd/mcp251xfd-regmap.c Gregor Herburger  2025-09-18  42  static int
182c647943b5c0 drivers/net/can/spi/mcp251xfd/mcp251xfd-regmap.c Gregor Herburger  2025-09-18  43  mcp251xfd_regmap_nocrc_gather_write(void *context,
182c647943b5c0 drivers/net/can/spi/mcp251xfd/mcp251xfd-regmap.c Gregor Herburger  2025-09-18  44  				    const void *reg_p, size_t reg_len,
182c647943b5c0 drivers/net/can/spi/mcp251xfd/mcp251xfd-regmap.c Gregor Herburger  2025-09-18  45  				    const void *val, size_t val_len)
182c647943b5c0 drivers/net/can/spi/mcp251xfd/mcp251xfd-regmap.c Gregor Herburger  2025-09-18  46  {
182c647943b5c0 drivers/net/can/spi/mcp251xfd/mcp251xfd-regmap.c Gregor Herburger  2025-09-18  47  	const u16 byte_exclude = MCP251XFD_REG_IOCON +
182c647943b5c0 drivers/net/can/spi/mcp251xfd/mcp251xfd-regmap.c Gregor Herburger  2025-09-18  48  				 mcp251xfd_first_byte_set(MCP251XFD_REG_IOCON_GPIO_MASK);
182c647943b5c0 drivers/net/can/spi/mcp251xfd/mcp251xfd-regmap.c Gregor Herburger  2025-09-18 @49  	u16 reg = be16_to_cpu(*(u16 *)reg_p) & MCP251XFD_SPI_ADDRESS_MASK;
182c647943b5c0 drivers/net/can/spi/mcp251xfd/mcp251xfd-regmap.c Gregor Herburger  2025-09-18  50  	int ret;
182c647943b5c0 drivers/net/can/spi/mcp251xfd/mcp251xfd-regmap.c Gregor Herburger  2025-09-18  51  
182c647943b5c0 drivers/net/can/spi/mcp251xfd/mcp251xfd-regmap.c Gregor Herburger  2025-09-18  52  	/* Never write to bits 16..23 of IOCON register to avoid clearing of LAT0/LAT1
182c647943b5c0 drivers/net/can/spi/mcp251xfd/mcp251xfd-regmap.c Gregor Herburger  2025-09-18  53  	 *
182c647943b5c0 drivers/net/can/spi/mcp251xfd/mcp251xfd-regmap.c Gregor Herburger  2025-09-18  54  	 * According to MCP2518FD Errata DS80000789E 5 writing IOCON register using one
182c647943b5c0 drivers/net/can/spi/mcp251xfd/mcp251xfd-regmap.c Gregor Herburger  2025-09-18  55  	 * SPI write command clears LAT0/LAT1.
182c647943b5c0 drivers/net/can/spi/mcp251xfd/mcp251xfd-regmap.c Gregor Herburger  2025-09-18  56  	 *
182c647943b5c0 drivers/net/can/spi/mcp251xfd/mcp251xfd-regmap.c Gregor Herburger  2025-09-18  57  	 * Errata Fix/Work Around suggests to write registers with single byte
182c647943b5c0 drivers/net/can/spi/mcp251xfd/mcp251xfd-regmap.c Gregor Herburger  2025-09-18  58  	 * write instructions. However, it seems that the byte at 0xe06(IOCON[23:16])
182c647943b5c0 drivers/net/can/spi/mcp251xfd/mcp251xfd-regmap.c Gregor Herburger  2025-09-18  59  	 * is for read-only access and writing to it causes the clearing of LAT0/LAT1.
182c647943b5c0 drivers/net/can/spi/mcp251xfd/mcp251xfd-regmap.c Gregor Herburger  2025-09-18  60  	 */
182c647943b5c0 drivers/net/can/spi/mcp251xfd/mcp251xfd-regmap.c Gregor Herburger  2025-09-18  61  	if (reg <= byte_exclude && reg + val_len > byte_exclude) {
182c647943b5c0 drivers/net/can/spi/mcp251xfd/mcp251xfd-regmap.c Gregor Herburger  2025-09-18  62  		size_t len = byte_exclude - reg;
182c647943b5c0 drivers/net/can/spi/mcp251xfd/mcp251xfd-regmap.c Gregor Herburger  2025-09-18  63  
182c647943b5c0 drivers/net/can/spi/mcp251xfd/mcp251xfd-regmap.c Gregor Herburger  2025-09-18  64  		/* Write up to 0xe05 */
182c647943b5c0 drivers/net/can/spi/mcp251xfd/mcp251xfd-regmap.c Gregor Herburger  2025-09-18  65  		ret = _mcp251xfd_regmap_nocrc_gather_write(context, reg_p, reg_len, val, len);
182c647943b5c0 drivers/net/can/spi/mcp251xfd/mcp251xfd-regmap.c Gregor Herburger  2025-09-18  66  		if (ret)
182c647943b5c0 drivers/net/can/spi/mcp251xfd/mcp251xfd-regmap.c Gregor Herburger  2025-09-18  67  			return ret;
182c647943b5c0 drivers/net/can/spi/mcp251xfd/mcp251xfd-regmap.c Gregor Herburger  2025-09-18  68  
182c647943b5c0 drivers/net/can/spi/mcp251xfd/mcp251xfd-regmap.c Gregor Herburger  2025-09-18  69  		/* Write from 0xe07 on */
182c647943b5c0 drivers/net/can/spi/mcp251xfd/mcp251xfd-regmap.c Gregor Herburger  2025-09-18  70  		reg += len + 1;
182c647943b5c0 drivers/net/can/spi/mcp251xfd/mcp251xfd-regmap.c Gregor Herburger  2025-09-18 @71  		reg = cpu_to_be16(MCP251XFD_SPI_INSTRUCTION_WRITE | reg);
182c647943b5c0 drivers/net/can/spi/mcp251xfd/mcp251xfd-regmap.c Gregor Herburger  2025-09-18  72  		return _mcp251xfd_regmap_nocrc_gather_write(context, &reg, reg_len,
182c647943b5c0 drivers/net/can/spi/mcp251xfd/mcp251xfd-regmap.c Gregor Herburger  2025-09-18  73  							    val + len + 1,
182c647943b5c0 drivers/net/can/spi/mcp251xfd/mcp251xfd-regmap.c Gregor Herburger  2025-09-18  74  							    val_len - len - 1);
182c647943b5c0 drivers/net/can/spi/mcp251xfd/mcp251xfd-regmap.c Gregor Herburger  2025-09-18  75  	}
182c647943b5c0 drivers/net/can/spi/mcp251xfd/mcp251xfd-regmap.c Gregor Herburger  2025-09-18  76  
182c647943b5c0 drivers/net/can/spi/mcp251xfd/mcp251xfd-regmap.c Gregor Herburger  2025-09-18  77  	return _mcp251xfd_regmap_nocrc_gather_write(context, reg_p, reg_len,
182c647943b5c0 drivers/net/can/spi/mcp251xfd/mcp251xfd-regmap.c Gregor Herburger  2025-09-18  78  						  val, val_len);
182c647943b5c0 drivers/net/can/spi/mcp251xfd/mcp251xfd-regmap.c Gregor Herburger  2025-09-18  79  }
182c647943b5c0 drivers/net/can/spi/mcp251xfd/mcp251xfd-regmap.c Gregor Herburger  2025-09-18  80  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v4 5/6] can: mcp251xfd: add gpio functionality

[Viken Dadhaniya]

On 9/18/2025 4:28 PM, Marc Kleine-Budde wrote:
> On 18.09.2025 05:46:44, Bartosz Golaszewski wrote:
>>> diff --git a/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c b/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c
>>> index ea41f04ae1a6..8c253091f498 100644
>>> --- a/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c
>>> +++ b/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c
>>> @@ -16,6 +16,7 @@
>>>  #include <linux/bitfield.h>
>>>  #include <linux/clk.h>
>>>  #include <linux/device.h>
>>> +#include <linux/gpio/driver.h>
>>>  #include <linux/mod_devicetable.h>
>>>  #include <linux/module.h>
>>>  #include <linux/pm_runtime.h>
>>> @@ -1797,6 +1798,178 @@ static int mcp251xfd_register_check_rx_int(struct mcp251xfd_priv *priv)
>>>  	return 0;
>>>  }
>>>
>>> +#ifdef CONFIG_GPIOLIB
>>
>> Any reason why you don't just depend on GPIOLIB in Kconfig? There's no
>> reason to make it optional if the device always has the GPIO pins.
> 
> I don't mind having the ifdef. But it's up to you.
> 
> [...]

Sure, I’ll add depends on GPIOLIB in the Kconfig and remove the #ifdef CONFIG_GPIOLIB
from the driver in the next patch.

> 
>>> +static void mcp251xfd_gpio_set(struct gpio_chip *chip, unsigned int offset,
>>> +			       int value)
>>
>> You must be rebased on pre v6.17 code, this will not compile with current
>> mainline.
> 
> You mean "post" v6.17? Best rebase to latest net-next/main, which
> already contains the new signatures for the GPIO callbacks.
> 
> regards,
> Marc
> 

Sure, I will update in next patch.

Thanks
Viken

Re: [PATCH v4 5/6] can: mcp251xfd: add gpio functionality

[Bartosz Golaszewski]

On Thu, Sep 18, 2025 at 12:58 PM Marc Kleine-Budde <mkl@pengutronix.de> wrote:
>
> On 18.09.2025 05:46:44, Bartosz Golaszewski wrote:
> > > diff --git a/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c b/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c
> > > index ea41f04ae1a6..8c253091f498 100644
> > > --- a/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c
> > > +++ b/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c
> > > @@ -16,6 +16,7 @@
> > >  #include <linux/bitfield.h>
> > >  #include <linux/clk.h>
> > >  #include <linux/device.h>
> > > +#include <linux/gpio/driver.h>
> > >  #include <linux/mod_devicetable.h>
> > >  #include <linux/module.h>
> > >  #include <linux/pm_runtime.h>
> > > @@ -1797,6 +1798,178 @@ static int mcp251xfd_register_check_rx_int(struct mcp251xfd_priv *priv)
> > >     return 0;
> > >  }
> > >
> > > +#ifdef CONFIG_GPIOLIB
> >
> > Any reason why you don't just depend on GPIOLIB in Kconfig? There's no
> > reason to make it optional if the device always has the GPIO pins.
>
> I don't mind having the ifdef. But it's up to you.
>
> [...]
>
> > > +static void mcp251xfd_gpio_set(struct gpio_chip *chip, unsigned int offset,
> > > +                          int value)
> >
> > You must be rebased on pre v6.17 code, this will not compile with current
> > mainline.
>
> You mean "post" v6.17? Best rebase to latest net-next/main, which
> already contains the new signatures for the GPIO callbacks.
>

No, you read that right. The signature of the set() and set_multiple()
callbacks changed in v6.17-rc1 so Viken must have rebased his changes
on v6.16 or earlier.

Bartosz

Re: [PATCH v4 5/6] can: mcp251xfd: add gpio functionality

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 869 bytes --]

On 22.09.2025 16:28:53, Bartosz Golaszewski wrote:
> > > You must be rebased on pre v6.17 code, this will not compile with current
> > > mainline.
> >
> > You mean "post" v6.17? Best rebase to latest net-next/main, which
> > already contains the new signatures for the GPIO callbacks.
> 
> No, you read that right. The signature of the set() and set_multiple()
> callbacks changed in v6.17-rc1 so Viken must have rebased his changes
> on v6.16 or earlier.

I'm not sure if I understand you correctly. This series must apply on
current net-next/main, which is v6.17-rc6.

regards,
Marc

-- 
Pengutronix e.K.                 | Marc Kleine-Budde          |
Embedded Linux                   | https://www.pengutronix.de |
Vertretung Nürnberg              | Phone: +49-5121-206917-129 |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-9   |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v4 5/6] can: mcp251xfd: add gpio functionality

[Bartosz Golaszewski]

On Mon, Sep 22, 2025 at 4:43 PM Marc Kleine-Budde <mkl@pengutronix.de> wrote:
>
> On 22.09.2025 16:28:53, Bartosz Golaszewski wrote:
> > > > You must be rebased on pre v6.17 code, this will not compile with current
> > > > mainline.
> > >
> > > You mean "post" v6.17? Best rebase to latest net-next/main, which
> > > already contains the new signatures for the GPIO callbacks.
> >
> > No, you read that right. The signature of the set() and set_multiple()
> > callbacks changed in v6.17-rc1 so Viken must have rebased his changes
> > on v6.16 or earlier.
>
> I'm not sure if I understand you correctly. This series must apply on
> current net-next/main, which is v6.17-rc6.
>

The GPIO driver interface changed between v6.16 and v6.17-rc1. This
series uses the old interface. It will not apply on top of v6.17-rc6.

Bartosz

Re: [PATCH v4 5/6] can: mcp251xfd: add gpio functionality

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 1308 bytes --]

On 22.09.2025 16:49:07, Bartosz Golaszewski wrote:
> On Mon, Sep 22, 2025 at 4:43 PM Marc Kleine-Budde <mkl@pengutronix.de> wrote:
> >
> > On 22.09.2025 16:28:53, Bartosz Golaszewski wrote:
> > > > > You must be rebased on pre v6.17 code, this will not compile with current
> > > > > mainline.
> > > >
> > > > You mean "post" v6.17? Best rebase to latest net-next/main, which
> > > > already contains the new signatures for the GPIO callbacks.
> > >
> > > No, you read that right. The signature of the set() and set_multiple()
> > > callbacks changed in v6.17-rc1 so Viken must have rebased his changes
> > > on v6.16 or earlier.
> >
> > I'm not sure if I understand you correctly. This series must apply on
> > current net-next/main, which is v6.17-rc6.
> 
> The GPIO driver interface changed between v6.16 and v6.17-rc1. This
> series uses the old interface. It will not apply on top of v6.17-rc6.

ACK, apparently we had a communication problem about what we exactly
pre/post and earlier means.

regards,
Marc

-- 
Pengutronix e.K.                 | Marc Kleine-Budde          |
Embedded Linux                   | https://www.pengutronix.de |
Vertretung Nürnberg              | Phone: +49-5121-206917-129 |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-9   |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]