Re: [PATCH v10 4/5] phy: move and rename Airoha PCIe PHY driver to dedicated directory

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [Low] The driver Kconfig symbol, file names, and platform driver name are incorrectly renamed using the 'AN7581' prefix instead of 'EN7581', creating a mismatch with the hardware name and device tree bindings.
- [Low] The include guard in `phy-an7581-pcie-regs.h` was not updated after the file was renamed.
- [Low] Incorrect SPDX-License-Identifier comment style for a C header file.
- [Low] The newly added `Kconfig` and `Makefile` for the `drivers/phy/airoha/` directory are missing from the `MAINTAINERS` file.
--

commit 6b7c73648fcebc4627d82c8ab33128623756a453
Author: Christian Marangi <ansuelsmth@gmail.com>

phy: move and rename Airoha PCIe PHY driver to dedicated directory

This commit moves the PCIe PHY driver for the Airoha AN7581 SoC to a dedicated
directory to keep the generic PHY directory tidy. It also renames the driver
and files to include the specific SoC name in preparation for adding AN7583
support, and updates the Kconfig symbol accordingly.

> To keep the generic PHY directory tidy, move the PCIe PHY driver for
> Airoha AN7581 SoC to a dedicated directory.
> 
> Also rename the driver and add the relevant SoC name to the .c and .h
> file in preparation for support of PCIe and USB PHY driver for Airoha
> AN7583 SoC

[Severity: Low]
This isn't a bug, but there might be a typo in the commit message. Is the
SoC name EN7581 instead of AN7581, given the existing device tree bindings
mention en7581?

> diff --git a/MAINTAINERS b/MAINTAINERS
> index cb7836a27b177..bbfaca572118c 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -765,8 +765,8 @@ M:	Lorenzo Bianconi <lorenzo@kernel.org>
>  L:	linux-arm-kernel@lists.infradead.org (moderated for non-subscribers)
>  S:	Maintained
>  F:	Documentation/devicetree/bindings/phy/airoha,en7581-pcie-phy.yaml
> -F:	drivers/phy/phy-airoha-pcie-regs.h
> -F:	drivers/phy/phy-airoha-pcie.c
> +F:	drivers/phy/airoha/phy-an7581-pcie-regs.h
> +F:	drivers/phy/airoha/phy-an7581-pcie.c

[Severity: Low]
By explicitly listing only the source files, the newly added Kconfig and
Makefile in the drivers/phy/airoha/ directory are left without a maintainer
mapping. Would it be better to use a directory wildcard like
F: drivers/phy/airoha/ to include the new build files?

> diff --git a/drivers/phy/airoha/Kconfig b/drivers/phy/airoha/Kconfig
> new file mode 100644
> index 0000000000000..9a1b625a77012
> --- /dev/null
> +++ b/drivers/phy/airoha/Kconfig
> @@ -0,0 +1,13 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +#
> +# Phy drivers for Airoha devices
> +#
> +config PHY_AIROHA_AN7581_PCIE
> +	tristate "Airoha AN7581 PCIe-PHY Driver"

[Severity: Low]
Does this Kconfig symbol and prompt incorrectly use the AN7581 prefix
despite the SoC hardware and device tree binding being named EN7581?

> diff --git a/drivers/phy/phy-airoha-pcie-regs.h b/drivers/phy/airoha/phy-an7581-pcie-regs.h
> similarity index 99%
> rename from drivers/phy/phy-airoha-pcie-regs.h
> rename to drivers/phy/airoha/phy-an7581-pcie-regs.h
> index 58572c7937223..b938a7b468fee 100644
> --- a/drivers/phy/phy-airoha-pcie-regs.h
> +++ b/drivers/phy/airoha/phy-an7581-pcie-regs.h
> @@ -1,4 +1,4 @@
> -/* SPDX-License-Identifier: GPL-2.0-only */
> +// SPDX-License-Identifier: GPL-2.0-only

[Severity: Low]
Does this change to a C++ style line comment violate the kernel's SPDX
license identifier rules for C header files, which require the /* ... */
block comment style?

[Severity: Low]
Also, since this file was renamed, does the include guard further down in the
file (currently #ifndef _PHY_AIROHA_PCIE_H) need to be updated to match the
new filename?

> diff --git a/drivers/phy/phy-airoha-pcie.c b/drivers/phy/airoha/phy-an7581-pcie.c
> similarity index 99%
> rename from drivers/phy/phy-airoha-pcie.c
> rename to drivers/phy/airoha/phy-an7581-pcie.c
> index 56e9ade8a9fd3..81ddf0e7638b7 100644
> --- a/drivers/phy/phy-airoha-pcie.c
> +++ b/drivers/phy/airoha/phy-an7581-pcie.c

[ ... ]

> @@ -1279,12 +1279,12 @@ MODULE_DEVICE_TABLE(of, airoha_pcie_phy_of_match);
>  static struct platform_driver airoha_pcie_phy_driver = {
>  	.probe	= airoha_pcie_phy_probe,
>  	.driver	= {
> -		.name = "airoha-pcie-phy",
> +		.name = "airoha-an7581-pcie-phy",

[Severity: Low]
Does this platform driver name incorrectly use the misspelled an7581 prefix
instead of en7581?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260702093812.15918-1-ansuelsmth@gmail.com?part=4