Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Meaningful code and commit message mismatch: The patch modifies userspace-visible `gpio-line-names` for existing `gpio@22` and `gpio@24` expanders on both LPDB and RPDB without documenting these changes.
--
--- Patch [10]: [PATCH v4 10/12] ARM: dts: aspeed: anacapa: Add LPDB fuse monitor GPIO expander ---
commit dcdaa17a966db9099c36ac9d58c5a7e8e75f9c5d
Author: Rex Fu <[email protected]>
ARM: dts: aspeed: anacapa: Add LPDB fuse monitor GPIO expander
This patch adds the PCA9554 GPIO expander for PDB fuse monitor signals on the
Anacapa BMC board. It adds definitions under the i2c8 multiplexer for both
left and right PDBs.
> ARM: dts: aspeed: anacapa: Add LPDB fuse monitor GPIO expander
>
> Add the PCA9554 GPIO expander for PDB fuse monitor signals.
[Severity: Medium]
This isn't a bug in the code, but does the commit message fully describe the
changes in this patch?
The commit message solely claims to "Add LPDB fuse monitor GPIO expander", but
this patch also adds the RPDB expander and modifies userspace-visible
gpio-line-names for existing gpio@22 and gpio@24 expanders on both LPDB and
RPDB.
For example, it renames "RPDB_I2C_TEMP75_U8_ALERT_R_N" to
"RPDB_I2C_TMP432_U8_ALERT_R_N", and adds new labels like "RPDB_SKU_ID0" and
"LPDB_VERID_A0".
Could the commit message be updated to reflect these functional hardware label
changes and the addition of the RPDB expander?
> diff --git a/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-anacapa-dvt.dts b/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-anacapa-dvt.dts
> index 0eb547ad0ec16..f09dd1abe77d8 100644
> --- a/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-anacapa-dvt.dts
> +++ b/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-anacapa-dvt.dts
> @@ -8,6 +8,93 @@ / {
> compatible = "facebook,anacapa-bmc-dvt", "aspeed,ast2600";
> };
>
> +// MB - PDB
> +&i2c8 {
> + i2c-mux@72 {
> + i2c8mux0ch0: i2c@0 {
> + gpio@22 {
> + gpio-line-names =
> + "RPDB_FAN_FULL_SPEED_R_N", "RPDB_I2C_TMP432_U8_ALERT_R_N",
[Severity: Medium]
This is where the undocumented RPDB expander definitions and gpio-line-names
modifications are introduced in i2c8mux0ch0. Should these be split into a
separate patch or documented in the commit message?
[ ... ]
> + i2c8mux0ch1: i2c@1 {
> + gpio@22 {
> + gpio-line-names =
> + "LPDB_FAN_FULL_SPEED_R_N", "LPDB_I2C_TEMP75_U8_ALERT_R_N",
[Severity: Medium]
Similarly here for i2c8mux0ch1 in the Left PDB, the patch introduces
undocumented label additions and modifications to the existing gpio@22 and
gpio@24 expanders. Should these changes also be mentioned in the commit
message?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260805-anacapa-devlop-phase-devicetree-v4-0-5d67e0efa6c5@gmail.com?part=10
lmpx.com only provides a reader for public news (NNTP) servers. It is not
affiliated with the servers or forums shown here and is not responsible for
the content of articles, which is written by their respective authors.