From: Krzysztof Kozlowski <krzk@kernel.org> To: Lukasz Luba <l.luba@partner.samsung.com> Cc: devicetree@vger.kernel.org, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, linux-pm@vger.kernel.org, "linux-samsung-soc@vger.kernel.org" <linux-samsung-soc@vger.kernel.org>, linux-arm-kernel@lists.infradead.org, "Bartłomiej Żołnierkiewicz" <b.zolnierkie@samsung.com>, kgene@kernel.org, mark.rutland@arm.com, robh+dt@kernel.org, "Chanwoo Choi" <cw00.choi@samsung.com>, kyungmin.park@samsung.com, "Marek Szyprowski" <m.szyprowski@samsung.com>, s.nawrocki@samsung.com, myungjoo.ham@samsung.com, willy.mh.wolff.ml@gmail.com Subject: Re: [PATCH v2 3/3] dt-bindings: ddr: Add bindings for Samsung LPDDR3 memories Date: Thu, 19 Sep 2019 09:28:18 +0200 [thread overview] Message-ID: <CAJKOXPfBueaf2UVik8x2L_D_PTxdkvRukuWL3xh_jDiLKtt7pQ@mail.gmail.com> (raw) In-Reply-To: <4a0d1d2c-cd1d-5df4-d4b1-f2dd1ef3bb72@partner.samsung.com> On Thu, 19 Sep 2019 at 08:49, Lukasz Luba <l.luba@partner.samsung.com> wrote: > > Hi Krzysztof, > > On 9/18/19 8:51 PM, Krzysztof Kozlowski wrote: > > On Mon, 16 Sep 2019 at 12:07, Lukasz Luba <l.luba@partner.samsung.com> wrote: > >> > >> Add compatible for Samsung k3qf2f20db LPDDR3 memory bindings. > >> Introduce minor fixes in the old documentation. > >> > >> Signed-off-by: Lukasz Luba <l.luba@partner.samsung.com> > >> --- > >> Documentation/devicetree/bindings/ddr/lpddr3.txt | 9 ++++++--- > >> 1 file changed, 6 insertions(+), 3 deletions(-) > >> > >> diff --git a/Documentation/devicetree/bindings/ddr/lpddr3.txt b/Documentation/devicetree/bindings/ddr/lpddr3.txt > >> index 3b2485b84b3f..49afe794daaa 100644 > >> --- a/Documentation/devicetree/bindings/ddr/lpddr3.txt > >> +++ b/Documentation/devicetree/bindings/ddr/lpddr3.txt > >> @@ -1,7 +1,9 @@ > >> * LPDDR3 SDRAM memories compliant to JEDEC JESD209-3C > >> > >> Required properties: > >> -- compatible : Should be - "jedec,lpddr3" > >> +- compatible : should be one of the following: > >> + Generic default - "jedec,lpddr3". > > > > The convention is first compatible, then description. I gave you the > > example to base on - at25. Why making it different? > > I have checked at25 that you pointed me to and also checked at24, which > has a bit longer "compatible" section. > > I found that there are many "jedec,spi-nor" compatible devices, which I > thought would be a better example for my "jedec,lpddr3". > For example, two configurations, where you have a single labels or dual > (with specific device) > arch/arm/boot/dts/imx6dl-rex-basic.dts: > compatible = "sst,sst25vf016b", "jedec,spi-nor"; > arch/arm/boot/dts/imx6q-ba16.dtsi: > compatible = "jedec,spi-nor"; > > The 'compatible' in documentation for the "jedec,spi-nor" is slightly > different (similar to at24). > Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt > It has a long explanation, which is also OK. So I thought that it is > quite flexible what you put in there. It is flexible but I see clear pattern in existing sources: jedec,spi-nor.txt compatible : May include a device-specific .. ... Supported chip names: at25df321a ... at25.txt: - compatible : Should be "<vendor>,<type>", and generic value "atmel,at25". Example "<vendor>,<type>" values: "anvo,anv32e61w" "microchip,25lc040" In these cases the doc says that "compatible should be" and then you have the list of values. Your example says that the compatible should be "Generic default" or "For Samsung 542x SoC"... :) The difference is slight but putting the value first is a simple and elegant solution. In your case one has to go to the end of sentence to find the most important information - the compatible value. > I have also checked Cadance QSPI controller. > Documentation/devicetree/bindings/mtd/cadence-quadspi.txt > The controller might be built-in into different vendor SoC's > and the "compatible" is ready to reflect it in similar fashion but > with a short explanation in this section. I see. I do not find this pattern as much readable as jedec-spi-nor or at25 therefore I mentioned them as an example to base on ("Exactly the same as AT24 or AT25 EEPROM bindings."). We can avoid also this entire discussion with YAML (which also follows approach of at25 - value first). > Therefore, what you see in the patch draw heavily on Cadence's qspi, > with a bit of inspiration from jedec,spi-nor usage. > > Should I change it to at25 "compatible" style and send next patch? Yes, please. Or go to YAML and make entire discussion obsolete. Best regards, Krzysztof
WARNING: multiple messages have this Message-ID (diff)
From: Krzysztof Kozlowski <krzk@kernel.org> To: Lukasz Luba <l.luba@partner.samsung.com> Cc: mark.rutland@arm.com, devicetree@vger.kernel.org, willy.mh.wolff.ml@gmail.com, "linux-samsung-soc@vger.kernel.org" <linux-samsung-soc@vger.kernel.org>, "Bartłomiej Żołnierkiewicz" <b.zolnierkie@samsung.com>, linux-pm@vger.kernel.org, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, robh+dt@kernel.org, "Chanwoo Choi" <cw00.choi@samsung.com>, kyungmin.park@samsung.com, kgene@kernel.org, myungjoo.ham@samsung.com, s.nawrocki@samsung.com, linux-arm-kernel@lists.infradead.org, "Marek Szyprowski" <m.szyprowski@samsung.com> Subject: Re: [PATCH v2 3/3] dt-bindings: ddr: Add bindings for Samsung LPDDR3 memories Date: Thu, 19 Sep 2019 09:28:18 +0200 [thread overview] Message-ID: <CAJKOXPfBueaf2UVik8x2L_D_PTxdkvRukuWL3xh_jDiLKtt7pQ@mail.gmail.com> (raw) In-Reply-To: <4a0d1d2c-cd1d-5df4-d4b1-f2dd1ef3bb72@partner.samsung.com> On Thu, 19 Sep 2019 at 08:49, Lukasz Luba <l.luba@partner.samsung.com> wrote: > > Hi Krzysztof, > > On 9/18/19 8:51 PM, Krzysztof Kozlowski wrote: > > On Mon, 16 Sep 2019 at 12:07, Lukasz Luba <l.luba@partner.samsung.com> wrote: > >> > >> Add compatible for Samsung k3qf2f20db LPDDR3 memory bindings. > >> Introduce minor fixes in the old documentation. > >> > >> Signed-off-by: Lukasz Luba <l.luba@partner.samsung.com> > >> --- > >> Documentation/devicetree/bindings/ddr/lpddr3.txt | 9 ++++++--- > >> 1 file changed, 6 insertions(+), 3 deletions(-) > >> > >> diff --git a/Documentation/devicetree/bindings/ddr/lpddr3.txt b/Documentation/devicetree/bindings/ddr/lpddr3.txt > >> index 3b2485b84b3f..49afe794daaa 100644 > >> --- a/Documentation/devicetree/bindings/ddr/lpddr3.txt > >> +++ b/Documentation/devicetree/bindings/ddr/lpddr3.txt > >> @@ -1,7 +1,9 @@ > >> * LPDDR3 SDRAM memories compliant to JEDEC JESD209-3C > >> > >> Required properties: > >> -- compatible : Should be - "jedec,lpddr3" > >> +- compatible : should be one of the following: > >> + Generic default - "jedec,lpddr3". > > > > The convention is first compatible, then description. I gave you the > > example to base on - at25. Why making it different? > > I have checked at25 that you pointed me to and also checked at24, which > has a bit longer "compatible" section. > > I found that there are many "jedec,spi-nor" compatible devices, which I > thought would be a better example for my "jedec,lpddr3". > For example, two configurations, where you have a single labels or dual > (with specific device) > arch/arm/boot/dts/imx6dl-rex-basic.dts: > compatible = "sst,sst25vf016b", "jedec,spi-nor"; > arch/arm/boot/dts/imx6q-ba16.dtsi: > compatible = "jedec,spi-nor"; > > The 'compatible' in documentation for the "jedec,spi-nor" is slightly > different (similar to at24). > Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt > It has a long explanation, which is also OK. So I thought that it is > quite flexible what you put in there. It is flexible but I see clear pattern in existing sources: jedec,spi-nor.txt compatible : May include a device-specific .. ... Supported chip names: at25df321a ... at25.txt: - compatible : Should be "<vendor>,<type>", and generic value "atmel,at25". Example "<vendor>,<type>" values: "anvo,anv32e61w" "microchip,25lc040" In these cases the doc says that "compatible should be" and then you have the list of values. Your example says that the compatible should be "Generic default" or "For Samsung 542x SoC"... :) The difference is slight but putting the value first is a simple and elegant solution. In your case one has to go to the end of sentence to find the most important information - the compatible value. > I have also checked Cadance QSPI controller. > Documentation/devicetree/bindings/mtd/cadence-quadspi.txt > The controller might be built-in into different vendor SoC's > and the "compatible" is ready to reflect it in similar fashion but > with a short explanation in this section. I see. I do not find this pattern as much readable as jedec-spi-nor or at25 therefore I mentioned them as an example to base on ("Exactly the same as AT24 or AT25 EEPROM bindings."). We can avoid also this entire discussion with YAML (which also follows approach of at25 - value first). > Therefore, what you see in the patch draw heavily on Cadence's qspi, > with a bit of inspiration from jedec,spi-nor usage. > > Should I change it to at25 "compatible" style and send next patch? Yes, please. Or go to YAML and make entire discussion obsolete. Best regards, Krzysztof _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2019-09-19 7:28 UTC|newest] Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top [not found] <CGME20190916100716eucas1p213ef29ba8bd288dfc6b5f05138c9a558@eucas1p2.samsung.com> 2019-09-16 10:07 ` [PATCH v2 0/3] Exynos5 DMC minor fixes Lukasz Luba 2019-09-16 10:07 ` Lukasz Luba [not found] ` <CGME20190916100717eucas1p1b8d24c74c4d0bb385aa3455cf98c76bd@eucas1p1.samsung.com> 2019-09-16 10:07 ` [PATCH v2 1/3] memory: Exynos5422: minor fixes in DMC Lukasz Luba 2019-09-16 10:07 ` Lukasz Luba 2019-09-18 18:55 ` Krzysztof Kozlowski 2019-09-18 18:55 ` Krzysztof Kozlowski [not found] ` <CGME20190916100718eucas1p1efcbabdf9dbe17a062ae83b8c19ac256@eucas1p1.samsung.com> 2019-09-16 10:07 ` [PATCH v2 2/3] ARM: dts: exynos: fix too long line in memory device Lukasz Luba 2019-09-16 10:07 ` Lukasz Luba 2019-09-18 18:55 ` Krzysztof Kozlowski 2019-09-18 18:55 ` Krzysztof Kozlowski [not found] ` <CGME20190916100719eucas1p206fe95982b774840b5d6e62ba9c42c79@eucas1p2.samsung.com> 2019-09-16 10:07 ` [PATCH v2 3/3] dt-bindings: ddr: Add bindings for Samsung LPDDR3 memories Lukasz Luba 2019-09-16 10:07 ` Lukasz Luba 2019-09-18 18:51 ` Krzysztof Kozlowski 2019-09-18 18:51 ` Krzysztof Kozlowski 2019-09-19 6:49 ` Lukasz Luba 2019-09-19 6:49 ` Lukasz Luba 2019-09-19 7:28 ` Krzysztof Kozlowski [this message] 2019-09-19 7:28 ` Krzysztof Kozlowski 2019-09-19 7:43 ` Lukasz Luba 2019-09-19 7:43 ` Lukasz Luba
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=CAJKOXPfBueaf2UVik8x2L_D_PTxdkvRukuWL3xh_jDiLKtt7pQ@mail.gmail.com \ --to=krzk@kernel.org \ --cc=b.zolnierkie@samsung.com \ --cc=cw00.choi@samsung.com \ --cc=devicetree@vger.kernel.org \ --cc=kgene@kernel.org \ --cc=kyungmin.park@samsung.com \ --cc=l.luba@partner.samsung.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-pm@vger.kernel.org \ --cc=linux-samsung-soc@vger.kernel.org \ --cc=m.szyprowski@samsung.com \ --cc=mark.rutland@arm.com \ --cc=myungjoo.ham@samsung.com \ --cc=robh+dt@kernel.org \ --cc=s.nawrocki@samsung.com \ --cc=willy.mh.wolff.ml@gmail.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.