Re: [PATCH v5 2/2] OMAP4 HSMMC: Adding card detect support for MMC1 Controller

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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


[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux