From mboxrd@z Thu Jan 1 00:00:00 1970 From: Krzysztof Kozlowski Subject: Re: [PATCH v4 1/4] Documentation: dt-bindings: Describe SROMc configuration Date: Fri, 30 Oct 2015 15:30:06 +0900 Message-ID: <56330E6E.50003@samsung.com> References: Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Return-path: In-reply-to: Sender: devicetree-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Pavel Fedin , devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org, linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org Cc: Rob Herring , Pawel Moll , Mark Rutland , Ian Campbell , Kumar Gala , Kukjin Kim List-Id: linux-samsung-soc@vger.kernel.org On 29.10.2015 21:42, Pavel Fedin wrote: > Add documentation for new subnode properties, allowing bank configuration. > Based on u-boot implementation, but heavily reworked. Please, carefully look at: Documentation/devicetree/bindings/net/gpmc-eth.txt Documentation/devicetree/bindings/bus/ti-gpmc.txt 1. Try to re-use existing bindings. Although I see existing bank-width and gpmc,device-width but yours samsung,srom-data-width seems better. Maybe there are other to re-use? 2. The array of "PMC, Tacp, Tcah, Tcoh, Tacc, Tcos, Tacs" is poorly extendable and not very descriptive/readable. You mapped directly device register which is the easier way for the driver but that is not the purpose of binding. 3. Please provide ranges for valid values and units. 4. PMC is not a timing. I doubt that TI GPMC could be used in Exynos SROM but please look at it and get useful stuff from it. Best regards, Krzysztof > > Signed-off-by: Pavel Fedin > --- > .../bindings/arm/samsung/exynos-srom.txt | 50 +++++++++++++++++++++- > 1 file changed, 48 insertions(+), 2 deletions(-) > > diff --git a/Documentation/devicetree/bindings/arm/samsung/exynos-srom.txt b/Documentation/devicetree/bindings/arm/samsung/exynos-srom.txt > index 33886d5..02ecc7f 100644 > --- a/Documentation/devicetree/bindings/arm/samsung/exynos-srom.txt > +++ b/Documentation/devicetree/bindings/arm/samsung/exynos-srom.txt > @@ -5,8 +5,54 @@ Required properties: > > - reg: offset and length of the register set > > -Example: > +- #address-cells, #size-cells : should be '1' if the device has sub-nodes > + with 'reg' property. > +- ranges: allows valid 1:1 translation between child's address space and > + parent's address space > + > +Sub-nodes: > +The SROM controller can be used to attach external peripherials. In this case > +device nodes should be added as subnodes to the SROMc node. These subnodes, > +except regular device specification, should contain the following properties, > +describing configuration of the relevant SROM bank: > + > +Required properties: > +- samsung,srom-bank : bank number (0 - 3) > + > +- samsung,srom-timing : array of 7 integers: PMC, Tacp, Tcah, Tcoh, Tacc, Tcos, > + Tacs > + > +Optional properties: > +- samsung,srom-data-width : data width in bytes (1 or 2). If omitted, default > + of 1 is used. > + > +Example: basic definition, no banks are configured > sromc@12570000 { > compatible = "samsung,exynos-srom"; > - reg = <0x12570000 0x10>; > + reg = <0x12570000 0x14>; > + }; > + > +Example: SROMc with smsc 911x ethernet chip on bank 3 > + sromc@12570000 { > + #address-cells = <1>; > + #size-cells = <1>; > + ranges; > + > + compatible = "samsung,exynos-srom"; > + reg = <0x12570000 0x14>; > + > + ethernet@07000000 { > + compatible = "smsc,lan9115"; > + reg = <0x07000000 0x10000>; > + phy-mode = "mii"; > + interrupt-parent = <&gpx0>; > + interrupts = <5 8>; > + reg-io-width = <2>; > + smsc,irq-push-pull; > + smsc,force-internal-phy; > + > + samsung,srom-bank = <3>; > + samsung,srom-data-width = <2>; > + samsung,srom-timing = <1 9 12 1 9 1 1>; > + }; > }; > -- 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 From: k.kozlowski@samsung.com (Krzysztof Kozlowski) Date: Fri, 30 Oct 2015 15:30:06 +0900 Subject: [PATCH v4 1/4] Documentation: dt-bindings: Describe SROMc configuration In-Reply-To: References: Message-ID: <56330E6E.50003@samsung.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On 29.10.2015 21:42, Pavel Fedin wrote: > Add documentation for new subnode properties, allowing bank configuration. > Based on u-boot implementation, but heavily reworked. Please, carefully look at: Documentation/devicetree/bindings/net/gpmc-eth.txt Documentation/devicetree/bindings/bus/ti-gpmc.txt 1. Try to re-use existing bindings. Although I see existing bank-width and gpmc,device-width but yours samsung,srom-data-width seems better. Maybe there are other to re-use? 2. The array of "PMC, Tacp, Tcah, Tcoh, Tacc, Tcos, Tacs" is poorly extendable and not very descriptive/readable. You mapped directly device register which is the easier way for the driver but that is not the purpose of binding. 3. Please provide ranges for valid values and units. 4. PMC is not a timing. I doubt that TI GPMC could be used in Exynos SROM but please look at it and get useful stuff from it. Best regards, Krzysztof > > Signed-off-by: Pavel Fedin > --- > .../bindings/arm/samsung/exynos-srom.txt | 50 +++++++++++++++++++++- > 1 file changed, 48 insertions(+), 2 deletions(-) > > diff --git a/Documentation/devicetree/bindings/arm/samsung/exynos-srom.txt b/Documentation/devicetree/bindings/arm/samsung/exynos-srom.txt > index 33886d5..02ecc7f 100644 > --- a/Documentation/devicetree/bindings/arm/samsung/exynos-srom.txt > +++ b/Documentation/devicetree/bindings/arm/samsung/exynos-srom.txt > @@ -5,8 +5,54 @@ Required properties: > > - reg: offset and length of the register set > > -Example: > +- #address-cells, #size-cells : should be '1' if the device has sub-nodes > + with 'reg' property. > +- ranges: allows valid 1:1 translation between child's address space and > + parent's address space > + > +Sub-nodes: > +The SROM controller can be used to attach external peripherials. In this case > +device nodes should be added as subnodes to the SROMc node. These subnodes, > +except regular device specification, should contain the following properties, > +describing configuration of the relevant SROM bank: > + > +Required properties: > +- samsung,srom-bank : bank number (0 - 3) > + > +- samsung,srom-timing : array of 7 integers: PMC, Tacp, Tcah, Tcoh, Tacc, Tcos, > + Tacs > + > +Optional properties: > +- samsung,srom-data-width : data width in bytes (1 or 2). If omitted, default > + of 1 is used. > + > +Example: basic definition, no banks are configured > sromc at 12570000 { > compatible = "samsung,exynos-srom"; > - reg = <0x12570000 0x10>; > + reg = <0x12570000 0x14>; > + }; > + > +Example: SROMc with smsc 911x ethernet chip on bank 3 > + sromc at 12570000 { > + #address-cells = <1>; > + #size-cells = <1>; > + ranges; > + > + compatible = "samsung,exynos-srom"; > + reg = <0x12570000 0x14>; > + > + ethernet at 07000000 { > + compatible = "smsc,lan9115"; > + reg = <0x07000000 0x10000>; > + phy-mode = "mii"; > + interrupt-parent = <&gpx0>; > + interrupts = <5 8>; > + reg-io-width = <2>; > + smsc,irq-push-pull; > + smsc,force-internal-phy; > + > + samsung,srom-bank = <3>; > + samsung,srom-data-width = <2>; > + samsung,srom-timing = <1 9 12 1 9 1 1>; > + }; > }; > From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758732AbbJ3GaU (ORCPT ); Fri, 30 Oct 2015 02:30:20 -0400 Received: from mailout4.w1.samsung.com ([210.118.77.14]:49426 "EHLO mailout4.w1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1758568AbbJ3GaS (ORCPT ); Fri, 30 Oct 2015 02:30:18 -0400 X-AuditID: cbfec7f4-f79c56d0000012ee-19-56330e77e278 Subject: Re: [PATCH v4 1/4] Documentation: dt-bindings: Describe SROMc configuration To: Pavel Fedin , devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-samsung-soc@vger.kernel.org, linux-kernel@vger.kernel.org References: Cc: Rob Herring , Pawel Moll , Mark Rutland , Ian Campbell , Kumar Gala , Kukjin Kim From: Krzysztof Kozlowski X-Enigmail-Draft-Status: N1110 Message-id: <56330E6E.50003@samsung.com> Date: Fri, 30 Oct 2015 15:30:06 +0900 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 MIME-version: 1.0 In-reply-to: Content-type: text/plain; charset=windows-1252 Content-transfer-encoding: 7bit X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFupgkeLIzCtJLcpLzFFi42I5/e/4Nd1yPuMwg9nv9CzmHznHatH/ZiGr xblXKxktXr8wtOh//JrZYtPja6wWl3fNYbOYcX4fk8XS6xeZLF5dWsVmMWH6WhaL1r1H2B14 PNbMW8Pocbmvl8lj5fIvbB6bVnWyeWxeUu/Rt2UVo8fnTXIB7FFcNimpOZllqUX6dglcGVf/ zGIrOCxZcfP3E9YGxo0iXYycHBICJhL7Vr1ghbDFJC7cW8/WxcjFISSwlFFi+pUDbCAJIYEv jBId18NAbGGBMIktRzrYQYpEBNYwSiw/d5ARoqiNUeL+9EKQBLPAAyD7chNYgk3AWGLz8iVs ECvkJHq7J7GA2LwCGhJdWw+CxVkEVCXa7kxjArFFBSIkJk5oYIWoEZT4MfkeWD2nQIzEhvkH geIcQAv0JO5f1AIJMwvIS2xe85Z5AqPgLCQdsxCqZiGpWsDIvIpRNLU0uaA4KT3XUK84Mbe4 NC9dLzk/dxMjJH6+7GBcfMzqEKMAB6MSD++PBKMwIdbEsuLK3EOMEhzMSiK8vy8BhXhTEiur Uovy44tKc1KLDzFKc7AoifPO3fU+REggPbEkNTs1tSC1CCbLxMEp1cCo3N8QXvqykvHzZVdb 609XmLKLKwyOb1Q5WXcj+6+u7be6jhsbjd+sXhgyYeNDjUUF+wP2PjCeIxJ3u6XEY4ma8i/f e+XbvkcbaQs8sKgQMhPkWFHy3fZpiA7Pwwcn+fr6j6kb7zB8s1DFzt3044/42t+SXx+yLdI+ /NatzEyDTfJA9x795eVKLMUZiYZazEXFiQCliSIVmwIAAA== Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 29.10.2015 21:42, Pavel Fedin wrote: > Add documentation for new subnode properties, allowing bank configuration. > Based on u-boot implementation, but heavily reworked. Please, carefully look at: Documentation/devicetree/bindings/net/gpmc-eth.txt Documentation/devicetree/bindings/bus/ti-gpmc.txt 1. Try to re-use existing bindings. Although I see existing bank-width and gpmc,device-width but yours samsung,srom-data-width seems better. Maybe there are other to re-use? 2. The array of "PMC, Tacp, Tcah, Tcoh, Tacc, Tcos, Tacs" is poorly extendable and not very descriptive/readable. You mapped directly device register which is the easier way for the driver but that is not the purpose of binding. 3. Please provide ranges for valid values and units. 4. PMC is not a timing. I doubt that TI GPMC could be used in Exynos SROM but please look at it and get useful stuff from it. Best regards, Krzysztof > > Signed-off-by: Pavel Fedin > --- > .../bindings/arm/samsung/exynos-srom.txt | 50 +++++++++++++++++++++- > 1 file changed, 48 insertions(+), 2 deletions(-) > > diff --git a/Documentation/devicetree/bindings/arm/samsung/exynos-srom.txt b/Documentation/devicetree/bindings/arm/samsung/exynos-srom.txt > index 33886d5..02ecc7f 100644 > --- a/Documentation/devicetree/bindings/arm/samsung/exynos-srom.txt > +++ b/Documentation/devicetree/bindings/arm/samsung/exynos-srom.txt > @@ -5,8 +5,54 @@ Required properties: > > - reg: offset and length of the register set > > -Example: > +- #address-cells, #size-cells : should be '1' if the device has sub-nodes > + with 'reg' property. > +- ranges: allows valid 1:1 translation between child's address space and > + parent's address space > + > +Sub-nodes: > +The SROM controller can be used to attach external peripherials. In this case > +device nodes should be added as subnodes to the SROMc node. These subnodes, > +except regular device specification, should contain the following properties, > +describing configuration of the relevant SROM bank: > + > +Required properties: > +- samsung,srom-bank : bank number (0 - 3) > + > +- samsung,srom-timing : array of 7 integers: PMC, Tacp, Tcah, Tcoh, Tacc, Tcos, > + Tacs > + > +Optional properties: > +- samsung,srom-data-width : data width in bytes (1 or 2). If omitted, default > + of 1 is used. > + > +Example: basic definition, no banks are configured > sromc@12570000 { > compatible = "samsung,exynos-srom"; > - reg = <0x12570000 0x10>; > + reg = <0x12570000 0x14>; > + }; > + > +Example: SROMc with smsc 911x ethernet chip on bank 3 > + sromc@12570000 { > + #address-cells = <1>; > + #size-cells = <1>; > + ranges; > + > + compatible = "samsung,exynos-srom"; > + reg = <0x12570000 0x14>; > + > + ethernet@07000000 { > + compatible = "smsc,lan9115"; > + reg = <0x07000000 0x10000>; > + phy-mode = "mii"; > + interrupt-parent = <&gpx0>; > + interrupts = <5 8>; > + reg-io-width = <2>; > + smsc,irq-push-pull; > + smsc,force-internal-phy; > + > + samsung,srom-bank = <3>; > + samsung,srom-data-width = <2>; > + samsung,srom-timing = <1 9 12 1 9 1 1>; > + }; > }; >