linux-kernel - Re: [PATCH 1/3] mfd: Add realtek USB card reader driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Mon, 20 Jan 2014 13:18:17 -0800
From:	Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To:	Roger <rogerable@...ltek.com>
Cc:	Lee Jones <lee.jones@...aro.org>,
	Samuel Ortiz <sameo@...ux.intel.com>,
	Chris Ball <cjb@...top.org>,
	Maxim Levitsky <maximlevitsky@...il.com>,
	Alex Dubov <oakad@...oo.com>,
	Dan Carpenter <dan.carpenter@...cle.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-mmc@...r.kernel.org" <linux-mmc@...r.kernel.org>,
	"driverdev-devel@...uxdriverproject.org" 
	<driverdev-devel@...uxdriverproject.org>,
	Wei_wang <wei_wang@...lsil.com.cn>,
	"micky_ching@...lsil.com.cn" <micky_ching@...lsil.com.cn>
Subject: Re: [PATCH 1/3] mfd: Add realtek USB card reader driver

On Mon, Jan 20, 2014 at 04:55:52PM +0800, Roger wrote:
> On 01/16/2014 05:35 PM, Lee Jones wrote:
> >>>>+static int rtsx_usb_bulk_transfer_sglist(struct rtsx_ucr *ucr,
> >>>>+             unsigned int pipe, struct scatterlist *sg, int num_sg,
> >>>>+             unsigned int length, unsigned int *act_len, int timeout)
> >>>>+{
> >>>>+     int ret;
> >>>>+
> >>>>+     dev_dbg(&ucr->pusb_intf->dev, "%s: xfer %u bytes, %d entries\n",
> >>>>+                     __func__, length, num_sg);
> >>>>+     ret = usb_sg_init(&ucr->current_sg, ucr->pusb_dev, pipe, 0,
> >>>>+                     sg, num_sg, length, GFP_NOIO);
> >>>>+     if (ret)
> >>>>+             return ret;
> >>>>+
> >>>>+     ucr->sg_timer.expires = jiffies + msecs_to_jiffies(timeout);
> >>>>+     add_timer(&ucr->sg_timer);
> >>>>+     usb_sg_wait(&ucr->current_sg);
> >>>>+     del_timer(&ucr->sg_timer);
> >>>>+
> >>>>+     if (act_len)
> >>>>+             *act_len = ucr->current_sg.bytes;
> >>>>+
> >>>>+     return ucr->current_sg.status;
> >>>>+}
> >>>
> >>>Code looks fine, but shouldn't this live in a USB driver?
> >>>
> >>We have studied drivers in usb/storage, the place that most likely
> >>to put the driver. All of them are for STANDARD usb mass storage
> >>class(something like USB_CLASS_MASS_STORAGE). But this is not the
> >>same case. This driver is for our vendor-class device with
> >>completely different protocol. It is really an USB interfaced flash
> >>card command converter(or channel) device rather than a real
> >>storage. It also has two derived modules(rtsx_usb_sdmmc,
> >>rtsx_usb_memstick) for other two subsystems.
> >>
> >>We also have another driver: rtsx_pcr.c resident in mfd/ for similar
> >>devices but of PCIe interface. It is nature for us to choose the
> >>same place for this variant.
> >
> >Very well, as long as it truly does not fit in drivers/usb. It would
> >be good to have one of the USB folk give the nod though.
> >
> I found Greg K-H is exactly one of the maintainers of USB subsystem as I
> read the MAINTAINERS document.
> 
> Greg, would you please comment this subsystem concern or introduce other
> members? Thanks.

It's the middle of the merge window, nothing can happen until after
3.14-rc1 is out.  So how about resending all of this in two weeks after
that happens so we can all properly discuss it?

As for where the driver should be located, if it shares logic with the
usb storage driver, then it should be in drivers/usb/storage, otherwise
put it wherever makes most sense, there's no need to put all USB drivers
under drivers/usb/ at all.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/