On Mon, Feb 21, 2011 at 10:59 PM, Mike Rapoport <mike@xxxxxxxxxxxxxx> wrote: > Hi Olof, > Sorry for jumping late, haven't thought about the proposal below yesterday. > I'm not objecting to applying the patch as is, we can refactor the code > afterwards as well. Thanks for the input. And yeah, I'd say we can refactor later when we see how things will fit well with flat devicetrees. More below. >> static void __init tegra_harmony_init(void) >> { >> tegra_common_init(); >> @@ -85,6 +111,10 @@ static void __init tegra_harmony_init(void) >> >> harmony_pinmux_init(); >> >> + tegra_sdhci_device1.dev.platform_data = &sdhci_pdata1; >> + tegra_sdhci_device2.dev.platform_data = &sdhci_pdata2; >> + tegra_sdhci_device4.dev.platform_data = &sdhci_pdata4; >> + > > I think that instead of copying explicit initialization of the platform_data > member from board to board we can do something similar to what PXA does. The > board file would have to call something like > tegra_set_device_X_info(struct tegra_device_X_platform_data *data); > and then static registration of platform devices in the board files can be dropped. > > For instance, the devices.c would have > > void __init tegra_register_device(struct platform_device *dev, void *data) > { > int ret; > > dev->dev.platform_data = data; > > ret = platform_device_register(dev); > if (ret) > dev_err(&dev->dev, "unable to register device: %d\n", ret); > } > > void __init tegra_set_sdhci1_info(struct tegra_sdhci_platform_data *info) > { > tegra_register_device(&tegra_sdhci_device1, info); > } > > and the board files would just call tegra_set_sdhci1_info to pass the platform > data for the tegra_sdhci_device1 and register the device. > > This way we could reduce amount of copy/paste between the board files. Besides, > if/when tegra will be using device trees, handling of the device registration in > common place would make the transition easier. I don't see how that reduces the amount of copy and paste (significantly), since you still need the platform_data defined per board so you'll need to add the calls there. Maybe a generic "set_platform_data" hook instead of doing the pointer assignment open-coded would be an improvement though. I'm personally not a fan of the style where data is described as code, i.e. where there would be a tegra_set_<dev>_info function for every possible device out there. If anything that adds to the amount of copy-and-paste (per device), when it could just be handled with one common function being passed the appropriate data (i.e. like tegra_register_device). Anyway, I'm not looking to argue over it. :) I think we'll see when the FDT pieces start to take shape what kind of model will work well, and there's no reason to do premature refactoring. -Olof -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html