On Mon. 11 Nov. 2024 at 17:55, Sean Nyekjaer <sean@xxxxxxxxxx> wrote:
> nWKRQ supports an output voltage of either the internal reference voltage
> (3.6V) or the reference voltage of the digital interface 0 - 6V.
> Add the devicetree option ti,nwkrq-voltage-sel to be able to select
> between them.
> Default is kept as the internal reference voltage.
>
> Signed-off-by: Sean Nyekjaer <sean@xxxxxxxxxx>
> ---
> drivers/net/can/m_can/tcan4x5x-core.c | 35 +++++++++++++++++++++++++++++++++++
> drivers/net/can/m_can/tcan4x5x.h | 2 ++
> 2 files changed, 37 insertions(+)
>
> diff --git a/drivers/net/can/m_can/tcan4x5x-core.c b/drivers/net/can/m_can/tcan4x5x-core.c
> index 2f73bf3abad889c222f15c39a3d43de1a1cf5fbb..264bba830be50033347056da994102f8b614e51b 100644
> --- a/drivers/net/can/m_can/tcan4x5x-core.c
> +++ b/drivers/net/can/m_can/tcan4x5x-core.c
> @@ -92,6 +92,8 @@
> #define TCAN4X5X_MODE_STANDBY BIT(6)
> #define TCAN4X5X_MODE_NORMAL BIT(7)
>
> +#define TCAN4X5X_NWKRQ_VOLTAGE_MASK BIT(19)
> +
> #define TCAN4X5X_DISABLE_WAKE_MSK (BIT(31) | BIT(30))
> #define TCAN4X5X_DISABLE_INH_MSK BIT(9)
>
> @@ -267,6 +269,11 @@ static int tcan4x5x_init(struct m_can_classdev *cdev)
> if (ret)
> return ret;
>
> + ret = regmap_update_bits(tcan4x5x->regmap, TCAN4X5X_CONFIG,
> + TCAN4X5X_NWKRQ_VOLTAGE_MASK, tcan4x5x->nwkrq_voltage);
> + if (ret)
> + return ret;
> +
> return ret;
> }
>
> @@ -318,6 +325,28 @@ static const struct tcan4x5x_version_info
> return &tcan4x5x_versions[TCAN4X5X];
> }
>
> +static int tcan4x5x_get_dt_data(struct m_can_classdev *cdev)
> +{
> + struct tcan4x5x_priv *tcan4x5x = cdev_to_priv(cdev);
> + struct device_node *np = cdev->dev->of_node;
> + u8 prop;
> + int ret;
> +
> + ret = of_property_read_u8(np, "ti,nwkrq-voltage-sel", &prop);
> + if (!ret) {
> + if (prop <= 1)
> + tcan4x5x->nwkrq_voltage = prop;
> + else
> + dev_warn(cdev->dev,
> + "nwkrq-voltage-sel have invalid option: %u\n",
> + prop);
> + } else {
> + tcan4x5x->nwkrq_voltage = 0;
> + }
If the
if (prop <= 1)
condition fails, you print a warning, but you are not assigning a
value to tcan4x5x->nwkrq_voltage. Is this intentional?
What about:
tcan4x5x->nwkrq_voltage = 0;
ret = of_property_read_u8(np, "ti,nwkrq-voltage-sel", &prop);
if (!ret) {
if (prop <= 1)
tcan4x5x->nwkrq_voltage = prop;
else
dev_warn(cdev->dev,
"nwkrq-voltage-sel have invalid option: %u\n",
prop);
}
so that you make sure that tcan4x5x->nwkrq_voltage always gets a
default zero value? Else, if you can make sure that tcan4x5x is always
zero initialized, you can just drop the
tcan4x5x->nwkrq_voltage = 0;
thing.
> + return 0;
> +}
> +
> static int tcan4x5x_get_gpios(struct m_can_classdev *cdev,
> const struct tcan4x5x_version_info *version_info)
> {
> @@ -453,6 +482,12 @@ static int tcan4x5x_can_probe(struct spi_device *spi)
> goto out_power;
> }
>
> + ret = tcan4x5x_get_dt_data(mcan_class);
> + if (ret) {
> + dev_err(&spi->dev, "Getting dt data failed %pe\n", ERR_PTR(ret));
> + goto out_power;
> + }
> +
> tcan4x5x_check_wake(priv);
>
> ret = tcan4x5x_write_tcan_reg(mcan_class, TCAN4X5X_INT_EN, 0);
> diff --git a/drivers/net/can/m_can/tcan4x5x.h b/drivers/net/can/m_can/tcan4x5x.h
> index e62c030d3e1e5a713c997e7c8ecad4a44aff4e6a..04ebe5c64f4f7056a62e72e717cb85dd3817ab9c 100644
> --- a/drivers/net/can/m_can/tcan4x5x.h
> +++ b/drivers/net/can/m_can/tcan4x5x.h
> @@ -42,6 +42,8 @@ struct tcan4x5x_priv {
>
> struct tcan4x5x_map_buf map_buf_rx;
> struct tcan4x5x_map_buf map_buf_tx;
> +
> + u8 nwkrq_voltage;
> };
>
> static inline void
>
> --
> 2.46.2
>
>
