diff mbox series

[RFC,1/2] regmap: add miim bus support

Message ID 489e8a2d22dc8a5aaa3600289669c3bf0a15ba19.1617914861.git.sander@svanheule.net (mailing list archive)
State RFC
Headers show
Series MIIM regmap and RTL8231 GPIO expander support | expand

Checks

Context Check Description
netdev/tree_selection success Not a local patch

Commit Message

Sander Vanheule April 8, 2021, 8:52 p.m. UTC
Basic support for MIIM bus access. Support only includes clause-22
register access, with 5-bit addresses, and 16-bit wide registers.

Signed-off-by: Sander Vanheule <sander@svanheule.net>
---
 drivers/base/regmap/Kconfig       |  6 +++-
 drivers/base/regmap/Makefile      |  1 +
 drivers/base/regmap/regmap-miim.c | 58 +++++++++++++++++++++++++++++++
 include/linux/regmap.h            | 36 +++++++++++++++++++
 4 files changed, 100 insertions(+), 1 deletion(-)
 create mode 100644 drivers/base/regmap/regmap-miim.c

Comments

Mark Brown April 9, 2021, 4:07 p.m. UTC | #1
On Thu, Apr 08, 2021 at 10:52:34PM +0200, Sander Vanheule wrote:
> Basic support for MIIM bus access. Support only includes clause-22
> register access, with 5-bit addresses, and 16-bit wide registers.

What is "MIIM"?  A quick search isn't showing up useful hits for that.
Why not just call this MDIO like the rest of the kernel is doing, it
seems like using something else is at best going to make it harder to
discover this code?  If MIIM is some subset or something it's not
obvious how we're limited to that.
Sander Vanheule April 9, 2021, 6:14 p.m. UTC | #2
Hi Mark,

On Fri, 2021-04-09 at 17:07 +0100, Mark Brown wrote:
> On Thu, Apr 08, 2021 at 10:52:34PM +0200, Sander Vanheule wrote:
> > Basic support for MIIM bus access. Support only includes clause-22
> > register access, with 5-bit addresses, and 16-bit wide registers.
> 
> What is "MIIM"?  A quick search isn't showing up useful hits for that.
> Why not just call this MDIO like the rest of the kernel is doing, it
> seems like using something else is at best going to make it harder to
> discover this code?  If MIIM is some subset or something it's not
> obvious how we're limited to that.

