Re: [PATCH net-next v25 04/10] mfd: an8855: Add support for Airoha AN8855 Switch

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] mfd: overlapping regmap readable ranges for General Purpose and GPU timers
- [Low] mfd: structure and variable naming violates MFD API guidelines
--

commit 05d4dc95b969f0fcf66d11398aafad3e7eb7c7f1
Author: Christian Marangi <ansuelsmth@gmail.com>

mfd: an8855: Add support for Airoha AN8855 Switch

This commit adds support for the Airoha AN8855 Switch which provides
a DSA switch and a NVMEM provider. It utilizes the mdio-regmap driver
and registers a regmap for each internal PHY.

> diff --git a/drivers/mfd/airoha-an8855.c b/drivers/mfd/airoha-an8855.c
> new file mode 100644
> index 0000000000000..a2778511dfefc
> --- /dev/null
> +++ b/drivers/mfd/airoha-an8855.c
[ ... ]
> +static const struct regmap_range an8855_readable_ranges[] = {
[ ... ]
> +	regmap_reg_range(0x10009000, 0x10009fff), /* PDMA */
> +	regmap_reg_range(0x1000a100, 0x1000a2ff), /* General Purpose Timer */
> +	regmap_reg_range(0x1000a200, 0x1000a2ff), /* GPU timer */

[Severity: Medium]
Is this range definition correct? The General Purpose Timer range
(0x1000a100 to 0x1000a2ff) fully overlaps with the GPU timer range
(0x1000a200 to 0x1000a2ff).

Could this be a typo in the hardware memory map definition? It seems like
the General Purpose Timer range might have been intended to end at
0x1000a1ff to avoid overlapping and potential register access boundary
issues.

[ ... ]
> +static int an8855_core_probe(struct mdio_device *mdiodev)
> +{
> +	struct an8855_phy_bus phy_bus_infos[AN8855_MAX_PHY_PORT] = { };
> +	struct device *dev = &mdiodev->dev;
> +	struct an8855_core *priv;
> +	struct regmap *regmap;
> +	int num_phys;
> +	int i, ret;
> +
> +	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);

[Severity: Low]
This isn't a bug, but does this variable and structure naming align with MFD
subsystem conventions?

The MFD guidelines suggest avoiding generic variable names like priv, and
instead recommend using ddata for the instance variable. Also, should the
struct name itself avoid hardcoding implementation details like core?
Naming it something closer to struct an8855 might better adhere to the
subsystem guidelines.

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