Re: [RESEND PATCH v3 1/2] mfd: da9063: Fix revision handling to correctly select reg tables

2020-07-16 Thread Lee Jones
On Mon, 13 Jul 2020, Adam Thomson wrote:

> The current implementation performs checking in the i2c_probe()
> function of the variant_code but does this immediately after the
> containing struct has been initialised as all zero. This means the
> check for variant code will always default to using the BB tables
> and will never select AD. The variant code is subsequently set
> by device_init() and later used by the RTC so really it's a little
> fortunate this mismatch works.
> 
> This update adds raw I2C read access functionality to read the chip
> and variant/revision information (common to all revisions) so that
> it can subsequently correctly choose the proper regmap tables for
> real initialisation.
> 
> Signed-off-by: Adam Thomson 
> ---
> 
> v3:
>  - Replaced magic numbers around I2C paged access code with enums and defines
>  - Small style tidy ups as requested by Lee Jones
> 
> v2:
>  - Use raw I2C read access instead of a temporary regmap to interrogate chip 
> and
>variant id registers
> 
>  drivers/mfd/da9063-core.c|  31 --
>  drivers/mfd/da9063-i2c.c | 184 
> +++
>  include/linux/mfd/da9063/registers.h |  15 ++-
>  3 files changed, 177 insertions(+), 53 deletions(-)

Applied, thanks.

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog


[RESEND PATCH v3 1/2] mfd: da9063: Fix revision handling to correctly select reg tables

2020-07-13 Thread Adam Thomson
The current implementation performs checking in the i2c_probe()
function of the variant_code but does this immediately after the
containing struct has been initialised as all zero. This means the
check for variant code will always default to using the BB tables
and will never select AD. The variant code is subsequently set
by device_init() and later used by the RTC so really it's a little
fortunate this mismatch works.

This update adds raw I2C read access functionality to read the chip
and variant/revision information (common to all revisions) so that
it can subsequently correctly choose the proper regmap tables for
real initialisation.

Signed-off-by: Adam Thomson 
---

v3:
 - Replaced magic numbers around I2C paged access code with enums and defines
 - Small style tidy ups as requested by Lee Jones

v2:
 - Use raw I2C read access instead of a temporary regmap to interrogate chip and
   variant id registers

 drivers/mfd/da9063-core.c|  31 --
 drivers/mfd/da9063-i2c.c | 184 +++
 include/linux/mfd/da9063/registers.h |  15 ++-
 3 files changed, 177 insertions(+), 53 deletions(-)

diff --git a/drivers/mfd/da9063-core.c b/drivers/mfd/da9063-core.c
index b125f90d..a353d52 100644
--- a/drivers/mfd/da9063-core.c
+++ b/drivers/mfd/da9063-core.c
@@ -160,7 +160,6 @@ static int da9063_clear_fault_log(struct da9063 *da9063)
 
 int da9063_device_init(struct da9063 *da9063, unsigned int irq)
 {
-   int model, variant_id, variant_code;
int ret;
 
ret = da9063_clear_fault_log(da9063);
@@ -171,36 +170,6 @@ int da9063_device_init(struct da9063 *da9063, unsigned int 
irq)
da9063->irq_base = -1;
da9063->chip_irq = irq;
 
-   ret = regmap_read(da9063->regmap, DA9063_REG_CHIP_ID, );
-   if (ret < 0) {
-   dev_err(da9063->dev, "Cannot read chip model id.\n");
-   return -EIO;
-   }
-   if (model != PMIC_CHIP_ID_DA9063) {
-   dev_err(da9063->dev, "Invalid chip model id: 0x%02x\n", model);
-   return -ENODEV;
-   }
-
-   ret = regmap_read(da9063->regmap, DA9063_REG_CHIP_VARIANT, _id);
-   if (ret < 0) {
-   dev_err(da9063->dev, "Cannot read chip variant id.\n");
-   return -EIO;
-   }
-
-   variant_code = variant_id >> DA9063_CHIP_VARIANT_SHIFT;
-
-   dev_info(da9063->dev,
-"Device detected (chip-ID: 0x%02X, var-ID: 0x%02X)\n",
-model, variant_id);
-
-   if (variant_code < PMIC_DA9063_BB && variant_code != PMIC_DA9063_AD) {
-   dev_err(da9063->dev,
-   "Cannot support variant code: 0x%02X\n", variant_code);
-   return -ENODEV;
-   }
-
-   da9063->variant_code = variant_code;
-
ret = da9063_irq_init(da9063);
if (ret) {
dev_err(da9063->dev, "Cannot initialize interrupts.\n");
diff --git a/drivers/mfd/da9063-i2c.c b/drivers/mfd/da9063-i2c.c
index 455de74..4815489 100644
--- a/drivers/mfd/da9063-i2c.c
+++ b/drivers/mfd/da9063-i2c.c
@@ -22,12 +22,124 @@
 #include 
 #include 
 
+/*
+ * Raw I2C access required for just accessing chip and variant info before we
+ * know which device is present. The info read from the device using this
+ * approach is then used to select the correct regmap tables.
+ */
+
+#define DA9063_REG_PAGE_SIZE   0x100
+#define DA9063_REG_PAGED_ADDR_MASK 0xFF
+
+enum da9063_page_sel_buf_fmt {
+   DA9063_PAGE_SEL_BUF_PAGE_REG = 0,
+   DA9063_PAGE_SEL_BUF_PAGE_VAL,
+   DA9063_PAGE_SEL_BUF_SIZE,
+};
+
+enum da9063_paged_read_msgs {
+   DA9063_PAGED_READ_MSG_PAGE_SEL = 0,
+   DA9063_PAGED_READ_MSG_REG_SEL,
+   DA9063_PAGED_READ_MSG_DATA,
+   DA9063_PAGED_READ_MSG_CNT,
+};
+
+static int da9063_i2c_blockreg_read(struct i2c_client *client, u16 addr,
+   u8 *buf, int count)
+{
+   struct i2c_msg xfer[DA9063_PAGED_READ_MSG_CNT];
+   u8 page_sel_buf[DA9063_PAGE_SEL_BUF_SIZE];
+   u8 page_num, paged_addr;
+   int ret;
+
+   /* Determine page info based on register address */
+   page_num = (addr / DA9063_REG_PAGE_SIZE);
+   if (page_num > 1) {
+   dev_err(>dev, "Invalid register address provided\n");
+   return -EINVAL;
+   }
+
+   paged_addr = (addr % DA9063_REG_PAGE_SIZE) & DA9063_REG_PAGED_ADDR_MASK;
+   page_sel_buf[DA9063_PAGE_SEL_BUF_PAGE_REG] = DA9063_REG_PAGE_CON;
+   page_sel_buf[DA9063_PAGE_SEL_BUF_PAGE_VAL] =
+   (page_num << DA9063_I2C_PAGE_SEL_SHIFT) & DA9063_REG_PAGE_MASK;
+
+   /* Write reg address, page selection */
+   xfer[DA9063_PAGED_READ_MSG_PAGE_SEL].addr = client->addr;
+   xfer[DA9063_PAGED_READ_MSG_PAGE_SEL].flags = 0;
+   xfer[DA9063_PAGED_READ_MSG_PAGE_SEL].len = DA9063_PAGE_SEL_BUF_SIZE;
+   xfer[DA9063_PAGED_READ_MSG_PAGE_SEL].buf = page_sel_buf;
+
+   /* Select register address */
+