linux-usb.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Re: [PATCH 1/1] usb: Add MA USB Host driver
       [not found] ` <61e63056-31f9-9d4b-60c1-8cbf0372d34f@displaylink.com>
@ 2020-02-11 16:39   ` gregkh
  2020-02-12  9:41     ` [External] " Vladimir Stankovic
  2020-02-11 16:39   ` gregkh
  2020-02-11 16:41   ` gregkh
  2 siblings, 1 reply; 6+ messages in thread
From: gregkh @ 2020-02-11 16:39 UTC (permalink / raw)
  To: Vladimir Stankovic
  Cc: linux-kernel, linux-usb, Petar Kovacevic, Stefan Lugonjic,
	Nikola Simic, Marko Miljkovic

On Tue, Feb 11, 2020 at 04:21:24PM +0000, Vladimir Stankovic wrote:
>   39 files changed, 8668 insertions(+)

This is a bit hard, if not impossible, to review all in one huge patch.

Can you resend this as a patch series, breaking it down into logical
chunks, like all other kernel patches have?

Also, why so many individual files?  For only 8k lines, 39 files seems
like a huge number.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/1] usb: Add MA USB Host driver
       [not found] ` <61e63056-31f9-9d4b-60c1-8cbf0372d34f@displaylink.com>
  2020-02-11 16:39   ` [PATCH 1/1] usb: Add MA USB Host driver gregkh
@ 2020-02-11 16:39   ` gregkh
  2020-02-11 16:41   ` gregkh
  2 siblings, 0 replies; 6+ messages in thread
From: gregkh @ 2020-02-11 16:39 UTC (permalink / raw)
  To: Vladimir Stankovic
  Cc: linux-kernel, linux-usb, Petar Kovacevic, Stefan Lugonjic,
	Nikola Simic, Marko Miljkovic

On Tue, Feb 11, 2020 at 04:21:24PM +0000, Vladimir Stankovic wrote:
> MA-USB Host driver provides USB connectivity over an available
> network, allowing host device to access remote USB devices attached
> to one or more MA USB devices (accessible via network).
> 
> This driver has been developed to enable the host to communicate
> with DisplayLink products supporting MA USB protocol (MA USB device,
> in terms of MA USB Specification).
> 
> MA-USB protocol used by MA-USB Host driver has been implemented in
> accordance with MA USB Specification Release 1.0b.
> 
> This driver depends on the functions provided by DisplayLink's
> user-space driver.
> 
> Signed-off-by: Vladimir Stankovic <vladimir.stankovic@displaylink.com>
> ---
>   drivers/usb/Kconfig                           |    2 +
>   drivers/usb/Makefile                          |    2 +
>   drivers/usb/mausb_host/Kconfig                |   15 +
>   drivers/usb/mausb_host/Makefile               |   30 +
>   .../usb/mausb_host/include/common/ma_usb.h    |  873 +++++++++
>   .../mausb_host/include/common/mausb_address.h |   38 +
>   .../include/common/mausb_driver_status.h      |   21 +
>   .../mausb_host/include/common/mausb_event.h   |  228 +++
>   drivers/usb/mausb_host/include/hcd/hub.h      |  115 ++
>   drivers/usb/mausb_host/include/hcd/vhcd.h     |   79 +
>   .../usb/mausb_host/include/hpal/data_common.h |   66 +
>   drivers/usb/mausb_host/include/hpal/data_in.h |   18 +
>   .../usb/mausb_host/include/hpal/data_out.h    |   22 +
>   drivers/usb/mausb_host/include/hpal/hpal.h    |  184 ++
>   .../usb/mausb_host/include/hpal/isoch_in.h    |   22 +
>   .../usb/mausb_host/include/hpal/isoch_out.h   |   20 +
>   .../mausb_host/include/hpal/mausb_events.h    |   99 +
>   .../include/hpal/network_callbacks.h          |   20 +
>   .../mausb_host/include/link/mausb_ip_link.h   |   87 +
>   .../include/utils/mausb_data_iterator.h       |   52 +
>   .../usb/mausb_host/include/utils/mausb_logs.h |   35 +
>   .../usb/mausb_host/include/utils/mausb_mmap.h |   21 +
>   .../include/utils/mausb_ring_buffer.h         |   67 +
>   .../mausb_host/include/utils/mausb_version.h  |   17 +
>   drivers/usb/mausb_host/src/hcd/hub.c          | 1684 +++++++++++++++++
>   drivers/usb/mausb_host/src/hcd/module_init.c  |  219 +++
>   drivers/usb/mausb_host/src/hcd/vhcd.c         |  291 +++
>   drivers/usb/mausb_host/src/hpal/data_common.c |  141 ++
>   drivers/usb/mausb_host/src/hpal/data_in.c     |   73 +
>   drivers/usb/mausb_host/src/hpal/data_out.c    |  202 ++
>   drivers/usb/mausb_host/src/hpal/hpal.c        | 1306 +++++++++++++
>   drivers/usb/mausb_host/src/hpal/isoch_in.c    |  241 +++
>   drivers/usb/mausb_host/src/hpal/isoch_out.c   |  361 ++++
>   .../usb/mausb_host/src/hpal/mausb_events.c    |  659 +++++++
>   .../mausb_host/src/hpal/network_callbacks.c   |  160 ++
>   .../usb/mausb_host/src/link/mausb_ip_link.c   |  354 ++++
>   .../src/utils/mausb_data_iterator.c           |  295 +++
>   drivers/usb/mausb_host/src/utils/mausb_mmap.c |  381 ++++
>   .../mausb_host/src/utils/mausb_ring_buffer.c  |  168 ++
>   39 files changed, 8668 insertions(+)
>   create mode 100644 drivers/usb/mausb_host/Kconfig
>   create mode 100644 drivers/usb/mausb_host/Makefile
>   create mode 100644 drivers/usb/mausb_host/include/common/ma_usb.h
>   create mode 100644 drivers/usb/mausb_host/include/common/mausb_address.h
>   create mode 100644 
> drivers/usb/mausb_host/include/common/mausb_driver_status.h
>   create mode 100644 drivers/usb/mausb_host/include/common/mausb_event.h
>   create mode 100644 drivers/usb/mausb_host/include/hcd/hub.h
>   create mode 100644 drivers/usb/mausb_host/include/hcd/vhcd.h
>   create mode 100644 drivers/usb/mausb_host/include/hpal/data_common.h
>   create mode 100644 drivers/usb/mausb_host/include/hpal/data_in.h
>   create mode 100644 drivers/usb/mausb_host/include/hpal/data_out.h
>   create mode 100644 drivers/usb/mausb_host/include/hpal/hpal.h
>   create mode 100644 drivers/usb/mausb_host/include/hpal/isoch_in.h
>   create mode 100644 drivers/usb/mausb_host/include/hpal/isoch_out.h
>   create mode 100644 drivers/usb/mausb_host/include/hpal/mausb_events.h
>   create mode 100644 drivers/usb/mausb_host/include/hpal/network_callbacks.h
>   create mode 100644 drivers/usb/mausb_host/include/link/mausb_ip_link.h
>   create mode 100644 
> drivers/usb/mausb_host/include/utils/mausb_data_iterator.h
>   create mode 100644 drivers/usb/mausb_host/include/utils/mausb_logs.h
>   create mode 100644 drivers/usb/mausb_host/include/utils/mausb_mmap.h
>   create mode 100644 
> drivers/usb/mausb_host/include/utils/mausb_ring_buffer.h
>   create mode 100644 drivers/usb/mausb_host/include/utils/mausb_version.h
>   create mode 100644 drivers/usb/mausb_host/src/hcd/hub.c
>   create mode 100644 drivers/usb/mausb_host/src/hcd/module_init.c
>   create mode 100644 drivers/usb/mausb_host/src/hcd/vhcd.c
>   create mode 100644 drivers/usb/mausb_host/src/hpal/data_common.c
>   create mode 100644 drivers/usb/mausb_host/src/hpal/data_in.c
>   create mode 100644 drivers/usb/mausb_host/src/hpal/data_out.c
>   create mode 100644 drivers/usb/mausb_host/src/hpal/hpal.c
>   create mode 100644 drivers/usb/mausb_host/src/hpal/isoch_in.c
>   create mode 100644 drivers/usb/mausb_host/src/hpal/isoch_out.c
>   create mode 100644 drivers/usb/mausb_host/src/hpal/mausb_events.c
>   create mode 100644 drivers/usb/mausb_host/src/hpal/network_callbacks.c
>   create mode 100644 drivers/usb/mausb_host/src/link/mausb_ip_link.c
>   create mode 100644 drivers/usb/mausb_host/src/utils/mausb_data_iterator.c
>   create mode 100644 drivers/usb/mausb_host/src/utils/mausb_mmap.c
>   create mode 100644 drivers/usb/mausb_host/src/utils/mausb_ring_buffer.c
> 
> 

> From 0324b3a49d0ef41480a0a42ab27f70edeb007980 Mon Sep 17 00:00:00 2001
> From: Vladimir Stankovic <vladimir.stankovic@displaylink.com>
> Date: Tue, 11 Feb 2020 16:03:43 +0100
> Subject: [PATCH] usb: Add MA USB Host driver
> 
> MA-USB Host driver provides USB connectivity over an available
> network, allowing host device to access remote USB devices attached
> to one or more MA USB devices (accessible via network).

<snip>

You attached the patch, as well as put the description above in the
email as well, so I couldn't apply this even if I wanted to :(

Please just submit the patch all inline, like all other kernel patches
are.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/1] usb: Add MA USB Host driver
       [not found] ` <61e63056-31f9-9d4b-60c1-8cbf0372d34f@displaylink.com>
  2020-02-11 16:39   ` [PATCH 1/1] usb: Add MA USB Host driver gregkh
  2020-02-11 16:39   ` gregkh
