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

Viken Dadhaniya posted 6 patches 2 weeks ago
There is a newer version of this series
[PATCH v4 5/6] can: mcp251xfd: add gpio functionality
Posted by Viken Dadhaniya 2 weeks ago
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
Re: [PATCH v4 5/6] can: mcp251xfd: add gpio functionality
Posted by Bartosz Golaszewski 1 week, 6 days ago
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
Posted by Marc Kleine-Budde 1 week, 6 days ago
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   |
Re: [PATCH v4 5/6] can: mcp251xfd: add gpio functionality
Posted by Bartosz Golaszewski 1 week, 2 days ago
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
Posted by Marc Kleine-Budde 1 week, 2 days ago
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   |
Re: [PATCH v4 5/6] can: mcp251xfd: add gpio functionality
Posted by Bartosz Golaszewski 1 week, 2 days ago
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
Posted by Marc Kleine-Budde 1 week, 2 days ago
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   |
Re: [PATCH v4 5/6] can: mcp251xfd: add gpio functionality
Posted by Viken Dadhaniya 1 week, 2 days ago

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