| Message ID | 1403582324-9485-2-git-send-email-addy.ke@rock-chips.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Jun 24, 2014 at 04:58:43AM +0100, addy ke wrote: > Signed-off-by: addy ke <addy.ke@rock-chips.com> > --- > .../devicetree/bindings/spi/spi-rockchip.txt | 51 ++++++++++++++++++++++ > 1 file changed, 51 insertions(+) > create mode 100644 Documentation/devicetree/bindings/spi/spi-rockchip.txt > > diff --git a/Documentation/devicetree/bindings/spi/spi-rockchip.txt b/Documentation/devicetree/bindings/spi/spi-rockchip.txt > new file mode 100644 > index 0000000..ce9c881 > --- /dev/null > +++ b/Documentation/devicetree/bindings/spi/spi-rockchip.txt > @@ -0,0 +1,51 @@ > +* Rockchip SPI Controller > + > +The Rockchip SPI controller is used to interface with various devices such as flash > +and display controllers using the SPI communication interface. > + > +Required SoC Specific Properties: > + > +- compatible: should be one of the following. > + - rockchip,rk3066-spi: for rk3066, rk3188 and rk3288 platforms. Are you sure you don't want specifc strings for rk3188 and rk3288 (in addtion to the common "rockchip,rk3066-spi")? > +- reg: physical base address of the controller and length of memory mapped > + region. > +- interrupts: The interrupt number to the cpu. The interrupt specifier format > + depends on the interrupt controller. > +- clocks: Must contain an entry for each entry in clock-names. > +- clock-names: Shall be "spiclk" for the transfer-clock, and "apb_pclk" for > + the peripheral clock. > + > +Optional properties: > +- dmas: DMA specifiers for tx and rx dma. See the DMA client binding, > + Documentation/devicetree/bindings/dma/dma.txt > +- dma-names: DMA request names should include "tx" and "rx" if present. > + > +Example: > + > +- SoC Specific Portion: > + > + spi0: spi@ff110000 { > + compatible = "rockchip,rockchip-spi"; This does not match the description of the compatible property. > + reg = <0xff110000 0x1000>; > + dmas = <&pdma1 11>, <&pdma1 12>; > + dma-names = "tx", "rx"; > + #address-cells = <1>; > + #size-cells = <0>; These weren't mentioned. > + interrupts = <GIC_SPI 44 IRQ_TYPE_LEVEL_HIGH>; > + pinctrl-names = "default"; > + pinctrl-0 = <&spi0_clk &spi0_tx &spi0_rx &spi0_cs0 &spi0_cs1>; pinctrl was not mentioned. > + clocks = <&cru SCLK_SPI0>, <&cru PCLK_SPI0>; > + clock-names = "spiclk", "apb_pclk"; > + status = "disabled"; Any reason for the status? > + }; > + > +- Board Specific Portion: > + > + &spi0 { > + status = "okay"; > + spi_test@00 { > + compatible = "rockchip,spi_test"; Huh? Mark.
Am Dienstag, 24. Juni 2014, 11:18:02 schrieb Mark Rutland: > On Tue, Jun 24, 2014 at 04:58:43AM +0100, addy ke wrote: > > Signed-off-by: addy ke <addy.ke@rock-chips.com> > > --- > > > > .../devicetree/bindings/spi/spi-rockchip.txt | 51 > > ++++++++++++++++++++++ 1 file changed, 51 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/spi/spi-rockchip.txt > > > > diff --git a/Documentation/devicetree/bindings/spi/spi-rockchip.txt > > b/Documentation/devicetree/bindings/spi/spi-rockchip.txt new file mode > > 100644 > > index 0000000..ce9c881 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/spi/spi-rockchip.txt > > @@ -0,0 +1,51 @@ > > +* Rockchip SPI Controller > > + > > +The Rockchip SPI controller is used to interface with various devices > > such as flash +and display controllers using the SPI communication > > interface. > > + > > +Required SoC Specific Properties: > > + > > +- compatible: should be one of the following. > > + - rockchip,rk3066-spi: for rk3066, rk3188 and rk3288 platforms. > > Are you sure you don't want specifc strings for rk3188 and rk3288 (in > addtion to the common "rockchip,rk3066-spi")? Wasn't the convention that "later" platforms that are compatible to an earlier one, reuse this compatible string instead of introducing a new one? From what I've heard so far, the specific spi controller got introduced with the rk3066 [earlier SoCs used a different implementation] and didn't change for rk3188 and rk3288. Addy may be able to verify this. > > +- reg: physical base address of the controller and length of memory > > mapped > > + region. > > +- interrupts: The interrupt number to the cpu. The interrupt specifier > > format + depends on the interrupt controller. > > +- clocks: Must contain an entry for each entry in clock-names. > > +- clock-names: Shall be "spiclk" for the transfer-clock, and "apb_pclk" > > for + the peripheral clock. > > + > > +Optional properties: > > +- dmas: DMA specifiers for tx and rx dma. See the DMA client binding, > > + Documentation/devicetree/bindings/dma/dma.txt > > +- dma-names: DMA request names should include "tx" and "rx" if present. > > + > > +Example: > > + > > +- SoC Specific Portion: > > + > > + spi0: spi@ff110000 { > > + compatible = "rockchip,rockchip-spi"; > > This does not match the description of the compatible property. > > > + reg = <0xff110000 0x1000>; > > + dmas = <&pdma1 11>, <&pdma1 12>; > > + dma-names = "tx", "rx"; > > + #address-cells = <1>; > > + #size-cells = <0>; > > These weren't mentioned. > > > + interrupts = <GIC_SPI 44 IRQ_TYPE_LEVEL_HIGH>; > > + pinctrl-names = "default"; > > + pinctrl-0 = <&spi0_clk &spi0_tx &spi0_rx &spi0_cs0 &spi0_cs1>; > > pinctrl was not mentioned. > > > + clocks = <&cru SCLK_SPI0>, <&cru PCLK_SPI0>; > > + clock-names = "spiclk", "apb_pclk"; > > + status = "disabled"; > > Any reason for the status? I guess to have the spi controller only be enabled when a board is using it as below. But it may be an implementation detail which could be omitted from the binding doc. > > > + }; > > + > > +- Board Specific Portion: > > + > > + &spi0 { > > + status = "okay"; > > + spi_test@00 { > > + compatible = "rockchip,spi_test"; > > Huh? SPI declares it's devices similar to i2c, so while the example might profit from a more casual device, I'm not exactly sure what is the problem here. Thanks Heiko
On Tue, Jun 24, 2014 at 11:18:02AM +0100, Mark Rutland wrote: > On Tue, Jun 24, 2014 at 04:58:43AM +0100, addy ke wrote: > > +- Board Specific Portion: > > + > > + &spi0 { > > + status = "okay"; > > + spi_test@00 { > > + compatible = "rockchip,spi_test"; > Huh? I expect that's just intended as an example of how to set up a device and they're doing something like use a driver that generates known patterns. We'd normally omit such examples from the binding document since it's generic and covered well enough elsehwere.
On Tue, Jun 24, 2014 at 11:32:21AM +0100, Heiko Stübner wrote: > Am Dienstag, 24. Juni 2014, 11:18:02 schrieb Mark Rutland: > > On Tue, Jun 24, 2014 at 04:58:43AM +0100, addy ke wrote: > > > Signed-off-by: addy ke <addy.ke@rock-chips.com> > > > --- > > > > > > .../devicetree/bindings/spi/spi-rockchip.txt | 51 > > > ++++++++++++++++++++++ 1 file changed, 51 insertions(+) > > > create mode 100644 Documentation/devicetree/bindings/spi/spi-rockchip.txt > > > > > > diff --git a/Documentation/devicetree/bindings/spi/spi-rockchip.txt > > > b/Documentation/devicetree/bindings/spi/spi-rockchip.txt new file mode > > > 100644 > > > index 0000000..ce9c881 > > > --- /dev/null > > > +++ b/Documentation/devicetree/bindings/spi/spi-rockchip.txt > > > @@ -0,0 +1,51 @@ > > > +* Rockchip SPI Controller > > > + > > > +The Rockchip SPI controller is used to interface with various devices > > > such as flash +and display controllers using the SPI communication > > > interface. > > > + > > > +Required SoC Specific Properties: > > > + > > > +- compatible: should be one of the following. > > > + - rockchip,rk3066-spi: for rk3066, rk3188 and rk3288 platforms. > > > > Are you sure you don't want specifc strings for rk3188 and rk3288 (in > > addtion to the common "rockchip,rk3066-spi")? > > Wasn't the convention that "later" platforms that are compatible to an earlier > one, reuse this compatible string instead of introducing a new one? That's why I said in addition to the common one. I'd only expect the driver to look for "rockchip,rk3066-spi", but a DTB could have: compatible = "rockchip,rk3188-spi", "rockchip,rk3066-spi"; Seeding the DTBs with the extra strings early makes it more likely that we can rely on them later. If we don't happen to need them they only clutter some DTBs. > From what I've heard so far, the specific spi controller got introduced with > the rk3066 [earlier SoCs used a different implementation] and didn't change for > rk3188 and rk3288. Addy may be able to verify this. > > > > > +- reg: physical base address of the controller and length of memory > > > mapped > > > + region. > > > +- interrupts: The interrupt number to the cpu. The interrupt specifier > > > format + depends on the interrupt controller. > > > +- clocks: Must contain an entry for each entry in clock-names. > > > +- clock-names: Shall be "spiclk" for the transfer-clock, and "apb_pclk" > > > for + the peripheral clock. > > > + > > > +Optional properties: > > > +- dmas: DMA specifiers for tx and rx dma. See the DMA client binding, > > > + Documentation/devicetree/bindings/dma/dma.txt > > > +- dma-names: DMA request names should include "tx" and "rx" if present. > > > + > > > +Example: > > > + > > > +- SoC Specific Portion: > > > + > > > + spi0: spi@ff110000 { > > > + compatible = "rockchip,rockchip-spi"; > > > > This does not match the description of the compatible property. > > > > > + reg = <0xff110000 0x1000>; > > > + dmas = <&pdma1 11>, <&pdma1 12>; > > > + dma-names = "tx", "rx"; > > > + #address-cells = <1>; > > > + #size-cells = <0>; > > > > These weren't mentioned. > > > > > + interrupts = <GIC_SPI 44 IRQ_TYPE_LEVEL_HIGH>; > > > + pinctrl-names = "default"; > > > + pinctrl-0 = <&spi0_clk &spi0_tx &spi0_rx &spi0_cs0 &spi0_cs1>; > > > > pinctrl was not mentioned. > > > > > + clocks = <&cru SCLK_SPI0>, <&cru PCLK_SPI0>; > > > + clock-names = "spiclk", "apb_pclk"; > > > + status = "disabled"; > > > > Any reason for the status? > > I guess to have the spi controller only be enabled when a board is using it as > below. But it may be an implementation detail which could be omitted from the > binding doc. Yes please :) > > > > > > + }; > > > + > > > +- Board Specific Portion: > > > + > > > + &spi0 { > > > + status = "okay"; > > > + spi_test@00 { > > > + compatible = "rockchip,spi_test"; > > > > Huh? > > SPI declares it's devices similar to i2c, so while the example might profit > from a more casual device, I'm not exactly sure what is the problem here. Sure. Given that this is a known pattern for SPI controllers, do we need this in each binding doc? Mark.
diff --git a/Documentation/devicetree/bindings/spi/spi-rockchip.txt b/Documentation/devicetree/bindings/spi/spi-rockchip.txt new file mode 100644 index 0000000..ce9c881 --- /dev/null +++ b/Documentation/devicetree/bindings/spi/spi-rockchip.txt @@ -0,0 +1,51 @@ +* Rockchip SPI Controller + +The Rockchip SPI controller is used to interface with various devices such as flash +and display controllers using the SPI communication interface. + +Required SoC Specific Properties: + +- compatible: should be one of the following. + - rockchip,rk3066-spi: for rk3066, rk3188 and rk3288 platforms. +- reg: physical base address of the controller and length of memory mapped + region. +- interrupts: The interrupt number to the cpu. The interrupt specifier format + depends on the interrupt controller. +- clocks: Must contain an entry for each entry in clock-names. +- clock-names: Shall be "spiclk" for the transfer-clock, and "apb_pclk" for + the peripheral clock. + +Optional properties: +- dmas: DMA specifiers for tx and rx dma. See the DMA client binding, + Documentation/devicetree/bindings/dma/dma.txt +- dma-names: DMA request names should include "tx" and "rx" if present. + +Example: + +- SoC Specific Portion: + + spi0: spi@ff110000 { + compatible = "rockchip,rockchip-spi"; + reg = <0xff110000 0x1000>; + dmas = <&pdma1 11>, <&pdma1 12>; + dma-names = "tx", "rx"; + #address-cells = <1>; + #size-cells = <0>; + interrupts = <GIC_SPI 44 IRQ_TYPE_LEVEL_HIGH>; + pinctrl-names = "default"; + pinctrl-0 = <&spi0_clk &spi0_tx &spi0_rx &spi0_cs0 &spi0_cs1>; + clocks = <&cru SCLK_SPI0>, <&cru PCLK_SPI0>; + clock-names = "spiclk", "apb_pclk"; + status = "disabled"; + }; + +- Board Specific Portion: + + &spi0 { + status = "okay"; + spi_test@00 { + compatible = "rockchip,spi_test"; + reg = <0>; + spi-max-frequency = <24000000>; + }; + };
Signed-off-by: addy ke <addy.ke@rock-chips.com> --- .../devicetree/bindings/spi/spi-rockchip.txt | 51 ++++++++++++++++++++++ 1 file changed, 51 insertions(+) create mode 100644 Documentation/devicetree/bindings/spi/spi-rockchip.txt