Hi Rob, Thank you for your comments.
On 6/3/2016 07:16, Rob Herring wrote:
On Tue, May 31, 2016 at 02:58:23PM +0800, Songjun Wu wrote:DT binding documentation for ISC driver. Signed-off-by: Songjun Wu <songjun...@atmel.com> --- Changes in v3: - Remove the 'atmel,sensor-preferred'. - Modify the isc clock node according to the Rob's remarks. Changes in v2: - Remove the unit address of the endpoint. - Add the unit address to the clock node. - Avoid using underscores in node names. - Drop the "0x" in the unit address of the i2c node. - Modify the description of 'atmel,sensor-preferred'. - Add the description for the ISC internal clock. .../devicetree/bindings/media/atmel-isc.txt | 88 ++++++++++++++++++++++ 1 file changed, 88 insertions(+) create mode 100644 Documentation/devicetree/bindings/media/atmel-isc.txt diff --git a/Documentation/devicetree/bindings/media/atmel-isc.txt b/Documentation/devicetree/bindings/media/atmel-isc.txt new file mode 100644 index 0000000..2ae1d60 --- /dev/null +++ b/Documentation/devicetree/bindings/media/atmel-isc.txt @@ -0,0 +1,88 @@ +Atmel Image Sensor Controller (ISC) +---------------------------------------------- + +Required properties for ISC: +- compatible + Must be "atmel,sama5d2-isc" +- reg + Physical base address and length of the registers set for the device; +- interrupts + Should contain IRQ line for the ISI; +- clocks + List of clock specifiers, corresponding to entries in + the clock-names property; + Please refer to clock-bindings.txt. +- clock-names + Required elements: "hclock", "ispck". +- pinctrl-names, pinctrl-0 + Please refer to pinctrl-bindings.txt. +- isc-ispck + The clock for the ISC digital pipeline. + - compatible + Must be "atmel,sama5d2-isc-ispck". + - clock-cells + From common clock binding; should be set to 0. + - clocks + The clock source phandles. +- isc-mck + The clock for the image sensor. + - compatible + Must be "atmel,sama5d2-isc-mck". + - clock-cells + From common clock binding; should be set to 0. + - clocks + The clock source phandles. + +ISC supports a single port node with parallel bus. It should contain one +'port' child node with child 'endpoint' node. Please refer to the bindings +defined in Documentation/devicetree/bindings/media/video-interfaces.txt. + +Example: +isc: isc@f0008000 { + compatible = "atmel,sama5d2-isc"; + reg = <0xf0008000 0x4000>; + interrupts = <46 IRQ_TYPE_LEVEL_HIGH 5>; + clocks = <&isc_clk>, <&isc_ispck>; + clock-names = "hclock", "ispck"; + + port { + #address-cells = <1>; + #size-cells = <0>; + + isc_0: endpoint { + remote-endpoint = <&ov7740_0>; + hsync-active = <1>; + vsync-active = <0>; + pclk-sample = <1>; + }; + }; + + isc_ispck: isc-ispck@0 {Drop the unit-address. You should only have one if you have a reg property.
Accept. Thank you.
+ compatible = "atmel,sama5d2-isc-ispck"; + #clock-cells = <0>; + clocks = <&isc_clk>, <&iscck>; + }; + + isc_mck: isc-mck@1 {ditto. I still think these should be implied by atmel,sama5d2-isc and not in DT. The fact that you don't have any registers for them pretty much indicates that. It is also strange that isc-ispck is used by isc. If that is the only user, there is certainly no need to put it in DT.
The isc-ispck in only used by isc, it can be removed from DT. Thank you.
+ compatible = "atmel,sama5d2-isc-mck"; + #clock-cells = <0>; + clocks = <&isc_clk>, <&iscck>, <&isc_gclk>; + }; +}; + +i2c1: i2c@fc028000 { + ov7740: camera@21 { + compatible = "ovti,ov7740"; + reg = <0x21>; + + clocks = <&isc_mck>; + clock-names = "xvclk"; + assigned-clocks = <&isc_mck>; + assigned-clock-rates = <24000000>; + + port { + ov7740_0: endpoint { + remote-endpoint = <&isc_0>; + }; + }; +}; -- 2.7.4
