public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Eli Billauer <eli.billauer@gmail.com>
To: Greg KH <gregkh@linuxfoundation.org>
Cc: arnd@arndb.de, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] char: xillybus: Add driver for XillyUSB (Xillybus variant for USB)
Date: Thu, 07 Jan 2021 13:18:05 +0200	[thread overview]
Message-ID: <5FF6EDED.40408@gmail.com> (raw)
In-Reply-To: <X/XH5Q6APKKt4kRR@kroah.com>

Hello, Greg.

I'm afraid we're not on the same page. As mentioned in the original 
patch description, XillyUSB and the existing Xillybus variant presents a 
nearly identical API. User space programs see no difference when using 
one or the other, except for different device file names. In that sense, 
it's exactly like tty devices. But unlike ttys, there are no ioctls or 
any other special API functions to export. Xillybus' API consists only 
of the basic file operations, which behave like you'd expect from a 
pipe, more or less.

So even though one would usually expect this API similarity to take the 
form of common core routines, as it did with xillybus_pcie and 
xillybus_of, there is very little of that regarding XillyUSB. This is 
what I tried to explain in my previous mail: Why this common core is 
very slim.

I can see the benefit of maintaining the formal structure of a single 
entry point for the fops for all Xillybus variants. However my 
understanding while writing the XillyUSB driver was that sticking to 
this structure would be quite artificial, and that it would mess up 
things more than anything else.

Or have I misunderstood your concern about this patch?

Thanks and regards,
    Eli

On 06/01/21 16:23, Greg KH wrote:
> On Wed, Jan 06, 2021 at 04:03:08PM +0200, Eli Billauer wrote:
>    
>> >  Hello Greg,
>> >  
>> >  Merging XillyUSB's driver into xillybus_core.c was of course the initial
>> >  idea. Practically, it turned out that this doesn't reduce the number of code
>> >  lines nor makes the code easier to understand: The XillyUSB driver is a
>> >  completely different deal internally, in almost every aspect of it.
>> >  
>> >  Indeed, the two drivers do basically the same thing: They create a pipe-like
>> >  API using a hardware interface that is based upon buffers. This is what most
>> >  of the code in both drivers is about. As this underlying hardware interface
>> >  is so fundamentally different, there is little in common between the
>> >  drivers.
>> >  
>> >  The existing xillybus_core.c driver is based upon direct memory register +
>> >  DMA interaction with the hardware. XillyUSB relies on the USB framework for
>> >  all communication. I'll try to demonstrate the practical differences with
>> >  two examples.
>> >  
>> >  (1) Sending commands to the hardware: The existing Xillybus driver just
>> >  writes to registers in memory space. Its XillyUSB counterpart calls
>> >  xillyusb_send_opcode() to prepare a little packet for transmission over USB,
>> >  and may possibly sleep if there's a (temporary) lack of resources to
>> >  complete that task.
>> >  
>> >  (2) Data handling: The existing Xillybus driver just copies user data to and
>> >  from DMA buffers. Its main business is to maintain and juggle these buffers
>> >  with the hardware. The XillyUSB driver, on the other hand, manages a pool of
>> >  URBs to efficiently shuffle the data to and from the hardware. The main
>> >  challenge is to keep the data flowing at 400 MB/s.
>> >  
>> >  This goes on for every single aspect of the two drivers: They do the same
>> >  things essentially, but the actual actions are completely different, as they
>> >  have different means to do get the job done. And completely different
>> >  challenges.
>>      
> That's fine, but I'm talking about the userspace api.  You should not be
> creating two different userspace apis just because the bus transport
> changed for the hardware.
>
> We don't do that for things like tty devices, right?  :)
>
> So please, share the same core code that exports the api to userspace to
> be common, do not create a new one, like you did here.
>
>    


  reply	other threads:[~2021-01-07 11:19 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-13 17:05 [PATCH] char: xillybus: Add driver for XillyUSB (Xillybus variant for USB) eli.billauer
2020-12-13 17:22 ` Randy Dunlap
2020-12-13 17:30   ` Eli Billauer
2021-01-05 13:47 ` Greg KH
2021-01-06 14:03   ` Eli Billauer
2021-01-06 14:23     ` Greg KH
2021-01-07 11:18       ` Eli Billauer [this message]
2021-01-07 11:39         ` Greg KH
2021-01-08 10:59           ` Eli Billauer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5FF6EDED.40408@gmail.com \
    --to=eli.billauer@gmail.com \
    --cc=arnd@arndb.de \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox