Re: [PATCH v2 1/2] media: soc_camera: pxa_camera documentation device-tree support

2014-06-26 Thread Mark Rutland
On Wed, Jun 25, 2014 at 08:44:31PM +0100, Robert Jarzmik wrote:
 Mark Rutland mark.rutl...@arm.com writes:
 
  On Sat, Jun 21, 2014 at 11:21:46PM +0100, Robert Jarzmik wrote:
  +Required properties:
  + - compatible: Should be marvell,pxa27x-qci
 
  Is that x a wildcard? Or is 'x' part of the name of a particular unit?
 It's kind of a wildcard for a group of platforms
 It stands for the 3 PXA27x SoCs I'm aware of : PXA270, PXA271, and PXA272. The
 difference between them is different core frequency range and embedded RAM.
 
  We prefer not to have wildcard compatible strings in DT.
 OK, then let's go for marvell,pxa270-qci.

That sounds fine to me.

 
  + - reg: register base and size
  + - interrupts: the interrupt number
  + - any required generic properties defined in video-interfaces.txt
  +
  +Optional properties:
  + - clock-frequency: host interface is driving MCLK, and MCLK rate is this 
  rate
 
  Is MCLK an input or an output of this block?
 An output clock.
 
  If the former, why isn't this described as a clock?
 It's a good point. I'll try to add that too. The little trouble I have is that
 the PXA clocks are not _yet_ in device-tree. Putting a clock description will
 make this patch dependant on the clock framework patches [1], right ?

Yes, this will.

 
  
  +Example:
  +
  +  pxa_camera: pxa_camera@5000 {
  +  compatible = marvell,pxa27x-qci;
  +  reg = 0x5000 0x1000;
  +  interrupts = 33;
  +
  +  clocks = pxa2xx_clks 24;
  +  clock-names = camera;
 
  These weren't mentioned above. Is the clock input line really called
  camera?
 This is another clock, an input clock, independant of the former one. This is
 the clock actually fed to make this IP block work. This is dependant on the
 clock framework patches [1].

Ok. This clock should be mentioned in the properties description above.

Thanks,
Mark.
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 1/2] media: soc_camera: pxa_camera documentation device-tree support

2014-06-25 Thread Mark Rutland
On Sat, Jun 21, 2014 at 11:21:46PM +0100, Robert Jarzmik wrote:
 Add device-tree bindings documentation for pxa_camera driver.
 
 Signed-off-by: Robert Jarzmik robert.jarz...@free.fr
 ---
  .../devicetree/bindings/media/pxa-camera.txt   | 39 
 ++
  1 file changed, 39 insertions(+)
  create mode 100644 Documentation/devicetree/bindings/media/pxa-camera.txt
 
 diff --git a/Documentation/devicetree/bindings/media/pxa-camera.txt 
 b/Documentation/devicetree/bindings/media/pxa-camera.txt
 new file mode 100644
 index 000..9835aae
 --- /dev/null
 +++ b/Documentation/devicetree/bindings/media/pxa-camera.txt
 @@ -0,0 +1,39 @@
 +Marvell PXA camera host interface
 +
 +Required properties:
 + - compatible: Should be marvell,pxa27x-qci

Is that x a wildcard? Or is 'x' part of the name of a particular unit?

We prefer not to have wildcard compatible strings in DT.

 + - reg: register base and size
 + - interrupts: the interrupt number
 + - any required generic properties defined in video-interfaces.txt
 +
 +Optional properties:
 + - clock-frequency: host interface is driving MCLK, and MCLK rate is this 
 rate

Is MCLK an input or an output of this block?

If the former, why isn't this described as a clock?

 +
 +Example:
 +
 + pxa_camera: pxa_camera@5000 {
 + compatible = marvell,pxa27x-qci;
 + reg = 0x5000 0x1000;
 + interrupts = 33;
 +
 + clocks = pxa2xx_clks 24;
 + clock-names = camera;

These weren't mentioned above. Is the clock input line really called
camera?

Mark.
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 1/2] media: soc_camera: pxa_camera documentation device-tree support

2014-06-25 Thread Robert Jarzmik
Mark Rutland mark.rutl...@arm.com writes:

 On Sat, Jun 21, 2014 at 11:21:46PM +0100, Robert Jarzmik wrote:
 +Required properties:
 + - compatible: Should be marvell,pxa27x-qci

 Is that x a wildcard? Or is 'x' part of the name of a particular unit?
It's kind of a wildcard for a group of platforms
It stands for the 3 PXA27x SoCs I'm aware of : PXA270, PXA271, and PXA272. The
difference between them is different core frequency range and embedded RAM.

 We prefer not to have wildcard compatible strings in DT.
OK, then let's go for marvell,pxa270-qci.



 + - reg: register base and size
 + - interrupts: the interrupt number
 + - any required generic properties defined in video-interfaces.txt
 +
 +Optional properties:
 + - clock-frequency: host interface is driving MCLK, and MCLK rate is this 
 rate

 Is MCLK an input or an output of this block?
An output clock.

 If the former, why isn't this described as a clock?
It's a good point. I'll try to add that too. The little trouble I have is that
the PXA clocks are not _yet_ in device-tree. Putting a clock description will
make this patch dependant on the clock framework patches [1], right ?

 
 +Example:
 +
 +pxa_camera: pxa_camera@5000 {
 +compatible = marvell,pxa27x-qci;
 +reg = 0x5000 0x1000;
 +interrupts = 33;
 +
 +clocks = pxa2xx_clks 24;
 +clock-names = camera;

 These weren't mentioned above. Is the clock input line really called
 camera?
This is another clock, an input clock, independant of the former one. This is
the clock actually fed to make this IP block work. This is dependant on the
clock framework patches [1].

Cheers.

-- 
Robert

[1] http://www.spinics.net/lists/arm-kernel/msg337521.html
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html