Linux 3.4-rc2 register mapping and paging on I2C device.

[dd diasemi]

Hello Mark,

I need to implement a I2C MFD driver that requires register page
switching. It is not supported by regmap currently and requires
additional effort to make it clean. What is the best (most clean) way
of implementing a driver with register page switching?

I have come up with few alternatives:
1. Write the driver without regmap, with separate I2C routines.
2. After a call to devm_regmap_init_i2c(), exchange regmap.bus.read,
regmap.bus.write and regmap.bus.gather_write with specific handlers
(supporting register page switching). Those handlers would call
original I2C bus handlers indirectly, by passing altered registers.
3. Update regmap API to enable support for register page switching.

Let me know what you think,

many thanks,

Krystian
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majo...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

[Mark Brown]

On Mon, Apr 30, 2012 at 06:31:37PM +0100, dd diasemi wrote:

> 3. Update regmap API to enable support for register page switching.

Clearly this is the most sensible option. Open coding this in all the
drivers that need it is going to lead to a lot of duplicated effort.

[dd diasemi]

Below you can find my proposition of API for regmap paging support. It
is sufficient for my driver needs.
If you are OK with this proposition or you find any issues with it, let me know.

Kind Regards,
Krystian

---
diff --git a/drivers/base/regmap/Kconfig b/drivers/base/regmap/Kconfig
index 0f6c7fb..590d72c 100644
--- a/drivers/base/regmap/Kconfig
+++ b/drivers/base/regmap/Kconfig
@@ -16,3 +16,6 @@ config REGMAP_SPI

config REGMAP_IRQ
bool
+
+config REGMAP_PAGING
+ bool
diff --git a/include/linux/regmap.h b/include/linux/regmap.h
index a90abb6..81a11c1 100644
--- a/include/linux/regmap.h
+++ b/include/linux/regmap.h
@@ -50,6 +50,15 @@ struct reg_default {
* @pad_bits: Number of bits of padding between register and value.
* @val_bits: Number of bits in a register value, mandatory.
*
+ * @page_bits: Number of bits of register page number. These are the top-most
+ * bits exdending register address. These bits will be written
+ * to page register, for page switch.
+ * @page_reg: Page register used for page update (available for any page).
+ * @page_shift: Formatting bit shift for page number, made prior to write to
+ * page register.
+ * @nonpageable_reg: Optional callback returning true if the register
+ * needs NO page switching (eg. page is irrelevant).
+ *
* @writeable_reg: Optional callback returning true if the register
* can be written to.
* @readable_reg: Optional callback returning true if the register
@@ -81,6 +90,13 @@ struct regmap_config {
int pad_bits;
int val_bits;

+#ifdef CONFIG_REGMAP_PAGING
+ int page_bits;
+ int page_shift;
+ unsigned int page_reg;
+ bool (*nonpageable_reg)(struct device *dev, unsigned int reg);
+#endif
+
bool (*writeable_reg)(struct device *dev, unsigned int reg);
bool (*readable_reg)(struct device *dev, unsigned int reg);
bool (*volatile_reg)(struct device *dev, unsigned int reg);

[Mark Brown]

On Fri, May 04, 2012 at 11:35:58AM +0200, dd diasemi wrote:

> Below you can find my proposition of API for regmap paging support. It
> is sufficient for my driver needs.
> If you are OK with this proposition or you find any issues with it, let me know.

Please submit an actual patch, this needs to be supported by things like
the cache infrastructure as well. One thing that's not obvious with
just this is how things like register defaults and iterating all the
known registers will work.

> +config REGMAP_PAGING
> + bool

No, this needs to work with both paged and non-paged devices.

[dd diasemi]

Proposed implementation will extend virtually register range by number
of bits defined in .page_bits.
For example, for SPI register range 0 - 0x7F and 4 pages, we have
.page_bits = 2 and we define register mapping for range 0 - 0x1FF.
If .page_bits is not set and equal to 0, register range is going to be
defined as 0 - 0x7F and register mapping will work standard
(unchanged) way.

I plan to manage paging after caching and before actual address
formatting and bus write. The intention is to make register paging
transparent to regmap user and therefore it should cover situation,
when page boundaries are crossed by bulk read / write.
Proposed config flag - REGMAP_PAGING is here only to optimize kernel
compilation in case, when no drivers actually uses paging feature.

If you are asking about the details, I guess, comments may need to
improvement anyway to give a clearer view, how the API could be used,
right?

Kind Regards,
Krystian

[Mark Brown]

On Fri, May 04, 2012 at 01:40:18PM +0200, dd diasemi wrote:

> If you are asking about the details, I guess, comments may need to
> improvement anyway to give a clearer view, how the API could be used,
> right?

Well, it's more that I'm thinking about how this gets implemented - even
if things are nice and simple from a user point of view we have to make
sure that the implementation doesn't conflict with other features or
make the code too complex to think about.