On Mon, Jan 14, 2013 at 12:03:04PM +0100, Marc Kleine-Budde wrote:

On 01/14/2013 11:53 AM, Felipe Balbi wrote:

quoted

Hi,

On Mon, Jan 14, 2013 at 11:50:41AM +0100, Marc Kleine-Budde wrote:

quoted

On 01/14/2013 11:39 AM, Felipe Balbi wrote:

quoted

On Mon, Jan 14, 2013 at 11:34:05AM +0100, Marc Kleine-Budde wrote:

quoted

On 01/14/2013 11:24 AM, Felipe Balbi wrote:

quoted

On Mon, Jan 14, 2013 at 11:18:53AM +0100, Marc Kleine-Budde wrote:

quoted

On 01/14/2013 11:16 AM, Felipe Balbi wrote:

quoted

Hi,

On Mon, Jan 14, 2013 at 06:12:39PM +0800, Peter Chen wrote:

quoted

@@ -2756,22 +2753,41 @@ static int fsl_udc_otg_resume(struct device *dev)
 
 	return fsl_udc_resume(NULL);
 }
-
 /*-------------------------------------------------------------------------
 	Register entry point for the peripheral controller driver
 --------------------------------------------------------------------------*/
-
+static const struct platform_device_id fsl_udc_devtype[] = {
+	{
+		.name = "imx-udc-mx25",
+		.driver_data = IMX25_UDC,
+	}, {
+		.name = "imx-udc-mx27",
+		.driver_data = IMX27_UDC,
+	}, {
+		.name = "imx-udc-mx31",
+		.driver_data = IMX31_UDC,
+	}, {
+		.name = "imx-udc-mx35",
+		.driver_data = IMX35_UDC,
+	}, {
+		.name = "imx-udc-mx51",
+		.driver_data = IMX51_UDC,
+	}
+};

I wonder if your driver-data is actually needed since you can use string
comparisson to achieve the exact same outcome.

Why use a string compare, if the kernel infrastructure already does this
for you?

what do you mean ? What kernel infrastructure is doing waht for me ?

The kernel infrastructure is doing the string compare for you to match
the device against the driver (via platform_device_id->name). You get
the a pointer to the driver_data for free. So you don't need any string
compare in the driver later.

but current driver data is just duplicating name with an integer, it's
pretty useless driver data.

I don't think so - another argument:
Less code. As struct platform_device_id is a static array the space is
allocated anyway. So it doesn't make any difference if driver_data is
NULL or not. Later you just need to make an integer comparison instead
of a call to a strcmp(), if you have a switch/case and IMX*_UDC is an
enum, the compiler will warn you if you've missed an IMX variant.

fair enough, but then don't create a different enum value for each imx
instance if they're mostly the same. Differentiate only what's actually
different.

Usually there isn't any Changelog between IP cores used in the different
fsl processors (at least available outside of fsl), that makes it quite
difficult to say if something found on one imx is really the same as on
the other one. And they (usually) don't provide any versioning
information in a register or the documentation.

just my 2¢

$SUBJECT is trying to differentiate a single feature (or maybe two) to
replace cpu_is_xxx(), then expose that on driver_data without creating
one enum value for each release from fsl.

-- 
balbi