Subject: [PATCH v3 5/6] can: tcan4x5x: Add support for tcan4552/4553

tcan4552 and tcan4553 do not have wake or state pins, so they are
currently not compatible with the generic driver. The generic driver
uses tcan4x5x_disable_state() and tcan4x5x_disable_wake() if the gpios
are not defined. These functions use register bits that are not
available in tcan4552/4553.

This patch adds support by introducing version information to reflect if
the chip has wake and state pins. Also the version is now checked.

Signed-off-by: Markus Schneider-Pargmann <[email protected]>
---
drivers/net/can/m_can/tcan4x5x-core.c | 104 ++++++++++++++++++++++----
1 file changed, 90 insertions(+), 14 deletions(-)

diff --git a/drivers/net/can/m_can/tcan4x5x-core.c b/drivers/net/can/m_can/tcan4x5x-core.c
index fb9375fa20ec..2d329b4e4f52 100644
--- a/drivers/net/can/m_can/tcan4x5x-core.c
+++ b/drivers/net/can/m_can/tcan4x5x-core.c
@@ -7,6 +7,7 @@
#define TCAN4X5X_EXT_CLK_DEF 40000000

#define TCAN4X5X_DEV_ID1 0x00
+#define TCAN4X5X_DEV_ID1_TCAN 0x4e414354 /* ASCII TCAN */
#define TCAN4X5X_DEV_ID2 0x04
#define TCAN4X5X_REV 0x08
#define TCAN4X5X_STATUS 0x0C
@@ -103,6 +104,37 @@
#define TCAN4X5X_WD_3_S_TIMER BIT(29)
#define TCAN4X5X_WD_6_S_TIMER (BIT(28) | BIT(29))

+struct tcan4x5x_version_info {
+ const char *name;
+ u32 id2_register;
+
+ bool has_wake_pin;
+ bool has_state_pin;
+};
+
+enum {
+ TCAN4552 = 0,
+ TCAN4553,
+ TCAN4X5X,
+};
+
+static const struct tcan4x5x_version_info tcan4x5x_versions[] = {
+ [TCAN4552] = {
+ .name = "4552",
+ .id2_register = 0x32353534,
+ },
+ [TCAN4553] = {
+ .name = "4553",
+ .id2_register = 0x32353534,
+ },
+ /* generic version with no id2_register at the end */
+ [TCAN4X5X] = {
+ .name = "generic",
+ .has_wake_pin = true,
+ .has_state_pin = true,
+ },
+};
+
static inline struct tcan4x5x_priv *cdev_to_priv(struct m_can_classdev *cdev)
{
return container_of(cdev, struct tcan4x5x_priv, cdev);
@@ -254,18 +286,53 @@ static int tcan4x5x_disable_state(struct m_can_classdev *cdev)
TCAN4X5X_DISABLE_INH_MSK, 0x01);
}

