lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Mon, 6 Jun 2016 14:44:24 +0800 From: "Wu, Songjun" <songjun.wu@...el.com> To: Boris Brezillon <boris.brezillon@...e-electrons.com>, Rob Herring <robh@...nel.org> CC: Mark Rutland <mark.rutland@....com>, <devicetree@...r.kernel.org>, Pawel Moll <pawel.moll@....com>, Ian Campbell <ijc+devicetree@...lion.org.uk>, <nicolas.ferre@...el.com>, <linux-kernel@...r.kernel.org>, <laurent.pinchart@...asonboard.com>, Kumar Gala <galak@...eaurora.org>, <g.liakhovetski@....de>, <linux-arm-kernel@...ts.infradead.org> Subject: Re: [PATCH v3 2/2] [media] atmel-isc: DT binding for Image Sensor Controller driver On 6/3/2016 19:10, Boris Brezillon wrote: > On Thu, 2 Jun 2016 18:16:09 -0500 > Rob Herring <robh@...nel.org> 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.wu@...el.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@...08000 { >>> + 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. >> >>> + 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. > > I had a look at the block diagram, and it seems you are partially right. > The only clock that is really exported by the ISC is isc_mck (isc_pck is > not). > So I'd suggest dropping these clk sub-nodes, putting the #clock-cells, > and clock-names properties in the isc node, and then referencing the > isc node in you i2c device. > > isc: isc@...08000 { > compatible = "atmel,sama5d2-isc"; > reg = <0xf0008000 0x4000>; > interrupts = <46 IRQ_TYPE_LEVEL_HIGH 5>; > clocks = <&isc_clk>, <&isc_ispck>; > clock-names = "hclock", "ispck"; > #clock-cells = <0>; > clock-output-names = "isc-mck"; > }; > > i2c1: i2c@...28000 { > /* ... */ > ov7740: camera@21 { > /* ... */ > clocks = <&isc>; > clock-names = "xvclk"; > } > }; > > If I'm wrong and the ISC IP is really exposing several clks, you just > have to change #clock-cells to 1, and use clocks = <&isc X>; in the i2c > device node. > > Rob, Songjun, would you agree on this representation? > Agree, I think it's a good solution. Thank you for your comments. >> >> >>> + compatible = "atmel,sama5d2-isc-mck"; >>> + #clock-cells = <0>; >>> + clocks = <&isc_clk>, <&iscck>, <&isc_gclk>; >>> + }; >>> +}; >>> + >>> +i2c1: i2c@...28000 { >>> + 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 >>> >> >> _______________________________________________ >> linux-arm-kernel mailing list >> linux-arm-kernel@...ts.infradead.org >> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > > >
Powered by blists - more mailing lists