Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753816AbbH0Qrl (ORCPT ); Thu, 27 Aug 2015 12:47:41 -0400 Received: from mail-pa0-f48.google.com ([209.85.220.48]:36367 "EHLO mail-pa0-f48.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752002AbbH0Qrj (ORCPT ); Thu, 27 Aug 2015 12:47:39 -0400 Date: Thu, 27 Aug 2015 09:47:36 -0700 From: Brian Norris To: Stefan Agner Cc: dwmw2@infradead.org, sebastian@breakpoint.cc, robh+dt@kernel.org, pawel.moll@arm.com, mark.rutland@arm.com, ijc+devicetree@hellion.org.uk, galak@codeaurora.org, shawn.guo@linaro.org, kernel@pengutronix.de, boris.brezillon@free-electrons.com, marb@ixxat.de, aaron@tastycactus.com, bpringlemeir@gmail.com, linux-mtd@lists.infradead.org, devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, albert.aribaud@3adev.fr, klimov.linux@gmail.com, Bill Pringlemeir Subject: Re: [PATCH v10 1/5] mtd: nand: vf610_nfc: Freescale NFC for VF610, MPC5125 and others Message-ID: <20150827164736.GY81844@google.com> References: <1438594050-4595-1-git-send-email-stefan@agner.ch> <1438594050-4595-2-git-send-email-stefan@agner.ch> <20150825203412.GM81844@google.com> 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 Content-Length: 2158 Lines: 53 On Wed, Aug 26, 2015 at 06:10:05PM -0700, Stefan Agner wrote: > On 2015-08-25 13:34, Brian Norris wrote: > > One more thing... > > > > On Mon, Aug 03, 2015 at 11:27:26AM +0200, Stefan Agner wrote: > >> --- /dev/null > >> +++ b/drivers/mtd/nand/vf610_nfc.c > >> @@ -0,0 +1,645 @@ > > ... > >> +struct vf610_nfc { > >> + struct mtd_info mtd; > >> + struct nand_chip chip; > >> + struct device *dev; > >> + void __iomem *regs; > >> + struct completion cmd_done; > >> + uint buf_offset; > >> + int page_sz; > > > > AFAICT (even with the 2nd patch), you never really use this field. You > > just set it/increment it, but don't use it for anything. Kill it? > > It is used in the write path, I think I meant to use it for subpage > writes, when I thought it would just mean to transfer only parts of the > page to the controller. Ah, you're right. Sorry, I missed that. I got mixed up seeing most of your uses of 'page_sz' were for a local variable of the same name, not this field. > However, as the subpage discussion basically concluded in not using it > for now on this controller, we can as well transfer the complete page > (page_sz). Or is there another case in which vf610_nfc_write_buf could > be called with less than page_sz? I'll leave that up to you. I'm perfectly fine leaving it in, now that I see its proper use. Just in case things change in the future, I think it does help to clarify the flow of information a little. Although, I might recommend a change in naming, since it could get confused with the actual page size -- which is normally constant -- whereas this field changes dynamically depending on the command-in-flight. Perhaps the struct could have 'write_len' (to help represent an action) and the local variable in vf610_nfc_command() could be 'tfr_len' (to distinguish how it isn't necessarily identical to 'write_len')? Just throwing (likely bad) ideas out there. Regards, Brian -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/