Re: [PATCH 4/5] usb: xhci: dbc: Add a dbc raw driver to provide a raw interface on DbC

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Jun 12, 2019 at 08:49:11AM +0000, Regupathy, Rajaram wrote:
> 
> 
> > -----Original Message-----
> > From: Greg KH [mailto:gregkh@xxxxxxxxxxxxxxxxxxx]
> > Sent: Tuesday, June 11, 2019 6:04 PM
> > To: Regupathy, Rajaram <rajaram.regupathy@xxxxxxxxx>
> > Cc: Mathias Nyman <mathias.nyman@xxxxxxxxxxxxxxx>; Pandey, Prabhat Chand
> > <prabhat.chand.pandey@xxxxxxxxx>; linux-usb@xxxxxxxxxxxxxxx; Nyman,
> > Mathias <mathias.nyman@xxxxxxxxx>; K V, Abhilash <abhilash.k.v@xxxxxxxxx>;
> > Balaji, M <m.balaji@xxxxxxxxx>
> > Subject: Re: [PATCH 4/5] usb: xhci: dbc: Add a dbc raw driver to provide a raw
> > interface on DbC
> > Importance: High

Please fix your quoting logic, this is never needed in an email.

And I don't find these emails of "High" importance :)

> > > > > The larger goal here is to have DbC as a unified debug
> > > > > infrastructure for
> > > > different debug methods like KGDB or early printk and leverage the
> > > > benefits of a dedicated debug infra (DbC) brings in.
> > > >
> > > > Have you modified kgdb for this?  Do you have patches for that?
> > > No kgdb changes. For kgdb to work we added necessary wrapper functions
> > required on the dbc_tty interface which already part of kernel. We have
> > functionally verified and shall push the patch subsequently. Am I missing
> > something ?
> > 
> > You are missing the justification of a custom api that requires all userspace
> > tools to be modified instead of using the existing tty api that everything "just
> > works" with today.
> 
> I was referring to the poll methods required for KGDB/GDB to work which is missing in dbc_tty driver in the kernel.

Again, fix your line-wrapping :(

Anyway, just poing kgdb at the different tty device and it should "just
work", right?  No need to change anything that I can see.  If not,
please send patches to get that working so people can use that today.

> > > > Who can use this interface in the "real world", is it only
> > > > developers that have access to the special hardware dongle?  Or can
> > > > anyone use this on their laptops for getting console access in a way
> > > > that is somehow "better" than the existing interface?
> > >
> > > No special hardware is required. As indicated earlier developers need a USB A-
> > A debug cable and anyone can use it to get console access.
> > 
> > Where can I get one of those?
> Here is one example:  https://www.amazon.com/SIIG-SuperSpeed-Cable-Meters-CB-US0212-S1/dp/B0032ANCBO

Ah, nice!  I'll try to see if I can get that in my country...

Nope, not available in Europe from what I can tell, I'll have to wait
until the next time I'm in the US :(

> > > Yes it is better that existing usb-serial converters with each having proprietary
> > drivers . This is a plug and play solution providing super speed interface.
> > 
> > I don't understand, what is "proprietary" about the existing tty api?
> > It's a generic tty device node, right?  What am I missing?
> 
> I am referring to usb-serial controller drivers as in "drivers/usb/serial/Kconfig" which has vendors configs leading to much of kernel maintenance.  DbC driver would provide necessary functionality without any of those.

Again, line-wrapping...

I have no idea what you are referring to "leading to much of kernel
maintenance."  You don't need to include _all_ usb-serial drivers, just
the usb-serial core and the specific driver.  No different from any
other serial port device, which Linux has supported for decades now.

Use the functionality we have already, do not create duplicate code just
because you want to, that's a sure way to get your patches rejected.

And this _is_ a serial connection to the other device, why pretend that
it is not?  Again, what is the reason why you can not use what you have
today?

> > > > And just how much "faster" is all of this than the current tty
> > > > interface?  What is lacking in the tty interface today that you need
> > > > this new, special one?  Can you just not fix any bottleneck in the
> > > > tty driver if you are not properly saturating the bus?
> > >
> > > IMHO, tty is a legacy interface designed for the purpose it serves
> > > for. Modern High speed IO will not fit into tty framework and
> > > refactoring it will not bring any real value.  We have captured the
> > > initial performance numbers in the documentation patch 5/5.  Please
> > > correct me if I am missing something
> > 
> > Why will "modern high speed IO" not fit into the tty framework?  Where is the
> > bottleneck?  We have tty devices that seem to run at "line speed"
> > on a firewire connection today, and that should be faster than whatever this
> > host controller can pump out, right?
> 
> Though I am not aware of the design thoughts behind firewire-tty driver which is in staging, I see
> the to do list and git logs indicate buffer over flow issues indicating the tty framework cannot handle high speed IO. 

That is that one driver, not yours.

> Thus our rationale of why tty may not serve the purpose as indicated below gets ratified 
> - Performance & stability ( multiple layers, >1GB file copy CRC errors)
> - Error Recovery  ( lack of framework to propagate transport error handling to the "real" class drivers)

Do you have these problems today with the existing kernel driver?  If
so, please let us know and fix them!  Don't create a whole new thing
just because you don't like the tty layer, otherwise you will be forced
to end up creating all of the same logic again.

> > My main objection here is a lack of justification to require userspace to write to
> > a new, unknown and undocumented interface, because of an unknown speed
> > issue in the existing codebase.
> 
> As for us clarity on unknown/undocumented, ADB is adequately documented and known tool. Please note this is just one example and the interface can be leveraged by other debug tools for better performance..  Having a thinner kernel implementation has well known advantage including stability and maintenance and is not new to USB drivers

What "thinner" kernel implementation?  Nothing is as "thin" as the code
that is already written and is not going away :)

Again, do not duplicate functionality for no good reason, and I have yet
to see a definitive reason why the current in-kernel code is not
acceptable.

> > Would you take patches submitted in such a way if you were in my place?
> I would be happy to address all your concerns and open to adopt any better approach that solves the problem .

That was not the answer to my question.

greg k-h



[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux