From mboxrd@z Thu Jan  1 00:00:00 1970
From: Artem Bityutskiy <dedekind1@gmail.com>
Subject: Re: [PATCH 2/2] mtd: msm_nand: Add initial msm nand driver support.
Date: Sat, 16 Apr 2011 11:31:52 +0300
Message-ID: <1302942712.2308.3.camel@koala>
References: <1298940450-27365-1-git-send-email-mnalajal@codeaurora.org>
	 <1298940450-27365-2-git-send-email-mnalajal@codeaurora.org>
	 <1302861097.3220.12.camel@localhost>  <4DA85B24.50807@codeaurora.org>
	 <1302942029.2308.1.camel@koala>
Reply-To: dedekind1@gmail.com
Mime-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: QUOTED-PRINTABLE
Return-path: <linux-arm-msm-owner@vger.kernel.org>
Received: from mail-ey0-f174.google.com ([209.85.215.174]:49523 "EHLO
	mail-ey0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1751837Ab1DPIb5 (ORCPT
	<rfc822;linux-arm-msm@vger.kernel.org>);
	Sat, 16 Apr 2011 04:31:57 -0400
In-Reply-To: <1302942029.2308.1.camel@koala>
Sender: linux-arm-msm-owner@vger.kernel.org
List-Id: linux-arm-msm@vger.kernel.org
To: Murali Nalajala <mnalajal@codeaurora.org>
Cc: dwmw2@infradead.org, linux-mtd@lists.infradead.org, linux-arm-msm@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, Dima Zavin <dima@android.com>, Brian Swetland <swetland@google.com>, Arve =?ISO-8859-1?Q?Hj=F8nnev=E5g?= <arve@android.com>

On Sat, 2011-04-16 at 11:20 +0300, Artem Bityutskiy wrote:
> > > +#define MSM_NAND_REG(off) (msm_nand_phys + (off))
> > > +
> > > +#define MSM_NAND_FLASH_CMD            MSM_NAND_REG(0x0000)
> > > +#define MSM_NAND_ADDR0                MSM_NAND_REG(0x0004)
> > >
> > > Could you please make the macros to take the "struct msm_nand_chi=
p
> > > *chip" argument instead, and store the pase address there. Do not=
 hide
> > > the fact that those macros are actually functions, not constant -=
 this
> > > is error prone.
> > >
> > > Besides, I'm do not know your HW, but if you have several control=
lers
> > > with various base addresses - your driver won't work.
> >=20
> > you are correct, we have multiple controllers, which breaks this lo=
gic=20
> > in future.
>=20
> So then make your macros to accept the base address as an argument
> instead please.

Or better make macros to be constants, and always use something like
base + MSM_NAND_FLASH_CMD - this is the standard approach.
--=20
Best Regards,
Artem Bityutskiy (=D0=91=D0=B8=D1=82=D1=8E=D1=86=D0=BA=D0=B8=D0=B9 =D0=90=
=D1=80=D1=82=D1=91=D0=BC)