@ 2020-02-11 16:41   ` gregkh
  2020-02-12  9:43     ` Vladimir Stankovic
  2 siblings, 1 reply; 6+ messages in thread
From: gregkh @ 2020-02-11 16:41 UTC (permalink / raw)
  To: Vladimir Stankovic
  Cc: linux-kernel, linux-usb, Petar Kovacevic, Stefan Lugonjic,
	Nikola Simic, Marko Miljkovic

On Tue, Feb 11, 2020 at 04:21:24PM +0000, Vladimir Stankovic wrote:
> +++ b/drivers/usb/mausb_host/include/common/ma_usb.h
> @@ -0,0 +1,873 @@
> +/* SPDX-License-Identifier: GPL-2.0 */

Nice.

> +/*
> + * Copyright (c) 2019 - 2020 DisplayLink (UK) Ltd.
> + *
> + * This file is subject to the terms and conditions of the GNU General Public
> + * License v2. See the file COPYING in the main directory of this archive for
> + * more details.

Note, COPYING does not say what you think it says here :)

Please remove all "license boilerplate" from all of these files, with
the use of SPDX you do not need it, and, as is the case here, it can
often be wrong.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [External] Re: [PATCH 1/1] usb: Add MA USB Host driver
  2020-02-11 16:39   ` [PATCH 1/1] usb: Add MA USB Host driver gregkh
@ 2020-02-12  9:41     ` Vladimir Stankovic
  2020-02-12 13:22       ` gregkh
  0 siblings, 1 reply; 6+ messages in thread
From: Vladimir Stankovic @ 2020-02-12  9:41 UTC (permalink / raw)
  To: gregkh
  Cc: linux-kernel, linux-usb, Petar Kovacevic, Stefan Lugonjic,
	Nikola Simic, Marko Miljkovic

On 11.2.20. 17:39, gregkh@linuxfoundation.org wrote:
> On Tue, Feb 11, 2020 at 04:21:24PM +0000, Vladimir Stankovic wrote:
>>    39 files changed, 8668 insertions(+)
> 
> This is a bit hard, if not impossible, to review all in one huge patch.
> 
> Can you resend this as a patch series, breaking it down into logical
> chunks, like all other kernel patches have?
> 
> Also, why so many individual files?  For only 8k lines, 39 files seems
> like a huge number.
> 
> thanks,
> 
> greg k-h
> 
Will break it down into patch series and resend.

In regards of the file count, our intention was to ease the 
troubleshooting efforts during development and have a clear separation 
between logical parts of MA-USB implementation (i.e data in/out, isoch 
in/out, etc.; each source file representing logical chunk).

Regards,
Vladimir.


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/1] usb: Add MA USB Host driver
  2020-02-11 16:41   ` gregkh
@ 2020-02-12  9:43     ` Vladimir Stankovic
  0 siblings, 0 replies; 6+ messages in thread
From: Vladimir Stankovic @ 2020-02-12  9:43 UTC (permalink / raw)
  To: gregkh
  Cc: linux-kernel, linux-usb, Petar Kovacevic, Stefan Lugonjic,
	Nikola Simic, Marko Miljkovic

On 11.2.20. 17:41, gregkh@linuxfoundation.org wrote:
> On Tue, Feb 11, 2020 at 04:21:24PM +0000, Vladimir Stankovic wrote:
>> +++ b/drivers/usb/mausb_host/include/common/ma_usb.h
>> @@ -0,0 +1,873 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
> 
> Nice.
> 
>> +/*
>> + * Copyright (c) 2019 - 2020 DisplayLink (UK) Ltd.
>> + *
>> + * This file is subject to the terms and conditions of the GNU General Public
>> + * License v2. See the file COPYING in the main directory of this archive for
>> + * more details.
> 
> Note, COPYING does not say what you think it says here :)
> 
> Please remove all "license boilerplate" from all of these files, with
> the use of SPDX you do not need it, and, as is the case here, it can
> often be wrong.
> 
> thanks,
> 
> greg k-h
> 
Agree. Will address.

Regards,
Vladimir.


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [External] Re: [PATCH 1/1] usb: Add MA USB Host driver
  2020-02-12  9:41     ` [External] " Vladimir Stankovic
@ 2020-02-12 13:22       ` gregkh
  0 siblings, 0 replies; 6+ messages in thread
From: gregkh @ 2020-02-12 13:22 UTC (permalink / raw)
  To: Vladimir Stankovic
  Cc: linux-kernel, linux-usb, Petar Kovacevic, Stefan Lugonjic,
	Nikola Simic, Marko Miljkovic

On Wed, Feb 12, 2020 at 09:41:12AM +0000, Vladimir Stankovic wrote:
> On 11.2.20. 17:39, gregkh@linuxfoundation.org wrote:
> > On Tue, Feb 11, 2020 at 04:21:24PM +0000, Vladimir Stankovic wrote:
> >>    39 files changed, 8668 insertions(+)
> > 
> > This is a bit hard, if not impossible, to review all in one huge patch.
> > 
> > Can you resend this as a patch series, breaking it down into logical
> > chunks, like all other kernel patches have?
> > 
> > Also, why so many individual files?  For only 8k lines, 39 files seems
> > like a huge number.
> > 
> > thanks,
> > 
> > greg k-h
> > 
> Will break it down into patch series and resend.
> 
> In regards of the file count, our intention was to ease the 
> troubleshooting efforts during development and have a clear separation 
> between logical parts of MA-USB implementation (i.e data in/out, isoch 
> in/out, etc.; each source file representing logical chunk).

Yeah, but at the very least, only have a single .h "internal" file, you
have whole files for just 1 or 2 function prototypes, that's totally
overkill.

Simple is good, you don't want to have to bounce around through multiple
files for a simple 8k line driver, that's not a good idea.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-02-12 13:22 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <VI1PR10MB1965B4B61D7A9808B2EA095591180@VI1PR10MB1965.EURPRD10.PROD.OUTLOOK.COM>
     [not found] ` <61e63056-31f9-9d4b-60c1-8cbf0372d34f@displaylink.com>
2020-02-11 16:39   ` [PATCH 1/1] usb: Add MA USB Host driver gregkh
2020-02-12  9:41     ` [External] " Vladimir Stankovic
2020-02-12 13:22       ` gregkh
2020-02-11 16:39   ` gregkh
2020-02-11 16:41   ` gregkh
2020-02-12  9:43     ` Vladimir Stankovic

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).