Hi Josh,
On Wednesday 19 March 2014 17:15:02 Josh Wu wrote:
> On 3/18/2014 9:36 PM, Laurent Pinchart wrote:
> > On Tuesday 18 March 2014 19:19:54 Josh Wu wrote:
> >> This patch add the DT support for Atmel ISI driver.
> >> It use the same v4l2 DT interface that defined in video-interfaces.txt.
> >>
> >> Signed-off-by: Josh Wu <josh.wu@xxxxxxxxx>
> >> Cc: devicetree@xxxxxxxxxxxxxxx
> >> ---
> >>
> >> .../devicetree/bindings/media/atmel-isi.txt | 51 ++++++++++++++
> >> drivers/media/platform/soc_camera/atmel-isi.c | 33 ++++++++++++-
> >> 2 files changed, 82 insertions(+), 2 deletions(-)
> >> create mode 100644
> >> Documentation/devicetree/bindings/media/atmel-isi.txt
> >>
> >> diff --git a/Documentation/devicetree/bindings/media/atmel-isi.txt
> >> b/Documentation/devicetree/bindings/media/atmel-isi.txt new file mode
> >> 100644
> >> index 0000000..07f00eb
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/media/atmel-isi.txt
> >> @@ -0,0 +1,51 @@
> >> +Atmel Image Sensor Interface (ISI) SoC Camera Subsystem
> >> +----------------------------------------------
> >> +
> >> +Required properties:
> >> +- compatible: must be "atmel,at91sam9g45-isi"
> >> +- 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;
> >> +- clock-names: must contain "isi_clk", which is the isi peripherial
> >> clock.
> >> + "isi_mck" is optinal, it is the master clock output to
> >> sensor.
> >
> > The mck clock should be handled by the sensor driver instead. I know we
> > have a legacy mode in the atmel-isi driver to manage that clock
> > internally, but let's not propagate that to DT.
>
> I agree with you.
>
> I put the isi_mck as optional here because current the sensor driver
> code only managed the v4l2 clock not the common clock.
> There should add additional code to manager mck clock.
> So if you want to ISI work for now, you should put the isi_mck in
> atmel-isi DT node.
>
> But for sure I can remove the isi_mck in atmel-isi DT document. In the
> future it will be add in sensor's DT document.
I think that's the way to go, yes. I know we have existing platforms that
require sensor clock management in the ISI driver. That should be fixed, and a
move to DT is a perfect opportunity to do so :-)
> > I would also drop the "isi_" prefix from the isi_clk name.
>
> hmm, I think "isi_clk" indicates it is a ISI peripheral clock. And
> which is consistent with other peripheral clock name in sama5.
I believe the "isi_" prefix is redundant, given that the clock-names property
is inside the ISI DT node. However, if this style matches the rest of the
platform there's no need to change it.
> > You should also describe the port node. You can just mention the related
> > bindings document, and state that the ISI has a single port.
>
> OK. will add in the v2.
>
> >> +Optional properties:
> >> +- atmel,isi-disable-preview: a boolean property to disable the preview
> >> channel;
> >
> > That doesn't really sound like a hardware property to me. Isn't it full
> > mode related to software configuration instead, which should be performed
> > at runtime by userspace ?
>
> yes, this configuration can be disable/enable by driver according to
> user select format.
> I will remove it in v2. Thanks.
>
> Best Regards,
> Josh Wu
>
> >> +
> >> +Example:
> >> + isi: isi@f0034000 {
> >> + compatible = "atmel,at91sam9g45-isi";
> >> + reg = <0xf0034000 0x4000>;
> >> + interrupts = <37 IRQ_TYPE_LEVEL_HIGH 5>;
> >> +
> >> + clocks = <&isi_clk>, <&pck1>;
> >> + clock-names = "isi_clk", "isi_mck";
> >> +
> >> + pinctrl-names = "default";
> >> + pinctrl-0 = <&pinctrl_isi &pinctrl_pck1_as_isi_mck>;
> >> +
> >> + port {
> >> + #address-cells = <1>;
> >> + #size-cells = <0>;
> >> +
> >> + isi_0: endpoint {
> >> + remote-endpoint = <&ov2640_0>;
> >> + };
> >> + };
> >> + };
> >> +
> >> + i2c1: i2c@f0018000 {
> >> + ov2640: camera@0x30 {
> >> + compatible = "omnivision,ov2640";
> >> + reg = <0x30>;
> >> +
> >> + port {
> >> + ov2640_0: endpoint {
> >> + remote-endpoint = <&isi_0>;
> >> + bus-width = <8>;
> >> + };
> >> + };
> >> + };
> >> + };
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
