On Tue, Jun 29, 2010 at 1:48 PM, Adrian Hunter <adrian.hunter@xxxxxxxxx> wrote: > kishore kadiyala wrote: >> >> On Tue, Jun 29, 2010 at 1:00 PM, Adrian Hunter <adrian.hunter@xxxxxxxxx> >> wrote: >>> >>> ext kishore kadiyala wrote: >>>> >>>> Adrian , >>>> >>>> Sorry for the late response >>>> >>>> <snip> >>>>> >>>>> As per my email 5/5/10, I would suggest the only change to omap_hsmmc >>>>> is: >>>> >>>> Agreed and followed the changes mostly but made some more changes on >>>> top >>>> of it. >>>> >>>> <snip> >>>>> >>>>> And that the late init function is used to do the rest e.g. >>>>> find a home for these 3 functions: >>>> >>>> I agree just having the 3 functions makes it work. >>>> >>>>> static int omap4_twl6030_hsmmc_late_init(struct device *dev) >>>>> { >>>>> int ret = 0; >>>>> struct platform_device *pdev = container_of(dev, >>>>> struct platform_device, dev); >>>>> struct omap_mmc_platform_data *pdata = dev->platform_data; >>>>> >>>>> /* MMC1 Card detect Configuration */ >>>>> if (pdev->id == 0) { >>>>> ret = omap4_hsmmc1_card_detect_config(); >>>>> if (ret < 0) >>>>> pr_err("Unable to configure Card detect for >>>>> MMC1\n"); >>>>> pdata->slots[0].card_detect = twl6030_mmc_card_detect; >>>>> pdata->slots[0].card_detect_irq = TWL6030_IRQ_BASE + >>>>> MMCDETECT_INTR_OFFSET; >>>>> } >>>>> return ret; >>>> >>>> <snip> >>>> >>>> Few Comments below: >>>> >>>> 1) In the above function, initializing "card_detect" in the driver as >>>> done in omap_hsmmc_gpio_init might be more readable and this has been >>>> done in nongpio_init instead. >>>> Even having initialization of "card_detect_irq" inside nongpio_init is >>>> fine. >>> >>> The problem is that referencing twl6030 from omap_hsmmc.c is not ok. >>> The driver must work with any platform and that is the reason that >>> platform data provides callbacks. >> >> ok, in that case how about having handler initialized in >> mach-omap2/hsmmc.c for both gpio and non-gpio case. > > Unless twl6030 is part of OMAP4 then it doesn't belong in hsmmc.c either ok, I'll make changes as suggested and post next version. Regards, Kishore > >> >> -Kishore >>>> >>>> 2)Also calling omap_hsmmc_gpio_init in case of a card detect line >>>> which is not GPIO >>>> doesn't make sense though it assigns -EINVAL to switch_pin in case of >>>> invalid GPIO >>>> which is intended for a non-removable card . >>>> >>>> 3) And also having some thing like GPIO and NON_GPIO flag to >>>> distinguish might make sense. >>>> >>>> Regards, >>>> Kishore >>>> >>> >> > > -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html