MIIM stands for "MII management", i.e. the management bus for devices
with some form of MII interface. MDIO is also frequently used to refer
to the data pin of the bus (there's also MDC: the clock pin), so I
wanted to make the distinction.

The kernel has the mii_bus struct to describe the bus master, but like
you noted the bus is generaly refered to as an MDIO interface. I'm fine
with naming it MDIO to make it easier to spot.

Best,
Sander
Mark Brown April 9, 2021, 6:16 p.m. UTC | #3
On Fri, Apr 09, 2021 at 08:14:22PM +0200, Sander Vanheule wrote:

> The kernel has the mii_bus struct to describe the bus master, but like
> you noted the bus is generaly refered to as an MDIO interface. I'm fine
> with naming it MDIO to make it easier to spot.

Either mii_bus or mdio seem like an improvement - something matching
existing kernel terminology, I guess mdio is consistent with the API it
works with so...
Andrew Lunn April 9, 2021, 7:44 p.m. UTC | #4
On Fri, Apr 09, 2021 at 07:16:42PM +0100, Mark Brown wrote:
> On Fri, Apr 09, 2021 at 08:14:22PM +0200, Sander Vanheule wrote:
> 
> > The kernel has the mii_bus struct to describe the bus master, but like
> > you noted the bus is generaly refered to as an MDIO interface. I'm fine
> > with naming it MDIO to make it easier to spot.
> 
> Either mii_bus or mdio seem like an improvement - something matching
> existing kernel terminology, I guess mdio is consistent with the API it
> works with so...

I would also suggest mdio. The mii_bus code is an old framework which
does not see any work done to it.

     Andrew
diff mbox series

Patch

diff --git a/drivers/base/regmap/Kconfig b/drivers/base/regmap/Kconfig
index 50b1e2d06a25..5bcd9789284e 100644
--- a/drivers/base/regmap/Kconfig
+++ b/drivers/base/regmap/Kconfig
@@ -4,7 +4,7 @@ 
 # subsystems should select the appropriate symbols.
 
 config REGMAP
-	default y if (REGMAP_I2C || REGMAP_SPI || REGMAP_SPMI || REGMAP_W1 || REGMAP_AC97 || REGMAP_MMIO || REGMAP_IRQ || REGMAP_SOUNDWIRE || REGMAP_SOUNDWIRE_MBQ || REGMAP_SCCB || REGMAP_I3C || REGMAP_SPI_AVMM)
+	default y if (REGMAP_I2C || REGMAP_SPI || REGMAP_SPMI || REGMAP_W1 || REGMAP_AC97 || REGMAP_MMIO || REGMAP_IRQ || REGMAP_SOUNDWIRE || REGMAP_SOUNDWIRE_MBQ || REGMAP_SCCB || REGMAP_I3C || REGMAP_SPI_AVMM || REGMAP_MIIM)
 	select IRQ_DOMAIN if REGMAP_IRQ
 	bool
 
@@ -36,6 +36,10 @@  config REGMAP_W1
 	tristate
 	depends on W1
 
+config REGMAP_MIIM
+	tristate
+	depends on MDIO_DEVICE
+
 config REGMAP_MMIO
 	tristate
 
diff --git a/drivers/base/regmap/Makefile b/drivers/base/regmap/Makefile
index 33f63adb5b3d..d80920bd42ce 100644
--- a/drivers/base/regmap/Makefile
+++ b/drivers/base/regmap/Makefile
@@ -19,3 +19,4 @@  obj-$(CONFIG_REGMAP_SOUNDWIRE_MBQ) += regmap-sdw-mbq.o
 obj-$(CONFIG_REGMAP_SCCB) += regmap-sccb.o
 obj-$(CONFIG_REGMAP_I3C) += regmap-i3c.o
 obj-$(CONFIG_REGMAP_SPI_AVMM) += regmap-spi-avmm.o
+obj-$(CONFIG_REGMAP_MIIM) += regmap-miim.o
diff --git a/drivers/base/regmap/regmap-miim.c b/drivers/base/regmap/regmap-miim.c
new file mode 100644
index 000000000000..a560d2910417
--- /dev/null
+++ b/drivers/base/regmap/regmap-miim.c
@@ -0,0 +1,58 @@ 
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/errno.h>
+#include <linux/mdio.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+
+static int regmap_miim_read(void *context, unsigned int reg, unsigned int *val)
+{
+	struct mdio_device *mdio_dev = context;
+	int ret;
+
+	ret = mdiobus_read(mdio_dev->bus, mdio_dev->addr, reg);
+	*val = ret & 0xffff;
+
+	return ret < 0 ? ret : 0;
+}
+
+static int regmap_miim_write(void *context, unsigned int reg, unsigned int val)
+{
+	struct mdio_device *mdio_dev = context;
+
+	return mdiobus_write(mdio_dev->bus, mdio_dev->addr, reg, val);
+}
+
+static const struct regmap_bus regmap_miim_bus = {
+	.reg_write = regmap_miim_write,
+	.reg_read = regmap_miim_read,
+};
+
+struct regmap *__regmap_init_miim(struct mdio_device *mdio_dev,
+	const struct regmap_config *config, struct lock_class_key *lock_key,
+	const char *lock_name)
+{
+	if (config->reg_bits != 5 || config->val_bits != 16)
+		return ERR_PTR(-ENOTSUPP);
+
+	return __regmap_init(&mdio_dev->dev, &regmap_miim_bus, mdio_dev, config,
+		lock_key, lock_name);
+}
+EXPORT_SYMBOL_GPL(__regmap_init_miim);
+
+struct regmap *__devm_regmap_init_miim(struct mdio_device *mdio_dev,
+	const struct regmap_config *config, struct lock_class_key *lock_key,
+	const char *lock_name)
+{
+	if (config->reg_bits != 5 || config->val_bits != 16)
+		return ERR_PTR(-ENOTSUPP);
+
+	return __devm_regmap_init(&mdio_dev->dev, &regmap_miim_bus, mdio_dev,
+		config, lock_key, lock_name);
+}
+EXPORT_SYMBOL_GPL(__devm_regmap_init_miim);
+
+MODULE_AUTHOR("Sander Vanheule <sander@svanheule.net>");
+MODULE_DESCRIPTION("Regmap MIIM Module");
+MODULE_LICENSE("GPL v2");
+
diff --git a/include/linux/regmap.h b/include/linux/regmap.h
index 2cc4ecd36298..f25630f793c3 100644
--- a/include/linux/regmap.h
+++ b/include/linux/regmap.h
@@ -27,6 +27,7 @@  struct device_node;
 struct i2c_client;
 struct i3c_device;
 struct irq_domain;
+struct mdio_device;
 struct slim_device;
 struct spi_device;
 struct spmi_device;
@@ -538,6 +539,10 @@  struct regmap *__regmap_init_i2c(struct i2c_client *i2c,
 				 const struct regmap_config *config,
 				 struct lock_class_key *lock_key,
 				 const char *lock_name);
+struct regmap *__regmap_init_miim(struct mdio_device *mdio_dev,
+				 const struct regmap_config *config,
+				 struct lock_class_key *lock_key,
+				 const char *lock_name);
 struct regmap *__regmap_init_sccb(struct i2c_client *i2c,
 				  const struct regmap_config *config,
 				  struct lock_class_key *lock_key,
@@ -594,6 +599,10 @@  struct regmap *__devm_regmap_init_i2c(struct i2c_client *i2c,
 				      const struct regmap_config *config,
 				      struct lock_class_key *lock_key,
 				      const char *lock_name);
+struct regmap *__devm_regmap_init_miim(struct mdio_device *mdio_dev,
+				      const struct regmap_config *config,
+				      struct lock_class_key *lock_key,
+				      const char *lock_name);
 struct regmap *__devm_regmap_init_sccb(struct i2c_client *i2c,
 				       const struct regmap_config *config,
 				       struct lock_class_key *lock_key,
@@ -697,6 +706,19 @@  int regmap_attach_dev(struct device *dev, struct regmap *map,
 	__regmap_lockdep_wrapper(__regmap_init_i2c, #config,		\
 				i2c, config)
 
+/**
+ * regmap_init_miim() - Initialise register map
+ *
+ * @mdio_dev: Device that will be interacted with
+ * @config: Configuration for register map
+ *
+ * The return value will be an ERR_PTR() on error or a valid pointer to
+ * a struct regmap.
+ */
+#define regmap_init_miim(mdio_dev, config)				\
+	__regmap_lockdep_wrapper(__regmap_init_miim, #config,		\
+				mdio_dev, config)
+
 /**
  * regmap_init_sccb() - Initialise register map
  *
@@ -888,6 +910,20 @@  bool regmap_ac97_default_volatile(struct device *dev, unsigned int reg);
 	__regmap_lockdep_wrapper(__devm_regmap_init_i2c, #config,	\
 				i2c, config)
 
+/**
+ * devm_regmap_init_miim() - Initialise managed register map
+ *
+ * @mdio_dev: Device that will be interacted with
+ * @config: Configuration for register map
+ *
+ * The return value will be an ERR_PTR() on error or a valid pointer
+ * to a struct regmap.  The regmap will be automatically freed by the
+ * device management code.
+ */
+#define devm_regmap_init_miim(mdio_dev, config)				\
+	__regmap_lockdep_wrapper(__devm_regmap_init_miim, #config,	\
+				mdio_dev, config)
+
 /**
  * devm_regmap_init_sccb() - Initialise managed register map
  *