lkml.org 
[lkml]   [2014]   [Mar]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v5 1/3] mfd: Add realtek USB card reader driver
On 03/26/2014 10:36 PM, Oliver Neukum wrote:
> On Tue, 2014-03-25 at 18:44 +0800, rogerable@realtek.com wrote:
>> From: Roger Tseng <rogerable@realtek.com>
>

>> +static int rtsx_usb_probe(struct usb_interface *intf,
>> + const struct usb_device_id *id)
>> +{
>> + struct usb_device *usb_dev = interface_to_usbdev(intf);
>> + struct rtsx_ucr *ucr;
>> + int ret;
>> +
>> + dev_dbg(&intf->dev,
>> + ": Realtek USB Card Reader found at bus %03d address %03d\n",
>> + usb_dev->bus->busnum, usb_dev->devnum);
>> +
>> + ucr = devm_kzalloc(&intf->dev, sizeof(*ucr), GFP_KERNEL);
>> + if (!ucr)
>> + return -ENOMEM;
>> +
>> + ucr->pusb_dev = usb_dev;
>> +
>> + ucr->iobuf = usb_alloc_coherent(ucr->pusb_dev, IOBUF_SIZE,
>> + GFP_KERNEL, &ucr->iobuf_dma);
>> + if (!ucr->iobuf)
>> + return -ENOMEM;
>> +
>> + usb_set_intfdata(intf, ucr);
>> +
>> + ucr->vendor_id = id->idVendor;
>> + ucr->product_id = id->idProduct;
>> + ucr->cmd_buf = ucr->rsp_buf = ucr->iobuf;
>> +
>> + mutex_init(&ucr->dev_mutex);
>> +
>> + ucr->pusb_intf = intf;
>> +
>> + /* initialize */
>> + ret = rtsx_usb_init_chip(ucr);
>> + if (ret)
>> + goto out_init_fail;
>> +
>> + ret = mfd_add_devices(&intf->dev, usb_dev->devnum, rtsx_usb_cells,
>> + ARRAY_SIZE(rtsx_usb_cells), NULL, 0, NULL);
>
> Race condition. What prevents the mfd layer from using this device
> before you've finished the next steps of the initialisation?
>
OK, I'll put the timer init before mfd_add_devices.
>> + if (ret)
>> + goto out_init_fail;
>> +
>> + /* initialize USB SG transfer timer */
>> + init_timer(&ucr->sg_timer);
>> + setup_timer(&ucr->sg_timer, rtsx_usb_sg_timed_out, (unsigned long) ucr);
>> +#ifdef CONFIG_PM
>> + intf->needs_remote_wakeup = 1;
>
> Why?
Our reader supports remote wake-up from card slot event(insertion,
removal). It should be enabled to let the driver be able to detect the
newly inserted card.

>> + usb_enable_autosuspend(usb_dev);
>> +#endif
>> +
>> + return 0;
>> +
>> +out_init_fail:
>> + usb_free_coherent(ucr->pusb_dev, IOBUF_SIZE, ucr->iobuf,
>> + ucr->iobuf_dma);
>> + return ret;
>> +}
>> +
...
>> +static int rtsx_usb_suspend(struct usb_interface *intf, pm_message_t message)
>> +{
>> + struct rtsx_ucr *ucr =
>> + (struct rtsx_ucr *)usb_get_intfdata(intf);
>> +
>> + dev_dbg(&intf->dev, "%s called with pm message 0x%04u\n",
>> + __func__, message.event);
>> +
>> + mutex_lock(&ucr->dev_mutex);
>> + rtsx_usb_turn_off_led(ucr);
>
> When is this turned on again?
>

The LED is not in a permanent cut-off state after here. It is called to
guarantee the LED is off during suspend to save more power. It could be
lit up in the card host drivers(e.g. rtsx_usb_sdmmc.c) anytime whenever
necessary.

This has been asked may times. I'm considering putting some comment in
next revision.

Best regards,
Roger Tseng


\
 
 \ /
  Last update: 2014-03-28 04:41    [W:0.167 / U:20.580 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site