From mboxrd@z Thu Jan 1 00:00:00 1970 From: Marek Vasut To: Krzysztof Kozlowski Subject: Re: [PATCH 11/11] dt: Exynos: add Snow SPI NOR node. Date: Thu, 4 Jun 2015 17:20:54 +0200 References: <3234c12c90eabbeeb6d33604a324231937e309ec.1433364398.git.hramrach@gmail.com> <556FB222.2070406@samsung.com> In-Reply-To: <556FB222.2070406@samsung.com> MIME-Version: 1.0 Content-Type: Text/Plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Message-Id: <201506041720.54666.marex@denx.de> Cc: Mark Rutland , Geert Uytterhoeven , linux-mtd@lists.infradead.org, Alison Chaiken , "Bean Huo =?utf-8?q?=E9=9C=8D=E6=96=8C=E6=96=8C?= \(beanhuo\)" , Michal Suchanek , Russell King , Vinod Koul , =?utf-8?q?Rafa=C5=82_Mi=C5=82ecki?= , linux-samsung-soc@vger.kernel.org, Kukjin Kim , Ben Hutchings , devicetree@vger.kernel.org, Pawel Moll , Ian Campbell , Kumar Gala , Mark Brown , Dan Williams , linux-arm-kernel@lists.infradead.org, pavel@denx.de, "grmoore@altera.com" , linux-kernel@vger.kernel.org, linux-spi@vger.kernel.org, Huang Shijie , Rob Herring , Han Xu , Knut Wohlrab , dmaengine@vger.kernel.org, Brian Norris , David Woodhouse List-Id: Linux MTD discussion mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , On Thursday, June 04, 2015 at 04:04:18 AM, Krzysztof Kozlowski wrote: > On 04.06.2015 06:26, Michal Suchanek wrote: > > The Snow has onboard SPI NOR flash which contains the bootloader. > > > > Add DT node for this flash chip. The flash is rated 133MHz but the pl330 > > controller can transfer only up to 128 bytes at this speed so use more > > conservative settings. Even at 40MHz pl330 can transfer at most 64k with > > the current driver. > > > > Signed-off-by: Michal Suchanek > > --- > > > > arch/arm/boot/dts/exynos5250-snow.dts | 12 ++++++++++++ > > 1 file changed, 12 insertions(+) > > > > diff --git a/arch/arm/boot/dts/exynos5250-snow.dts > > b/arch/arm/boot/dts/exynos5250-snow.dts index 1fa72cf..38e4cda 100644 > > --- a/arch/arm/boot/dts/exynos5250-snow.dts > > +++ b/arch/arm/boot/dts/exynos5250-snow.dts > > @@ -691,6 +691,18 @@ > > > > num-cs = <1>; > > cs-gpios = <&gpa2 5 0>; > > status = "okay"; > > > > + flash: m25p80@0 { > > The indentation looks odd. This should be at the same level as "status". > > > + #address-cells = <1>; > > + #size-cells = <1>; > > + compatible = "jedec,spi-nor"; > > + reg = <0>; > > + spi-max-frequency = <40000000>; > > So actually you wanted 133 MHz but as a workaround for DMA issue you use > 40 MHz, right? Could you add here a small TODO note in comment about it? I disagree. There is a problem, the problem should actually be analyzed and fixed, not just postponed with some TODO nonsense. Best regards, Marek Vasut From mboxrd@z Thu Jan 1 00:00:00 1970 From: Marek Vasut Subject: Re: [PATCH 11/11] dt: Exynos: add Snow SPI NOR node. Date: Thu, 4 Jun 2015 17:20:54 +0200 Message-ID: <201506041720.54666.marex@denx.de> References: <3234c12c90eabbeeb6d33604a324231937e309ec.1433364398.git.hramrach@gmail.com> <556FB222.2070406@samsung.com> Mime-Version: 1.0 Content-Type: Text/Plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <556FB222.2070406@samsung.com> Sender: linux-kernel-owner@vger.kernel.org To: Krzysztof Kozlowski Cc: Michal Suchanek , Rob Herring , Pawel Moll , Mark Rutland , Ian Campbell , Kumar Gala , Russell King , Kukjin Kim , Vinod Koul , Dan Williams , David Woodhouse , Brian Norris , Han Xu , Mark Brown , Geert Uytterhoeven , =?utf-8?q?Rafa=C5=82_Mi=C5=82ecki?= , Alison Chaiken , Huang Shijie , Ben Hutchings , Knut Wohlrab , "Bean Huo =?utf-8?q?=E9=9C=8D=E6=96=8C=E6=96=8C?= (beanhuo)" , grmoore@alter List-Id: linux-samsung-soc@vger.kernel.org On Thursday, June 04, 2015 at 04:04:18 AM, Krzysztof Kozlowski wrote: > On 04.06.2015 06:26, Michal Suchanek wrote: > > The Snow has onboard SPI NOR flash which contains the bootloader. > > > > Add DT node for this flash chip. The flash is rated 133MHz but the pl330 > > controller can transfer only up to 128 bytes at this speed so use more > > conservative settings. Even at 40MHz pl330 can transfer at most 64k with > > the current driver. > > > > Signed-off-by: Michal Suchanek > > --- > > > > arch/arm/boot/dts/exynos5250-snow.dts | 12 ++++++++++++ > > 1 file changed, 12 insertions(+) > > > > diff --git a/arch/arm/boot/dts/exynos5250-snow.dts > > b/arch/arm/boot/dts/exynos5250-snow.dts index 1fa72cf..38e4cda 100644 > > --- a/arch/arm/boot/dts/exynos5250-snow.dts > > +++ b/arch/arm/boot/dts/exynos5250-snow.dts > > @@ -691,6 +691,18 @@ > > > > num-cs = <1>; > > cs-gpios = <&gpa2 5 0>; > > status = "okay"; > > > > + flash: m25p80@0 { > > The indentation looks odd. This should be at the same level as "status". > > > + #address-cells = <1>; > > + #size-cells = <1>; > > + compatible = "jedec,spi-nor"; > > + reg = <0>; > > + spi-max-frequency = <40000000>; > > So actually you wanted 133 MHz but as a workaround for DMA issue you use > 40 MHz, right? Could you add here a small TODO note in comment about it? I disagree. There is a problem, the problem should actually be analyzed and fixed, not just postponed with some TODO nonsense. Best regards, Marek Vasut From mboxrd@z Thu Jan 1 00:00:00 1970 From: Marek Vasut Subject: Re: [PATCH 11/11] dt: Exynos: add Snow SPI NOR node. Date: Thu, 4 Jun 2015 17:20:54 +0200 Message-ID: <201506041720.54666.marex@denx.de> References: <3234c12c90eabbeeb6d33604a324231937e309ec.1433364398.git.hramrach@gmail.com> <556FB222.2070406@samsung.com> Mime-Version: 1.0 Content-Type: Text/Plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Cc: Michal Suchanek , Rob Herring , Pawel Moll , Mark Rutland , Ian Campbell , Kumar Gala , Russell King , Kukjin Kim , Vinod Koul , Dan Williams , David Woodhouse , Brian Norris , Han Xu , Mark Brown , Geert Uytterhoeven , =?utf-8?q?Rafa=C5=82_Mi=C5=82ecki?= , Alison Chaiken , Huang Shijie , Ben Hutchings , Knut Wohlrab , "Bean Huo =?utf-8?q?=E9=9C=8D=E6=96=8C=E6=96=8C?= (beanhuo)" , "grmoore@alter To: Krzysztof Kozlowski Return-path: In-Reply-To: <556FB222.2070406@samsung.com> Sender: linux-kernel-owner@vger.kernel.org List-Id: linux-spi.vger.kernel.org On Thursday, June 04, 2015 at 04:04:18 AM, Krzysztof Kozlowski wrote: > On 04.06.2015 06:26, Michal Suchanek wrote: > > The Snow has onboard SPI NOR flash which contains the bootloader. > > > > Add DT node for this flash chip. The flash is rated 133MHz but the pl330 > > controller can transfer only up to 128 bytes at this speed so use more > > conservative settings. Even at 40MHz pl330 can transfer at most 64k with > > the current driver. > > > > Signed-off-by: Michal Suchanek > > --- > > > > arch/arm/boot/dts/exynos5250-snow.dts | 12 ++++++++++++ > > 1 file changed, 12 insertions(+) > > > > diff --git a/arch/arm/boot/dts/exynos5250-snow.dts > > b/arch/arm/boot/dts/exynos5250-snow.dts index 1fa72cf..38e4cda 100644 > > --- a/arch/arm/boot/dts/exynos5250-snow.dts > > +++ b/arch/arm/boot/dts/exynos5250-snow.dts > > @@ -691,6 +691,18 @@ > > > > num-cs = <1>; > > cs-gpios = <&gpa2 5 0>; > > status = "okay"; > > > > + flash: m25p80@0 { > > The indentation looks odd. This should be at the same level as "status". > > > + #address-cells = <1>; > > + #size-cells = <1>; > > + compatible = "jedec,spi-nor"; > > + reg = <0>; > > + spi-max-frequency = <40000000>; > > So actually you wanted 133 MHz but as a workaround for DMA issue you use > 40 MHz, right? Could you add here a small TODO note in comment about it? I disagree. There is a problem, the problem should actually be analyzed and fixed, not just postponed with some TODO nonsense. Best regards, Marek Vasut From mboxrd@z Thu Jan 1 00:00:00 1970 From: marex@denx.de (Marek Vasut) Date: Thu, 4 Jun 2015 17:20:54 +0200 Subject: [PATCH 11/11] dt: Exynos: add Snow SPI NOR node. In-Reply-To: <556FB222.2070406@samsung.com> References: <3234c12c90eabbeeb6d33604a324231937e309ec.1433364398.git.hramrach@gmail.com> <556FB222.2070406@samsung.com> Message-ID: <201506041720.54666.marex@denx.de> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Thursday, June 04, 2015 at 04:04:18 AM, Krzysztof Kozlowski wrote: > On 04.06.2015 06:26, Michal Suchanek wrote: > > The Snow has onboard SPI NOR flash which contains the bootloader. > > > > Add DT node for this flash chip. The flash is rated 133MHz but the pl330 > > controller can transfer only up to 128 bytes at this speed so use more > > conservative settings. Even at 40MHz pl330 can transfer at most 64k with > > the current driver. > > > > Signed-off-by: Michal Suchanek > > --- > > > > arch/arm/boot/dts/exynos5250-snow.dts | 12 ++++++++++++ > > 1 file changed, 12 insertions(+) > > > > diff --git a/arch/arm/boot/dts/exynos5250-snow.dts > > b/arch/arm/boot/dts/exynos5250-snow.dts index 1fa72cf..38e4cda 100644 > > --- a/arch/arm/boot/dts/exynos5250-snow.dts > > +++ b/arch/arm/boot/dts/exynos5250-snow.dts > > @@ -691,6 +691,18 @@ > > > > num-cs = <1>; > > cs-gpios = <&gpa2 5 0>; > > status = "okay"; > > > > + flash: m25p80 at 0 { > > The indentation looks odd. This should be at the same level as "status". > > > + #address-cells = <1>; > > + #size-cells = <1>; > > + compatible = "jedec,spi-nor"; > > + reg = <0>; > > + spi-max-frequency = <40000000>; > > So actually you wanted 133 MHz but as a workaround for DMA issue you use > 40 MHz, right? Could you add here a small TODO note in comment about it? I disagree. There is a problem, the problem should actually be analyzed and fixed, not just postponed with some TODO nonsense. Best regards, Marek Vasut From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753534AbbFDPaF (ORCPT ); Thu, 4 Jun 2015 11:30:05 -0400 Received: from mail-out.m-online.net ([212.18.0.10]:51982 "EHLO mail-out.m-online.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752044AbbFDP3j (ORCPT ); Thu, 4 Jun 2015 11:29:39 -0400 X-Auth-Info: Gdc6EKqOLIma0HY8eM9Uro28gk2YCFjv8A94j5LbsDU= From: Marek Vasut To: Krzysztof Kozlowski Subject: Re: [PATCH 11/11] dt: Exynos: add Snow SPI NOR node. Date: Thu, 4 Jun 2015 17:20:54 +0200 User-Agent: KMail/1.13.7 (Linux/3.14-2-amd64; KDE/4.13.1; x86_64; ; ) Cc: Michal Suchanek , Rob Herring , Pawel Moll , Mark Rutland , Ian Campbell , Kumar Gala , Russell King , Kukjin Kim , Vinod Koul , Dan Williams , David Woodhouse , Brian Norris , Han Xu , Mark Brown , Geert Uytterhoeven , =?utf-8?q?Rafa=C5=82_Mi=C5=82ecki?= , Alison Chaiken , Huang Shijie , Ben Hutchings , Knut Wohlrab , "Bean Huo =?utf-8?q?=E9=9C=8D=E6=96=8C=E6=96=8C?= (beanhuo)" , "grmoore@altera.com" , devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-samsung-soc@vger.kernel.org, dmaengine@vger.kernel.org, linux-mtd@lists.infradead.org, linux-spi@vger.kernel.org, pavel@denx.de References: <3234c12c90eabbeeb6d33604a324231937e309ec.1433364398.git.hramrach@gmail.com> <556FB222.2070406@samsung.com> In-Reply-To: <556FB222.2070406@samsung.com> MIME-Version: 1.0 Content-Type: Text/Plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Message-Id: <201506041720.54666.marex@denx.de> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thursday, June 04, 2015 at 04:04:18 AM, Krzysztof Kozlowski wrote: > On 04.06.2015 06:26, Michal Suchanek wrote: > > The Snow has onboard SPI NOR flash which contains the bootloader. > > > > Add DT node for this flash chip. The flash is rated 133MHz but the pl330 > > controller can transfer only up to 128 bytes at this speed so use more > > conservative settings. Even at 40MHz pl330 can transfer at most 64k with > > the current driver. > > > > Signed-off-by: Michal Suchanek > > --- > > > > arch/arm/boot/dts/exynos5250-snow.dts | 12 ++++++++++++ > > 1 file changed, 12 insertions(+) > > > > diff --git a/arch/arm/boot/dts/exynos5250-snow.dts > > b/arch/arm/boot/dts/exynos5250-snow.dts index 1fa72cf..38e4cda 100644 > > --- a/arch/arm/boot/dts/exynos5250-snow.dts > > +++ b/arch/arm/boot/dts/exynos5250-snow.dts > > @@ -691,6 +691,18 @@ > > > > num-cs = <1>; > > cs-gpios = <&gpa2 5 0>; > > status = "okay"; > > > > + flash: m25p80@0 { > > The indentation looks odd. This should be at the same level as "status". > > > + #address-cells = <1>; > > + #size-cells = <1>; > > + compatible = "jedec,spi-nor"; > > + reg = <0>; > > + spi-max-frequency = <40000000>; > > So actually you wanted 133 MHz but as a workaround for DMA issue you use > 40 MHz, right? Could you add here a small TODO note in comment about it? I disagree. There is a problem, the problem should actually be analyzed and fixed, not just postponed with some TODO nonsense. Best regards, Marek Vasut