Felipe Balbi <[email protected]> writes: > Hi, > > On Wed, Sep 22, 2010 at 07:29:10PM -0500, Kalliguddi, Hema wrote: >>+#define MAX_OMAP_MUSB_HWMOD_NAME_LEN 16 > > this isn't used anywhere. > >>@@ -75,31 +62,30 @@ static struct musb_hdrc_platform_data mu >> >> static u64 musb_dmamask = DMA_BIT_MASK(32); >> >>-static struct platform_device musb_device = { >>- .name = "musb_hdrc", >>- .id = -1, >>- .dev = { >>- .dma_mask = &musb_dmamask, >>- .coherent_dma_mask = DMA_BIT_MASK(32), >>- .platform_data = &musb_plat, >>+static struct omap_device_pm_latency omap_musb_latency[] = { >>+ { >>+ .deactivate_func = omap_device_idle_hwmods, >>+ .activate_func = omap_device_enable_hwmods, >>+ .flags = OMAP_DEVICE_LATENCY_AUTO_ADJUST, >> }, >>- .num_resources = ARRAY_SIZE(musb_resources), >>- .resource = musb_resources, >> }; >> >> void __init usb_musb_init(struct omap_musb_board_data *board_data) >> { >>- if (cpu_is_omap243x()) { >>- musb_resources[0].start = OMAP243X_HS_BASE; >>- } else if (cpu_is_omap34xx()) { >>- musb_resources[0].start = OMAP34XX_HSUSB_OTG_BASE; >>- } else if (cpu_is_omap44xx()) { >>- musb_resources[0].start = OMAP44XX_HSUSB_OTG_BASE; >>- musb_resources[1].start = OMAP44XX_IRQ_HS_USB_MC_N; >>- musb_resources[2].start = OMAP44XX_IRQ_HS_USB_DMA_N; >>+ struct omap_hwmod *oh; >>+ struct omap_device *od; >>+ struct platform_device *pdev; >>+ struct device *dev; >>+ int bus_id = -1; >>+ const char *oh_name = "usb_otg_hs"; >>+ struct musb_hdrc_platform_data *pdata; >>+ >>+ oh = omap_hwmod_lookup(oh_name); >>+ >>+ if (!oh) { >>+ pr_err("Could not look up %s\n", oh_name); >>+ return; >> } > > Paul, Kevin, to me it looks like a duplication that all devices will > have to: > > oh = omap_hwmod_lookup("my_hwmod_name"); > omap_device_build("my_device_name", bus_id, oh, pdata, sizeof(*pdata)); > > could the omap_hwmod_lookup() part be moved to omap_device_build ? Or > maybe create a omap_hwmod_lookup_and_build(oh_name, dev_name, bus_id, > pdata, sizeof(*pdata)) ??
I don't think that this is too much extra work. Also, many drivers are not doing a single hwmod lookup, they are using an iterator to iterate over a bunch of hwmods in a given class (c.f. omap_hwmod_for_each_by_class) So, IMO, keeping the lookup and build separate is better. >>@@ -110,8 +96,23 @@ void __init usb_musb_init(struct omap_mu >> musb_plat.mode = board_data->mode; >> musb_plat.extvbus = board_data->extvbus; >> >>- if (platform_device_register(&musb_device) < 0) >>- printk(KERN_ERR "Unable to register HS-USB (MUSB) device\n"); >>+ pdata = &musb_plat; >>+ >>+ od = omap_device_build(name, bus_id, oh, pdata, >>+ sizeof(struct musb_hdrc_platform_data), > > use sizeof(*pdata), if we change the name of that structure (very > unlikely, but still) it'll avoid unwanted compile breakage. > >>+ pdev = &od->pdev; >>+ dev = &pdev->dev; >>+ get_device(dev); >>+ dev->dma_mask = &musb_dmamask; >>+ dev->coherent_dma_mask = musb_dmamask; >>+ put_device(dev); > > I think this is also a duplication, it's gonna on all hwmod device > registration, no ? Only for devices setting DMA masks. This is no more duplication than we have today for all platform_devices. Kevin -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
