All of lore.kernel.org
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: Sudeep Dutt <sudeep.dutt@intel.com>
Cc: Peter P Waskiewicz Jr <peter.p.waskiewicz.jr@intel.com>,
	"Yaozu (Eddie) Dong" <eddie.dong@intel.com>,
	Arnd Bergmann <arnd@arndb.de>,
	"Michael S. Tsirkin" <mst@redhat.com>,
	Harshavardhan R Kharche <harshavardhan.r.kharche@intel.com>,
	linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
	virtualization@lists.linux-foundation.org,
	Ashutosh Dixit <ashutosh.dixit@intel.com>,
	Rob Landley <rob@landley.net>,
	Caz Yokoyama <Caz.Yokoyama@intel.com>,
	Dasaratharaman Chandramouli
	<dasaratharaman.chandramouli@intel.com>
Subject: Re: [PATCH 1/5] Intel MIC Host Driver for X100 family.
Date: Wed, 31 Jul 2013 18:45:12 -0700	[thread overview]
Message-ID: <20130801014512.GC6941@kroah.com> (raw)
In-Reply-To: <be8c714ed743143c427058fc0b75722f2eeeee1c.1374717252.git.sudeep.dutt@intel.com>

On Wed, Jul 24, 2013 at 08:31:32PM -0700, Sudeep Dutt wrote:
> This patch enables the following:
> a) Initializes the Intel MIC X100 PCIe devices.
> b) Boots and shuts down the card via sysfs entries.
> c) Allocates and maps a device page for communication with the
> card driver and updates the device page address via scratchpad
> registers.
> d) Provides sysfs entries for family, stepping, state, shutdown
> status, kernel command line, IP address, ramdisk and log buffer
> information.

That's a lot to do in one patch, almost 4 thousand lines.  Can't you
break this up into some more smaller, logical, and reviewable, pieces?

For example, I have no idea what b) is, and how to separate it from the
things you do for a) and c).

thanks,

greg k-h

WARNING: multiple messages have this Message-ID (diff)
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: Sudeep Dutt <sudeep.dutt@intel.com>
Cc: Arnd Bergmann <arnd@arndb.de>,
	Rusty Russell <rusty@rustcorp.com.au>,
	"Michael S. Tsirkin" <mst@redhat.com>,
	Rob Landley <rob@landley.net>,
	linux-kernel@vger.kernel.org,
	virtualization@lists.linux-foundation.org,
	linux-doc@vger.kernel.org, Nikhil Rao <nikhil.rao@intel.com>,
	Ashutosh Dixit <ashutosh.dixit@intel.com>,
	Caz Yokoyama <Caz.Yokoyama@intel.com>,
	Dasaratharaman Chandramouli 
	<dasaratharaman.chandramouli@intel.com>,
	Harshavardhan R Kharche <harshavardhan.r.kharche@intel.com>,
	"Yaozu (Eddie) Dong" <eddie.dong@intel.com>,
	Peter P Waskiewicz Jr <peter.p.waskiewicz.jr@intel.com>
Subject: Re: [PATCH 1/5] Intel MIC Host Driver for X100 family.
Date: Wed, 31 Jul 2013 18:45:12 -0700	[thread overview]
Message-ID: <20130801014512.GC6941@kroah.com> (raw)
In-Reply-To: <be8c714ed743143c427058fc0b75722f2eeeee1c.1374717252.git.sudeep.dutt@intel.com>

On Wed, Jul 24, 2013 at 08:31:32PM -0700, Sudeep Dutt wrote:
> This patch enables the following:
> a) Initializes the Intel MIC X100 PCIe devices.
> b) Boots and shuts down the card via sysfs entries.
> c) Allocates and maps a device page for communication with the
> card driver and updates the device page address via scratchpad
> registers.
> d) Provides sysfs entries for family, stepping, state, shutdown
> status, kernel command line, IP address, ramdisk and log buffer
> information.

That's a lot to do in one patch, almost 4 thousand lines.  Can't you
break this up into some more smaller, logical, and reviewable, pieces?

For example, I have no idea what b) is, and how to separate it from the
things you do for a) and c).

thanks,

greg k-h

  parent reply	other threads:[~2013-08-01  1:45 UTC|newest]

Thread overview: 53+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-07-25  3:31 [PATCH 0/5] Enable Drivers for Intel MIC X100 Coprocessors Sudeep Dutt
2013-07-25  3:31 ` [PATCH 1/5] Intel MIC Host Driver for X100 family Sudeep Dutt
2013-08-01  1:44   ` Greg Kroah-Hartman
2013-08-01  1:44     ` Greg Kroah-Hartman
2013-08-01  1:45   ` Greg Kroah-Hartman [this message]
2013-08-01  1:45     ` Greg Kroah-Hartman
2013-08-02  0:38     ` Sudeep Dutt
2013-08-02  0:38       ` Sudeep Dutt
2013-08-01  1:51   ` Greg Kroah-Hartman
2013-08-01  1:51     ` Greg Kroah-Hartman
2013-08-02  0:36     ` Sudeep Dutt
2013-08-02  0:46       ` Greg Kroah-Hartman
2013-08-02  0:46         ` Greg Kroah-Hartman
2013-08-02  0:36     ` Sudeep Dutt
2013-07-25  3:31 ` Sudeep Dutt
2013-07-25  3:31 ` [PATCH 2/5] Intel MIC Card " Sudeep Dutt
2013-07-25  3:31 ` Sudeep Dutt
2013-07-25  3:31 ` [PATCH 3/5] Intel MIC Host Driver Changes for Virtio Devices Sudeep Dutt
2013-07-25  3:31 ` Sudeep Dutt
2013-07-29  7:05   ` Michael S. Tsirkin
2013-07-29  7:05     ` Michael S. Tsirkin
2013-08-02  0:40     ` Sudeep Dutt
2013-08-02  0:40       ` Sudeep Dutt
2013-08-06  3:47       ` Rusty Russell
2013-08-06  3:47       ` Rusty Russell
2013-07-25  3:31 ` [PATCH 4/5] Intel MIC Card " Sudeep Dutt
2013-07-25  3:31 ` Sudeep Dutt
2013-07-25  4:41   ` Greg Kroah-Hartman
2013-07-25  4:41     ` Greg Kroah-Hartman
2013-07-25 18:00     ` Sudeep Dutt
2013-07-25 18:00     ` Sudeep Dutt
2013-07-29  1:58   ` Rusty Russell
2013-07-29  1:58   ` Rusty Russell
2013-07-25  3:31 ` [PATCH 5/5] Sample Implementation of Intel MIC User Space Daemon Sudeep Dutt
2013-07-25  3:31 ` Sudeep Dutt
2013-08-01  1:46 ` [PATCH 0/5] Enable Drivers for Intel MIC X100 Coprocessors Greg Kroah-Hartman
2013-08-01  1:46 ` Greg Kroah-Hartman
2013-08-01  7:45   ` Asias He
2013-08-01  7:45     ` Asias He
2013-08-02  0:37     ` Sudeep Dutt
2013-08-02  0:37     ` Sudeep Dutt
2013-08-02  0:34   ` Sudeep Dutt
2013-08-02  0:34     ` Sudeep Dutt
2013-08-13 12:43 ` Pavel Machek
2013-08-13 12:43   ` Pavel Machek
2013-08-14 20:24   ` Sudeep Dutt
2013-08-14 20:24     ` Sudeep Dutt
2013-08-15 10:14     ` Pavel Machek
2013-08-15 10:14     ` Pavel Machek
2013-08-16 16:59       ` Sudeep Dutt
2013-08-16 17:08         ` Waskiewicz Jr, Peter P
2013-08-16 17:08         ` Waskiewicz Jr, Peter P
2013-08-16 16:59       ` Sudeep Dutt

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=20130801014512.GC6941@kroah.com \
    --to=gregkh@linuxfoundation.org \
    --cc=Caz.Yokoyama@intel.com \
    --cc=arnd@arndb.de \
    --cc=ashutosh.dixit@intel.com \
    --cc=dasaratharaman.chandramouli@intel.com \
    --cc=eddie.dong@intel.com \
    --cc=harshavardhan.r.kharche@intel.com \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mst@redhat.com \
    --cc=peter.p.waskiewicz.jr@intel.com \
    --cc=rob@landley.net \
    --cc=sudeep.dutt@intel.com \
    --cc=virtualization@lists.linux-foundation.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.