On 02 November 2016 14:29, Lee Jones wrote:

On Mon, 31 Oct 2016, Steve Twiss wrote:

quoted

From: Steve Twiss <redacted>

@@ -475,7 +855,25 @@ static int da9062_i2c_probe(struct i2c_client *i2c,
 		return -EINVAL;
 	}

-	chip->regmap = devm_regmap_init_i2c(i2c, &da9062_regmap_config);
+	switch (chip->chip_type) {
+	case(COMPAT_TYPE_DA9061):
+		cell = da9061_devs;
+		cell_num = ARRAY_SIZE(da9061_devs);
+		irq_chip = &da9061_irq_chip;
+		config = &da9061_regmap_config;
+		break;
+	case(COMPAT_TYPE_DA9062):
+		cell = da9062_devs;
+		cell_num = ARRAY_SIZE(da9062_devs);
+		irq_chip = &da9062_irq_chip;
+		config = &da9062_regmap_config;
+		break;
+	default:
+		dev_err(chip->dev, "Unrecognised chip type\n");
+		return -ENODEV;
+	}

I very much dislike when MFD and OF functionality is mixed.

In your case you can use da9062_get_device_type() to dynamically
interrogate the device and register using the correct MFD cells that
way.

Hi Lee,

It's the device tree that decides what the chip type is. It's not chip
interrogation in this case. The ordering dictates this I think: to access the
hardware ID register, a regmap definition is needed first. But because the
correct I2C register map requires a knowledge of what chip is being used,
it becomes a circular dependency.

To solve this dependency, I define the chip type (DA9061 or DA9062) in the
device tree and assign the correct regmap first before accessing any registers.

quoted

+	chip->regmap = devm_regmap_init_i2c(i2c, config);
 	if (IS_ERR(chip->regmap)) {
 		ret = PTR_ERR(chip->regmap);
 		dev_err(chip->dev, "Failed to allocate register map: %d\n",

@@ -493,7 +891,7 @@ static int da9062_i2c_probe(struct i2c_client *i2c,

 	ret = regmap_add_irq_chip(chip->regmap, i2c->irq,
 			IRQF_TRIGGER_LOW | IRQF_ONESHOT |IRQF_SHARED,
-			-1, &da9062_irq_chip,
+			-1, irq_chip,

What is -1?

.. it's a request for an irq_base.
http://lxr.free-electrons.com/source/kernel/irq/irqdesc.c#L477

Is there a reason I shouldn't be doing that?
There doesn't seem to be a #define anywhere, and using -1 seems
to be the standard in the kernel at the moment.

Regards,
Steve