[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZBC6lV44CIYsJA88@localhost.localdomain>
Date: Tue, 14 Mar 2023 19:19:01 +0100
From: Michal Kubiak <michal.kubiak@...el.com>
To: Markus Schneider-Pargmann <msp@...libre.com>
CC: Marc Kleine-Budde <mkl@...gutronix.de>,
Chandrasekar Ramakrishnan <rcsekar@...sung.com>,
Wolfgang Grandegger <wg@...ndegger.com>,
Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
Vincent MAILHOL <mailhol.vincent@...adoo.fr>,
Simon Horman <simon.horman@...igine.com>,
<linux-can@...r.kernel.org>, <netdev@...r.kernel.org>,
<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 5/5] can: tcan4x5x: Add support for tcan4552/4553
On Tue, Mar 14, 2023 at 04:12:01PM +0100, Markus Schneider-Pargmann wrote:
> 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
> ---
> drivers/net/can/m_can/tcan4x5x-core.c | 113 ++++++++++++++++++++------
> 1 file changed, 89 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/net/can/m_can/tcan4x5x-core.c b/drivers/net/can/m_can/tcan4x5x-core.c
> index fb9375fa20ec..e7fa509dacc9 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,13 @@
> #define TCAN4X5X_WD_3_S_TIMER BIT(29)
> #define TCAN4X5X_WD_6_S_TIMER (BIT(28) | BIT(29))
>
> +struct tcan4x5x_version_info {
> + u32 id2_register;
> +
> + bool has_wake_pin;
> + bool has_state_pin;
> +};
> +
> static inline struct tcan4x5x_priv *cdev_to_priv(struct m_can_classdev *cdev)
> {
> return container_of(cdev, struct tcan4x5x_priv, cdev);
> @@ -254,18 +262,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 int tcan4x5x_verify_version(
> + struct tcan4x5x_priv *priv,
> + const struct tcan4x5x_version_info *version_info)
> +{
> + u32 val;
> + int ret;
> +
> + ret = regmap_read(priv->regmap, TCAN4X5X_DEV_ID1, &val);
> + if (ret)
> + return ret;
> +
> + if (val != TCAN4X5X_DEV_ID1_TCAN) {
> + dev_err(&priv->spi->dev, "Not a tcan device %x\n", val);
> + return -ENODEV;
> + }
> +
> + if (!version_info->id2_register)
> + return 0;
> +
> + ret = regmap_read(priv->regmap, TCAN4X5X_DEV_ID2, &val);
> + if (ret)
> + return ret;
> +
> + if (version_info->id2_register != val) {
> + dev_err(&priv->spi->dev, "Not the specified TCAN device, id2: %x != %x\n",
> + version_info->id2_register, val);
> + return -ENODEV;
> + }
> +
> + return 0;
> +}
> +
> +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 +320,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;
> @@ -301,8 +346,13 @@ static int tcan4x5x_can_probe(struct spi_device *spi)
> {
> struct tcan4x5x_priv *priv;
> struct m_can_classdev *mcan_class;
> + const struct tcan4x5x_version_info *version_info;
> int freq, ret;
Nitpick: RCT.
>
> + version_info = of_device_get_match_data(&spi->dev);
> + if (!version_info)
> + version_info = (void *)spi_get_device_id(spi)->driver_data;
> +
> mcan_class = m_can_class_allocate_dev(&spi->dev,
> sizeof(struct tcan4x5x_priv));
> if (!mcan_class)
> @@ -361,7 +411,11 @@ static int tcan4x5x_can_probe(struct spi_device *spi)
> if (ret)
> goto out_m_can_class_free_dev;
>
> - ret = tcan4x5x_get_gpios(mcan_class);
> + ret = tcan4x5x_verify_version(priv, version_info);
> + if (ret)
> + goto out_power;
> +
> + ret = tcan4x5x_get_gpios(mcan_class, version_info);
> if (ret)
> goto out_power;
>
> @@ -394,21 +448,32 @@ static void tcan4x5x_can_remove(struct spi_device *spi)
> m_can_class_free_dev(priv->cdev.net);
> }
>
> +static const struct tcan4x5x_version_info tcan4x5x_generic = {
> + .has_state_pin = true,
> + .has_wake_pin = true,
> +};
> +
> +static const struct tcan4x5x_version_info tcan4x5x_tcan4552 = {
> + .id2_register = 0x32353534, /* ASCII = 4552 */
> +};
> +
> +static const struct tcan4x5x_version_info tcan4x5x_tcan4553 = {
> + .id2_register = 0x33353534, /* ASCII = 4553 */
> +};
> +
> static const struct of_device_id tcan4x5x_of_match[] = {
> - {
> - .compatible = "ti,tcan4x5x",
> - }, {
> - /* sentinel */
> - },
> + { .compatible = "ti,tcan4x5x", .data = &tcan4x5x_generic },
> + { .compatible = "ti,tcan4552", .data = &tcan4x5x_tcan4552 },
> + { .compatible = "ti,tcan4553", .data = &tcan4x5x_tcan4553 },
> + { /* sentinel */ }
> };
> MODULE_DEVICE_TABLE(of, tcan4x5x_of_match);
>
> static const struct spi_device_id tcan4x5x_id_table[] = {
> - {
> - .name = "tcan4x5x",
> - }, {
> - /* sentinel */
> - },
> + { .name = "tcan4x5x", .driver_data = (unsigned long) &tcan4x5x_generic, },
> + { .name = "tcan4552", .driver_data = (unsigned long) &tcan4x5x_tcan4552, },
> + { .name = "tcan4553", .driver_data = (unsigned long) &tcan4x5x_tcan4553, },
> + { /* sentinel */ }
> };
> MODULE_DEVICE_TABLE(spi, tcan4x5x_id_table);
Thanks,
Michal
>
> --
> 2.39.2
>
Powered by blists - more mailing lists