From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from ch1ehsobe002.messaging.microsoft.com ([216.32.181.182] helo=ch1outboundpool.messaging.microsoft.com) by merlin.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1UXmYM-0001Xz-3U for linux-mtd@lists.infradead.org; Thu, 02 May 2013 05:59:39 +0000 Message-ID: <51820127.5060003@freescale.com> Date: Thu, 2 May 2013 14:01:11 +0800 From: Huang Shijie MIME-Version: 1.0 To: "Gupta, Pekon" Subject: Re: [PATCH V4 5/9] mtd: replace the hardcode with the onfi_feature() References: <1366967337-5534-1-git-send-email-b32955@freescale.com> <1366967337-5534-6-git-send-email-b32955@freescale.com> <20980858CB6D3A4BAE95CA194937D5E73E9BF544@DBDE04.ent.ti.com> <5181CC19.8000904@freescale.com> <20980858CB6D3A4BAE95CA194937D5E73E9C09FC@DBDE04.ent.ti.com> In-Reply-To: <20980858CB6D3A4BAE95CA194937D5E73E9C09FC@DBDE04.ent.ti.com> Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: quoted-printable Cc: "dwmw2@infradead.org" , "computersforpeace@gmail.com" , "linux-mtd@lists.infradead.org" , "linux-kernel@vger.kernel.org" , "dedekind1@gmail.com" List-Id: Linux MTD discussion mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , =E4=BA=8E 2013=E5=B9=B405=E6=9C=8802=E6=97=A5 13:42, Gupta, Pekon =E5=86=99= =E9=81=93: >>>> - *busw =3D 0; >>>> - if (le16_to_cpu(p->features)& 1) >>>> - *busw =3D NAND_BUSWIDTH_16; >>>> + >>>> + *busw =3D (onfi_feature(chip)& ONFI_FEATURE_16_BIT_BUS) ? >>>> + NAND_BUSWIDTH_16 : 0; >>> Is this really needed ? you have already checked the 'onfi_version' >> above in >>> nand_flash_detect_onfi() .. >>> if (!chip->onfi_version) { >>> pr_info("%s: unsupported ONFI version: %d\n", >> __func__, val); >>> return 0; >>> } >>> >>> >> I think checking the onfi_version has no relationship with this patch.= :) >> This patch is just replace the hardcode for 16-bit onfi nand check. >> > [Pekon]: [Patch 3/9]: add a helper to get the supported features > I mean, do you really need this helper function ? > +static inline int onfi_feature(struct nand_chip *chip) > +{ > + return chip->onfi_version ? le16_to_cpu(chip->onfi_params.featu= res) : 0; > + } > > Following change should have been enough.. > *busw =3D (le16_to_cpu(p->features)& ONFI_FEATURE_16_BIT_BUS) ? > NAND_BUSWIDTH_16 : 0; yes. for this function, it's enough. But we may use the onfi_feature() in other places, such as some drivers. = That's reason why i added this helper. thanks Huang Shijie