-static int tcan4x5x_get_gpios(struct m_can_classdev *cdev)
+static const struct tcan4x5x_version_info
+*tcan4x5x_find_version(struct tcan4x5x_priv *priv)
+{
+ u32 val;
+ int ret;
+
+ ret = regmap_read(priv->regmap, TCAN4X5X_DEV_ID1, &val);
+ if (ret)
+ return ERR_PTR(ret);
+
+ if (val != TCAN4X5X_DEV_ID1_TCAN) {
+ dev_err(&priv->spi->dev, "Not a tcan device %x\n", val);
+ return ERR_PTR(-ENODEV);
+ }
+
+ ret = regmap_read(priv->regmap, TCAN4X5X_DEV_ID2, &val);
+ if (ret)
+ return ERR_PTR(ret);
+
+ for (int i = 0; i != ARRAY_SIZE(tcan4x5x_versions); ++i) {
+ const struct tcan4x5x_version_info *vinfo = &tcan4x5x_versions[i];
+
+ if (!vinfo->id2_register || val == vinfo->id2_register) {
+ dev_info(&priv->spi->dev, "Detected TCAN device version %s\n",
+ vinfo->name);
+ return vinfo;
+ }
+ }
+
+ return &tcan4x5x_versions[TCAN4X5X];
+}
+
+static int tcan4x5x_get_gpios(struct m_can_classdev *cdev,
+ const struct tcan4x5x_version_info *version_info)
{
struct tcan4x5x_priv *tcan4x5x = cdev_to_priv(cdev);
int ret;

- tcan4x5x->device_wake_gpio = devm_gpiod_get(cdev->dev, "device-wake",
- GPIOD_OUT_HIGH);
- if (IS_ERR(tcan4x5x->device_wake_gpio)) {
- if (PTR_ERR(tcan4x5x->device_wake_gpio) == -EPROBE_DEFER)
- return -EPROBE_DEFER;
+ if (version_info->has_wake_pin) {
+ tcan4x5x->device_wake_gpio = devm_gpiod_get(cdev->dev, "device-wake",
+ GPIOD_OUT_HIGH);
+ if (IS_ERR(tcan4x5x->device_wake_gpio)) {
+ if (PTR_ERR(tcan4x5x->device_wake_gpio) == -EPROBE_DEFER)
+ return -EPROBE_DEFER;

- tcan4x5x_disable_wake(cdev);
+ tcan4x5x_disable_wake(cdev);
+ }
}

tcan4x5x->reset_gpio = devm_gpiod_get_optional(cdev->dev, "reset",
@@ -277,12 +344,14 @@ static int tcan4x5x_get_gpios(struct m_can_classdev *cdev)
if (ret)
return ret;

- tcan4x5x->device_state_gpio = devm_gpiod_get_optional(cdev->dev,
- "device-state",
- GPIOD_IN);
- if (IS_ERR(tcan4x5x->device_state_gpio)) {
- tcan4x5x->device_state_gpio = NULL;
- tcan4x5x_disable_state(cdev);
+ if (version_info->has_state_pin) {
+ tcan4x5x->device_state_gpio = devm_gpiod_get_optional(cdev->dev,
+ "device-state",
+ GPIOD_IN);
+ if (IS_ERR(tcan4x5x->device_state_gpio)) {
+ tcan4x5x->device_state_gpio = NULL;
+ tcan4x5x_disable_state(cdev);
+ }
}

return 0;
@@ -299,6 +368,7 @@ static struct m_can_ops tcan4x5x_ops = {

static int tcan4x5x_can_probe(struct spi_device *spi)
{
+ const struct tcan4x5x_version_info *version_info;
struct tcan4x5x_priv *priv;
struct m_can_classdev *mcan_class;
int freq, ret;
@@ -361,7 +431,13 @@ static int tcan4x5x_can_probe(struct spi_device *spi)
if (ret)
goto out_m_can_class_free_dev;

- ret = tcan4x5x_get_gpios(mcan_class);
+ version_info = tcan4x5x_find_version(priv);
+ if (IS_ERR(version_info)) {
+ ret = PTR_ERR(version_info);
+ goto out_power;
+ }
+
+ ret = tcan4x5x_get_gpios(mcan_class, version_info);
if (ret)
goto out_power;

--
2.40.1



2023-09-15 22:45:09

by Sean Anderson

[permalink] [raw]
Subject: Re: [PATCH v3 5/6] can: tcan4x5x: Add support for tcan4552/4553

On 7/21/23 09:50, Markus Schneider-Pargmann wrote:
> +static const struct tcan4x5x_version_info tcan4x5x_versions[] = {
> + [TCAN4552] = {
> + .name = "4552",
> + .id2_register = 0x32353534,
> + },
> + [TCAN4553] = {
> + .name = "4553",
> + .id2_register = 0x32353534,

Should this be 0x33353534?

> + },
> + /* generic version with no id2_register at the end */
> + [TCAN4X5X] = {
> + .name = "generic",
> + .has_wake_pin = true,
> + .has_state_pin = true,
> + },
> +};

--Sean

Subject: Re: [PATCH v3 5/6] can: tcan4x5x: Add support for tcan4552/4553

Hi Sean,

On Fri, Sep 15, 2023 at 11:36:49AM -0400, Sean Anderson wrote:
> On 7/21/23 09:50, Markus Schneider-Pargmann wrote:
> > +static const struct tcan4x5x_version_info tcan4x5x_versions[] = {
> > + [TCAN4552] = {
> > + .name = "4552",
> > + .id2_register = 0x32353534,
> > + },
> > + [TCAN4553] = {
> > + .name = "4553",
> > + .id2_register = 0x32353534,
>
> Should this be 0x33353534?

Thank you for noticing! Yes. Probably slipped through in the last
refactoring. I will send a fix.

Best,
Markus