* Re: [PATCH 1/3] doc: dt: add documentation for Mediatek spi-nor controller
2015-09-08 9:49 ` [PATCH 1/3] doc: dt: add documentation for Mediatek spi-nor controller Bayi Cheng
@ 2015-09-08 11:34 ` Jagan Teki
2015-09-15 6:49 ` bayi.cheng
2015-09-09 5:47 ` Sascha Hauer
` (2 subsequent siblings)
3 siblings, 1 reply; 10+ messages in thread
From: Jagan Teki @ 2015-09-08 11:34 UTC (permalink / raw)
To: Bayi Cheng
Cc: David Woodhouse, Brian Norris, Mark Rutland, devicetree,
Pawel Moll, Ian Campbell, Sascha Hauer,
linux-kernel@vger.kernel.org, Daniel Kurtz, Rob Herring,
linux-mediatek, Kumar Gala, Matthias Brugger,
linux-mtd@lists.infradead.org,
linux-arm-kernel@lists.infradead.org
On 8 September 2015 at 15:19, Bayi Cheng <bayi.cheng@mediatek.com> wrote:
> Add device tree binding documentation for serial flash with
> Mediatek serial flash controller
>
> Signed-off-by: Bayi Cheng <bayi.cheng@mediatek.com>
> ---
> Documentation/devicetree/bindings/mtd/mtk_nor.txt | 25 +++++++++++++++++++++++
> 1 file changed, 25 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mtd/mtk_nor.txt
>
> diff --git a/Documentation/devicetree/bindings/mtd/mtk_nor.txt b/Documentation/devicetree/bindings/mtd/mtk_nor.txt
> new file mode 100644
> index 0000000..0eca0cd
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mtd/mtk_nor.txt
IMHO - looks like the name of the driver file not resembles controller
driver, usually spi-nor framework has a name include 'nor' and related
controller drivers uses simple notion with vendor/soc_name-quadspi or
something similar.
cadence-quadspi.c
fsl-quadspi.c
Simple suggestion - mtk-quadspi.c (since it supports quad)
> @@ -0,0 +1,25 @@
> +* MTD SPI nor driver for MTK MT81xx (and similar) serial flash controller
> +
> +MTK MT81xx serial flash controller is designed for serial Flash device.
> +It supports one Flash device with signal mode, dual mode and quad mode.
> +
> +Required properties:
> +- compatible: should be "mediatek,mt8173-nor";
> +- reg: physical base address and length of the controller's register
> +- clocks: spi nor source clock
> +- clock-names: "spi_clk", "axi_clk", "mux_clk", "sf_clk"
> +
> +See Documentation/devicetree/bindings/clock/clock-bindings.txt
As this related to clock-names, just add this line in-continuous with
clock-name property description.
> +and Documentation/mtd/spi-nor.txt for details.
This explicit mentioning about spi-nor documentation is may not
required as this .txt will describe how spi-nor works and not related
to bindings.
> +
> +Example:
> +nor_flash: nor@1100d000 {
> + compatible = "mediatek,mt8173-nor";
> + reg = <0 0x1100d000 0 0xe0>;
> + clocks = <&pericfg CLK_PERI_SPI>,
> + <&topckgen CLK_TOP_AXI_SEL>,
> + <&topckgen CLK_TOP_UNIVPLL2_D8>,
> + <&topckgen CLK_TOP_SPINFI_IFR_SEL>;
> + clock-names = "spi_clk", "axi_clk", "mux_clk", "sf_clk";
> +};
> +
> --
> 1.8.1.1.dirty
>
thanks!
--
Jagan | openedev.
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH 1/3] doc: dt: add documentation for Mediatek spi-nor controller
2015-09-08 11:34 ` Jagan Teki
@ 2015-09-15 6:49 ` bayi.cheng
0 siblings, 0 replies; 10+ messages in thread
From: bayi.cheng @ 2015-09-15 6:49 UTC (permalink / raw)
To: Jagan Teki
Cc: Mark Rutland, devicetree, Pawel Moll, Ian Campbell, Sascha Hauer,
linux-kernel@vger.kernel.org, Rob Herring, linux-mediatek,
Kumar Gala, Matthias Brugger, linux-mtd@lists.infradead.org,
Brian Norris, David Woodhouse,
linux-arm-kernel@lists.infradead.org
On Tue, 2015-09-08 at 17:04 +0530, Jagan Teki wrote:
> On 8 September 2015 at 15:19, Bayi Cheng <bayi.cheng@mediatek.com> wrote:
> > Add device tree binding documentation for serial flash with
> > Mediatek serial flash controller
> >
> > Signed-off-by: Bayi Cheng <bayi.cheng@mediatek.com>
> > ---
> > Documentation/devicetree/bindings/mtd/mtk_nor.txt | 25 +++++++++++++++++++++++
> > 1 file changed, 25 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/mtd/mtk_nor.txt
> >
> > diff --git a/Documentation/devicetree/bindings/mtd/mtk_nor.txt b/Documentation/devicetree/bindings/mtd/mtk_nor.txt
> > new file mode 100644
> > index 0000000..0eca0cd
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mtd/mtk_nor.txt
>
> IMHO - looks like the name of the driver file not resembles controller
> driver, usually spi-nor framework has a name include 'nor' and related
> controller drivers uses simple notion with vendor/soc_name-quadspi or
> something similar.
>
> cadence-quadspi.c
> fsl-quadspi.c
>
> Simple suggestion - mtk-quadspi.c (since it supports quad)
Ok, Thanks for your instruct! I will change the name of the driver file
from mtk_nor.c to mtk-quadspi.c . at the same time, change the
mtk_nor.txt to mtk-quadspi.txt .
>
> > @@ -0,0 +1,25 @@
> > +* MTD SPI nor driver for MTK MT81xx (and similar) serial flash controller
> > +
> > +MTK MT81xx serial flash controller is designed for serial Flash device.
> > +It supports one Flash device with signal mode, dual mode and quad mode.
> > +
> > +Required properties:
> > +- compatible: should be "mediatek,mt8173-nor";
> > +- reg: physical base address and length of the controller's register
> > +- clocks: spi nor source clock
> > +- clock-names: "spi_clk", "axi_clk", "mux_clk", "sf_clk"
> > +
> > +See Documentation/devicetree/bindings/clock/clock-bindings.txt
>
> As this related to clock-names, just add this line in-continuous with
> clock-name property description.
>
OK, I will change it.
> > +and Documentation/mtd/spi-nor.txt for details.
>
> This explicit mentioning about spi-nor documentation is may not
> required as this .txt will describe how spi-nor works and not related
> to bindings.
>
yes, you are right, and I will drop this line
> > +
> > +Example:
> > +nor_flash: nor@1100d000 {
> > + compatible = "mediatek,mt8173-nor";
> > + reg = <0 0x1100d000 0 0xe0>;
> > + clocks = <&pericfg CLK_PERI_SPI>,
> > + <&topckgen CLK_TOP_AXI_SEL>,
> > + <&topckgen CLK_TOP_UNIVPLL2_D8>,
> > + <&topckgen CLK_TOP_SPINFI_IFR_SEL>;
> > + clock-names = "spi_clk", "axi_clk", "mux_clk", "sf_clk";
> > +};
> > +
> > --
> > 1.8.1.1.dirty
> >
>
> thanks!
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] doc: dt: add documentation for Mediatek spi-nor controller
2015-09-08 9:49 ` [PATCH 1/3] doc: dt: add documentation for Mediatek spi-nor controller Bayi Cheng
2015-09-08 11:34 ` Jagan Teki
@ 2015-09-09 5:47 ` Sascha Hauer
2015-09-15 6:39 ` bayi.cheng
2015-09-11 21:47 ` Brian Norris
2015-09-11 21:49 ` Brian Norris
3 siblings, 1 reply; 10+ messages in thread
From: Sascha Hauer @ 2015-09-09 5:47 UTC (permalink / raw)
To: Bayi Cheng
Cc: David Woodhouse, Brian Norris, Rob Herring, Pawel Moll,
Mark Rutland, Ian Campbell, Kumar Gala, Matthias Brugger,
Daniel Kurtz, devicetree, linux-arm-kernel, linux-mediatek,
linux-kernel, linux-mtd
On Tue, Sep 08, 2015 at 05:49:54PM +0800, Bayi Cheng wrote:
> Add device tree binding documentation for serial flash with
> Mediatek serial flash controller
>
> Signed-off-by: Bayi Cheng <bayi.cheng@mediatek.com>
> ---
> Documentation/devicetree/bindings/mtd/mtk_nor.txt | 25 +++++++++++++++++++++++
> 1 file changed, 25 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mtd/mtk_nor.txt
>
> diff --git a/Documentation/devicetree/bindings/mtd/mtk_nor.txt b/Documentation/devicetree/bindings/mtd/mtk_nor.txt
> new file mode 100644
> index 0000000..0eca0cd
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mtd/mtk_nor.txt
> @@ -0,0 +1,25 @@
> +* MTD SPI nor driver for MTK MT81xx (and similar) serial flash controller
> +
> +MTK MT81xx serial flash controller is designed for serial Flash device.
> +It supports one Flash device with signal mode, dual mode and quad mode.
> +
> +Required properties:
> +- compatible: should be "mediatek,mt8173-nor";
> +- reg: physical base address and length of the controller's register
> +- clocks: spi nor source clock
> +- clock-names: "spi_clk", "axi_clk", "mux_clk", "sf_clk"
Drop the _clk suffix. It's the clock-names property which already makes
it clear that these are clock names.
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] doc: dt: add documentation for Mediatek spi-nor controller
2015-09-09 5:47 ` Sascha Hauer
@ 2015-09-15 6:39 ` bayi.cheng
0 siblings, 0 replies; 10+ messages in thread
From: bayi.cheng @ 2015-09-15 6:39 UTC (permalink / raw)
To: Sascha Hauer
Cc: David Woodhouse, Brian Norris, Rob Herring, Pawel Moll,
Mark Rutland, Ian Campbell, Kumar Gala, Matthias Brugger,
Daniel Kurtz, devicetree, linux-arm-kernel, linux-mediatek,
linux-kernel, linux-mtd
On Wed, 2015-09-09 at 07:47 +0200, Sascha Hauer wrote:
> On Tue, Sep 08, 2015 at 05:49:54PM +0800, Bayi Cheng wrote:
> > Add device tree binding documentation for serial flash with
> > Mediatek serial flash controller
> >
> > Signed-off-by: Bayi Cheng <bayi.cheng@mediatek.com>
> > ---
> > Documentation/devicetree/bindings/mtd/mtk_nor.txt | 25 +++++++++++++++++++++++
> > 1 file changed, 25 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/mtd/mtk_nor.txt
> >
> > diff --git a/Documentation/devicetree/bindings/mtd/mtk_nor.txt b/Documentation/devicetree/bindings/mtd/mtk_nor.txt
> > new file mode 100644
> > index 0000000..0eca0cd
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mtd/mtk_nor.txt
> > @@ -0,0 +1,25 @@
> > +* MTD SPI nor driver for MTK MT81xx (and similar) serial flash controller
> > +
> > +MTK MT81xx serial flash controller is designed for serial Flash device.
> > +It supports one Flash device with signal mode, dual mode and quad mode.
> > +
> > +Required properties:
> > +- compatible: should be "mediatek,mt8173-nor";
> > +- reg: physical base address and length of the controller's register
> > +- clocks: spi nor source clock
> > +- clock-names: "spi_clk", "axi_clk", "mux_clk", "sf_clk"
>
> Drop the _clk suffix. It's the clock-names property which already makes
> it clear that these are clock names.
>
> Sascha
>
>
OK, I will drop the _clk suffix here!
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] doc: dt: add documentation for Mediatek spi-nor controller
2015-09-08 9:49 ` [PATCH 1/3] doc: dt: add documentation for Mediatek spi-nor controller Bayi Cheng
2015-09-08 11:34 ` Jagan Teki
2015-09-09 5:47 ` Sascha Hauer
@ 2015-09-11 21:47 ` Brian Norris
2015-09-11 21:49 ` Brian Norris
3 siblings, 0 replies; 10+ messages in thread
From: Brian Norris @ 2015-09-11 21:47 UTC (permalink / raw)
To: Bayi Cheng
Cc: David Woodhouse, Rob Herring, Pawel Moll, Mark Rutland,
Ian Campbell, Kumar Gala, Matthias Brugger, Daniel Kurtz,
Sascha Hauer, devicetree, linux-arm-kernel, linux-mediatek,
linux-kernel, linux-mtd, Marek Vasut
On Tue, Sep 08, 2015 at 05:49:54PM +0800, Bayi Cheng wrote:
> Add device tree binding documentation for serial flash with
> Mediatek serial flash controller
>
> Signed-off-by: Bayi Cheng <bayi.cheng@mediatek.com>
> ---
> Documentation/devicetree/bindings/mtd/mtk_nor.txt | 25 +++++++++++++++++++++++
> 1 file changed, 25 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mtd/mtk_nor.txt
>
> diff --git a/Documentation/devicetree/bindings/mtd/mtk_nor.txt b/Documentation/devicetree/bindings/mtd/mtk_nor.txt
> new file mode 100644
> index 0000000..0eca0cd
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mtd/mtk_nor.txt
> @@ -0,0 +1,25 @@
> +* MTD SPI nor driver for MTK MT81xx (and similar) serial flash controller
> +
> +MTK MT81xx serial flash controller is designed for serial Flash device.
> +It supports one Flash device with signal mode, dual mode and quad mode.
> +
> +Required properties:
> +- compatible: should be "mediatek,mt8173-nor";
> +- reg: physical base address and length of the controller's register
> +- clocks: spi nor source clock
> +- clock-names: "spi_clk", "axi_clk", "mux_clk", "sf_clk"
> +
> +See Documentation/devicetree/bindings/clock/clock-bindings.txt
> +and Documentation/mtd/spi-nor.txt for details.
> +
> +Example:
> +nor_flash: nor@1100d000 {
> + compatible = "mediatek,mt8173-nor";
> + reg = <0 0x1100d000 0 0xe0>;
> + clocks = <&pericfg CLK_PERI_SPI>,
> + <&topckgen CLK_TOP_AXI_SEL>,
> + <&topckgen CLK_TOP_UNIVPLL2_D8>,
> + <&topckgen CLK_TOP_SPINFI_IFR_SEL>;
> + clock-names = "spi_clk", "axi_clk", "mux_clk", "sf_clk";
> +};
I understand that for now, you only support a single flash, and you
don't need any extra flash-specific DT properties, but in the interest
of being more generic and more in-line with other drivers, can you
include:
* #addres-cells (= <1>) and #size-cells (= <0>) properties
* sub-node(s) representing the flash; reference [1], and there's a
good example in a recent submission [2]
So I'd expect something like:
nor_flash: nor@1100d000 {
compatible = "mediatek,mt8173-nor";
...
#address-cells = <1>;
#size-cells = <0>;
flash@0 {
compatible = "jedec,spi-nor";
reg = <0>;
...
};
};
This patch is also relevant [3] (hopefully I'll get to merge that one
soon); you'll want to use the sub-node (not the main node) when
initializing the flash device.
I think maybe we'll want to codify some of this in a "SPI NOR
controller" document, so we can make sure more developers follow this
when designing their binding.
Brian
[1] https://www.kernel.org/doc/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt
[2] http://lists.infradead.org/pipermail/linux-mtd/2015-August/061439.html
[3] http://lists.infradead.org/pipermail/linux-mtd/2015-September/061637.html
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH 1/3] doc: dt: add documentation for Mediatek spi-nor controller
2015-09-08 9:49 ` [PATCH 1/3] doc: dt: add documentation for Mediatek spi-nor controller Bayi Cheng
` (2 preceding siblings ...)
2015-09-11 21:47 ` Brian Norris
@ 2015-09-11 21:49 ` Brian Norris
2015-09-15 6:53 ` bayi.cheng
3 siblings, 1 reply; 10+ messages in thread
From: Brian Norris @ 2015-09-11 21:49 UTC (permalink / raw)
To: Bayi Cheng
Cc: David Woodhouse, Rob Herring, Pawel Moll, Mark Rutland,
Ian Campbell, Kumar Gala, Matthias Brugger, Daniel Kurtz,
Sascha Hauer, devicetree, linux-arm-kernel, linux-mediatek,
linux-kernel, linux-mtd
One more thing:
On Tue, Sep 08, 2015 at 05:49:54PM +0800, Bayi Cheng wrote:
> +- clocks: spi nor source clock
^^ you only list one clock here
> +- clock-names: "spi_clk", "axi_clk", "mux_clk", "sf_clk"
But you have 4 names here.
...
> + clocks = <&pericfg CLK_PERI_SPI>,
> + <&topckgen CLK_TOP_AXI_SEL>,
> + <&topckgen CLK_TOP_UNIVPLL2_D8>,
> + <&topckgen CLK_TOP_SPINFI_IFR_SEL>;
And you provide 4 clocks.
> + clock-names = "spi_clk", "axi_clk", "mux_clk", "sf_clk";
Please list all 4 under the "clocks" property, not just under the
"clock-names" property.
Brian
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH 1/3] doc: dt: add documentation for Mediatek spi-nor controller
2015-09-11 21:49 ` Brian Norris
@ 2015-09-15 6:53 ` bayi.cheng
0 siblings, 0 replies; 10+ messages in thread
From: bayi.cheng @ 2015-09-15 6:53 UTC (permalink / raw)
To: Brian Norris
Cc: David Woodhouse, Rob Herring, Pawel Moll, Mark Rutland,
Ian Campbell, Kumar Gala, Matthias Brugger, Daniel Kurtz,
Sascha Hauer, devicetree, linux-arm-kernel, linux-mediatek,
linux-kernel, linux-mtd
On Fri, 2015-09-11 at 14:49 -0700, Brian Norris wrote:
> One more thing:
>
> On Tue, Sep 08, 2015 at 05:49:54PM +0800, Bayi Cheng wrote:
> > +- clocks: spi nor source clock
>
> ^^ you only list one clock here
>
Ok, I will add other clocks
> > +- clock-names: "spi_clk", "axi_clk", "mux_clk", "sf_clk"
>
> But you have 4 names here.
>
> ...
> > + clocks = <&pericfg CLK_PERI_SPI>,
> > + <&topckgen CLK_TOP_AXI_SEL>,
> > + <&topckgen CLK_TOP_UNIVPLL2_D8>,
> > + <&topckgen CLK_TOP_SPINFI_IFR_SEL>;
>
> And you provide 4 clocks.
>
> > + clock-names = "spi_clk", "axi_clk", "mux_clk", "sf_clk";
>
> Please list all 4 under the "clocks" property, not just under the
> "clock-names" property.
>
OK, Thanks for your remind!
> Brian
^ permalink raw reply [flat|nested] 10+ messages in thread