Re: [PATCH v2] cirrus-logic-framebuffer-i2c-support.patch

["Antonino A. Daplas" <adaplas@pol.net>]

Krzysztof Halasa wrote:
> "Antonino A. Daplas" <adaplas@pol.net> writes:
> 
>> Why don't you create a separate function for this, ie cirrusfb_create_i2c()
>> or something. This way, we eliminate the #ifdef/#endif inside the function.
> 
> #ifdef inside a function isn't a problem, while unnecessary complication
> (= worse readability) is.

There are better reasons (ie, smaller patch), but worse readability is not
one of them.

I could more easily grasp the code flow of cirrusfb_register() if you
just inserted "cirrusfb_create_i2c_buses()" instead of:

+		cinfo->bit_cirrus_data.setsda = alpine_setsda;
+		cinfo->bit_cirrus_data.setscl = alpine_setscl;
+		cinfo->bit_cirrus_data.getsda = alpine_getsda;
+		cinfo->bit_cirrus_data.getscl = alpine_getscl;
+		cinfo->bit_cirrus_data.udelay = 5;
+		cinfo->bit_cirrus_data.mdelay = 1;
+		cinfo->bit_cirrus_data.timeout = HZ;
+		cinfo->bit_cirrus_data.data = cinfo;
+		cinfo->cirrus_ops.owner = THIS_MODULE;
+		cinfo->cirrus_ops.id = I2C_HW_B_CIRRUS;
+		cinfo->cirrus_ops.class = I2C_CLASS_DDC;
+		cinfo->cirrus_ops.algo_data = &cinfo->bit_cirrus_data;
+		cinfo->cirrus_ops.dev.parent = info->device;
+		strlcpy(cinfo->cirrus_ops.name, "Cirrus Logic DDC I2C adapter",
+			I2C_NAME_SIZE);
+		if (!(err = i2c_bit_add_bus(&cinfo->cirrus_ops))) {
+			printk(KERN_DEBUG "Initialized Cirrus Logic I2C"
+			       " adapter\n");
 			cinfo->i2c_used = 1;
 		} else
-			printk(KERN_WARNING "Unable to initialize Alpine I2C"
-			       "adapter (result = %i)\n", err);
+			printk(KERN_WARNING "Unable to initialize Cirrus"
+			       " Logic I2C adapter (result = %i)\n", err);

Tony