On 14.11.2024 13:53:34, Vincent Mailhol wrote:
> 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
The tcan4x5x_priv is allocated in the netdev priv, which is initialized
with 0x0.
>
> tcan4x5x->nwkrq_voltage = 0;
>
> thing.
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:
signature.asc
Description: PGP signature
