mfd: update i2c driver for max8925

static inline int max8925_read_device(struct i2c_client *i2c,

(quoted diff) +0 -15

 				      int reg, int bytes, void *dest)
 {
-	unsigned char data;
-	unsigned char *buf;
 	int ret;
 
-	buf = kzalloc(bytes + 1, GFP_KERNEL);
-	if (!buf)
-		return -ENOMEM;
-
-	data = (unsigned char)reg;
-	ret = i2c_master_send(i2c, &data, 1);
-	if (ret < 0)
-		return ret;
-
-	ret = i2c_master_recv(i2c, buf, bytes + 1);
-	if (ret < 0)
-		return ret;
-	memcpy(dest, buf, bytes);

(quoted diff) +0 -0

@@ -142,28 +138,56 @@ static int __devinit max8925_probe(struct
 i2c_client *client,
 				   const struct i2c_device_id *id)
 {
 	struct max8925_platform_data *pdata = client->dev.platform_data;

(quoted diff) +23 -0

+	if (!init_gpm) {
+		init_gpm = 1;
+		chip = kzalloc(sizeof(struct max8925_chip), GFP_KERNEL);
+		if (chip == NULL)
+			return -ENOMEM;
+		chip->i2c = client;
+		chip->dev = &client->dev;
+		i2c_set_clientdata(client, chip);
+		dev_set_drvdata(chip->dev, chip);
+		mutex_init(&chip->io_lock);
+	}
+	if (!init_rtc) {
+		init_rtc = 1;
+		memset(&rtc_info, 0, sizeof(struct i2c_board_info));
+		strlcpy(rtc_info.type, "max8925", I2C_NAME_SIZE);
+		rtc_info.platform_data = chip->i2c->dev.platform_data;
+		chip->rtc = i2c_new_probed_device(chip->i2c->adapter,
+						&rtc_info, addr_rtc);
+		i2c_set_clientdata(chip->rtc, chip);
+	} else if (!init_adc) {
+		init_adc = 1;
+		memset(&adc_info, 0, sizeof(struct i2c_board_info));
+		strlcpy(adc_info.type, "max8925", I2C_NAME_SIZE);

(quoted diff) +16 -3

 	return 0;
 }

@@ -171,10 +195,23 @@ static int __devinit max8925_probe(struct
 i2c_client *client,
 static int __devexit max8925_remove(struct i2c_client *client)
 {
 	struct max8925_chip *chip = i2c_get_clientdata(client);
+	static int init_gpm = 1, init_adc = 1, init_rtc = 1;
 
-	max8925_device_exit(chip);
-	i2c_set_clientdata(client, NULL);
-	kfree(chip);
+	if (client->addr == ADC_I2C_ADDR)
+		init_adc = 0;
+	else if (client->addr == RTC_I2C_ADDR)
+		init_rtc = 0;
+	else
+		init_gpm = 0;
+	if (!init_gpm && !init_rtc && !init_adc) {
+		max8925_device_exit(chip);
+		i2c_unregister_device(chip->adc);
+		i2c_unregister_device(chip->rtc);
+		i2c_set_clientdata(chip->adc, NULL);
+		i2c_set_clientdata(chip->rtc, NULL);
+		i2c_set_clientdata(chip->i2c, NULL);
+		kfree(chip);
+	}
 	return 0;
 }

Signature · 2 lines

-- 
1.5.6.5

Hi Haojian,

This has to be rewritten, but first let me try to understand something: You need the max9825 core to have pointers to the other i2c devices because of how it handles IRQs right ? In your max9825 irq handler you need to be able to access the rtc and the adc devices, right ? And then on yourrtc and power drivers, you actually only access the max8925 core i2c registers ? I would appreciate if most of your answers to those questions could morph into code comments to this probe routine.

Then, about the code design: Having your probe routine relying on those static variable is not ok. If I undersand you correctly, what you trying to achieve here is not calling max8925_device_init() before you actually have the core, the i2c and the adc pointer setup ?
If that's so, why not simply use i2c_new_dummy() instead of having to handle recursive calls of your probe routine ?

On Fri, Jan 29, 2010 at 2:54 PM, Samuel Ortiz <[email protected]> wrote:
I wants to initialize max8925 after all three components probed. In core driver, irq handling needs to access all three components. In power supply driver, both adc and generic components are accessed. It's the reason that using three i2c clients in one driver.

Yes, the code is not very clear. Now I use i2c_new_dummy() now. Now I attache new patches now.

Updates are in below.
1. use i2c_new_dummy() to replace i2c_new_probed_device().
2. add onkey driver
3. remove unused i2c pointer in rtc driver

Quoted 7 lines

Yes, the code is not very clear. Now I use i2c_new_dummy() now. Now I attache new patches now.

Updates are in below.
1. use i2c_new_dummy() to replace i2c_new_probed_device().
2. add onkey driver
3. remove unused i2c pointer in rtc driver

Very good, I just have one last comment:

Review static int __devinit max8925_probe(struct i2c_client *client, · line 141

Quoted 6 lines

(quoted diff) +1 -1

@@ -142,27 +138,28 @@ static int __devinit max8925_probe(struct i2c_client *client,
 ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?const struct i2c_device_id *id)
 ?{
 ? ? ? struct max8925_platform_data *pdata = client->dev.platform_data;
- ? ? struct max8925_chip *chip;
+ ? ? static struct max8925_chip *chip;

That no longer needs to be static.
The rest of the code looks fine to me. Once you remove that static definition, and you add some comments explaining why you need those 3 i2c pointers, I'll merge it.

Yes, the code is not very clear. Now I use i2c_new_dummy() now. Now I attache new patches now.

Updates are in below.
1. use i2c_new_dummy() to replace i2c_new_probed_device().
2. add onkey driver
3. remove unused i2c pointer in rtc driver

Thanks
Haojian