Messages in this thread | | | Date | Mon, 10 Jul 2006 18:07:36 +0800 | From | "Antonino A. Daplas" <> | Subject | Re: [PATCH v2] cirrus-logic-framebuffer-i2c-support.patch |
| |
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 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |