Re: [PATCH v3 2/3] mmc: Add realtek USB sdmmc host driver

2014-02-11 Thread Roger 

On 02/11/2014 05:50 PM, Ulf Hansson wrote:

On 11 February 2014 10:27, Roger  wrote:

On 02/10/2014 10:58 PM, Ulf Hansson wrote:


On 6 February 2014 15:35,   wrote:


From: Roger Tseng 

Realtek USB SD/MMC host driver provides mmc host support based on the
Realtek
USB card reader MFD driver.

Signed-off-by: Roger Tseng 
---
   drivers/mmc/host/Kconfig  |7 +
   drivers/mmc/host/Makefile |1 +
   drivers/mmc/host/rtsx_usb_sdmmc.c | 1500
+
   3 files changed, 1508 insertions(+)
   create mode 100644 drivers/mmc/host/rtsx_usb_sdmmc.c


[snip]


+#ifdef CONFIG_PM_RUNTIME



There are stubs for pm_runtime* functions, thus the ifdefs can be removed.
Please go though the complete patch and remove all instances.


+   pm_runtime_put(sdmmc_dev(host));



I don't know so much about USB mmc hosts hardware, but I just wanted
to find out if I have understood this correct.

You can't do fine grained power management of the USB parent device,
since it needs to be runtime resumed to be able keep the power the
card? Once it becomes runtime suspended, the power to the card will
thus also be dropped?


Yes, and to keep some internal state of the controller.


Okay.

But the internal state of the controller should be possible to restore
at runtime_resume, so that should not be the reason, right?



I checked again. Internal registers will hold its state during suspend.
Card power is the exact reason.

Kind regards
Ulf Hansson

.


___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v3 2/3] mmc: Add realtek USB sdmmc host driver

2014-02-11 Thread Ulf Hansson 
On 11 February 2014 10:27, Roger  wrote:
> On 02/10/2014 10:58 PM, Ulf Hansson wrote:
>>
>> On 6 February 2014 15:35,   wrote:
>>>
>>> From: Roger Tseng 
>>>
>>> Realtek USB SD/MMC host driver provides mmc host support based on the
>>> Realtek
>>> USB card reader MFD driver.
>>>
>>> Signed-off-by: Roger Tseng 
>>> ---
>>>   drivers/mmc/host/Kconfig  |7 +
>>>   drivers/mmc/host/Makefile |1 +
>>>   drivers/mmc/host/rtsx_usb_sdmmc.c | 1500
>>> +
>>>   3 files changed, 1508 insertions(+)
>>>   create mode 100644 drivers/mmc/host/rtsx_usb_sdmmc.c
>
> [snip]
>
>>> +#ifdef CONFIG_PM_RUNTIME
>>
>>
>> There are stubs for pm_runtime* functions, thus the ifdefs can be removed.
>> Please go though the complete patch and remove all instances.
>>
>>> +   pm_runtime_put(sdmmc_dev(host));
>>
>>
>> I don't know so much about USB mmc hosts hardware, but I just wanted
>> to find out if I have understood this correct.
>>
>> You can't do fine grained power management of the USB parent device,
>> since it needs to be runtime resumed to be able keep the power the
>> card? Once it becomes runtime suspended, the power to the card will
>> thus also be dropped?
>>
> Yes, and to keep some internal state of the controller.

Okay.

But the internal state of the controller should be possible to restore
at runtime_resume, so that should not be the reason, right?

>
> [snip]
>
>>> +#ifdef CONFIG_PM
>>
>>
>> I suppose this should be CONFIG_PM_SLEEP?
>>
> ...
>
>>> +   err = mmc_suspend_host(mmc);
>>
>>
>> This won't compile. The mmc_suspend_host API has been removed.
>>
> ...
>
>>> +   return mmc_resume_host(mmc);
>>
>>
>> This won't compile. The mmc_resume_host API has been removed.
>>
> ...
>>>
>>> +static struct platform_driver rtsx_usb_sdmmc_driver = {
>>> +   .probe  = rtsx_usb_sdmmc_drv_probe,
>>> +   .remove = rtsx_usb_sdmmc_drv_remove,
>>> +   .id_table   = rtsx_usb_sdmmc_ids,
>>> +   .suspend= rtsx_usb_sdmmc_suspend,
>>> +   .resume = rtsx_usb_sdmmc_resume,
>>
>>
>> Please use the modern pm_ops instead of the legacy suspend/resume
>> callbacks.
>> I suggest you then also switch to use the SIMPLE_DEV_PM_OPS macro.
>
>
> I just missed the removal of mmc_suspend|resume_host.
>
> I'll remove all these unnecessary mmc host pm things as done in commit
> ff71c4bcb0af2730d047989e485303ae4e1ce794 for drivers of our PCIe devices.
>
> Thanks for pointing this out.
>
>> Kind regards
>> Ulf Hansson
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v3 2/3] mmc: Add realtek USB sdmmc host driver

2014-02-11 Thread Roger 

On 02/10/2014 10:58 PM, Ulf Hansson wrote:

On 6 February 2014 15:35,   wrote:

From: Roger Tseng 

Realtek USB SD/MMC host driver provides mmc host support based on the Realtek
USB card reader MFD driver.

Signed-off-by: Roger Tseng 
---
  drivers/mmc/host/Kconfig  |7 +
  drivers/mmc/host/Makefile |1 +
  drivers/mmc/host/rtsx_usb_sdmmc.c | 1500 +
  3 files changed, 1508 insertions(+)
  create mode 100644 drivers/mmc/host/rtsx_usb_sdmmc.c

[snip]

+#ifdef CONFIG_PM_RUNTIME


There are stubs for pm_runtime* functions, thus the ifdefs can be removed.
Please go though the complete patch and remove all instances.


+   pm_runtime_put(sdmmc_dev(host));


I don't know so much about USB mmc hosts hardware, but I just wanted
to find out if I have understood this correct.

You can't do fine grained power management of the USB parent device,
since it needs to be runtime resumed to be able keep the power the
card? Once it becomes runtime suspended, the power to the card will
thus also be dropped?


Yes, and to keep some internal state of the controller.

[snip]

+#ifdef CONFIG_PM


I suppose this should be CONFIG_PM_SLEEP?


...

+   err = mmc_suspend_host(mmc);


This won't compile. The mmc_suspend_host API has been removed.


...

+   return mmc_resume_host(mmc);


This won't compile. The mmc_resume_host API has been removed.


...

+static struct platform_driver rtsx_usb_sdmmc_driver = {
+   .probe  = rtsx_usb_sdmmc_drv_probe,
+   .remove = rtsx_usb_sdmmc_drv_remove,
+   .id_table   = rtsx_usb_sdmmc_ids,
+   .suspend= rtsx_usb_sdmmc_suspend,
+   .resume = rtsx_usb_sdmmc_resume,


Please use the modern pm_ops instead of the legacy suspend/resume callbacks.
I suggest you then also switch to use the SIMPLE_DEV_PM_OPS macro.


I just missed the removal of mmc_suspend|resume_host.

I'll remove all these unnecessary mmc host pm things as done in commit 
ff71c4bcb0af2730d047989e485303ae4e1ce794 for drivers of our PCIe devices.


Thanks for pointing this out.


Kind regards
Ulf Hansson

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v3 2/3] mmc: Add realtek USB sdmmc host driver

2014-02-06 Thread rogerable 
From: Roger Tseng 

Realtek USB SD/MMC host driver provides mmc host support based on the Realtek
USB card reader MFD driver.

Signed-off-by: Roger Tseng 
---
 drivers/mmc/host/Kconfig  |7 +
 drivers/mmc/host/Makefile |1 +
 drivers/mmc/host/rtsx_usb_sdmmc.c | 1500 +
 3 files changed, 1508 insertions(+)
 create mode 100644 drivers/mmc/host/rtsx_usb_sdmmc.c

diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
index 7fc5099..16f9a23 100644
--- a/drivers/mmc/host/Kconfig
+++ b/drivers/mmc/host/Kconfig
@@ -665,3 +665,10 @@ config MMC_REALTEK_PCI
help
  Say Y here to include driver code to support SD/MMC card interface
  of Realtek PCI-E card reader
