Re: [PATCH v5 2/2] phy: add basic support for NXPs TJA1145 CAN transceiver

[Dimitri Fedrau <dima.fedrau@gmail.com>]

Am Thu, May 14, 2026 at 05:44:47AM +0000 schrieb sashiko-bot@kernel.org:
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
> - [High] The driver fails to enable the hardware power supplies (`vcc-supply`, `vio-supply`, `vbat-supply`) before communicating with the device.
> - [High] The driver does not observe the required mode transition delay when powering on the PHY.
> - [Medium] Missing rollback to standby mode in the error path of `tja1145_phy_power_on`.
> --
> 
> commit 288134e179992cb2e690ff7a97502e72f04e9d5a
> Author: Dimitri Fedrau <dimitri.fedrau@liebherr.com>
> 
> phy: add basic support for NXPs TJA1145 CAN transceiver
> 
> This commit adds basic driver support for the NXP TJA1145 CAN transceiver.
> It implements the PHY operations to bring the hardware up and down by
> switching between normal and standby modes using SPI commands.
> 
> > diff --git a/drivers/phy/phy-nxp-tja1145.c b/drivers/phy/phy-nxp-tja1145.c
> > new file mode 100644
> > index 0000000000000..1e8bd169743ab
> > --- /dev/null
> > +++ b/drivers/phy/phy-nxp-tja1145.c
> 
> [ ... ]
> 
> > +static int tja1145_phy_power_on(struct phy *phy)
> > +{
> > +	struct regmap *map = phy_get_drvdata(phy);
> > +	int ret;
> > +
> > +	/*
> > +	 * Switch operating mode to normal which is the active operating mode.
> > +	 * In this mode, the device is fully operational.
> > +	 */
> > +	ret = regmap_update_bits(map, TJA1145_MODE_CTRL, TJA1145_MODE_CTRL_MC,
> > +				 TJA1145_MODE_CTRL_NORMAL);
> > +	if (ret)
> > +		return ret;
> 
> According to the hardware datasheet, transitioning the transceiver to normal
> mode can take up to 40 us. Does this sequence need a hardware transition
> delay before sending the active mode command?
>
Cannot find anything related in the datasheet.

> With a fast SPI bus, the next command might arrive before the hardware state
> machine finishes transitioning, which could cause the active mode command to
> be ignored.
> 
> > +
> > +	/*
> > +	 * Switch to CAN operating mode active where the PHY can transmit and
> > +	 * receive data.
> > +	 */
> > +	return regmap_update_bits(map, TJA1145_CAN_CTRL, TJA1145_CAN_CTRL_CMC,
> > +				  TJA1145_CAN_CTRL_ACTIVE);
> 
> If this second regmap_update_bits() fails, the function returns an error
> without reverting the device to standby mode.
> 
Don't see a second regmap_update_bits().

> Since the PHY framework keeps the power_count at 0 on an error return,
> phy_power_off() will not be called during shutdown or suspend. Could this
> leave the hardware permanently stuck in normal mode, drawing power
> unintentionally?
> 
> > +}
> 
> [ ... ]
> 
> > +static int tja1145_probe(struct spi_device *spi)
> > +{
> > +	struct phy_provider *phy_provider;
> > +	struct device *dev = &spi->dev;
> > +	struct regmap *map;
> > +	struct phy *phy;
> > +	int ret;
> > +
> > +	map = devm_regmap_init_spi(spi, &tja1145_regmap_config);
> > +	if (IS_ERR(map))
> > +		return dev_err_probe(dev, PTR_ERR(map), "failed to init regmap\n");
> > +
> > +	ret = tja1145_check_ident(dev, map);
> 
> Will this fail to communicate with the hardware on platforms where power
> supplies are dynamically controlled?
>

Yes, but dyamically controlled power supplies are not implemented yet.
Support has to wait until there is need for it.

> The device tree binding defines vcc-supply, vio-supply, and vbat-supply as
> power sources, but they do not appear to be requested or enabled in this
> function before tja1145_check_ident() is called.
> 
> This might leave the PHY unpowered, causing the SPI read to return garbage
> data and fail the probe with -ENODEV.
> 
> > +	if (ret)
> > +		return dev_err_probe(dev, ret, "failed to identify device\n");
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260513-tja1145-support-v5-0-38720a7ee63e@liebherr.com?part=2
> 
> -- 
> linux-phy mailing list
> linux-phy@lists.infradead.org
> https://lists.infradead.org/mailman/listinfo/linux-phy

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy