From mboxrd@z Thu Jan 1 00:00:00 1970 From: Greg Kroah-Hartman Subject: Re: [PATCH 1/5] Intel MIC Host Driver for X100 family. Date: Wed, 31 Jul 2013 18:45:12 -0700 Message-ID: <20130801014512.GC6941@kroah.com> References: Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: virtualization-bounces@lists.linux-foundation.org Errors-To: virtualization-bounces@lists.linux-foundation.org To: Sudeep Dutt Cc: Peter P Waskiewicz Jr , "Yaozu (Eddie) Dong" , Arnd Bergmann , "Michael S. Tsirkin" , Harshavardhan R Kharche , linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org, virtualization@lists.linux-foundation.org, Ashutosh Dixit , Rob Landley , Caz Yokoyama , Dasaratharaman Chandramouli List-Id: virtualization@lists.linuxfoundation.org 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 From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753515Ab3HAHaw (ORCPT ); Thu, 1 Aug 2013 03:30:52 -0400 Received: from mail.linuxfoundation.org ([140.211.169.12]:41808 "EHLO mail.linuxfoundation.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753223Ab3HAHat (ORCPT ); Thu, 1 Aug 2013 03:30:49 -0400 Date: Wed, 31 Jul 2013 18:45:12 -0700 From: Greg Kroah-Hartman To: Sudeep Dutt Cc: Arnd Bergmann , Rusty Russell , "Michael S. Tsirkin" , Rob Landley , linux-kernel@vger.kernel.org, virtualization@lists.linux-foundation.org, linux-doc@vger.kernel.org, Nikhil Rao , Ashutosh Dixit , Caz Yokoyama , Dasaratharaman Chandramouli , Harshavardhan R Kharche , "Yaozu (Eddie) Dong" , Peter P Waskiewicz Jr Subject: Re: [PATCH 1/5] Intel MIC Host Driver for X100 family. Message-ID: <20130801014512.GC6941@kroah.com> References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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