From mboxrd@z Thu Jan 1 00:00:00 1970 Message-ID: <5289FB9C.6060204@ti.com> Date: Mon, 18 Nov 2013 13:35:56 +0200 From: Grygorii Strashko MIME-Version: 1.0 To: Santosh Shilimkar , "Khoronzhuk, Ivan" Subject: Re: [PATCH 10/12] mtd: nand: davinci: don't set timings if AEMIF is used References: <1384187188-5776-1-git-send-email-ivan.khoronzhuk@ti.com>, <1384187188-5776-11-git-send-email-ivan.khoronzhuk@ti.com> <4F5844B3A985794BA902E12C070812375F8D53@DNCE04.ent.ti.com> <528253B7.2060408@ti.com> In-Reply-To: <528253B7.2060408@ti.com> Content-Type: text/plain; charset="ISO-8859-1"; format=flowed Content-Transfer-Encoding: 7bit Cc: Mark Rutland , "devicetree@vger.kernel.org" , Russell King , Pawel Moll , Stephen Warren , Ian Campbell , Sekhar Nori , Kumar Gala , Rob Herring , "linux-kernel@vger.kernel.org" , "linux-mtd@lists.infradead.org" , Rob Landley , "linux-arm-kernel@lists.infradead.org" List-Id: Linux MTD discussion mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , On 11/12/2013 06:13 PM, Santosh Shilimkar wrote: > On Monday 11 November 2013 12:10 PM, Khoronzhuk, Ivan wrote: >> If Davinci AEMIF is used we don't need to set timings and bus width. >> It is done by AEMIF driver (drivers/memory/davinci-aemfi.c). >> >> Signed-off-by: Ivan Khoronzhuk >> --- >> drivers/mtd/nand/davinci_nand.c | 22 +++++++++++++++------- >> 1 file changed, 15 insertions(+), 7 deletions(-) >> >> diff --git a/drivers/mtd/nand/davinci_nand.c b/drivers/mtd/nand/davinci_nand.c >> index 4705214..879e915 100644 >> --- a/drivers/mtd/nand/davinci_nand.c >> +++ b/drivers/mtd/nand/davinci_nand.c >> @@ -742,27 +742,35 @@ static int __init nand_davinci_probe(struct platform_device *pdev) >> goto err_clk_enable; >> } >> >> +#if !IS_ENABLED(CONFIG_TI_DAVINCI_AEMIF) >> > Instead above #if, just use a variable. > bool aemif = IS_ENABLED(CONFIG_TI_DAVINCI_AEMIF) and then skip > the below code. #if block in the middle of the code looks ugly. Yes, this is the hack. The problem is that this part of code contains the call of Davinci platform function davinci_aemif_setup_timing() which is not accessible if Kernel is built for Keystone only. That's why "#if" has been used. This part of code has to be removed together with Davinci aemif platform code (aemif.c), once Davinci will be converted to DT and use new driver. The corresponding comment can be added here. Is it ok? > > Other than that patch looks fine to me. > > Regards, > Santosh > From mboxrd@z Thu Jan 1 00:00:00 1970 From: grygorii.strashko@ti.com (Grygorii Strashko) Date: Mon, 18 Nov 2013 13:35:56 +0200 Subject: [PATCH 10/12] mtd: nand: davinci: don't set timings if AEMIF is used In-Reply-To: <528253B7.2060408@ti.com> References: <1384187188-5776-1-git-send-email-ivan.khoronzhuk@ti.com>, <1384187188-5776-11-git-send-email-ivan.khoronzhuk@ti.com> <4F5844B3A985794BA902E12C070812375F8D53@DNCE04.ent.ti.com> <528253B7.2060408@ti.com> Message-ID: <5289FB9C.6060204@ti.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On 11/12/2013 06:13 PM, Santosh Shilimkar wrote: > On Monday 11 November 2013 12:10 PM, Khoronzhuk, Ivan wrote: >> If Davinci AEMIF is used we don't need to set timings and bus width. >> It is done by AEMIF driver (drivers/memory/davinci-aemfi.c). >> >> Signed-off-by: Ivan Khoronzhuk >> --- >> drivers/mtd/nand/davinci_nand.c | 22 +++++++++++++++------- >> 1 file changed, 15 insertions(+), 7 deletions(-) >> >> diff --git a/drivers/mtd/nand/davinci_nand.c b/drivers/mtd/nand/davinci_nand.c >> index 4705214..879e915 100644 >> --- a/drivers/mtd/nand/davinci_nand.c >> +++ b/drivers/mtd/nand/davinci_nand.c >> @@ -742,27 +742,35 @@ static int __init nand_davinci_probe(struct platform_device *pdev) >> goto err_clk_enable; >> } >> >> +#if !IS_ENABLED(CONFIG_TI_DAVINCI_AEMIF) >> > Instead above #if, just use a variable. > bool aemif = IS_ENABLED(CONFIG_TI_DAVINCI_AEMIF) and then skip > the below code. #if block in the middle of the code looks ugly. Yes, this is the hack. The problem is that this part of code contains the call of Davinci platform function davinci_aemif_setup_timing() which is not accessible if Kernel is built for Keystone only. That's why "#if" has been used. This part of code has to be removed together with Davinci aemif platform code (aemif.c), once Davinci will be converted to DT and use new driver. The corresponding comment can be added here. Is it ok? > > Other than that patch looks fine to me. > > Regards, > Santosh > From mboxrd@z Thu Jan 1 00:00:00 1970 From: Grygorii Strashko Subject: Re: [PATCH 10/12] mtd: nand: davinci: don't set timings if AEMIF is used Date: Mon, 18 Nov 2013 13:35:56 +0200 Message-ID: <5289FB9C.6060204@ti.com> References: <1384187188-5776-1-git-send-email-ivan.khoronzhuk@ti.com>,<1384187188-5776-11-git-send-email-ivan.khoronzhuk@ti.com> <4F5844B3A985794BA902E12C070812375F8D53@DNCE04.ent.ti.com> <528253B7.2060408@ti.com> Mime-Version: 1.0 Content-Type: text/plain; charset="ISO-8859-1"; format=flowed Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <528253B7.2060408-l0cyMroinI0@public.gmane.org> Sender: devicetree-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Santosh Shilimkar , "Khoronzhuk, Ivan" Cc: Rob Landley , Russell King , "devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" , Pawel Moll , Mark Rutland , Rob Herring , Stephen Warren , Kumar Gala , Ian Campbell , "linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" , "linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org" , "linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org" , Sekhar Nori List-Id: devicetree@vger.kernel.org On 11/12/2013 06:13 PM, Santosh Shilimkar wrote: > On Monday 11 November 2013 12:10 PM, Khoronzhuk, Ivan wrote: >> If Davinci AEMIF is used we don't need to set timings and bus width. >> It is done by AEMIF driver (drivers/memory/davinci-aemfi.c). >> >> Signed-off-by: Ivan Khoronzhuk >> --- >> drivers/mtd/nand/davinci_nand.c | 22 +++++++++++++++------- >> 1 file changed, 15 insertions(+), 7 deletions(-) >> >> diff --git a/drivers/mtd/nand/davinci_nand.c b/drivers/mtd/nand/davinci_nand.c >> index 4705214..879e915 100644 >> --- a/drivers/mtd/nand/davinci_nand.c >> +++ b/drivers/mtd/nand/davinci_nand.c >> @@ -742,27 +742,35 @@ static int __init nand_davinci_probe(struct platform_device *pdev) >> goto err_clk_enable; >> } >> >> +#if !IS_ENABLED(CONFIG_TI_DAVINCI_AEMIF) >> > Instead above #if, just use a variable. > bool aemif = IS_ENABLED(CONFIG_TI_DAVINCI_AEMIF) and then skip > the below code. #if block in the middle of the code looks ugly. Yes, this is the hack. The problem is that this part of code contains the call of Davinci platform function davinci_aemif_setup_timing() which is not accessible if Kernel is built for Keystone only. That's why "#if" has been used. This part of code has to be removed together with Davinci aemif platform code (aemif.c), once Davinci will be converted to DT and use new driver. The corresponding comment can be added here. Is it ok? > > Other than that patch looks fine to me. > > Regards, > Santosh > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751904Ab3KRLkL (ORCPT ); Mon, 18 Nov 2013 06:40:11 -0500 Received: from devils.ext.ti.com ([198.47.26.153]:50443 "EHLO devils.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751339Ab3KRLkD (ORCPT ); Mon, 18 Nov 2013 06:40:03 -0500 Message-ID: <5289FB9C.6060204@ti.com> Date: Mon, 18 Nov 2013 13:35:56 +0200 From: Grygorii Strashko User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.1.0 MIME-Version: 1.0 To: Santosh Shilimkar , "Khoronzhuk, Ivan" CC: Rob Landley , Russell King , "devicetree@vger.kernel.org" , Pawel Moll , Mark Rutland , Rob Herring , Stephen Warren , Kumar Gala , Ian Campbell , "linux-kernel@vger.kernel.org" , "linux-arm-kernel@lists.infradead.org" , "linux-mtd@lists.infradead.org" , Sekhar Nori Subject: Re: [PATCH 10/12] mtd: nand: davinci: don't set timings if AEMIF is used References: <1384187188-5776-1-git-send-email-ivan.khoronzhuk@ti.com>,<1384187188-5776-11-git-send-email-ivan.khoronzhuk@ti.com> <4F5844B3A985794BA902E12C070812375F8D53@DNCE04.ent.ti.com> <528253B7.2060408@ti.com> In-Reply-To: <528253B7.2060408@ti.com> Content-Type: text/plain; charset="ISO-8859-1"; format=flowed Content-Transfer-Encoding: 7bit X-Originating-IP: [10.167.145.75] X-EXCLAIMER-MD-CONFIG: f9c360f5-3d1e-4c3c-8703-f45bf52eff6b Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 11/12/2013 06:13 PM, Santosh Shilimkar wrote: > On Monday 11 November 2013 12:10 PM, Khoronzhuk, Ivan wrote: >> If Davinci AEMIF is used we don't need to set timings and bus width. >> It is done by AEMIF driver (drivers/memory/davinci-aemfi.c). >> >> Signed-off-by: Ivan Khoronzhuk >> --- >> drivers/mtd/nand/davinci_nand.c | 22 +++++++++++++++------- >> 1 file changed, 15 insertions(+), 7 deletions(-) >> >> diff --git a/drivers/mtd/nand/davinci_nand.c b/drivers/mtd/nand/davinci_nand.c >> index 4705214..879e915 100644 >> --- a/drivers/mtd/nand/davinci_nand.c >> +++ b/drivers/mtd/nand/davinci_nand.c >> @@ -742,27 +742,35 @@ static int __init nand_davinci_probe(struct platform_device *pdev) >> goto err_clk_enable; >> } >> >> +#if !IS_ENABLED(CONFIG_TI_DAVINCI_AEMIF) >> > Instead above #if, just use a variable. > bool aemif = IS_ENABLED(CONFIG_TI_DAVINCI_AEMIF) and then skip > the below code. #if block in the middle of the code looks ugly. Yes, this is the hack. The problem is that this part of code contains the call of Davinci platform function davinci_aemif_setup_timing() which is not accessible if Kernel is built for Keystone only. That's why "#if" has been used. This part of code has to be removed together with Davinci aemif platform code (aemif.c), once Davinci will be converted to DT and use new driver. The corresponding comment can be added here. Is it ok? > > Other than that patch looks fine to me. > > Regards, > Santosh >