+
+config MMC_REALTEK_USB
+   tristate "Realtek USB SD/MMC Card Interface Driver"
+   depends on MFD_RTSX_USB
+   help
+ Say Y here to include driver code to support SD/MMC card interface
+ of Realtek RTS5129/39 series card reader
diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
index c41d0c3..9d8d765 100644
--- a/drivers/mmc/host/Makefile
+++ b/drivers/mmc/host/Makefile
@@ -51,6 +51,7 @@ obj-$(CONFIG_MMC_USHC)+= ushc.o
 obj-$(CONFIG_MMC_WMT)  += wmt-sdmmc.o
 
 obj-$(CONFIG_MMC_REALTEK_PCI)  += rtsx_pci_sdmmc.o
+obj-$(CONFIG_MMC_REALTEK_USB)  += rtsx_usb_sdmmc.o
 
 obj-$(CONFIG_MMC_SDHCI_PLTFM)  += sdhci-pltfm.o
 obj-$(CONFIG_MMC_SDHCI_CNS3XXX)+= sdhci-cns3xxx.o
diff --git a/drivers/mmc/host/rtsx_usb_sdmmc.c 
b/drivers/mmc/host/rtsx_usb_sdmmc.c
new file mode 100644
index 000..952d80b
--- /dev/null
+++ b/drivers/mmc/host/rtsx_usb_sdmmc.c
@@ -0,0 +1,1500 @@
+/* Realtek USB SD/MMC Card Interface driver
+ *
+ * Copyright(c) 2009-2013 Realtek Semiconductor Corp. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see .
+ *
+ * Author:
+ *   Roger Tseng 
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+
+#if defined(CONFIG_LEDS_CLASS) || defined(CONFIG_LEDS_CLASS_MODULE)
+#include 
+#include 
+#define RTSX_USB_USE_LEDS_CLASS
+#endif
+
+struct rtsx_usb_sdmmc {
+   struct platform_device  *pdev;
+   struct rtsx_ucr *ucr;
+   struct mmc_host *mmc;
+   struct mmc_request  *mrq;
+
+   struct mutexhost_mutex;
+
+   u8  ssc_depth;
+   unsigned intclock;
+   boolvpclk;
+   booldouble_clk;
+   boolhost_removal;
+   boolcard_exist;
+   boolinitial_mode;
+   boolddr_mode;
+
+   unsigned char   power_mode;
+
+#if defined(CONFIG_LEDS_CLASS) || defined(CONFIG_LEDS_CLASS_MODULE)
+   struct led_classdev led;
+   charled_name[32];
+   struct work_struct  led_work;
+#endif
+};
+
+static inline struct device *sdmmc_dev(struct rtsx_usb_sdmmc *host)
+{
+   return &(host->pdev->dev);
+}
+
+static inline void sd_clear_error(struct rtsx_usb_sdmmc *host)
+{
+   struct rtsx_ucr *ucr = host->ucr;
+   rtsx_usb_ep0_write_register(ucr, CARD_STOP,
+ SD_STOP | SD_CLR_ERR,
+ SD_STOP | SD_CLR_ERR);
+
+   rtsx_usb_clear_dma_err(ucr);
+   rtsx_usb_clear_fsm_err(ucr);
+}
+
+#ifdef DEBUG
+static void sd_print_debug_regs(struct rtsx_usb_sdmmc *host)
+{
+   struct rtsx_ucr *ucr = host->ucr;
+   u8 val = 0;
+
+   rtsx_usb_ep0_read_register(ucr, SD_STAT1, &val);
+   dev_dbg(sdmmc_dev(host), "SD_STAT1: 0x%x\n", val);
+   rtsx_usb_ep0_read_register(ucr, SD_STAT2, &val);
+   dev_dbg(sdmmc_dev(host), "SD_STAT2: 0x%x\n", val);
+   rtsx_usb_ep0_read_register(ucr, SD_BUS_STAT, &val);
+   dev_dbg(sdmmc_dev(host), "SD_BUS_STAT: 0x%x\n", val);
+}
+#else
+#define sd_print_debug_regs(host)
+#endif /* DEBUG */
+
+static int sd_read_data(struct rtsx_usb_sdmmc *host, struct mmc_command *cmd,
+  u16 byte_cnt, u8 *buf, int buf_len, int timeout)
+{
+   struct rtsx_ucr *ucr = host->ucr;
+   int err;
+   u8 trans_mode;
+
+   if (!buf)
+   buf_len = 0;
+
+   rtsx_usb_init_cmd(ucr);
+   if (cmd != NULL) {
+   dev_dbg(sdmmc_dev(host), "%s