Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20190827231409.253037-1-jwerner@chromium.org> <CAA6KcBAykS+VkhkcF42PhGyNu8KAEoaYPgA9-ru_HCxKrAEZzg@mail.gmail.com>
In-Reply-To: <CAA6KcBAykS+VkhkcF42PhGyNu8KAEoaYPgA9-ru_HCxKrAEZzg@mail.gmail.com>
From:   Julius Werner <jwerner@chromium.org>
Date:   Tue, 27 Aug 2019 16:59:43 -0700
Message-ID: <CAODwPW-+pPtiBh8Swn-cFs--2YaG6fMDyAzLXRucoweV50hDwA@mail.gmail.com>
Subject: Re: [PATCH] usb: storage: Add ums-cros-aoa driver
To:     Matthew Dharm <mdharm-usb@one-eyed-alien.net>
Cc:     Alan Stern <stern@rowland.harvard.edu>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        LKML <linux-kernel@vger.kernel.org>, linux-usb@vger.kernel.org,
        USB Mass Storage on Linux 
        <usb-storage@lists.one-eyed-alien.net>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

> Why not do the mode switch from userspace?  I thought we were trying to move all the mode-switching stuff in that direction.....

I need to tie in to the main USB mass storage driver in a way that I
think would make it too complicated to move the mode switching part to
userspace. See the part I'm adding to initializers.c... that one has
to be in the kernel since it operates on the device after the mode
switch when it is claimed by the normal USB storage driver. But the
mode switch part has to communicate information to it to make sure it
picks up the right device (just relying on the normal USB device
matching isn't enough in this case, because all Android devices in AOA
mode identify themselves with that well-known VID/PID... I don't know
if there's any other kernel driver using this protocol today, but
there may be at some point and then it becomes important to make sure
you really grab the device you meant to grab). Some of that
information (the 'route' field) isn't even directly available from
userspace (I could use the device name string instead and that would
roughly come out to the same thing, but seems less clean to me).

So I could either do the mode switch in userspace and add a big custom
sysfs interface to the usb-storage driver to allow userspace to
configure all this, or I can add a small kernel shim driver like in
this patch. Considering how little code the shim driver actually needs
I expect it would come out to roughly the same amount of kernel code
in both cases, and I feel like this version is much simpler to follow
and fits cleaner in the existing "unusual device" driver
infrastructure.