[PATCH v2 0/2] Clause-22/Clause-45 MDIO regmap support fixups

[PATCH v2 0/2] Clause-22/Clause-45 MDIO regmap support fixups

[Sander Vanheule]

A proposed patch to make C22 access more strict [1], was posted in reply to an
RFC series which also added C45 register access [2]. It appears that as a
result, the original RFC patches got merged instead.

Access functions are corrected to return -ENXIO, instead of silently ignoring
any invalid high bits in the register offset.
Additionally, a warning about a discarded const qualifier is fixed.

[1] Proposed C22 patch:
https://lore.kernel.org/lkml/20210605083116.12786-1-sander@svanheule.net/

[2] RFC series:
https://lore.kernel.org/lkml/cover.1622743333.git.sander@svanheule.net/

---
Changes since v1:
Link: https://lore.kernel.org/lkml/cover.1623238313.git.sander@svanheule.net/
- Keep C45 access code, but apply fix for discarded pointer constness
- Return -ENXIO for invalid C45 access too, as done for invalid C22 access

Sander Vanheule (2):
  regmap: mdio: Fix regmap_bus pointer constness
  regmap: mdio: Reject invalid addresses

 drivers/base/regmap/regmap-mdio.c | 22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

-- 
2.31.1

[PATCH v2 1/2] regmap: mdio: Fix regmap_bus pointer constness

[Sander Vanheule]

A const qualifier was omitted in the declaration of the regmap_bus
pointer, resulting in the following errors:

drivers/base/regmap/regmap-mdio.c: In function ‘__regmap_init_mdio’:
drivers/base/regmap/regmap-mdio.c:87:7: warning: assignment discards
‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
   87 |   bus = &regmap_mdio_c22_bus;
      |       ^
drivers/base/regmap/regmap-mdio.c:89:7: warning: assignment discards
‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
   89 |   bus = &regmap_mdio_c45_bus;
      |       ^

Fix this by ensuring the pointer has the same qualifiers as the assigned
values.

Fixes: f083be9db060 ("regmap: mdio: Add clause-45 support")
Signed-off-by: Sander Vanheule <sander@svanheule.net>
---
 drivers/base/regmap/regmap-mdio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/base/regmap/regmap-mdio.c b/drivers/base/regmap/regmap-mdio.c
index cfb23afb19eb..b772b42809e2 100644
--- a/drivers/base/regmap/regmap-mdio.c
+++ b/drivers/base/regmap/regmap-mdio.c
@@ -69,7 +69,7 @@ struct regmap *__regmap_init_mdio(struct mdio_device *mdio_dev,
 	const struct regmap_config *config, struct lock_class_key *lock_key,
 	const char *lock_name)
 {
-	struct regmap_bus *bus;
+	const struct regmap_bus *bus;
 
 	if (config->reg_bits == 5 && config->val_bits == 16)
 		bus = &regmap_mdio_c22_bus;
-- 
2.31.1

[PATCH v2 2/2] regmap: mdio: Reject invalid addresses

[Sander Vanheule]

When an invalid register offset is provided, the upper bits are silently
discarded. Change this to return -ENXIO instead, to help catch potential
bugs.

Signed-off-by: Sander Vanheule <sander@svanheule.net>
---
 drivers/base/regmap/regmap-mdio.c | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/drivers/base/regmap/regmap-mdio.c b/drivers/base/regmap/regmap-mdio.c
index b772b42809e2..6a20201299f5 100644
--- a/drivers/base/regmap/regmap-mdio.c
+++ b/drivers/base/regmap/regmap-mdio.c
@@ -31,14 +31,20 @@ static int regmap_mdio_c22_read(void *context, unsigned int reg, unsigned int *v
 {
 	struct mdio_device *mdio_dev = context;
 
-	return regmap_mdio_read(mdio_dev, reg & REGNUM_C22_MASK, val);
+	if (unlikely(reg & ~REGNUM_C22_MASK))
+		return -ENXIO;
+
+	return regmap_mdio_read(mdio_dev, reg, val);
 }
 
 static int regmap_mdio_c22_write(void *context, unsigned int reg, unsigned int val)
 {
 	struct mdio_device *mdio_dev = context;
 
-	return regmap_mdio_write(mdio_dev, reg & REGNUM_C22_MASK, val);
+	if (unlikely(reg & ~REGNUM_C22_MASK))
+		return -ENXIO;
+
+	return mdiobus_write(mdio_dev->bus, mdio_dev->addr, reg, val);
 }
 
 static const struct regmap_bus regmap_mdio_c22_bus = {
@@ -50,14 +56,20 @@ static int regmap_mdio_c45_read(void *context, unsigned int reg, unsigned int *v
 {
 	struct mdio_device *mdio_dev = context;
 
-	return regmap_mdio_read(mdio_dev, MII_ADDR_C45 | (reg & REGNUM_C45_MASK), val);
+	if (unlikely(reg & ~REGNUM_C45_MASK))
+		return -ENXIO;
+
+	return regmap_mdio_read(mdio_dev, MII_ADDR_C45 | reg, val);
 }
 
 static int regmap_mdio_c45_write(void *context, unsigned int reg, unsigned int val)
 {
 	struct mdio_device *mdio_dev = context;
 
-	return regmap_mdio_write(mdio_dev, MII_ADDR_C45 | (reg & REGNUM_C45_MASK), val);
+	if (unlikely(reg & ~REGNUM_C45_MASK))
+		return -ENXIO;
+
+	return regmap_mdio_write(mdio_dev, MII_ADDR_C45 | reg, val);
 }
 
 static const struct regmap_bus regmap_mdio_c45_bus = {
-- 
2.31.1

Re: [PATCH v2 0/2] Clause-22/Clause-45 MDIO regmap support fixups

[Mark Brown]

On Wed, 9 Jun 2021 15:10:02 +0200, Sander Vanheule wrote:
> A proposed patch to make C22 access more strict [1], was posted in reply to an
> RFC series which also added C45 register access [2]. It appears that as a
> result, the original RFC patches got merged instead.
> 
> Access functions are corrected to return -ENXIO, instead of silently ignoring
> any invalid high bits in the register offset.
> Additionally, a warning about a discarded const qualifier is fixed.
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regmap.git for-next

Thanks!

[1/2] regmap: mdio: Fix regmap_bus pointer constness
      commit: ce62df2290bb86b39f826a835d6060bc3b43dae7
[2/2] regmap: mdio: Reject invalid addresses
      commit: 0df0240946b1ffbe852fa302c04c0d322229c9ce

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark