From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 682D6303A0D for ; Thu, 14 May 2026 05:44:48 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1778737488; cv=none; b=ZZpSSXvmGZZtHYWDU/lMDj8/xki18AveSzXRRU63foM8ejMh3GUzYc9pXAWLCllLWofQG6uGORkB4k+ZvrlaMkn8qRRBDuzFmJGGCGppdF1WpvVIFmCeOD5keEytCIw2wpES6ADmgNgGvQ2xkiWPPcUuxWgW88njA4My3JV+7rY= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1778737488; c=relaxed/simple; bh=H/+ligJMcqnsftu46EjlCwh92QGuF7zRIdAgI725xe0=; h=From:Subject:To:Cc:In-Reply-To:References:Content-Type:Date: Message-Id; b=giQgY/8A6XsUTG92wZ2ox1B27Ym04c7bpLlkx3CWIY1cQANdLJnTmUQ9QH41cKGRdtH4Lm94AjrvUlN3H6ZmgWG+RoN0JNUZ9sXoGdqAl6pVVMfZxw0XAG2chGzllpIrqZqbJg1IlPzrvZBDvLHsu1lIrMhdzdZ4wRmc2YksNH0= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=JSaNookr; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="JSaNookr" Received: by smtp.kernel.org (Postfix) with ESMTPSA id A59AFC2BCB7; Thu, 14 May 2026 05:44:47 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1778737488; bh=H/+ligJMcqnsftu46EjlCwh92QGuF7zRIdAgI725xe0=; h=From:Subject:Reply-To:To:Cc:In-Reply-To:References:Date:From; b=JSaNookrt/HD3/So/BLwd6sybK9u980Ub22mkOz8+WeEsL5ibSt867CVKnnAaVXQ7 PflsChz/JTH0X3oJ1xG1Vs2+l2grgm7ztZviOuFG8unGQ588+eiyb3L0VJwkI1Drtq P61SEuq/N+UbPEm18jjj1HWmzibMOsljFnJGZtaiS8ZjpUigBE7SP0TsNCbk7Gn+FB GEip4c1XmRieIwzsnSBVaUE8JQ7hnm9K0uCpAdm7k/wYUslMm5SRauO6GqjJpUjzB8 kitS94jOJ2m4A/LA9Bq3tziU58zgvZUVMLut0fbGkxPzQaJk67jYdlSMZ4cjwqnOL8 hPq7VD/bmNyUg== From: sashiko-bot@kernel.org Subject: Re: [PATCH v5 2/2] phy: add basic support for NXPs TJA1145 CAN transceiver Reply-To: sashiko-reviews@lists.linux.dev To: "Dimitri Fedrau via B4 Relay" Cc: olteanv@gmail.com, krzk+dt@kernel.org, vkoul@kernel.org, linux-phy@lists.infradead.org, conor+dt@kernel.org, robh@kernel.org, devicetree@vger.kernel.org, neil.armstrong@linaro.org In-Reply-To: <20260513-tja1145-support-v5-2-38720a7ee63e@liebherr.com> References: <20260513-tja1145-support-v5-2-38720a7ee63e@liebherr.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Date: Thu, 14 May 2026 05:44:47 +0000 Message-Id: <20260514054447.A59AFC2BCB7@smtp.kernel.org> Precedence: bulk X-Mailing-List: devicetree@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: 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-suppl= y`, `vio-supply`, `vbat-supply`) before communicating with the device. - [High] The driver does not observe the required mode transition delay whe= n powering on the PHY. - [Medium] Missing rollback to standby mode in the error path of `tja1145_p= hy_power_on`. -- commit 288134e179992cb2e690ff7a97502e72f04e9d5a Author: Dimitri Fedrau 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 =3D 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 =3D 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? 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. 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 =3D &spi->dev; > + struct regmap *map; > + struct phy *phy; > + int ret; > + > + map =3D 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 =3D tja1145_check_ident(dev, map); Will this fail to communicate with the hardware on platforms where power supplies are dynamically controlled? 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"); --=20 Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260513-tja1145-su= pport-v5-0-38720a7ee63e@liebherr.com?part=3D2 From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 51E4ECD4F39 for ; Thu, 14 May 2026 05:44:53 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:MIME-Version:Reply-To:List-Subscribe: List-Help:List-Post:List-Archive:List-Unsubscribe:List-Id:Message-Id:Date: References:In-Reply-To:Cc:To:Subject:From:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=sVZnGdZyDVNoDDXkq1vdNJ2pECQ5EQ9UKy3scQZBlg0=; b=Rab8uQmQV54wQe +vpQDqpZyxNYzLJJtSIQuxnUOJlvMc/E1HEPeHQBM1ffys6t6nomMsZId7toEr+qKGDsZOl67xtHJ 4hA1hQI1NqIks5PzmUnZd9l9RsZ55VJKyLMGm0GaGFZXhEAscOFgDaIqkQjZJun7uFHBFf46RanG/ Ls0Os/supd3n65eIDnTAvCqY57jtFddgYLIsbS2X5/dHUjC7euDoAMN7B+vGCF47CzsKEhtkijHXp lFpYfnZkIIjUkvfzzA7UpJaRgjkcz98AgeE5YJsMNnMiTjWhLDiTHScEZaDokzH53o3fLVSUDlA0J 5ghB2KOASAajNW5a5eXA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.99.1 #2 (Red Hat Linux)) id 1wNOsO-00000004eG9-1hrg; Thu, 14 May 2026 05:44:52 +0000 Received: from sea.source.kernel.org ([2600:3c0a:e001:78e:0:1991:8:25]) by bombadil.infradead.org with esmtps (Exim 4.99.1 #2 (Red Hat Linux)) id 1wNOsL-00000004eFb-3DQD for linux-phy@lists.infradead.org; Thu, 14 May 2026 05:44:51 +0000 Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by sea.source.kernel.org (Postfix) with ESMTP id 2EB6943B73; Thu, 14 May 2026 05:44:48 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id A59AFC2BCB7; Thu, 14 May 2026 05:44:47 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1778737488; bh=H/+ligJMcqnsftu46EjlCwh92QGuF7zRIdAgI725xe0=; h=From:Subject:Reply-To:To:Cc:In-Reply-To:References:Date:From; b=JSaNookrt/HD3/So/BLwd6sybK9u980Ub22mkOz8+WeEsL5ibSt867CVKnnAaVXQ7 PflsChz/JTH0X3oJ1xG1Vs2+l2grgm7ztZviOuFG8unGQ588+eiyb3L0VJwkI1Drtq P61SEuq/N+UbPEm18jjj1HWmzibMOsljFnJGZtaiS8ZjpUigBE7SP0TsNCbk7Gn+FB GEip4c1XmRieIwzsnSBVaUE8JQ7hnm9K0uCpAdm7k/wYUslMm5SRauO6GqjJpUjzB8 kitS94jOJ2m4A/LA9Bq3tziU58zgvZUVMLut0fbGkxPzQaJk67jYdlSMZ4cjwqnOL8 hPq7VD/bmNyUg== From: sashiko-bot@kernel.org Subject: Re: [PATCH v5 2/2] phy: add basic support for NXPs TJA1145 CAN transceiver To: "Dimitri Fedrau via B4 Relay" Cc: olteanv@gmail.com, krzk+dt@kernel.org, vkoul@kernel.org, linux-phy@lists.infradead.org, conor+dt@kernel.org, robh@kernel.org, devicetree@vger.kernel.org, neil.armstrong@linaro.org In-Reply-To: <20260513-tja1145-support-v5-2-38720a7ee63e@liebherr.com> References: <20260513-tja1145-support-v5-2-38720a7ee63e@liebherr.com> Date: Thu, 14 May 2026 05:44:47 +0000 Message-Id: <20260514054447.A59AFC2BCB7@smtp.kernel.org> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.9.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20260513_224449_850477_A514730D X-CRM114-Status: GOOD ( 19.43 ) X-BeenThere: linux-phy@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Linux Phy Mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: sashiko-reviews@lists.linux.dev MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Sender: "linux-phy" Errors-To: linux-phy-bounces+linux-phy=archiver.kernel.org@lists.infradead.org VGhhbmsgeW91IGZvciB5b3VyIGNvbnRyaWJ1dGlvbiEgU2FzaGlrbyBBSSByZXZpZXcgZm91bmQg MyBwb3RlbnRpYWwgaXNzdWUocykgdG8gY29uc2lkZXI6Ci0gW0hpZ2hdIFRoZSBkcml2ZXIgZmFp bHMgdG8gZW5hYmxlIHRoZSBoYXJkd2FyZSBwb3dlciBzdXBwbGllcyAoYHZjYy1zdXBwbHlgLCBg dmlvLXN1cHBseWAsIGB2YmF0LXN1cHBseWApIGJlZm9yZSBjb21tdW5pY2F0aW5nIHdpdGggdGhl IGRldmljZS4KLSBbSGlnaF0gVGhlIGRyaXZlciBkb2VzIG5vdCBvYnNlcnZlIHRoZSByZXF1aXJl ZCBtb2RlIHRyYW5zaXRpb24gZGVsYXkgd2hlbiBwb3dlcmluZyBvbiB0aGUgUEhZLgotIFtNZWRp dW1dIE1pc3Npbmcgcm9sbGJhY2sgdG8gc3RhbmRieSBtb2RlIGluIHRoZSBlcnJvciBwYXRoIG9m IGB0amExMTQ1X3BoeV9wb3dlcl9vbmAuCi0tCgpjb21taXQgMjg4MTM0ZTE3OTk5MmNiMmU2OTBm ZjdhOTc1MDJlNzJmMDRlOWQ1YQpBdXRob3I6IERpbWl0cmkgRmVkcmF1IDxkaW1pdHJpLmZlZHJh dUBsaWViaGVyci5jb20+CgpwaHk6IGFkZCBiYXNpYyBzdXBwb3J0IGZvciBOWFBzIFRKQTExNDUg Q0FOIHRyYW5zY2VpdmVyCgpUaGlzIGNvbW1pdCBhZGRzIGJhc2ljIGRyaXZlciBzdXBwb3J0IGZv ciB0aGUgTlhQIFRKQTExNDUgQ0FOIHRyYW5zY2VpdmVyLgpJdCBpbXBsZW1lbnRzIHRoZSBQSFkg b3BlcmF0aW9ucyB0byBicmluZyB0aGUgaGFyZHdhcmUgdXAgYW5kIGRvd24gYnkKc3dpdGNoaW5n IGJldHdlZW4gbm9ybWFsIGFuZCBzdGFuZGJ5IG1vZGVzIHVzaW5nIFNQSSBjb21tYW5kcy4KCj4g ZGlmZiAtLWdpdCBhL2RyaXZlcnMvcGh5L3BoeS1ueHAtdGphMTE0NS5jIGIvZHJpdmVycy9waHkv cGh5LW54cC10amExMTQ1LmMKPiBuZXcgZmlsZSBtb2RlIDEwMDY0NAo+IGluZGV4IDAwMDAwMDAw MDAwMDAuLjFlOGJkMTY5NzQzYWIKPiAtLS0gL2Rldi9udWxsCj4gKysrIGIvZHJpdmVycy9waHkv cGh5LW54cC10amExMTQ1LmMKClsgLi4uIF0KCj4gK3N0YXRpYyBpbnQgdGphMTE0NV9waHlfcG93 ZXJfb24oc3RydWN0IHBoeSAqcGh5KQo+ICt7Cj4gKwlzdHJ1Y3QgcmVnbWFwICptYXAgPSBwaHlf Z2V0X2RydmRhdGEocGh5KTsKPiArCWludCByZXQ7Cj4gKwo+ICsJLyoKPiArCSAqIFN3aXRjaCBv cGVyYXRpbmcgbW9kZSB0byBub3JtYWwgd2hpY2ggaXMgdGhlIGFjdGl2ZSBvcGVyYXRpbmcgbW9k ZS4KPiArCSAqIEluIHRoaXMgbW9kZSwgdGhlIGRldmljZSBpcyBmdWxseSBvcGVyYXRpb25hbC4K PiArCSAqLwo+ICsJcmV0ID0gcmVnbWFwX3VwZGF0ZV9iaXRzKG1hcCwgVEpBMTE0NV9NT0RFX0NU UkwsIFRKQTExNDVfTU9ERV9DVFJMX01DLAo+ICsJCQkJIFRKQTExNDVfTU9ERV9DVFJMX05PUk1B TCk7Cj4gKwlpZiAocmV0KQo+ICsJCXJldHVybiByZXQ7CgpBY2NvcmRpbmcgdG8gdGhlIGhhcmR3 YXJlIGRhdGFzaGVldCwgdHJhbnNpdGlvbmluZyB0aGUgdHJhbnNjZWl2ZXIgdG8gbm9ybWFsCm1v ZGUgY2FuIHRha2UgdXAgdG8gNDAgdXMuIERvZXMgdGhpcyBzZXF1ZW5jZSBuZWVkIGEgaGFyZHdh cmUgdHJhbnNpdGlvbgpkZWxheSBiZWZvcmUgc2VuZGluZyB0aGUgYWN0aXZlIG1vZGUgY29tbWFu ZD8KCldpdGggYSBmYXN0IFNQSSBidXMsIHRoZSBuZXh0IGNvbW1hbmQgbWlnaHQgYXJyaXZlIGJl Zm9yZSB0aGUgaGFyZHdhcmUgc3RhdGUKbWFjaGluZSBmaW5pc2hlcyB0cmFuc2l0aW9uaW5nLCB3 aGljaCBjb3VsZCBjYXVzZSB0aGUgYWN0aXZlIG1vZGUgY29tbWFuZCB0bwpiZSBpZ25vcmVkLgoK PiArCj4gKwkvKgo+ICsJICogU3dpdGNoIHRvIENBTiBvcGVyYXRpbmcgbW9kZSBhY3RpdmUgd2hl cmUgdGhlIFBIWSBjYW4gdHJhbnNtaXQgYW5kCj4gKwkgKiByZWNlaXZlIGRhdGEuCj4gKwkgKi8K PiArCXJldHVybiByZWdtYXBfdXBkYXRlX2JpdHMobWFwLCBUSkExMTQ1X0NBTl9DVFJMLCBUSkEx MTQ1X0NBTl9DVFJMX0NNQywKPiArCQkJCSAgVEpBMTE0NV9DQU5fQ1RSTF9BQ1RJVkUpOwoKSWYg dGhpcyBzZWNvbmQgcmVnbWFwX3VwZGF0ZV9iaXRzKCkgZmFpbHMsIHRoZSBmdW5jdGlvbiByZXR1 cm5zIGFuIGVycm9yCndpdGhvdXQgcmV2ZXJ0aW5nIHRoZSBkZXZpY2UgdG8gc3RhbmRieSBtb2Rl LgoKU2luY2UgdGhlIFBIWSBmcmFtZXdvcmsga2VlcHMgdGhlIHBvd2VyX2NvdW50IGF0IDAgb24g YW4gZXJyb3IgcmV0dXJuLApwaHlfcG93ZXJfb2ZmKCkgd2lsbCBub3QgYmUgY2FsbGVkIGR1cmlu ZyBzaHV0ZG93biBvciBzdXNwZW5kLiBDb3VsZCB0aGlzCmxlYXZlIHRoZSBoYXJkd2FyZSBwZXJt YW5lbnRseSBzdHVjayBpbiBub3JtYWwgbW9kZSwgZHJhd2luZyBwb3dlcgp1bmludGVudGlvbmFs bHk/Cgo+ICt9CgpbIC4uLiBdCgo+ICtzdGF0aWMgaW50IHRqYTExNDVfcHJvYmUoc3RydWN0IHNw aV9kZXZpY2UgKnNwaSkKPiArewo+ICsJc3RydWN0IHBoeV9wcm92aWRlciAqcGh5X3Byb3ZpZGVy Owo+ICsJc3RydWN0IGRldmljZSAqZGV2ID0gJnNwaS0+ZGV2Owo+ICsJc3RydWN0IHJlZ21hcCAq bWFwOwo+ICsJc3RydWN0IHBoeSAqcGh5Owo+ICsJaW50IHJldDsKPiArCj4gKwltYXAgPSBkZXZt X3JlZ21hcF9pbml0X3NwaShzcGksICZ0amExMTQ1X3JlZ21hcF9jb25maWcpOwo+ICsJaWYgKElT X0VSUihtYXApKQo+ICsJCXJldHVybiBkZXZfZXJyX3Byb2JlKGRldiwgUFRSX0VSUihtYXApLCAi ZmFpbGVkIHRvIGluaXQgcmVnbWFwXG4iKTsKPiArCj4gKwlyZXQgPSB0amExMTQ1X2NoZWNrX2lk ZW50KGRldiwgbWFwKTsKCldpbGwgdGhpcyBmYWlsIHRvIGNvbW11bmljYXRlIHdpdGggdGhlIGhh cmR3YXJlIG9uIHBsYXRmb3JtcyB3aGVyZSBwb3dlcgpzdXBwbGllcyBhcmUgZHluYW1pY2FsbHkg Y29udHJvbGxlZD8KClRoZSBkZXZpY2UgdHJlZSBiaW5kaW5nIGRlZmluZXMgdmNjLXN1cHBseSwg dmlvLXN1cHBseSwgYW5kIHZiYXQtc3VwcGx5IGFzCnBvd2VyIHNvdXJjZXMsIGJ1dCB0aGV5IGRv IG5vdCBhcHBlYXIgdG8gYmUgcmVxdWVzdGVkIG9yIGVuYWJsZWQgaW4gdGhpcwpmdW5jdGlvbiBi ZWZvcmUgdGphMTE0NV9jaGVja19pZGVudCgpIGlzIGNhbGxlZC4KClRoaXMgbWlnaHQgbGVhdmUg dGhlIFBIWSB1bnBvd2VyZWQsIGNhdXNpbmcgdGhlIFNQSSByZWFkIHRvIHJldHVybiBnYXJiYWdl CmRhdGEgYW5kIGZhaWwgdGhlIHByb2JlIHdpdGggLUVOT0RFVi4KCj4gKwlpZiAocmV0KQo+ICsJ CXJldHVybiBkZXZfZXJyX3Byb2JlKGRldiwgcmV0LCAiZmFpbGVkIHRvIGlkZW50aWZ5IGRldmlj ZVxuIik7CgotLSAKU2FzaGlrbyBBSSByZXZpZXcgwrcgaHR0cHM6Ly9zYXNoaWtvLmRldi8jL3Bh dGNoc2V0LzIwMjYwNTEzLXRqYTExNDUtc3VwcG9ydC12NS0wLTM4NzIwYTdlZTYzZUBsaWViaGVy ci5jb20/cGFydD0yCgotLSAKbGludXgtcGh5IG1haWxpbmcgbGlzdApsaW51eC1waHlAbGlzdHMu aW5mcmFkZWFkLm9yZwpodHRwczovL2xpc3RzLmluZnJhZGVhZC5vcmcvbWFpbG1hbi9saXN0aW5m by9saW51eC1waHkK