Add device tree support for Exynos4 camera interface
Subsystems (5)
Sylwester Nawrocki
maintainerv6 cover Add device tree support for Exynos4 camera interface
This series adds devicetree support for the front and rear camera of the Exynos4412 SoC Trats2 board. It converts related drivers to use the v4l2-async API. The SoC output clocks are provided to external image image sensors through the common clock API.
This iteration includes three small fixes, detailed description can be found in patches 7/10, 8/10.
I'd like to ask for a DT binding maintainer review/Ack of patch 3/10. I'm going to push this series to the media tree on beginning of next week.
Thanks,
Sylwester
Sylwester Nawrocki (10):
Documentation: dt: Add binding documentation for S5K6A3 image sensor
Documentation: dt: Add binding documentation for S5C73M3 camera
Documentation: devicetree: Update Samsung FIMC DT binding
V4L: Add driver for s5k6a3 image sensor
V4L: s5c73m3: Add device tree support
exynos4-is: Use external s5k6a3 sensor driver
exynos4-is: Add clock provider for the SCLK_CAM clock outputs
exynos4-is: Add support for asynchronous subdevices registration
ARM: dts: Add rear camera nodes for Exynos4412 TRATS2 board
ARM: dts: exynos4: Update camera clk provider and s5k6a3 sensor node
.../devicetree/bindings/media/samsung-fimc.txt | 34 +- .../devicetree/bindings/media/samsung-s5c73m3.txt | 97 +++++ .../devicetree/bindings/media/samsung-s5k6a3.txt | 33 ++ arch/arm/boot/dts/exynos4.dtsi | 6 +- arch/arm/boot/dts/exynos4412-trats2.dts | 86 ++++- drivers/media/i2c/Kconfig | 8 + drivers/media/i2c/Makefile | 1 + drivers/media/i2c/s5c73m3/s5c73m3-core.c | 207 ++++++++--- drivers/media/i2c/s5c73m3/s5c73m3-spi.c | 6 + drivers/media/i2c/s5c73m3/s5c73m3.h | 4 + drivers/media/i2c/s5k6a3.c | 389 ++++++++++++++++++++ drivers/media/platform/exynos4-is/fimc-is-regs.c | 2 +- drivers/media/platform/exynos4-is/fimc-is-sensor.c | 285 +------------- drivers/media/platform/exynos4-is/fimc-is-sensor.h | 49 +-- drivers/media/platform/exynos4-is/fimc-is.c | 97 ++--- drivers/media/platform/exynos4-is/fimc-is.h | 4 +- drivers/media/platform/exynos4-is/media-dev.c | 340 ++++++++++++----- drivers/media/platform/exynos4-is/media-dev.h | 32 +- 18 files changed, 1136 insertions(+), 544 deletions(-) create mode 100644 Documentation/devicetree/bindings/media/samsung-s5c73m3.txt create mode 100644 Documentation/devicetree/bindings/media/samsung-s5k6a3.txt create mode 100644 drivers/media/i2c/s5k6a3.c
--
1.7.9.5
Sylwester Nawrocki
maintainerPATCH v6 1/10 Documentation: dt: Add binding documentation for S5K6A3 image sensor
This patch adds binding documentation for the Samsung S5K6A3(YX) raw image sensor.
---
Changes since v5:
- none.
Changes since v2:
- rephrased 'clocks' and 'clock-names' properties' description;
--- .../devicetree/bindings/media/samsung-s5k6a3.txt | 33 ++++++++++++++++++++ 1 file changed, 33 insertions(+) create mode 100644 Documentation/devicetree/bindings/media/samsung-s5k6a3.txt
Documentation/devicetree/bindings/media/samsung-s5k6a3.txt new file +33 -0
@@ -0,0 +1,33 @@ +Samsung S5K6A3(YX) raw image sensor +--------------------------------- + +S5K6A3(YX) is a raw image sensor with MIPI CSI-2 and CCP2 image data interfaces +and CCI (I2C compatible) control bus. + +Required properties: + +- compatible : "samsung,s5k6a3"; +- reg : I2C slave address of the sensor; +- svdda-supply : core voltage supply; +- svddio-supply : I/O voltage supply; +- afvdd-supply : AF (actuator) voltage supply; +- gpios : specifier of a GPIO connected to the RESET pin; +- clocks : should contain list of phandle and clock specifier pairs + according to common clock bindings for the clocks described + in the clock-names property; +- clock-names : should contain "extclk" entry for the sensor's EXTCLK clock; + +Optional properties: + +- clock-frequency : the frequency at which the "extclk" clock should be + configured to operate, in Hz; if this property is not + specified default 24 MHz value will be used. + +The common video interfaces bindings (see video-interfaces.txt) should be +used to specify link to the image data receiver. The S5K6A3(YX) device +node should contain one 'port' child node with an 'endpoint' subnode. + +Following properties are valid for the endpoint node: + +- data-lanes : (optional) specifies MIPI CSI-2 data lanes as covered in + video-interfaces.txt. The sensor supports only one data lane.
Signature · 1 line
1.7.9.5
PATCH v6 1/10 Documentation: dt: Add binding documentation for S5K6A3 image sensor
Hi Sylwester,
Quoted 27 lines · Sylwester Nawrocki
This patch adds binding documentation for the Samsung S5K6A3(YX) raw image sensor.
Signed-off-by Sylwester Nawrocki <[email protected]>Acked-by Kyungmin Park <[email protected]>Acked-by Mark Rutland <[email protected]>---
Changes since v5:
- none.Changes since v2:
- rephrased 'clocks' and 'clock-names' properties' description;--- .../devicetree/bindings/media/samsung-s5k6a3.txt | 33 ++++++++++++++++++++ 1 file changed, 33 insertions(+) create mode 100644 Documentation/devicetree/bindings/media/samsung-s5k6a3.txtDocumentation/devicetree/bindings/media/samsung-s5k6a3.txt new file +4
-0@@ -0,0 +1,33 @@ +Samsung S5K6A3(YX) raw image sensor +--------------------------------- + +S5K6A3(YX) is a raw image sensor with MIPI CSI-2 and CCP2 image data interfaces
Documentation/devicetree/bindings/media/samsung-s5k6a3.txt line 14
Quoted 10 lines
Documentation/devicetree/bindings/media/samsung-s5k6a3.txt new file +10
-0+and CCI (I2C compatible) control bus. + +Required properties: + +- compatible : "samsung,s5k6a3"; +- reg : I2C slave address of the sensor; +- svdda-supply : core voltage supply; +- svddio-supply : I/O voltage supply; +- afvdd-supply : AF (actuator) voltage supply; +- gpios : specifier of a GPIO connected to the RESET pin;
Please use 'reset-gpios' for GPIOs connected to reset pins.
regards
Philipp
PATCH v6 1/10 Documentation: dt: Add binding documentation for S5K6A3 image sensor
Hi Phillip,
Quoted 17 lines · Philipp Zabel
Quoted 16 lines
+++
b/Documentation/devicetree/bindings/media/samsung-s5k6a3.txtQuoted 15 lines
@@ -0,0 +1,33 @@
+Samsung S5K6A3(YX) raw image sensor
+---------------------------------
+
+S5K6A3(YX) is a raw image sensor with MIPI CSI-2 and CCP2 image data interfaces +and CCI (I2C compatible) control bus.
+
+Required properties:
++- compatible : "samsung,s5k6a3"; +- reg : I2C slave address of the sensor; +- svdda-supply : core voltage supply; +- svddio-supply : I/O voltage supply; +- afvdd-supply : AF (actuator) voltage supply; +- gpios : specifier of a GPIO connected to the RESET pin;
Quoted 1 line
Please use 'reset-gpios' for GPIOs connected to reset pins.
I would prefer to keep it as is, I'm not adding a new driver in this series, just the binding documentation and doing some refactoring. So if I changed this now, the driver would need to be messed up with an additional code to support both 'gpios' and 'reset-gpios'. Are there any serious reasons to use this specific name ? It's not related to the reset signal DT bindings, is it ?
--
Regards,
Sylwester
Sylwester Nawrocki
maintainerPATCH v6 2/10 Documentation: dt: Add binding documentation for S5C73M3 camera
In reply to Sylwester NawrockiThis adds DT binding documentation for Samsung S5C73M3 camera sensor with an embedded ISP.
---
Changes since v5:
- none.
Changes since v4:
- added missing unit-address at the example SPI device node;
Changes since v3:
- DT binding documentation separated into this patch;
Changes since v2:
- rephrased 'clocks' and 'clock-names' properties' description;
--- .../devicetree/bindings/media/samsung-s5c73m3.txt | 97 ++++++++++++++++++++ 1 file changed, 97 insertions(+) create mode 100644 Documentation/devicetree/bindings/media/samsung-s5c73m3.txt
Documentation/devicetree/bindings/media/samsung-s5c73m3.txt new file +97 -0
@@ -0,0 +1,97 @@ +Samsung S5C73M3 8Mp camera ISP +------------------------------ + +The S5C73M3 camera ISP supports MIPI CSI-2 and parallel (ITU-R BT.656) video +data busses. The I2C bus is the main control bus and additionally the SPI bus +is used, mostly for transferring the firmware to and from the device. Two +slave device nodes corresponding to these control bus interfaces are required +and should be placed under respective bus controller nodes. + +I2C slave device node +--------------------- + +Required properties: + +- compatible : "samsung,s5c73m3"; +- reg : I2C slave address of the sensor; +- vdd-int-supply : digital power supply (1.2V); +- vdda-supply : analog power supply (1.2V); +- vdd-reg-supply : regulator input power supply (2.8V); +- vddio-host-supply : host I/O power supply (1.8V to 2.8V); +- vddio-cis-supply : CIS I/O power supply (1.2V to 1.8V); +- vdd-af-supply : lens power supply (2.8V); +- xshutdown-gpios : specifier of GPIO connected to the XSHUTDOWN pin; +- standby-gpios : specifier of GPIO connected to the STANDBY pin; +- clocks : should contain list of phandle and clock specifier pairs + according to common clock bindings for the clocks described + in the clock-names property; +- clock-names : should contain "cis_extclk" entry for the CIS_EXTCLK clock; + +Optional properties: + +- clock-frequency : the frequency at which the "cis_extclk" clock should be + configured to operate, in Hz; if this property is not + specified default 24 MHz value will be used. + +The common video interfaces bindings (see video-interfaces.txt) should be used +to specify link from the S5C73M3 to an external image data receiver. The S5C73M3 +device node should contain one 'port' child node with an 'endpoint' subnode for +this purpose. The data link from a raw image sensor to the S5C73M3 can be +similarly specified, but it is optional since the S5C73M3 ISP and a raw image +sensor are usually inseparable and form a hybrid module. + +Following properties are valid for the endpoint node(s): + +endpoint subnode +---------------- + +- data-lanes : (optional) specifies MIPI CSI-2 data lanes as covered in + video-interfaces.txt. This sensor doesn't support data lane remapping + and physical lane indexes in subsequent elements of the array should + be only consecutive ascending values. + +SPI device node +--------------- + +Required properties: + +- compatible : "samsung,s5c73m3"; + +For more details see description of the SPI busses bindings +(../spi/spi-bus.txt) and bindings of a specific bus controller. + +Example: + +i2c at 138A000000 { + ... + s5c73m3 at 3c { + compatible = "samsung,s5c73m3"; + reg = <0x3c>; + vdd-int-supply = <&buck9_reg>; + vdda-supply = <&ldo17_reg>; + vdd-reg-supply = <&cam_io_reg>; + vddio-host-supply = <&ldo18_reg>; + vddio-cis-supply = <&ldo9_reg>; + vdd-af-supply = <&cam_af_reg>; + clock-frequency = <24000000>; + clocks = <&clk 0>; + clock-names = "cis_extclk"; + reset-gpios = <&gpf1 3 1>; + standby-gpios = <&gpm0 1 1>; + port { + s5c73m3_ep: endpoint { + remote-endpoint = <&csis0_ep>; + data-lanes = <1 2 3 4>; + }; + }; + }; +}; + +spi at 1392000 { + ... + s5c73m3_spi: s5c73m3 at 0 { + compatible = "samsung,s5c73m3"; + reg = <0>; + ... + }; +};
Signature · 1 line
1.7.9.5
Sylwester Nawrocki
maintainerPATCH v6 3/10 Documentation: devicetree: Update Samsung FIMC DT binding
In reply to Sylwester NawrockiThis patch documents following updates of the Exynos4 SoC camera subsystem devicetree binding:
- addition of #clock-cells property to 'camera' node - the #clock-cells property is needed when the sensor sub-devices use clock provided by the camera host interface,
- addition of an optional clock-output-names property,
- change of the clock-frequency at image sensor node from mandatory to an optional property - the sensor devices can now control their clock themselves and there should be no need to require this property by the camera host device binding, a default frequency value can often be used.
---
Changes since v5:
- none.
Changes since v4:
- dropped a requirement of specific order of values in clocks/ clock-names properties (Mark) and reference to clock-names in clock-output-names property description (Mark).
--- .../devicetree/bindings/media/samsung-fimc.txt | 34 +++++++++++++++----- 1 file changed, 26 insertions(+), 8 deletions(-)
Documentation/devicetree/bindings/media/samsung-fimc.txt +26 -8
@@ -32,6 +32,21 @@ way around. The 'camera' node must include at least one 'fimc' child node. +Optional properties: + +- #clock-cells: from the common clock bindings (../clock/clock-bindings.txt), + must be 1. A clock provider is associated with the 'camera' node and it should + be referenced by external sensors that use clocks provided by the SoC on + CAM_*_CLKOUT pins. The clock specifier cell stores an index of a clock. + The indices are 0, 1 for CAM_A_CLKOUT, CAM_B_CLKOUT clocks respectively. + +- clock-output-names: from the common clock bindings, should contain names of + clocks registered by the camera subsystem corresponding to CAM_A_CLKOUT, + CAM_B_CLKOUT output clocks respectively. + +Note: #clock-cells and clock-output-names are mandatory properties if external +image sensor devices reference 'camera' device node as a clock provider. + 'fimc' device nodes -------------------
@@ -97,8 +112,8 @@ Image sensor nodes The sensor device nodes should be added to their control bus controller (e.g. I2C0) nodes and linked to a port node in the csis or the parallel-ports node, using the common video interfaces bindings, defined in video-interfaces.txt. -The implementation of this bindings requires clock-frequency property to be -present in the sensor device nodes. +An optional clock-frequency property needs to be present in the sensor device +nodes. Default value when this property is not present is 24 MHz. Example:
@@ -114,7 +129,7 @@ Example: vddio-supply = <...>; clock-frequency = <24000000>; - clocks = <...>; + clocks = <&camera 1>; clock-names = "mclk"; port {
@@ -135,7 +150,7 @@ Example: vddio-supply = <...>; clock-frequency = <24000000>; - clocks = <...>; + clocks = <&camera 0>; clock-names = "mclk"; port {
@@ -149,12 +164,15 @@ Example: camera { compatible = "samsung,fimc", "simple-bus"; - #address-cells = <1>; - #size-cells = <1>; - status = "okay"; - + clocks = <&clock 132>, <&clock 133>; + clock-names = "sclk_cam0", "sclk_cam1"; + #clock-cells = <1>; + clock-output-names = "cam_mclk_a", "cam_mclk_b"; pinctrl-names = "default"; pinctrl-0 = <&cam_port_a_clk_active>; + status = "okay"; + #address-cells = <1>; + #size-cells = <1>; /* parallel camera ports */ parallel-ports {
Signature · 1 line
1.7.9.5
PATCH v6 3/10 Documentation: devicetree: Update Samsung FIMC DT binding
Hi Sylwester,
Thank you for the patch.
Quoted 47 lines · On Thursday 06 March 2014 17:20:12 Sylwester Nawrocki wrote:
This patch documents following updates of the Exynos4 SoC camera subsystem devicetree binding:
- addition of #clock-cells property to 'camera' node - the #clock-cells property is needed when the sensor sub-devices use clock provided by the camera host interface,
- addition of an optional clock-output-names property,
- change of the clock-frequency at image sensor node from mandatory to an optional property - the sensor devices can now control their clock themselves and there should be no need to require this property by the camera host device binding, a default frequency value can often be used.Signed-off-by Sylwester Nawrocki <[email protected]>Acked-by Kyungmin Park <[email protected]>---
Changes since v5:
- none.Changes since v4:
- dropped a requirement of specific order of values in clocks/ clock-names properties (Mark) and reference to clock-names in clock-output-names property description (Mark).--- .../devicetree/bindings/media/samsung-fimc.txt | 34 ++++++++++++----- 1 file changed, 26 insertions(+), 8 deletions(-)(quoted diff) +0
-0
b/Documentation/devicetree/bindings/media/samsung-fimc.txtindex 96312f6..dbd4020 100644Documentation/devicetree/bindings/media/samsung-fimc.txt +4
-0@@ -32,6 +32,21 @@ way around. The 'camera' node must include at least one 'fimc' child node. +Optional properties: + +- #clock-cells: from the common clock bindings (../clock/clock-bindings.txt), + must be 1. A clock provider is associated with the 'camera' node and itshould
Documentation/devicetree/bindings/media/samsung-fimc.txt +5
-0+ be referenced by external sensors that use clocks provided by the SoC on + CAM_*_CLKOUT pins. The clock specifier cell stores an index of a clock. + The indices are 0, 1 for CAM_A_CLKOUT, CAM_B_CLKOUT clocks respectively. + +- clock-output-names: from the common clock bindings, should contain namesof
Documentation/devicetree/bindings/media/samsung-fimc.txt way around. · line 45
Quoted 2 lines
Documentation/devicetree/bindings/media/samsung-fimc.txt +2
-0+ clocks registered by the camera subsystem corresponding to CAM_A_CLKOUT, + CAM_B_CLKOUT output clocks respectively.
Wouldn't it be better to document the "cam_mclk_a" and "cam_mclk_b" names explicitly ? Or do you expect different names to be used in different DT files ? And as they correspond to the CAM_A_CLKOUT and CAM_B_CLKOUT pins, shouldn't they be named "cam_a_clkout" and "cam_b_clkout" ?
Documentation/devicetree/bindings/media/samsung-fimc.txt way around. · line 49
Quoted 5 lines
Documentation/devicetree/bindings/media/samsung-fimc.txt +2
-0+ +Note: #clock-cells and clock-output-names are mandatory properties ifexternal
Documentation/devicetree/bindings/media/samsung-fimc.txt +2
-0+image sensor devices reference 'camera' device node as a clock provider. +
What's the reason not to make them always mandatory ? Backward compatibility only ? If so wouldn't it make sense to document the properties as mandatory from now on, and treating them as optional in the driver for backward compatibility ?
Quoted 11 lines
'fimc' device nodes
-------------------Documentation/devicetree/bindings/media/samsung-fimc.txt +0
-0@@ -97,8 +112,8 @@ Image sensor nodes The sensor device nodes should be added to their control bus controller(e.g. I2C0) nodes and linked to a port node in the csis or the parallel-ports node, using the common video interfaces bindings, defined in video-interfaces.txt.
Documentation/devicetree/bindings/media/samsung-fimc.txt +1
-2-The implementation of this bindings requires clock-frequency property to be -present in the sensor device nodes. +An optional clock-frequency property needs to be present in the sensor
Quoted 2 lines
device
+nodes. Default value when this property is not present is 24 MHz.
This bothers me. Having the FIMC driver read the clock-frequence property from the sensor DT nodes feels like a layering violation. Shouldn't the sensor drivers call clk_set_rate() explicitly instead ?
Quoted 21 lines
Example:
Documentation/devicetree/bindings/media/samsung-fimc.txt +2
-2@@ -114,7 +129,7 @@ Example: vddio-supply = <...>; clock-frequency = <24000000>; - clocks = <...>; + clocks = <&camera 1>; clock-names = "mclk"; port {@@ -135,7 +150,7 @@ Example: vddio-supply = <...>; clock-frequency = <24000000>; - clocks = <...>; + clocks = <&camera 0>; clock-names = "mclk"; port {
Documentation/devicetree/bindings/media/samsung-fimc.txt Example: · line 168
Quoted 9 lines
Documentation/devicetree/bindings/media/samsung-fimc.txt +2
-4@@ -149,12 +164,15 @@ Example: camera { compatible = "samsung,fimc", "simple-bus"; - #address-cells = <1>; - #size-cells = <1>; - status = "okay"; - + clocks = <&clock 132>, <&clock 133>; + clock-names = "sclk_cam0", "sclk_cam1";
The documentation mentions that clock-names must contain "sclk_cam0", "sclk_cam1", "pxl_async0", "pxl_async1". Are the last two optional ? If so I think you should clarify the description of the clock-names property. This can be done in a separate patch.
Quoted 10 lines
Documentation/devicetree/bindings/media/samsung-fimc.txt +5
-0+ #clock-cells = <1>; + clock-output-names = "cam_mclk_a", "cam_mclk_b"; pinctrl-names = "default"; pinctrl-0 = <&cam_port_a_clk_active>; + status = "okay"; + #address-cells = <1>; + #size-cells = <1>; /* parallel camera ports */ parallel-ports {
Signature · 3 lines
Regards, Laurent Pinchart
Sylwester Nawrocki
maintainerPATCH v6 3/10 Documentation: devicetree: Update Samsung FIMC DT binding
Hi Laurent,
Thanks for your review.
On 11/03/14 13:30, Laurent Pinchart wrote:
[...]
Quoted 24 lines
Quoted 24 lines
--- .../devicetree/bindings/media/samsung-fimc.txt | 34 ++++++++++++----- 1 file changed, 26 insertions(+), 8 deletions(-)(quoted diff) +0
-0
b/Documentation/devicetree/bindings/media/samsung-fimc.txtindex 96312f6..dbd4020 100644Documentation/devicetree/bindings/media/samsung-fimc.txt +4
-0@@ -32,6 +32,21 @@ way around. The 'camera' node must include at least one 'fimc' child node. +Optional properties: + +- #clock-cells: from the common clock bindings (../clock/clock-bindings.txt), + must be 1. A clock provider is associated with the 'camera' node and itshould
Documentation/devicetree/bindings/media/samsung-fimc.txt +5
-0+ be referenced by external sensors that use clocks provided by the SoC on + CAM_*_CLKOUT pins. The clock specifier cell stores an index of a clock. + The indices are 0, 1 for CAM_A_CLKOUT, CAM_B_CLKOUT clocks respectively. + +- clock-output-names: from the common clock bindings, should contain names
Quoted 8 lines
Documentation/devicetree/bindings/media/samsung-fimc.txt way around. · line 45
Quoted 3 lines
of
Documentation/devicetree/bindings/media/samsung-fimc.txt +2
-0+ clocks registered by the camera subsystem corresponding to CAM_A_CLKOUT, + CAM_B_CLKOUT output clocks respectively.Wouldn't it be better to document the "
cam_mclk_a" and "cam_mclk_b" names explicitly ? Or do you expect different names to be used in different DT files ? And as they correspond to theCAM_A_CLKOUTandCAM_B_CLKOUTpins, shouldn't they be named "cam_a_clkout" and "cam_b_clkout" ?
Basically I could use fixed names for these clocks, I just wanted to keep a possibility to override them in dts to avoid any possible clock name collisions, rather than keep a list of different names per SoC in the driver. Right now fixed names could also be used for all SoCs I'm aware of, nevertheless I would prefer to keep the clock-output-names property. "cam_a_clkout", "cam_b_clkout" may be indeed better names, I'll change that.
Quoted 9 lines
Documentation/devicetree/bindings/media/samsung-fimc.txt way around. · line 49
Quoted 4 lines
+Note: #clock-cells and clock-output-names are mandatory properties if external
Documentation/devicetree/bindings/media/samsung-fimc.txt +2
-0+image sensor devices reference 'camera' device node as a clock provider. +What's the reason not to make them always mandatory ? Backward compatibility only ? If so wouldn't it make sense to document the properties as mandatory from now on, and treating them as optional in the driver for backward compatibility ?
Yes, it's for backwards compatibility only. It may be a good idea to just document them as required, since this is how the device is expected to be described in DT from now. I'll just make these a required properties, the driver already handles them as optional.
Quoted 17 lines
Quoted 11 lines
'fimc' device nodes
-------------------Documentation/devicetree/bindings/media/samsung-fimc.txt +0
-0@@ -97,8 +112,8 @@ Image sensor nodes The sensor device nodes should be added to their control bus controller(e.g. I2C0) nodes and linked to a port node in the csis or the parallel-ports node, using the common video interfaces bindings, defined in video-interfaces.txt.
Documentation/devicetree/bindings/media/samsung-fimc.txt +1
-2-The implementation of this bindings requires clock-frequency property to be -present in the sensor device nodes. +An optional clock-frequency property needs to be present in the sensorQuoted 2 lines
device
+nodes. Default value when this property is not present is 24 MHz.This bothers me. Having the FIMC driver read the clock-frequence property from the sensor DT nodes feels like a layering violation. Shouldn't the sensor drivers call
clk_set_rate()explicitly instead ?
It is supposed to do so, after this whole patch series. So the camera controller driver will not need such properties. What do you think about removing this sentence altogether ?
Quoted 22 lines
Quoted 22 lines
Example:
Documentation/devicetree/bindings/media/samsung-fimc.txt +2
-2@@ -114,7 +129,7 @@ Example: vddio-supply = <...>; clock-frequency = <24000000>; - clocks = <...>; + clocks = <&camera 1>; clock-names = "mclk"; port {@@ -135,7 +150,7 @@ Example: vddio-supply = <...>; clock-frequency = <24000000>; - clocks = <...>; + clocks = <&camera 0>; clock-names = "mclk"; port {@@ -149,12 +164,15 @@ Example:
Quoted 13 lines
Documentation/devicetree/bindings/media/samsung-fimc.txt Example: · line 168
Quoted 8 lines
Documentation/devicetree/bindings/media/samsung-fimc.txt +2
-4camera { compatible = "samsung,fimc", "simple-bus"; - #address-cells = <1>; - #size-cells = <1>; - status = "okay"; - + clocks = <&clock 132>, <&clock 133>; + clock-names = "sclk_cam0", "sclk_cam1";The documentation mentions that clock-names must contain "
sclk_cam0", "sclk_cam1", "pxl_async0", "pxl_async1". Are the last two optional ? If so I think you should clarify the description of the clock-names property. This can be done in a separate patch.
"pxl_async0", "pxl_async1" are mandatory, I'll add them also into this example dts.
Quoted 10 lines
Documentation/devicetree/bindings/media/samsung-fimc.txt Example: · line 178
Quoted 10 lines
Documentation/devicetree/bindings/media/samsung-fimc.txt +5
-0+ #clock-cells = <1>; + clock-output-names = "cam_mclk_a", "cam_mclk_b"; pinctrl-names = "default"; pinctrl-0 = <&cam_port_a_clk_active>; + status = "okay"; + #address-cells = <1>; + #size-cells = <1>; /* parallel camera ports */ parallel-ports {
--
Regards,
Sylwester
PATCH v6 3/10 Documentation: devicetree: Update Samsung FIMC DT binding
Hi Sylwester,
Quoted 3 lines · On Tuesday 11 March 2014 14:38:37 Sylwester Nawrocki wrote:
Hi Laurent,
Thanks for your review.
You're welcome.
Quoted 41 lines · Laurent Pinchart
[...]
Quoted 29 lines
Quoted 29 lines
---
.../devicetree/bindings/media/samsung-fimc.txt | 34 +++++++++----- 1 file changed, 26 insertions(+), 8 deletions(-)(quoted diff) +0
-0
b/Documentation/devicetree/bindings/media/samsung-fimc.txtindex 96312f6..dbd4020 100644Documentation/devicetree/bindings/media/samsung-fimc.txt +4
-0@@ -32,6 +32,21 @@ way around. The 'camera' node must include at least one 'fimc' child node. +Optional properties: + +- #clock-cells: from the common clock bindings (../clock/clock-bindings.txt), + must be 1. A clock provider is associated with the 'camera' node andit should
+ be referenced by external sensors that use clocks provided by the SoC on
+ CAM_*_CLKOUT pins. The clock specifier cell stores an index of a clock.
+ The indices are 0, 1 forCAM_A_CLKOUT,CAM_B_CLKOUTclocks respectively.
+
+- clock-output-names: from the common clock bindings, should contain names ofQuoted 8 lines
Quoted 3 lines
+ clocks registered by the camera subsystem corresponding to
CAM_A_CLKOUT,
+CAM_B_CLKOUToutput clocks respectively.Wouldn't it be better to document the "
cam_mclk_a" and "cam_mclk_b" names explicitly ? Or do you expect different names to be used in different DT files ? And as they correspond to theCAM_A_CLKOUTandCAM_B_CLKOUTpins, shouldn't they be named "cam_a_clkout" and "cam_b_clkout" ?
Quoted 7 lines
Basically I could use fixed names for these clocks, I just wanted to keep a possibility to override them in dts to avoid any possible clock name collisions, rather than keep a list of different names per SoC in the driver. Right now fixed names could also be used for all SoCs I'm aware of, nevertheless I would prefer to keep the clock-output-names property. "
cam_a_clkout", "cam_b_clkout" may be indeed better names, I'll change that.
OK, variable names are fine with me.
Quoted 10 lines
Quoted 9 lines
Documentation/devicetree/bindings/media/samsung-fimc.txt way around. · line 49
Quoted 4 lines
+Note: #clock-cells and clock-output-names are mandatory properties if external
Documentation/devicetree/bindings/media/samsung-fimc.txt +2
-0+image sensor devices reference 'camera' device node as a clock provider. +What's the reason not to make them always mandatory ? Backward compatibility only ? If so wouldn't it make sense to document the properties as mandatory from now on, and treating them as optional in the driver for backward compatibility ?
Quoted 4 lines
Yes, it's for backwards compatibility only. It may be a good idea to just document them as required, since this is how the device is expected to be described in DT from now. I'll just make these a required properties, the driver already handles them as optional.
OK.
Quoted 21 lines
Quoted 7 lines
Quoted 7 lines
'fimc' device nodes
-------------------Documentation/devicetree/bindings/media/samsung-fimc.txt +0
-0@@ -97,8 +112,8 @@ Image sensor nodes The sensor device nodes should be added to their control bus controllerQuoted 13 lines
Quoted 9 lines
(e.g. I2C0) nodes and linked to a port node in the csis or the parallel-ports node, using the common video interfaces bindings, defined in video-interfaces.txt.
-The implementation of this bindings requires clock-frequency property to beDocumentation/devicetree/bindings/media/samsung-fimc.txt +1
-1-present in the sensor device nodes. +An optional clock-frequency property needs to be present in the sensordevice
+nodes. Default value when this property is not present is 24 MHz.This bothers me. Having the FIMC driver read the clock-frequence property from the sensor DT nodes feels like a layering violation. Shouldn't the sensor drivers call
clk_set_rate()explicitly instead ?
Quoted 3 lines
It is supposed to do so, after this whole patch series. So the camera controller driver will not need such properties. What do you think about removing this sentence altogether ?
Sure. As the FIMC won't access the clock-frequency property of the sensor anymore after this patch series, let's just drop the mention of clock- frequency.
Quoted 42 lines
Quoted 30 lines
Quoted 30 lines
Example:
Documentation/devicetree/bindings/media/samsung-fimc.txt +1
-1@@ -114,7 +129,7 @@ Example: vddio-supply = <...>; clock-frequency = <24000000>; - clocks = <...>; + clocks = <&camera 1>; clock-names = "mclk";Documentation/devicetree/bindings/media/samsung-fimc.txt +1
-1port {@@ -135,7 +150,7 @@ Example: vddio-supply = <...>; clock-frequency = <24000000>; - clocks = <...>; + clocks = <&camera 0>; clock-names = "mclk";Documentation/devicetree/bindings/media/samsung-fimc.txt +0
-0port {@@ -149,12 +164,15 @@ Example: camera { compatible = "samsung,fimc", "simple-bus";Quoted 11 lines
Documentation/devicetree/bindings/media/samsung-fimc.txt Example: · line 168
Quoted 6 lines
Documentation/devicetree/bindings/media/samsung-fimc.txt +2
-4- #address-cells = <1>; - #size-cells = <1>; - status = "okay"; - + clocks = <&clock 132>, <&clock 133>; + clock-names = "sclk_cam0", "sclk_cam1";The documentation mentions that clock-names must contain "
sclk_cam0", "sclk_cam1", "pxl_async0", "pxl_async1". Are the last two optional ? If so I think you should clarify the description of the clock-names property. This can be done in a separate patch.
Quoted 2 lines
"
pxl_async0", "pxl_async1" are mandatory, I'll add them also into this example dts.
OK.
Quoted 12 lines
Quoted 12 lines
Quoted 12 lines
Documentation/devicetree/bindings/media/samsung-fimc.txt +5
-0+ #clock-cells = <1>; + clock-output-names = "cam_mclk_a", "cam_mclk_b"; pinctrl-names = "default"; pinctrl-0 = <&cam_port_a_clk_active>; + status = "okay"; + #address-cells = <1>; + #size-cells = <1>;Documentation/devicetree/bindings/media/samsung-fimc.txt +0
-0/* parallel camera ports */ parallel-ports {
Signature · 3 lines
Regards, Laurent Pinchart
Sylwester Nawrocki
maintainerPATCH v7 3/10 Documentation: devicetree: Update Samsung FIMC DT binding
This patch documents following updates of the Exynos4 SoC camera subsystem devicetree binding:
- addition of #clock-cells and clock-output-names properties to 'camera' node - these are now needed so the image sensor sub-devices can reference clocks provided by the camera host interface,
- dropped a note about required clock-frequency properties at the image sensor nodes; the sensor devices can now control their clock explicitly through the clk API and there is no need to require this property in the camera host interface binding.
---
Resending only single patch which changed.
Changes since v6:
- #clock-cells, clock-output-names documented as mandatory properties;
- renamed "cam_mclk_{a,b}" to "cam_{a,b}_clkout in the example dts, this now matches changes in exynos4.dtsi further in the patch series;
- marked "samsung,camclk-out" property as deprecated.
Changes since v5:
- none.
Changes since v4:
- dropped a requirement of specific order of values in clocks/ clock-names properties (Mark) and reference to clock-names in clock-output-names property description (Mark).
--- .../devicetree/bindings/media/samsung-fimc.txt | 46 +++++++++++++------- 1 file changed, 31 insertions(+), 15 deletions(-)
Documentation/devicetree/bindings/media/samsung-fimc.txt +31 -15
@@ -15,11 +15,21 @@ Common 'camera' node Required properties: -- compatible : must be "samsung,fimc", "simple-bus" -- clocks : list of clock specifiers, corresponding to entries in - the clock-names property; -- clock-names : must contain "sclk_cam0", "sclk_cam1", "pxl_async0", - "pxl_async1" entries, matching entries in the clocks property. +- compatible: must be "samsung,fimc", "simple-bus" +- clocks: list of clock specifiers, corresponding to entries in + the clock-names property; +- clock-names : must contain "sclk_cam0", "sclk_cam1", "pxl_async0", + "pxl_async1" entries, matching entries in the clocks property. + +- #clock-cells: from the common clock bindings (../clock/clock-bindings.txt), + must be 1. A clock provider is associated with the 'camera' node and it should + be referenced by external sensors that use clocks provided by the SoC on + CAM_*_CLKOUT pins. The clock specifier cell stores an index of a clock. + The indices are 0, 1 for CAM_A_CLKOUT, CAM_B_CLKOUT clocks respectively. + +- clock-output-names: from the common clock bindings, should contain names of + clocks registered by the camera subsystem corresponding to CAM_A_CLKOUT, + CAM_B_CLKOUT output clocks respectively. The pinctrl bindings defined in ../pinctrl/pinctrl-bindings.txt must be used to define a required pinctrl state named "default" and optional pinctrl states:
@@ -32,6 +42,7 @@ way around. The 'camera' node must include at least one 'fimc' child node. + 'fimc' device nodes -------------------
@@ -88,8 +99,8 @@ port nodes specifies data input - 0, 1 indicates input A, B respectively. Optional properties -- samsung,camclk-out : specifies clock output for remote sensor, - 0 - CAM_A_CLKOUT, 1 - CAM_B_CLKOUT; +- samsung,camclk-out (deprecated) : specifies clock output for remote sensor, + 0 - CAM_A_CLKOUT, 1 - CAM_B_CLKOUT; Image sensor nodes ------------------
@@ -97,8 +108,8 @@ Image sensor nodes The sensor device nodes should be added to their control bus controller (e.g. I2C0) nodes and linked to a port node in the csis or the parallel-ports node, using the common video interfaces bindings, defined in video-interfaces.txt. -The implementation of this bindings requires clock-frequency property to be -present in the sensor device nodes. +An optional clock-frequency property needs to be present in the sensor device +nodes. Default value when this property is not present is 24 MHz. Example:
@@ -114,7 +125,7 @@ Example: vddio-supply = <...>; clock-frequency = <24000000>; - clocks = <...>; + clocks = <&camera 1>; clock-names = "mclk"; port {
@@ -135,7 +146,7 @@ Example: vddio-supply = <...>; clock-frequency = <24000000>; - clocks = <...>; + clocks = <&camera 0>; clock-names = "mclk"; port {
@@ -149,12 +160,17 @@ Example: camera { compatible = "samsung,fimc", "simple-bus"; - #address-cells = <1>; - #size-cells = <1>; - status = "okay"; - + clocks = <&clock 132>, <&clock 133>, <&clock 351>, + <&clock 352>; + clock-names = "sclk_cam0", "sclk_cam1", "pxl_async0", + "pxl_async1"; + #clock-cells = <1>; + clock-output-names = "cam_a_clkout", "cam_b_clkout"; pinctrl-names = "default"; pinctrl-0 = <&cam_port_a_clk_active>; + status = "okay"; + #address-cells = <1>; + #size-cells = <1>; /* parallel camera ports */ parallel-ports {
--
1.7.9.5
Re: [PATCH v7 3/10] Documentation: devicetree: Update Samsung FIMC DT binding
Hi Sylwester,
Thank you for the patch.
Quoted 37 lines · On Tuesday 11 March 2014 17:00:35 Sylwester Nawrocki wrote:
This patch documents following updates of the Exynos4 SoC camera subsystem devicetree binding:
- addition of #clock-cells and clock-output-names properties to 'camera' node - these are now needed so the image sensor sub-devices can reference
clocks provided by the camera host interface,
- dropped a note about required clock-frequency properties at the image sensor nodes; the sensor devices can now control their clock explicitly through the clk API and there is no need to require this property in the camera host interface binding.Signed-off-by Sylwester Nawrocki <[email protected]>---
Resending only single patch which changed.Changes since v6:
- #clock-cells, clock-output-names documented as mandatory properties;
- renamed "cam_mclk_{a,b}" to "cam_{a,b}_clkout in the example dts, this now matches changes inexynos4.dtsifurther in the patch series;
- marked "samsung,camclk-out" property as deprecated.Changes since v5:
- none.Changes since v4:
- dropped a requirement of specific order of values in clocks/ clock-names properties (Mark) and reference to clock-names in clock-output-names property description (Mark).--- .../devicetree/bindings/media/samsung-fimc.txt | 46+++++++++++++------- 1 file changed, 31 insertions(+), 15 deletions(-)
(quoted diff) +0
-0
b/Documentation/devicetree/bindings/media/samsung-fimc.txtindex 96312f6..1908a5f 100644Documentation/devicetree/bindings/media/samsung-fimc.txt +0
-0
[snip]
Quoted 10 lines
Image sensor nodes
------------------Documentation/devicetree/bindings/media/samsung-fimc.txt +0
-0@@ -97,8 +108,8 @@ Image sensor nodes The sensor device nodes should be added to their control bus controller(e.g. I2C0) nodes and linked to a port node in the csis or the parallel-ports node, using the common video interfaces bindings, defined in video-interfaces.txt.
Documentation/devicetree/bindings/media/samsung-fimc.txt +1
-2-The implementation of this bindings requires clock-frequency property to be -present in the sensor device nodes. +An optional clock-frequency property needs to be present in the sensor
Quoted 2 lines
device
+nodes. Default value when this property is not present is 24 MHz.
I think you forgot to drop that sentence.
Signature · 3 lines
Regards, Laurent Pinchart
Sylwester Nawrocki
maintainerRe: [PATCH v7 3/10] Documentation: devicetree: Update Samsung FIMC DT binding
Hi Laurent,
Quoted 14 lines · Laurent Pinchart
Quoted 12 lines
Image sensor nodes
Quoted 11 lines
------------------
(quoted diff) +0
-0@@ -97,8 +108,8 @@ Image sensor nodes The sensor device nodes should be added to their control bus controller(e.g. I2C0) nodes and linked to a port node in the csis or the parallel-ports node, using the common video interfaces bindings, defined in video-interfaces.txt.
Documentation/devicetree/bindings/media/samsung-fimc.txt +1
-2-The implementation of this bindings requires clock-frequency property to be -present in the sensor device nodes. +An optional clock-frequency property needs to be present in the sensordevice
+nodes. Default value when this property is not present is 24 MHz.I think you forgot to drop that sentence.
Indeed, sorry, I'll fix that and repost.
--
Thanks,
Sylwester
Sylwester Nawrocki
maintainerPATCH v8 3/10 Documentation: devicetree: Update Samsung FIMC DT binding
In reply to Laurent PinchartThis patch documents following updates of the Exynos4 SoC camera subsystem devicetree binding:
- addition of #clock-cells and clock-output-names properties to 'camera' node - these are now needed so the image sensor sub-devices can reference clocks provided by the camera host interface,
- dropped a note about required clock-frequency properties at the image sensor nodes; the sensor devices can now control their clock explicitly through the clk API and there is no need to require this property in the camera host interface binding.
---
Changes since v7:
- dropped a note about clock-frequency property in sensor nodes.
Changes since v6:
- #clock-cells, clock-output-names documented as mandatory properties;
- renamed "cam_mclk_{a,b}" to "cam_{a,b}_clkout in the example dts, this now matches changes in exynos4.dtsi further in the patch series;
- marked "samsung,camclk-out" property as deprecated.
Changes since v5:
- none.
Changes since v4:
- dropped a requirement of specific order of values in clocks/ clock-names properties (Mark) and reference to clock-names in clock-output-names property description (Mark).
--- .../devicetree/bindings/media/samsung-fimc.txt | 44 +++++++++++++------- 1 file changed, 29 insertions(+), 15 deletions(-)
Documentation/devicetree/bindings/media/samsung-fimc.txt +29 -15
@@ -15,11 +15,21 @@ Common 'camera' node Required properties: -- compatible : must be "samsung,fimc", "simple-bus" -- clocks : list of clock specifiers, corresponding to entries in - the clock-names property; -- clock-names : must contain "sclk_cam0", "sclk_cam1", "pxl_async0", - "pxl_async1" entries, matching entries in the clocks property. +- compatible: must be "samsung,fimc", "simple-bus" +- clocks: list of clock specifiers, corresponding to entries in + the clock-names property; +- clock-names : must contain "sclk_cam0", "sclk_cam1", "pxl_async0", + "pxl_async1" entries, matching entries in the clocks property. + +- #clock-cells: from the common clock bindings (../clock/clock-bindings.txt), + must be 1. A clock provider is associated with the 'camera' node and it should + be referenced by external sensors that use clocks provided by the SoC on + CAM_*_CLKOUT pins. The clock specifier cell stores an index of a clock. + The indices are 0, 1 for CAM_A_CLKOUT, CAM_B_CLKOUT clocks respectively. + +- clock-output-names: from the common clock bindings, should contain names of + clocks registered by the camera subsystem corresponding to CAM_A_CLKOUT, + CAM_B_CLKOUT output clocks respectively. The pinctrl bindings defined in ../pinctrl/pinctrl-bindings.txt must be used to define a required pinctrl state named "default" and optional pinctrl states:
@@ -32,6 +42,7 @@ way around. The 'camera' node must include at least one 'fimc' child node. + 'fimc' device nodes -------------------
@@ -88,8 +99,8 @@ port nodes specifies data input - 0, 1 indicates input A, B respectively. Optional properties -- samsung,camclk-out : specifies clock output for remote sensor, - 0 - CAM_A_CLKOUT, 1 - CAM_B_CLKOUT; +- samsung,camclk-out (deprecated) : specifies clock output for remote sensor, + 0 - CAM_A_CLKOUT, 1 - CAM_B_CLKOUT; Image sensor nodes ------------------
@@ -97,8 +108,6 @@ Image sensor nodes The sensor device nodes should be added to their control bus controller (e.g. I2C0) nodes and linked to a port node in the csis or the parallel-ports node, using the common video interfaces bindings, defined in video-interfaces.txt. -The implementation of this bindings requires clock-frequency property to be -present in the sensor device nodes. Example:
@@ -114,7 +123,7 @@ Example: vddio-supply = <...>; clock-frequency = <24000000>; - clocks = <...>; + clocks = <&camera 1>; clock-names = "mclk"; port {
@@ -135,7 +144,7 @@ Example: vddio-supply = <...>; clock-frequency = <24000000>; - clocks = <...>; + clocks = <&camera 0>; clock-names = "mclk"; port {
@@ -149,12 +158,17 @@ Example: camera { compatible = "samsung,fimc", "simple-bus"; - #address-cells = <1>; - #size-cells = <1>; - status = "okay"; - + clocks = <&clock 132>, <&clock 133>, <&clock 351>, + <&clock 352>; + clock-names = "sclk_cam0", "sclk_cam1", "pxl_async0", + "pxl_async1"; + #clock-cells = <1>; + clock-output-names = "cam_a_clkout", "cam_b_clkout"; pinctrl-names = "default"; pinctrl-0 = <&cam_port_a_clk_active>; + status = "okay"; + #address-cells = <1>; + #size-cells = <1>; /* parallel camera ports */ parallel-ports {
--
1.7.9.5
Re: [PATCH v8 3/10] Documentation: devicetree: Update Samsung FIMC DT binding
Quoted 11 lines · On Tuesday 11 March 2014 17:34:30 Sylwester Nawrocki wrote:
This patch documents following updates of the Exynos4 SoC camera subsystem devicetree binding:
- addition of #clock-cells and clock-output-names properties to 'camera' node - these are now needed so the image sensor sub-devices can reference
clocks provided by the camera host interface,
- dropped a note about required clock-frequency properties at the image sensor nodes; the sensor devices can now control their clock explicitly through the clk API and there is no need to require this property in the camera host interface binding.
Quoted 1 line
Signed-off-by Sylwester Nawrocki <[email protected]>
Quoted 123 lines
---
Changes since v7:
- dropped a note about clock-frequency property in sensor nodes.Changes since v6:
- #clock-cells, clock-output-names documented as mandatory properties;
- renamed "cam_mclk_{a,b}" to "cam_{a,b}_clkout in the example dts, this now matches changes inexynos4.dtsifurther in the patch series;
- marked "samsung,camclk-out" property as deprecated.Changes since v5:
- none.Changes since v4:
- dropped a requirement of specific order of values in clocks/ clock-names properties (Mark) and reference to clock-names in clock-output-names property description (Mark).--- .../devicetree/bindings/media/samsung-fimc.txt | 44 +++++++++++------ 1 file changed, 29 insertions(+), 15 deletions(-)(quoted diff) +0
-0
b/Documentation/devicetree/bindings/media/samsung-fimc.txtindex 96312f6..922d6f8 100644Documentation/devicetree/bindings/media/samsung-fimc.txt +7
-5@@ -15,11 +15,21 @@ Common 'camera' node Required properties: -- compatible : must be "samsung,fimc", "simple-bus" -- clocks : list of clock specifiers, corresponding to entries in - the clock-names property; -- clock-names : must contain "sclk_cam0", "sclk_cam1", "pxl_async0", - "pxl_async1" entries, matching entries in the clocks property. +- compatible: must be "samsung,fimc", "simple-bus" +- clocks: list of clock specifiers, corresponding to entries in + the clock-names property; +- clock-names : must contain "sclk_cam0", "sclk_cam1", "pxl_async0", + "pxl_async1" entries, matching entries in the clocks property. + +- #clock-cells: from the common clock bindings(
../clock/clock-bindings.txt), + must be 1. A clock provider is associated with the 'camera' node and it should + be referenced by external sensors that use clocks provided by the SoC on + CAM_*_CLKOUT pins. The clock specifier cell stores an index of a clock. + The indices are 0, 1 forCAM_A_CLKOUT,CAM_B_CLKOUTclocks respectively. ++- clock-output-names: from the common clock bindings, should contain names of + clocks registered by the camera subsystem corresponding to CAM_A_CLKOUT, + CAM_B_CLKOUT output clocks respectively.The pinctrl bindings defined in
../pinctrl/pinctrl-bindings.txtmust be
used to define a required pinctrl state named "default" and optional pinctrl states: @@ -32,6 +42,7 @@ way around.The 'camera' node must include at least one 'fimc' child node.
Documentation/devicetree/bindings/media/samsung-fimc.txt +1
-0+ 'fimc' device nodes -------------------@@ -88,8 +99,8 @@ port nodes specifies data input - 0, 1 indicates input A,B respectively.
Optional properties
-- samsung,camclk-out : specifies clock output for remote sensor,
Documentation/devicetree/bindings/media/samsung-fimc.txt +1
-1- 0 - CAM_A_CLKOUT, 1 - CAM_B_CLKOUT; +- samsung,camclk-out (deprecated) : specifies clock output for remotesensor, + 0 -
CAM_A_CLKOUT, 1 -CAM_B_CLKOUT;Image sensor nodes
------------------Documentation/devicetree/bindings/media/samsung-fimc.txt +0
-0@@ -97,8 +108,6 @@ Image sensor nodes The sensor device nodes should be added to their control bus controller(e.g. I2C0) nodes and linked to a port node in the csis or the parallel-ports node, using the common video interfaces bindings, defined in video-interfaces.txt.
Documentation/devicetree/bindings/media/samsung-fimc.txt +11
-8-The implementation of this bindings requires clock-frequency property to be -present in the sensor device nodes. Example:@@ -114,7 +123,7 @@ Example: vddio-supply = <...>; clock-frequency = <24000000>; - clocks = <...>; + clocks = <&camera 1>; clock-names = "mclk"; port {@@ -135,7 +144,7 @@ Example: vddio-supply = <...>; clock-frequency = <24000000>; - clocks = <...>; + clocks = <&camera 0>; clock-names = "mclk"; port {@@ -149,12 +158,17 @@ Example: camera { compatible = "samsung,fimc", "simple-bus"; - #address-cells = <1>; - #size-cells = <1>; - status = "okay"; - + clocks = <&clock 132>, <&clock 133>, <&clock 351>, + <&clock 352>; + clock-names = "sclk_cam0", "sclk_cam1", "pxl_async0", + "pxl_async1"; + #clock-cells = <1>; + clock-output-names = "cam_a_clkout", "cam_b_clkout"; pinctrl-names = "default"; pinctrl-0 = <&cam_port_a_clk_active>; + status = "okay"; + #address-cells = <1>; + #size-cells = <1>; /* parallel camera ports */ parallel-ports {
Signature · 3 lines
Regards, Laurent Pinchart
Re: [PATCH v8 3/10] Documentation: devicetree: Update Samsung FIMC DT binding
Mark,
Could you please also review this patch? It is the only thing pending for this 10 patches series to be merged.
Thank you!
Mauro
Em Tue, 11 Mar 2014 17:38:23 +0100
Laurent Pinchart <[email protected]> escreveu:
Quoted 141 lines · On Tuesday 11 March 2014 17:34:30 Sylwester Nawrocki wrote:
Quoted 11 lines
This patch documents following updates of the Exynos4 SoC camera subsystem devicetree binding:
- addition of #clock-cells and clock-output-names properties to 'camera' node - these are now needed so the image sensor sub-devices can reference
clocks provided by the camera host interface,
- dropped a note about required clock-frequency properties at the image sensor nodes; the sensor devices can now control their clock explicitly through the clk API and there is no need to require this property in the camera host interface binding.Quoted 1 line
Signed-off-by Sylwester Nawrocki <[email protected]>Acked-by Laurent Pinchart <[email protected]>Quoted 123 lines
---
Changes since v7:
- dropped a note about clock-frequency property in sensor nodes.Changes since v6:
- #clock-cells, clock-output-names documented as mandatory properties;
- renamed "cam_mclk_{a,b}" to "cam_{a,b}_clkout in the example dts, this now matches changes inexynos4.dtsifurther in the patch series;
- marked "samsung,camclk-out" property as deprecated.Changes since v5:
- none.Changes since v4:
- dropped a requirement of specific order of values in clocks/ clock-names properties (Mark) and reference to clock-names in clock-output-names property description (Mark).--- .../devicetree/bindings/media/samsung-fimc.txt | 44 +++++++++++------ 1 file changed, 29 insertions(+), 15 deletions(-)(quoted diff) +0
-0
b/Documentation/devicetree/bindings/media/samsung-fimc.txtindex 96312f6..922d6f8 100644Documentation/devicetree/bindings/media/samsung-fimc.txt +7
-5@@ -15,11 +15,21 @@ Common 'camera' node Required properties: -- compatible : must be "samsung,fimc", "simple-bus" -- clocks : list of clock specifiers, corresponding to entries in - the clock-names property; -- clock-names : must contain "sclk_cam0", "sclk_cam1", "pxl_async0", - "pxl_async1" entries, matching entries in the clocks property. +- compatible: must be "samsung,fimc", "simple-bus" +- clocks: list of clock specifiers, corresponding to entries in + the clock-names property; +- clock-names : must contain "sclk_cam0", "sclk_cam1", "pxl_async0", + "pxl_async1" entries, matching entries in the clocks property. + +- #clock-cells: from the common clock bindings(
../clock/clock-bindings.txt), + must be 1. A clock provider is associated with the 'camera' node and it should + be referenced by external sensors that use clocks provided by the SoC on + CAM_*_CLKOUT pins. The clock specifier cell stores an index of a clock. + The indices are 0, 1 forCAM_A_CLKOUT,CAM_B_CLKOUTclocks respectively. ++- clock-output-names: from the common clock bindings, should contain names of + clocks registered by the camera subsystem corresponding to CAM_A_CLKOUT, + CAM_B_CLKOUT output clocks respectively.The pinctrl bindings defined in
../pinctrl/pinctrl-bindings.txtmust be
used to define a required pinctrl state named "default" and optional pinctrl states: @@ -32,6 +42,7 @@ way around.The 'camera' node must include at least one 'fimc' child node.
Documentation/devicetree/bindings/media/samsung-fimc.txt +1
-0+ 'fimc' device nodes -------------------@@ -88,8 +99,8 @@ port nodes specifies data input - 0, 1 indicates input A,B respectively.
Optional properties
-- samsung,camclk-out : specifies clock output for remote sensor,
Documentation/devicetree/bindings/media/samsung-fimc.txt +1
-1- 0 - CAM_A_CLKOUT, 1 - CAM_B_CLKOUT; +- samsung,camclk-out (deprecated) : specifies clock output for remotesensor, + 0 -
CAM_A_CLKOUT, 1 -CAM_B_CLKOUT;Image sensor nodes
------------------Documentation/devicetree/bindings/media/samsung-fimc.txt +0
-0@@ -97,8 +108,6 @@ Image sensor nodes The sensor device nodes should be added to their control bus controller(e.g. I2C0) nodes and linked to a port node in the csis or the parallel-ports node, using the common video interfaces bindings, defined in video-interfaces.txt.
Documentation/devicetree/bindings/media/samsung-fimc.txt +11
-8-The implementation of this bindings requires clock-frequency property to be -present in the sensor device nodes. Example:@@ -114,7 +123,7 @@ Example: vddio-supply = <...>; clock-frequency = <24000000>; - clocks = <...>; + clocks = <&camera 1>; clock-names = "mclk"; port {@@ -135,7 +144,7 @@ Example: vddio-supply = <...>; clock-frequency = <24000000>; - clocks = <...>; + clocks = <&camera 0>; clock-names = "mclk"; port {@@ -149,12 +158,17 @@ Example: camera { compatible = "samsung,fimc", "simple-bus"; - #address-cells = <1>; - #size-cells = <1>; - status = "okay"; - + clocks = <&clock 132>, <&clock 133>, <&clock 351>, + <&clock 352>; + clock-names = "sclk_cam0", "sclk_cam1", "pxl_async0", + "pxl_async1"; + #clock-cells = <1>; + clock-output-names = "cam_a_clkout", "cam_b_clkout"; pinctrl-names = "default"; pinctrl-0 = <&cam_port_a_clk_active>; + status = "okay"; + #address-cells = <1>; + #size-cells = <1>; /* parallel camera ports */ parallel-ports {
Signature · 2 lines
Regards, Mauro
Re: [PATCH v8 3/10] Documentation: devicetree: Update Samsung FIMC DT binding
In reply to Sylwester NawrockiQuoted 38 lines · Sylwester Nawrocki
This patch documents following updates of the Exynos4 SoC camera subsystem devicetree binding:
- addition of #clock-cells and clock-output-names properties to 'camera' node - these are now needed so the image sensor sub-devices can reference clocks provided by the camera host interface,
- dropped a note about required clock-frequency properties at the image sensor nodes; the sensor devices can now control their clock explicitly through the clk API and there is no need to require this property in the camera host interface binding.Signed-off-by Sylwester Nawrocki <[email protected]>---
Changes since v7:
- dropped a note about clock-frequency property in sensor nodes.Changes since v6:
- #clock-cells, clock-output-names documented as mandatory properties;
- renamed "cam_mclk_{a,b}" to "cam_{a,b}_clkout in the example dts, this now matches changes inexynos4.dtsifurther in the patch series;
- marked "samsung,camclk-out" property as deprecated.Changes since v5:
- none.Changes since v4:
- dropped a requirement of specific order of values in clocks/ clock-names properties (Mark) and reference to clock-names in clock-output-names property description (Mark).--- .../devicetree/bindings/media/samsung-fimc.txt | 44 +++++++++++++------- 1 file changed, 29 insertions(+), 15 deletions(-)Documentation/devicetree/bindings/media/samsung-fimc.txt +0
-0
Documentation/devicetree/bindings/media/samsung-fimc.txt Common 'camera' node · line 18
Quoted 4 lines
Documentation/devicetree/bindings/media/samsung-fimc.txt +0
-1@@ -15,11 +15,21 @@ Common 'camera' node Required properties: -- compatible : must be "samsung,fimc", "simple-bus"
While it was already the case, why is "samsung,fimc" also a simple bus?
If it has any properties which are required for the correct use of the child nodes, it is _not_ a simple-bus.
Quoted 9 lines
Documentation/devicetree/bindings/media/samsung-fimc.txt +5
-4-- clocks : list of clock specifiers, corresponding to entries in - the clock-names property; -- clock-names : must contain "sclk_cam0", "sclk_cam1", "pxl_async0", - "pxl_async1" entries, matching entries in the clocks property. +- compatible: must be "samsung,fimc", "simple-bus" +- clocks: list of clock specifiers, corresponding to entries in + the clock-names property; +- clock-names : must contain "sclk_cam0", "sclk_cam1", "pxl_async0", + "pxl_async1" entries, matching entries in the clocks property.
Documentation/devicetree/bindings/media/samsung-fimc.txt Common 'camera' node · line 32
Quoted 10 lines
Documentation/devicetree/bindings/media/samsung-fimc.txt +10
-0+ +- #clock-cells: from the common clock bindings (../clock/clock-bindings.txt), + must be 1. A clock provider is associated with the 'camera' node and it should + be referenced by external sensors that use clocks provided by the SoC on + CAM_*_CLKOUT pins. The clock specifier cell stores an index of a clock. + The indices are 0, 1 for CAM_A_CLKOUT, CAM_B_CLKOUT clocks respectively. + +- clock-output-names: from the common clock bindings, should contain names of + clocks registered by the camera subsystem corresponding to CAM_A_CLKOUT, + CAM_B_CLKOUT output clocks respectively.
And if we're adding more stuff required by child nodes it's definitely not a simple-bus.
Get rid of simple-bus. Get the driver for "samsung,fimc" to probe its children once it has set up.
Apologies for the late reply, and sorry to have to be negative on this.
Thanks,
Mark.
Quoted 75 lines
Documentation/devicetree/bindings/media/samsung-fimc.txt +14
-11The pinctrl bindings defined in ../pinctrl/pinctrl-bindings.txt must be used to define a required pinctrl state named "default" and optional pinctrl states:@@ -32,6 +42,7 @@ way around. The 'camera' node must include at least one 'fimc' child node. + 'fimc' device nodes -------------------@@ -88,8 +99,8 @@ port nodes specifies data input - 0, 1 indicates input A, B respectively. Optional properties -- samsung,camclk-out : specifies clock output for remote sensor, - 0 - CAM_A_CLKOUT, 1 - CAM_B_CLKOUT; +- samsung,camclk-out (deprecated) : specifies clock output for remote sensor, + 0 - CAM_A_CLKOUT, 1 - CAM_B_CLKOUT; Image sensor nodes ------------------@@ -97,8 +108,6 @@ Image sensor nodes The sensor device nodes should be added to their control bus controller (e.g. I2C0) nodes and linked to a port node in the csis or the parallel-ports node, using the common video interfaces bindings, defined in video-interfaces.txt. -The implementation of this bindings requires clock-frequency property to be -present in the sensor device nodes. Example:@@ -114,7 +123,7 @@ Example: vddio-supply = <...>; clock-frequency = <24000000>; - clocks = <...>; + clocks = <&camera 1>; clock-names = "mclk"; port {@@ -135,7 +144,7 @@ Example: vddio-supply = <...>; clock-frequency = <24000000>; - clocks = <...>; + clocks = <&camera 0>; clock-names = "mclk"; port {@@ -149,12 +158,17 @@ Example: camera { compatible = "samsung,fimc", "simple-bus"; - #address-cells = <1>; - #size-cells = <1>; - status = "okay"; - + clocks = <&clock 132>, <&clock 133>, <&clock 351>, + <&clock 352>; + clock-names = "sclk_cam0", "sclk_cam1", "pxl_async0", + "pxl_async1"; + #clock-cells = <1>; + clock-output-names = "cam_a_clkout", "cam_b_clkout"; pinctrl-names = "default"; pinctrl-0 = <&cam_port_a_clk_active>; + status = "okay"; + #address-cells = <1>; + #size-cells = <1>; /* parallel camera ports */ parallel-ports { --1.7.9.5
Sylwester Nawrocki
maintainerRe: [PATCH v8 3/10] Documentation: devicetree: Update Samsung FIMC DT binding
Hi Mark,
Thanks for your review.
Quoted 40 lines · Mark Rutland
Quoted 39 lines · Sylwester Nawrocki
Quoted 39 lines
This patch documents following updates of the Exynos4 SoC camera subsystem devicetree binding:
- addition of #clock-cells and clock-output-names properties to 'camera' node - these are now needed so the image sensor sub-devices can reference clocks provided by the camera host interface,
- dropped a note about required clock-frequency properties at the image sensor nodes; the sensor devices can now control their clock explicitly through the clk API and there is no need to require this property in the camera host interface binding.Signed-off-by Sylwester Nawrocki <[email protected]>---
Changes since v7:
- dropped a note about clock-frequency property in sensor nodes.Changes since v6:
- #clock-cells, clock-output-names documented as mandatory properties;
- renamed "cam_mclk_{a,b}" to "cam_{a,b}_clkout in the example dts, this now matches changes inexynos4.dtsifurther in the patch series;
- marked "samsung,camclk-out" property as deprecated.Changes since v5:
- none.Changes since v4:
- dropped a requirement of specific order of values in clocks/ clock-names properties (Mark) and reference to clock-names in clock-output-names property description (Mark).--- .../devicetree/bindings/media/samsung-fimc.txt | 44 +++++++++++++------- 1 file changed, 29 insertions(+), 15 deletions(-)Documentation/devicetree/bindings/media/samsung-fimc.txt +0
-0@@ -15,11 +15,21 @@ Common 'camera' node
Quoted 5 lines
Quoted 3 lines
Documentation/devicetree/bindings/media/samsung-fimc.txt Common 'camera' node · line 18
Quoted 3 lines
Documentation/devicetree/bindings/media/samsung-fimc.txt +0
-1Required properties: -- compatible : must be "samsung,fimc", "simple-bus"While it was already the case, why is "samsung,fimc" also a simple bus?
IIRC the main reason was to have children automatically instantiated in Linux, also "samsung,fimc" is a master node gathering the all related camera subsystem memory mapped IP blocks.
Quoted 2 lines
If it has any properties which are required for the correct use of the child nodes, it is _not_ a simple-bus.
Then this can be considered a separate issue, and could be addressed in a separate patch ?
Quoted 23 lines
Quoted 11 lines
Quoted 11 lines
Documentation/devicetree/bindings/media/samsung-fimc.txt +7
-4-- clocks : list of clock specifiers, corresponding to entries in - the clock-names property; -- clock-names : must contain "sclk_cam0", "sclk_cam1", "pxl_async0", - "pxl_async1" entries, matching entries in the clocks property. +- compatible: must be "samsung,fimc", "simple-bus" +- clocks: list of clock specifiers, corresponding to entries in + the clock-names property; +- clock-names : must contain "sclk_cam0", "sclk_cam1", "pxl_async0", + "pxl_async1" entries, matching entries in the clocks property. + +- #clock-cells: from the common clock bindings (../clock/clock-bindings.txt),Quoted 8 lines
Documentation/devicetree/bindings/media/samsung-fimc.txt Common 'camera' node · line 32
Quoted 8 lines
Documentation/devicetree/bindings/media/samsung-fimc.txt +8
-0+ must be 1. A clock provider is associated with the 'camera' node and it should + be referenced by external sensors that use clocks provided by the SoC on + CAM_*_CLKOUT pins. The clock specifier cell stores an index of a clock. + The indices are 0, 1 for CAM_A_CLKOUT, CAM_B_CLKOUT clocks respectively. + +- clock-output-names: from the common clock bindings, should contain names of + clocks registered by the camera subsystem corresponding to CAM_A_CLKOUT, + CAM_B_CLKOUT output clocks respectively.And if we're adding more stuff required by child nodes it's definitely not a simple-bus.
Quoted 4 lines
Get rid of simple-bus. Get the driver for "samsung,fimc" to probe its children once it has set up.
Apologies for the late reply, and sorry to have to be negative on this.
While I agree with you that the "simple-bus" might not be a best idea, I can't see how your suggestion could be now implemented in Linux, since AFAIK there is no API like of_platform_unpopulate(), so device objects can be removed upon the driver module removal.
--
Regards,
Sylwester
Sylwester Nawrocki
maintainerPATCH v6 4/10 V4L: Add driver for s5k6a3 image sensor
In reply to Sylwester NawrockiThis patch adds subdev driver for Samsung S5K6A3 raw image sensor. As it is intended at the moment to be used only with the Exynos FIMC-IS (camera ISP) subsystem it is pretty minimal subdev driver. It doesn't do any I2C communication since the sensor is controlled by the ISP and its own firmware.
This driver, if needed, can be updated in future into a regular subdev driver where the main CPU communicates with the sensor directly.
---
Changes since v5:
- added missing sd->internal_ops assignment (addressing comments from Baruch Siach),
- removed dependency on OF from Kconfig.
Changes since v4:
- none.
Changes since v3:
- clk_get() called moved from s_power() to driver probe() callback.
Changes since v1:
- added missing pm_runtime_disable(),
- removed subdev name overriding,
- s/S5K6A3_DEF_PIX/S5K6A3_DEFAULT_,
- merged patch adding v4l2-async API support,
- clock-frequency property is now optional and a default frequency value will be used when it is missing, rather than bailing out,
- reset GPIO made mandatory as it is required for proper power on/off sequences and all known boards use it,
- maximum image size is now 1412x1412 pixels, as specified in in the S5K6A3YX datasheet.
- regulator_enable()/disable() used instead of the regulator bulk API to ensure proper regulators enable/disable sequences.
--- drivers/media/i2c/Kconfig | 8 + drivers/media/i2c/Makefile | 1 + drivers/media/i2c/s5k6a3.c | 389 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 398 insertions(+) create mode 100644 drivers/media/i2c/s5k6a3.c
drivers/media/i2c/Kconfig +8 -0
@@ -579,6 +579,14 @@ config VIDEO_S5K6AA This is a V4L2 sensor-level driver for Samsung S5K6AA(FX) 1.3M camera sensor with an embedded SoC image signal processor. +config VIDEO_S5K6A3 + tristate "Samsung S5K6A3 sensor support" + depends on MEDIA_CAMERA_SUPPORT + depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API + ---help--- + This is a V4L2 sensor-level driver for Samsung S5K6A3 raw + camera sensor. + config VIDEO_S5K4ECGX tristate "Samsung S5K4ECGX sensor support" depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
drivers/media/i2c/Makefile +1 -0
@@ -66,6 +66,7 @@ obj-$(CONFIG_VIDEO_MT9V032) += mt9v032.o obj-$(CONFIG_VIDEO_SR030PC30) += sr030pc30.o obj-$(CONFIG_VIDEO_NOON010PC30) += noon010pc30.o obj-$(CONFIG_VIDEO_S5K6AA) += s5k6aa.o +obj-$(CONFIG_VIDEO_S5K6A3) += s5k6a3.o obj-$(CONFIG_VIDEO_S5K4ECGX) += s5k4ecgx.o obj-$(CONFIG_VIDEO_S5K5BAF) += s5k5baf.o obj-$(CONFIG_VIDEO_S5C73M3) += s5c73m3/
drivers/media/i2c/s5k6a3.c new file +389 -0
389 lines not on this page · raw message
Signature · 1 line
1.7.9.5
Sylwester Nawrocki
maintainerPATCH v6 5/10 V4L: s5c73m3: Add device tree support
In reply to Sylwester NawrockiThis patch adds the V4L2 asynchronous subdev registration and device tree support. Common clock API is used to control the sensor master clock from within the subdev.
---
Changes since v5:
- none.
Changes since v3:
- move clk_get from subdev s_power() to driver probe() callback and make it devm_clk_get(),
- pass any errors from dev_clk_get() through, rather than overwriting with EPROBE_DEFER. Patch [1] is required for things to work correctly after this change.
[1] http://www.spinics.net/lists/arm-kernel/msg306072.html
--- drivers/media/i2c/s5c73m3/s5c73m3-core.c | 207 ++++++++++++++++++++++-------- drivers/media/i2c/s5c73m3/s5c73m3-spi.c | 6 + drivers/media/i2c/s5c73m3/s5c73m3.h | 4 + 3 files changed, 167 insertions(+), 50 deletions(-)
drivers/media/i2c/s5c73m3/s5c73m3-core.c +157 -50
@@ -15,7 +15,7 @@* GNU General Public License for more details. */ -#include <linux/sizes.h> +#include <linux/clk.h> #include <linux/delay.h> #include <linux/firmware.h> #include <linux/gpio.h>
@@ -23,7 +23,9 @@#include <linux/init.h> #include <linux/media.h> #include <linux/module.h> +#include <linux/of_gpio.h> #include <linux/regulator/consumer.h> +#include <linux/sizes.h> #include <linux/slab.h> #include <linux/spi/spi.h> #include <linux/videodev2.h>
@@ -33,6 +35,7 @@#include <media/v4l2-subdev.h> #include <media/v4l2-mediabus.h> #include <media/s5c73m3.h> +#include <media/v4l2-of.h> #include "s5c73m3.h"
@@ -46,6 +49,8 @@ static int update_fw;module_param (update_fw, int,0644 );#define S5C73M3_EMBEDDED_DATA_MAXLEN SZ_4K +#define S5C73M3_MIPI_DATA_LANES 4 +#define S5C73M3_CLK_NAME "cis_extclk"static const char *const s5c73m3_supply_names[S5C73M3_MAX_SUPPLIES ] = {"vdd-int" ,/* Digital Core supply (1.2V), CAM_ISP_CORE_1.2V */
@@ -1355,9 +1360,20 @@ static int __s5c73m3_power_on(struct s5c73m3 *state)for (i =0 ; i <S5C73M3_MAX_SUPPLIES ; i++) { ret =regulator_enable (state->supplies[i].consumer);if (ret) - goto err; + goto err_reg_dis; } + ret =clk_set_rate (state->clock, state->mclk_frequency); +if (ret <0 ) + goto err_reg_dis; + + ret =clk_prepare_enable (state->clock); +if (ret <0 ) + goto err_reg_dis; + +v4l2_dbg (1 , s5c73m3_dbg, &state->oif_sd,"clock frequency: %ld\n" , +clk_get_rate (state->clock)); +s5c73m3_gpio_deassert (state,STBY );usleep_range (100 ,200 );
@@ -1365,7 +1381,8 @@ static int __s5c73m3_power_on(struct s5c73m3 *state)usleep_range (50 ,100 );return 0 ; -err: + +err_reg_dis:for (--i; i >=0 ; i--)regulator_disable (state->supplies[i].consumer);return ret;
@@ -1380,6 +1397,9 @@ static int __s5c73m3_power_off(struct s5c73m3 *state)if (s5c73m3_gpio_assert (state,STBY ))usleep_range (100 ,200 ); + +clk_disable_unprepare (state->clock); + state->streaming =0 ; state->isp_ready =0 ;
@@ -1388,6 +1408,7 @@ static int __s5c73m3_power_off(struct s5c73m3 *state)if (ret) goto err; } +return 0 ; err:for (++i; i <S5C73M3_MAX_SUPPLIES ; i++) {
@@ -1396,6 +1417,8 @@ err:v4l2_err (&state->oif_sd,"Failed to reenable %s: %d\n" , state->supplies[i].supply, r); } + +clk_prepare_enable (state->clock);return ret; }
@@ -1451,17 +1474,6 @@ static int s5c73m3_oif_registered(struct v4l2_subdev *sd)S5C73M3_JPEG_PAD , &state->oif_sd.entity,OIF_JPEG_PAD , MEDIA_LNK_FL_IMMUTABLE | MEDIA_LNK_FL_ENABLED); -mutex_lock (&state->lock); - ret =__s5c73m3_power_on (state); -if (ret ==0 ) -s5c73m3_get_fw_version (state); - -__s5c73m3_power_off (state); -mutex_unlock (&state->lock); - -v4l2_dbg (1 , s5c73m3_dbg, sd,"%s: Booting %s (%d)\n" , - __func__, ret ?"failed" :"succeeded" , ret); -return ret; }
@@ -1519,41 +1531,112 @@ static const struct v4l2_subdev_ops oif_subdev_ops = { .video = &s5c73m3_oif_video_ops, }; -static int s5c73m3_configure_gpios (struct s5c73m3 *state, -const struct s5c73m3_platform_data *pdata) +static int s5c73m3_configure_gpios (struct s5c73m3 *state) +{ +static const char *const gpio_names[] = { +"S5C73M3_STBY" ,"S5C73M3_RST" + }; +struct i2c_client *c = state->i2c_client; +struct s5c73m3_gpio *g = state->gpio; +int ret, i; + +for (i =0 ; i <GPIO_NUM ; ++i) { +unsigned int flags =GPIOF_DIR_OUT ; +if (g[i].level) + flags |=GPIOF_INIT_HIGH ; + ret =devm_gpio_request_one (&c->dev, g[i].gpio, flags, + gpio_names[i]); +if (ret) { +v4l2_err (c,"failed to request gpio %s\n" , + gpio_names[i]); +return ret; + } + } +return 0 ; +} + +static int s5c73m3_parse_gpios (struct s5c73m3 *state) +{ +static const char *const prop_names[] = { +"standby-gpios" ,"xshutdown-gpios" , + }; +struct device *dev = &state->i2c_client->dev; +struct device_node *node = dev->of_node; +int ret, i; + +for (i =0 ; i <GPIO_NUM ; ++i) { +enum of_gpio_flags of_flags; + + ret =of_get_named_gpio_flags (node, prop_names[i], +0 , &of_flags); +if (ret <0 ) { +dev_err (dev,"failed to parse %s DT property\n" , + prop_names[i]); +return -EINVAL ; + } + state->gpio[i].gpio = ret; + state->gpio[i].level = !(of_flags &OF_GPIO_ACTIVE_LOW ); + } +return 0 ; +} + +static int s5c73m3_get_platform_data (struct s5c73m3 *state) {struct device *dev = &state->i2c_client->dev; -const struct s5c73m3_gpio *gpio; -unsigned long flags; +const struct s5c73m3_platform_data *pdata = dev->platform_data; +struct device_node *node = dev->of_node; +struct device_node *node_ep; +struct v4l2_of_endpoint ep;int ret; - state->gpio[STBY ].gpio = -EINVAL ; - state->gpio[RST ].gpio = -EINVAL ; +if (!node) { +if (!pdata) { +dev_err (dev,"Platform data not specified\n" ); +return -EINVAL ; + } - gpio = &pdata->gpio_stby; -if (gpio_is_valid (gpio->gpio)) { - flags = (gpio->level ?GPIOF_OUT_INIT_HIGH :GPIOF_OUT_INIT_LOW ) - |GPIOF_EXPORT ; - ret =devm_gpio_request_one (dev, gpio->gpio, flags, -"S5C73M3_STBY" ); -if (ret <0 ) -return ret; + state->mclk_frequency = pdata->mclk_frequency; + state->gpio[STBY ] = pdata->gpio_stby; + state->gpio[RST ] = pdata->gpio_reset; +return 0 ; + } + + state->clock =devm_clk_get (dev,S5C73M3_CLK_NAME ); +if (IS_ERR (state->clock)) +return PTR_ERR (state->clock); - state->gpio[STBY ] = *gpio; +if (of_property_read_u32 (node,"clock-frequency" , + &state->mclk_frequency)) { + state->mclk_frequency =S5C73M3_DEFAULT_MCLK_FREQ ; +dev_info (dev,"using default %u Hz clock frequency\n" , + state->mclk_frequency); } - gpio = &pdata->gpio_reset; -if (gpio_is_valid (gpio->gpio)) { - flags = (gpio->level ?GPIOF_OUT_INIT_HIGH :GPIOF_OUT_INIT_LOW ) - |GPIOF_EXPORT ; - ret =devm_gpio_request_one (dev, gpio->gpio, flags, -"S5C73M3_RST" ); -if (ret <0 ) -return ret; + ret =s5c73m3_parse_gpios (state); +if (ret <0 ) +return -EINVAL ; - state->gpio[RST ] = *gpio; + node_ep =v4l2_of_get_next_endpoint (node,NULL ); +if (!node_ep) { +dev_warn (dev,"no endpoint defined for node: %s\n" , + node->full_name); +return 0 ; } +v4l2_of_parse_endpoint (node_ep, &ep); +of_node_put (node_ep); + +if (ep.bus_type !=V4L2_MBUS_CSI2 ) { +dev_err (dev,"unsupported bus type\n" ); +return -EINVAL ; + } +/* +* Number of MIPI CSI-2 data lanes is currently not configurable, +* always a default value of 4 lanes is used. +*/ +if (ep.bus.mipi_csi2.num_data_lanes !=S5C73M3_MIPI_DATA_LANES ) +dev_info (dev,"falling back to 4 MIPI CSI-2 data lanes\n" ); +return 0 ; }
@@ -1561,21 +1644,20 @@ static int s5c73m3_probe(struct i2c_client *client, const struct i2c_device_id *id) {struct device *dev = &client->dev; -const struct s5c73m3_platform_data *pdata = client->dev.platform_data;struct v4l2_subdev *sd;struct v4l2_subdev *oif_sd;struct s5c73m3 *state;int ret, i; -if (pdata ==NULL ) { -dev_err (&client->dev,"Platform data not specified\n" ); -return -EINVAL ; - } - state =devm_kzalloc (dev,sizeof (*state),GFP_KERNEL );if (!state)return -ENOMEM ; + state->i2c_client = client; + ret =s5c73m3_get_platform_data (state); +if (ret <0 ) +return ret; +mutex_init (&state->lock); sd = &state->sensor_sd; oif_sd = &state->oif_sd;
@@ -1613,11 +1695,7 @@ static int s5c73m3_probe(struct i2c_client *client,if (ret <0 )return ret; - state->mclk_frequency = pdata->mclk_frequency; - state->bus_type = pdata->bus_type; - state->i2c_client = client; - - ret =s5c73m3_configure_gpios (state, pdata); + ret =s5c73m3_configure_gpios (state);if (ret) goto out_err;
@@ -1651,9 +1729,29 @@ static int s5c73m3_probe(struct i2c_client *client,if (ret <0 ) goto out_err; + oif_sd->dev = dev; + + ret =__s5c73m3_power_on (state); +if (ret <0 ) + goto out_err1; + + ret =s5c73m3_get_fw_version (state); +__s5c73m3_power_off (state); + +if (ret <0 ) { +dev_err (dev,"Device detection failed: %d\n" , ret); + goto out_err1; + } + + ret =v4l2_async_register_subdev (oif_sd); +if (ret <0 ) + goto out_err1; +v4l2_info (sd,"%s: completed successfully\n" , __func__);return 0 ; +out_err1: +s5c73m3_unregister_spi_driver (state); out_err:media_entity_cleanup (&sd->entity);return ret;
@@ -1665,7 +1763,7 @@ static int s5c73m3_remove(struct i2c_client *client)struct s5c73m3 *state =oif_sd_to_s5c73m3 (oif_sd);struct v4l2_subdev *sensor_sd = &state->sensor_sd; -v4l2_device_unregister_subdev (oif_sd); +v4l2_async_unregister_subdev (oif_sd);v4l2_ctrl_handler_free (oif_sd->ctrl_handler);media_entity_cleanup (&oif_sd->entity);
@@ -1684,8 +1782,17 @@ static const struct i2c_device_id s5c73m3_id[] = { };MODULE_DEVICE_TABLE (i2c, s5c73m3_id); +#ifdef CONFIG_OF +static const struct of_device_id s5c73m3_of_match[] = { + { .compatible ="samsung,s5c73m3" }, + { } +}; +MODULE_DEVICE_TABLE (of, s5c73m3_of_match); +#endif +static struct i2c_driver s5c73m3_i2c_driver = { .driver = { + .of_match_table =of_match_ptr (s5c73m3_of_match), .name =DRIVER_NAME , }, .probe = s5c73m3_probe,
drivers/media/i2c/s5c73m3/s5c73m3-spi.c +6 -0
@@ -27,6 +27,11 @@#define S5C73M3_SPI_DRV_NAME "S5C73M3-SPI" +static const struct of_device_id s5c73m3_spi_ids[] = { + { .compatible ="samsung,s5c73m3" }, + { } +}; +enum spi_direction {SPI_DIR_RX ,SPI_DIR_TX
@@ -146,6 +151,7 @@ int s5c73m3_register_spi_driver(struct s5c73m3 *state) spidrv->driver.name =S5C73M3_SPI_DRV_NAME ; spidrv->driver.bus = &spi_bus_type; spidrv->driver.owner =THIS_MODULE ; + spidrv->driver.of_match_table = s5c73m3_spi_ids;return spi_register_driver (spidrv); }
drivers/media/i2c/s5c73m3/s5c73m3.h +4 -0
@@ -17,6 +17,7 @@#ifndef S5C73M3_H_ #define S5C73M3_H_ +#include <linux/clk.h> #include <linux/kernel.h> #include <linux/regulator/consumer.h> #include <media/v4l2-common.h>
@@ -321,6 +322,7 @@ enum s5c73m3_oif_pads {#define S5C73M3_MAX_SUPPLIES 6 +#define S5C73M3_DEFAULT_MCLK_FREQ 24000000Ustruct s5c73m3_ctrls {struct v4l2_ctrl_handler handler;
@@ -391,6 +393,8 @@ struct s5c73m3 {struct regulator_bulk_data supplies[S5C73M3_MAX_SUPPLIES ];struct s5c73m3_gpio gpio[GPIO_NUM ]; +struct clk *clock; +/* External master clock frequency */ u32 mclk_frequency;/* Video bus type - MIPI-CSI2/parallel */
Signature · 1 line
1.7.9.5
PATCH v6 5/10 V4L: s5c73m3: Add device tree support
Quoted 7 lines · Sylwester Nawrocki
This patch adds the V4L2 asynchronous subdev registration and device tree support. Common clock API is used to control the sensor master clock from within the subdev.
Signed-off-by Andrzej Hajda <[email protected]>Signed-off-by Sylwester Nawrocki <[email protected]>Acked-by Kyungmin Park <[email protected]>
This driver is in linux-next now, but
drivers/media/i2c/s5c73m3/s5c73m3-core.c static const struct v4l2_subdev_ops oif_subdev_ops = { · line 1624
Quoted 6 lines
drivers/media/i2c/s5c73m3/s5c73m3-core.c +5
-0+ node_ep =v4l2_of_get_next_endpoint (node,NULL ); +if (!node_ep) { +dev_warn (dev,"no endpoint defined for node: %s\n" , + node->full_name); +return 0 ; }
This function is not defined here, leading to build errors.
Arnd
Sylwester Nawrocki
maintainerPATCH v6 5/10 V4L: s5c73m3: Add device tree support
Quoted 11 lines · Arnd Bergmann
Quoted 7 lines · Sylwester Nawrocki
This patch adds the V4L2 asynchronous subdev registration and device tree support. Common clock API is used to control the sensor master clock from within the subdev.
Signed-off-by Andrzej Hajda <[email protected]>Signed-off-by Sylwester Nawrocki <[email protected]>Acked-by Kyungmin Park <[email protected]>This driver is in linux-next now, but
Quoted 8 lines
drivers/media/i2c/s5c73m3/s5c73m3-core.c static const struct v4l2_subdev_ops oif_subdev_ops = { · line 1624
Quoted 6 lines
drivers/media/i2c/s5c73m3/s5c73m3-core.c +5
-0+ node_ep =v4l2_of_get_next_endpoint (node,NULL ); +if (!node_ep) { +dev_warn (dev,"no endpoint defined for node: %s\n" , + node->full_name); +return 0 ; }This function is not defined here, leading to build errors.
*sigh* it seems this [1] patch series ended up somehow in -next, but it shouldn't. Mauro, could you please remove the 'exynos' branch from media-next tree ? This should fix the problem. Even though I have been trying to merge this patch to mainline for ages, I'm ready to resign from it for now, not to add to the mess we are already seeing [2].
[1] https://lkml.org/lkml/2014/3/6/352
[2] https://lkml.org/lkml/2014/3/17/547
Thanks,
Sylwester
Sylwester Nawrocki
maintainerPATCH v6 6/10 exynos4-is: Use external s5k6a3 sensor driver
In reply to Sylwester NawrockiThis patch removes the common fimc-is-sensor driver for image sensors that are normally controlled by the FIMC-IS firmware. The FIMC-IS driver now contains only a table of properties specific to each sensor. The sensor properties required for the ISP's firmware are parsed from device tree and retrieved from the internal table, which is selected based on the compatible property of an image sensor.
To use the Exynos4x12 internal ISP the S5K6A3 sensor driver (drivers/ media/i2c/s5k6a3.c) is now required.
---
Changes since v5:
- none.
--- drivers/media/platform/exynos4-is/fimc-is-regs.c | 2 +- drivers/media/platform/exynos4-is/fimc-is-sensor.c | 285 +------------------- drivers/media/platform/exynos4-is/fimc-is-sensor.h | 49 +--- drivers/media/platform/exynos4-is/fimc-is.c | 97 +++---- drivers/media/platform/exynos4-is/fimc-is.h | 4 +- 5 files changed, 57 insertions(+), 380 deletions(-)
drivers/media/platform/exynos4-is/fimc-is-regs.c +1 -1
@@ -112,7 +112,7 @@ void fimc_is_hw_set_sensor_num(struct fimc_is *is)mcuctl_write (IH_REPLY_DONE , is,MCUCTL_REG_ISSR (0 ));mcuctl_write (is->sensor_index, is,MCUCTL_REG_ISSR (1 ));mcuctl_write (IHC_GET_SENSOR_NUM , is,MCUCTL_REG_ISSR (2 )); -mcuctl_write (FIMC_IS_SENSOR_NUM , is,MCUCTL_REG_ISSR (3 )); +mcuctl_write (FIMC_IS_SENSORS_NUM , is,MCUCTL_REG_ISSR (3 )); }void fimc_is_hw_close_sensor (struct fimc_is *is,unsigned int index)
drivers/media/platform/exynos4-is/fimc-is-sensor.c +7 -278
310 lines not on this page · raw message
drivers/media/platform/exynos4-is/fimc-is-sensor.h +8 -41
77 lines not on this page · raw message
drivers/media/platform/exynos4-is/fimc-is.c +39 -58
159 lines not on this page · raw message
drivers/media/platform/exynos4-is/fimc-is.h +2 -2
@@ -39,7 +39,7 @@#define FIMC_IS_FW_LOAD_TIMEOUT 1000/* ms */ #define FIMC_IS_POWER_ON_TIMEOUT 1000/* us */ -#define FIMC_IS_SENSOR_NUM 2 +#define FIMC_IS_SENSORS_NUM 2/* Memory definitions */ #define FIMC_IS_CPU_MEM_SIZE (0xa00000)
@@ -253,7 +253,7 @@ struct fimc_is {struct firmware *f_w;struct fimc_isp isp; -struct fimc_is_sensor *sensor; +struct fimc_is_sensor sensor[FIMC_IS_SENSORS_NUM ];struct fimc_is_setfile setfile;struct vb2_alloc_ctx *alloc_ctx;
Signature · 1 line
1.7.9.5
Sylwester Nawrocki
maintainerPATCH v6 7/10 exynos4-is: Add clock provider for the SCLK_CAM clock outputs
In reply to Sylwester NawrockiThis patch adds clock provider so the the SCLK_CAM0/1 output clocks can be accessed by image sensor devices through the clk API.
---
Changes since v5:
- added a missing check if clocks provider is registered, in order to prevent control of the clock at the host interface side in case when clock is exposed through the clk API and handled by the sensor drivers. This allows new kernels to work with old DTB (old kernels will work with new DTB as well).
Changes since v4:
- retrieve clk parent name through __clk_get_name() on the input clock instead of improperly using clock-names (Mark).
Changes since v3:
- use clock-output-names DT property instead of hard coding names of registered clocks in the driver; first two entries of the clock-names property value are used to specify parent clocks of cam_{a,b}_clkout clocks, rather than hard coding it to sclk_cam{0,1} in the driver;
- addressed issues pointed out in review (Mauro).
Changes since v2:
- use 'camera' DT node drirectly as clock provider node, rather than creating additional clock-controller child node.
- DT binding documentation moved to a separate patch.
--- drivers/media/platform/exynos4-is/media-dev.c | 118 +++++++++++++++++++++++++ drivers/media/platform/exynos4-is/media-dev.h | 19 +++- 2 files changed, 136 insertions(+), 1 deletion(-)
drivers/media/platform/exynos4-is/media-dev.c +118 -0
@@ -11,6 +11,8 @@*/ #include <linux/bug.h> +#include <linux/clk.h> +#include <linux/clk-provider.h> #include <linux/device.h> #include <linux/errno.h> #include <linux/i2c.h>
@@ -1276,6 +1278,14 @@ int fimc_md_set_camclk(struct v4l2_subdev *sd, bool on)struct fimc_source_info *si =v4l2_get_subdev_hostdata (sd);struct fimc_md *fmd =entity_to_fimc_mdev (&sd->entity); +/* +* If there is a clock provider registered the sensors will +* handle their clock themselves, no need to control it on +* the host interface side. +*/ +if (fmd->clk_provider.num_clocks >0 ) +return 0 ; +return __fimc_md_set_camclk (fmd, si, on); }
@@ -1437,6 +1447,103 @@ static int fimc_md_get_pinctrl(struct fimc_md *fmd)return 0 ; } +#ifdef CONFIG_OF +static int cam_clk_prepare (struct clk_hw *hw) +{ +struct cam_clk *camclk =to_cam_clk (hw); +int ret; + +if (camclk->fmd->pmf ==NULL ) +return -ENODEV ; + + ret =pm_runtime_get_sync (camclk->fmd->pmf); +return ret <0 ? ret :0 ; +} + +static void cam_clk_unprepare (struct clk_hw *hw) +{ +struct cam_clk *camclk =to_cam_clk (hw); + +if (camclk->fmd->pmf ==NULL ) +return ; + +pm_runtime_put_sync (camclk->fmd->pmf); +} + +static const struct clk_ops cam_clk_ops = { + .prepare = cam_clk_prepare, + .unprepare = cam_clk_unprepare, +}; + +static void fimc_md_unregister_clk_provider (struct fimc_md *fmd) +{ +struct cam_clk_provider *cp = &fmd->clk_provider; +unsigned int i; + +if (cp->of_node) +of_clk_del_provider (cp->of_node); + +for (i =0 ; i < cp->num_clocks; i++) +clk_unregister (cp->clks[i]); +} + +static int fimc_md_register_clk_provider (struct fimc_md *fmd) +{ +struct cam_clk_provider *cp = &fmd->clk_provider; +struct device *dev = &fmd->pdev->dev; +int i, ret; + +for (i =0 ; i <FIMC_MAX_CAMCLKS ; i++) { +struct cam_clk *camclk = &cp->camclk[i]; +struct clk_init_data init; +const char *p_name; + + ret =of_property_read_string_index (dev->of_node, +"clock-output-names" , i, &init.name); +if (ret <0 ) +break ; + + p_name =__clk_get_name (fmd->camclk[i].clock); + +/* It's safe since clk_register() will duplicate the string. */ + init.parent_names = &p_name; + init.num_parents =1 ; + init.ops = &cam_clk_ops; + init.flags =CLK_SET_RATE_PARENT ; + camclk->hw.init = &init; + camclk->fmd = fmd; + + cp->clks[i] =clk_register (NULL , &camclk->hw); +if (IS_ERR (cp->clks[i])) { +dev_err (dev,"failed to register clock: %s (%ld)\n" , + init.name,PTR_ERR (cp->clks[i])); + ret =PTR_ERR (cp->clks[i]); + goto err; + } + cp->num_clocks++; + } + +if (cp->num_clocks ==0 ) { +dev_warn (dev,"clk provider not registered\n" ); +return 0 ; + } + + cp->clk_data.clks = cp->clks; + cp->clk_data.clk_num = cp->num_clocks; + cp->of_node = dev->of_node; + ret =of_clk_add_provider (dev->of_node, of_clk_src_onecell_get, + &cp->clk_data); +if (ret ==0 ) +return 0 ; +err: +fimc_md_unregister_clk_provider (fmd); +return ret; +} +#else +#define fimc_md_register_clk_provider (fmd) (0) +#define fimc_md_unregister_clk_provider (fmd) (0) +#endif +static int fimc_md_probe (struct platform_device *pdev) {struct device *dev = &pdev->dev;
@@ -1464,16 +1571,24 @@ static int fimc_md_probe(struct platform_device *pdev) fmd->use_isp =fimc_md_is_isp_available (dev->of_node); + ret =fimc_md_register_clk_provider (fmd); +if (ret <0 ) { +v4l2_err (v4l2_dev,"clock provider registration failed\n" ); +return ret; + } + ret =v4l2_device_register (dev, &fmd->v4l2_dev);if (ret <0 ) {v4l2_err (v4l2_dev,"Failed to register v4l2_device: %d\n" , ret);return ret; } + ret =media_device_register (&fmd->media_dev);if (ret <0 ) {v4l2_err (v4l2_dev,"Failed to register media device: %d\n" , ret); goto err_md; } + ret =fimc_md_get_clocks (fmd);if (ret) goto err_clk;
@@ -1507,6 +1622,7 @@ static int fimc_md_probe(struct platform_device *pdev) ret =fimc_md_create_links (fmd);if (ret) goto err_unlock; + ret =v4l2_device_register_subdev_nodes (&fmd->v4l2_dev);if (ret) goto err_unlock;
@@ -1527,6 +1643,7 @@ err_clk:media_device_unregister (&fmd->media_dev); err_md:v4l2_device_unregister (&fmd->v4l2_dev); +fimc_md_unregister_clk_provider (fmd);return ret; }
@@ -1537,6 +1654,7 @@ static int fimc_md_remove(struct platform_device *pdev)if (!fmd)return 0 ; +fimc_md_unregister_clk_provider (fmd);v4l2_device_unregister (&fmd->v4l2_dev);device_remove_file (&pdev->dev, &dev_attr_subdev_conf_mode);fimc_md_unregister_entities (fmd);
drivers/media/platform/exynos4-is/media-dev.h +18 -1
@@ -10,6 +10,7 @@#define FIMC_MDEVICE_H_ #include <linux/clk.h> +#include <linux/clk-provider.h> #include <linux/platform_device.h> #include <linux/mutex.h> #include <linux/of.h>
@@ -89,6 +90,12 @@ struct fimc_sensor_info {struct fimc_dev *host; }; +struct cam_clk { +struct clk_hw hw; +struct fimc_md *fmd; +}; +#define to_cam_clk (_hw) container_of(_hw, struct cam_clk, hw) +/** * struct fimc_md - fimc media device information * @csis: MIPI CSIS subdevs data
@@ -105,6 +112,7 @@ struct fimc_sensor_info {* @pinctrl: camera port pinctrl handle * @state_default: pinctrl default state handle * @state_idle: pinctrl idle state handle +* @cam_clk_provider: CAMCLK clock provider structure * @user_subdev_api: true if subdevs are not configured by the host driver * @slock: spinlock protecting @sensor array */
@@ -122,13 +130,22 @@ struct fimc_md {struct media_device media_dev;struct v4l2_device v4l2_dev;struct platform_device *pdev; +struct fimc_pinctrl {struct pinctrl *pinctrl;struct pinctrl_state *state_default;struct pinctrl_state *state_idle; } pinctl; -bool user_subdev_api; +struct cam_clk_provider { +struct clk *clks[FIMC_MAX_CAMCLKS ]; +struct clk_onecell_data clk_data; +struct device_node *of_node; +struct cam_clk camclk[FIMC_MAX_CAMCLKS ]; +int num_clocks; + } clk_provider; + +bool user_subdev_api;spinlock_t slock;struct list_head pipelines; };
--
1.7.9.5
Sylwester Nawrocki
maintainerPATCH v6 8/10 exynos4-is: Add support for asynchronous subdevices registration
In reply to Sylwester NawrockiAdd support for registering external sensor subdevs using v4l2-async API. The async API is used only for sensor subdevs and only for booting from DT.
---
Changes since v5:
- removed erroneous introduction of a redundant media_device_unregister() call.
Changes since v4:
- call v4l2_async_notifier_register() only if any camera sensor is specified in devicetree, so the driver can initialize even if only mem-to-mem functionality is used.
Changes since v2:
- none.
Changes since v1:
- register clock provider after registering FIMC devices so the clock can be actually used right after it is registered.Changes since v1:
- register clock provider after registering FIMC devices so the clock can be actually used right after it is registered.
--- drivers/media/platform/exynos4-is/media-dev.c | 238 ++++++++++++++----------- drivers/media/platform/exynos4-is/media-dev.h | 13 +- 2 files changed, 147 insertions(+), 104 deletions(-)
drivers/media/platform/exynos4-is/media-dev.c +135 -103
@@ -26,6 +26,7 @@#include <linux/pm_runtime.h> #include <linux/types.h> #include <linux/slab.h> +#include <media/v4l2-async.h> #include <media/v4l2-ctrls.h> #include <media/v4l2-of.h> #include <media/media-device.h>
@@ -220,6 +221,7 @@ static int __fimc_pipeline_open(struct exynos_media_pipeline *ep,if (ret <0 )return ret; } + ret =fimc_md_set_camclk (sd, true);if (ret <0 ) goto err_wbclk;
@@ -380,77 +382,18 @@ static void fimc_md_unregister_sensor(struct v4l2_subdev *sd)struct i2c_client *client =v4l2_get_subdevdata (sd);struct i2c_adapter *adapter; -if (!client) +if (!client || client->dev.of_node)return ;v4l2_device_unregister_subdev (sd); -if (!client->dev.of_node) { - adapter = client->adapter; -i2c_unregister_device (client); -if (adapter) -i2c_put_adapter (adapter); - } + adapter = client->adapter; +i2c_unregister_device (client); +if (adapter) +i2c_put_adapter (adapter); }#ifdef CONFIG_OF -/* Register I2C client subdev associated with @node. */ -static int fimc_md_of_add_sensor (struct fimc_md *fmd, -struct device_node *node,int index) -{ -struct fimc_sensor_info *si; -struct i2c_client *client; -struct v4l2_subdev *sd; -int ret; - -if (WARN_ON (index >=ARRAY_SIZE (fmd->sensor))) -return -EINVAL ; - si = &fmd->sensor[index]; - - client =of_find_i2c_device_by_node (node); -if (!client) -return -EPROBE_DEFER ; - -device_lock (&client->dev); - -if (!client->dev.driver || - !try_module_get (client->dev.driver->owner)) { - ret = -EPROBE_DEFER ; -v4l2_info (&fmd->v4l2_dev,"No driver found for %s\n" , - node->full_name); - goto dev_put; - } - -/* Enable sensor's master clock */ - ret =__fimc_md_set_camclk (fmd, &si->pdata, true); -if (ret <0 ) - goto mod_put; - sd =i2c_get_clientdata (client); - - ret =v4l2_device_register_subdev (&fmd->v4l2_dev, sd); -__fimc_md_set_camclk (fmd, &si->pdata, false); -if (ret <0 ) - goto mod_put; - -v4l2_set_subdev_hostdata (sd, &si->pdata); -if (si->pdata.fimc_bus_type ==FIMC_BUS_TYPE_ISP_WRITEBACK ) - sd->grp_id =GRP_ID_FIMC_IS_SENSOR ; -else - sd->grp_id =GRP_ID_SENSOR ; - - si->subdev = sd; -v4l2_info (&fmd->v4l2_dev,"Registered sensor subdevice: %s (%d)\n" , - sd->name, fmd->num_sensors); - fmd->num_sensors++; - -mod_put: -module_put (client->dev.driver->owner); -dev_put: -device_unlock (&client->dev); -put_device (&client->dev); -return ret; -} -/* Parse port node and register as a sub-device any sensor specified there. */ static int fimc_md_parse_port_node(struct fimc_md *fmd, struct device_node *port,
@@ -459,7 +402,6 @@ static int fimc_md_parse_port_node(struct fimc_md *fmd,struct device_node *rem, *ep, *np;struct fimc_source_info *pd;struct v4l2_of_endpoint endpoint; -int ret;u32 val; pd = &fmd->sensor[index].pdata;
@@ -487,6 +429,8 @@ static int fimc_md_parse_port_node(struct fimc_md *fmd,if (!of_property_read_u32 (rem,"clock-frequency" , &val)) pd->clk_frequency = val; +else + pd->clk_frequency =DEFAULT_SENSOR_CLK_FREQ ;if (pd->clk_frequency ==0 ) { v4l2_err(&fmd->v4l2_dev, "Wrong clock frequency at node %s\n",
@@ -526,10 +470,17 @@ static int fimc_md_parse_port_node(struct fimc_md *fmd,else pd->fimc_bus_type = pd->sensor_bus_type; - ret =fimc_md_of_add_sensor (fmd, rem, index); -of_node_put (rem); +if (WARN_ON (index >=ARRAY_SIZE (fmd->sensor))) +return -EINVAL ; -return ret; + fmd->sensor[index].asd.match_type =V4L2_ASYNC_MATCH_OF ; + fmd->sensor[index].asd.match.of.node = rem; + fmd->async_subdevs[index] = &fmd->sensor[index].asd; + + fmd->num_sensors++; + +of_node_put (rem); +return 0 ; }/* Register all SoC external sub-devices */
@@ -885,11 +836,13 @@ static void fimc_md_unregister_entities(struct fimc_md *fmd)v4l2_device_unregister_subdev (fmd->csis[i].sd); fmd->csis[i].sd =NULL ; } -for (i =0 ; i < fmd->num_sensors; i++) { -if (fmd->sensor[i].subdev ==NULL ) -continue ; -fimc_md_unregister_sensor (fmd->sensor[i].subdev); - fmd->sensor[i].subdev =NULL ; +if (fmd->pdev->dev.of_node ==NULL ) { +for (i =0 ; i < fmd->num_sensors; i++) { +if (fmd->sensor[i].subdev ==NULL ) +continue ; +fimc_md_unregister_sensor (fmd->sensor[i].subdev); + fmd->sensor[i].subdev =NULL ; + } }if (fmd->fimc_is)
@@ -1224,6 +1177,14 @@ static int __fimc_md_set_camclk(struct fimc_md *fmd,struct fimc_camclk_info *camclk;int ret =0 ; +/* +* When device tree is used the sensor drivers are supposed to +* control the clock themselves. This whole function will be +* removed once S5PV210 platform is converted to the device tree. +*/ +if (fmd->pdev->dev.of_node) +return 0 ; +if (WARN_ON (si->clk_id >=FIMC_MAX_CAMCLKS ) || !fmd || !fmd->pmf)return -EINVAL ;
@@ -1544,6 +1505,56 @@ err:#define fimc_md_unregister_clk_provider (fmd) (0)#endif +static int subdev_notifier_bound (struct v4l2_async_notifier *notifier, +struct v4l2_subdev *subdev, +struct v4l2_async_subdev *asd) +{ +struct fimc_md *fmd =notifier_to_fimc_md (notifier); +struct fimc_sensor_info *si =NULL ; +int i; + +/* Find platform data for this sensor subdev */ +for (i =0 ; i <ARRAY_SIZE (fmd->sensor); i++) +if (fmd->sensor[i].asd.match.of.node == subdev->dev->of_node) + si = &fmd->sensor[i]; + +if (si ==NULL ) +return -EINVAL ; + +v4l2_set_subdev_hostdata (subdev, &si->pdata); + +if (si->pdata.fimc_bus_type ==FIMC_BUS_TYPE_ISP_WRITEBACK ) + subdev->grp_id =GRP_ID_FIMC_IS_SENSOR ; +else + subdev->grp_id =GRP_ID_SENSOR ; + + si->subdev = subdev; + +v4l2_info (&fmd->v4l2_dev,"Registered sensor subdevice: %s (%d)\n" , + subdev->name, fmd->num_sensors); + + fmd->num_sensors++; + +return 0 ; +} + +static int subdev_notifier_complete (struct v4l2_async_notifier *notifier) +{ +struct fimc_md *fmd =notifier_to_fimc_md (notifier); +int ret; + +mutex_lock (&fmd->media_dev.graph_mutex); + + ret =fimc_md_create_links (fmd); +if (ret <0 ) + goto unlock; + + ret =v4l2_device_register_subdev_nodes (&fmd->v4l2_dev); +unlock: +mutex_unlock (&fmd->media_dev.graph_mutex); +return ret; +} +static int fimc_md_probe (struct platform_device *pdev) {struct device *dev = &pdev->dev;
@@ -1571,12 +1582,6 @@ static int fimc_md_probe(struct platform_device *pdev) fmd->use_isp =fimc_md_is_isp_available (dev->of_node); - ret =fimc_md_register_clk_provider (fmd); -if (ret <0 ) { -v4l2_err (v4l2_dev,"clock provider registration failed\n" ); -return ret; - } - ret =v4l2_device_register (dev, &fmd->v4l2_dev);if (ret <0 ) {v4l2_err (v4l2_dev,"Failed to register v4l2_device: %d\n" , ret);
@@ -1586,64 +1591,88 @@ static int fimc_md_probe(struct platform_device *pdev) ret =media_device_register (&fmd->media_dev);if (ret <0 ) {v4l2_err (v4l2_dev,"Failed to register media device: %d\n" , ret); - goto err_md; + goto err_v4l2_dev; } ret =fimc_md_get_clocks (fmd);if (ret) - goto err_clk; + goto err_md; fmd->user_subdev_api = (dev->of_node !=NULL ); -/* Protect the media graph while we're registering entities */ -mutex_lock (&fmd->media_dev.graph_mutex); - ret =fimc_md_get_pinctrl (fmd);if (ret <0 ) {if (ret !=EPROBE_DEFER )dev_err (dev,"Failed to get pinctrl: %d\n" , ret); - goto err_unlock; + goto err_clk; } +platform_set_drvdata (pdev, fmd); + +/* Protect the media graph while we're registering entities */ +mutex_lock (&fmd->media_dev.graph_mutex); +if (dev->of_node) ret =fimc_md_register_of_platform_entities (fmd, dev->of_node);else ret =bus_for_each_dev (&platform_bus_type,NULL , fmd, fimc_md_pdev_match); -if (ret) - goto err_unlock; +if (ret) { +mutex_unlock (&fmd->media_dev.graph_mutex); + goto err_clk; + }if (dev->platform_data || dev->of_node) { ret =fimc_md_register_sensor_entities (fmd); -if (ret) - goto err_unlock; +if (ret) { +mutex_unlock (&fmd->media_dev.graph_mutex); + goto err_m_ent; + } } - ret =fimc_md_create_links (fmd); -if (ret) - goto err_unlock; - - ret =v4l2_device_register_subdev_nodes (&fmd->v4l2_dev); -if (ret) - goto err_unlock; +mutex_unlock (&fmd->media_dev.graph_mutex); ret =device_create_file (&pdev->dev, &dev_attr_subdev_conf_mode);if (ret) - goto err_unlock; + goto err_m_ent; +/* +* FIMC platform devices need to be registered before the sclk_cam +* clocks provider, as one of these devices needs to be activated +* to enable the clock. +*/ + ret =fimc_md_register_clk_provider (fmd); +if (ret <0 ) { +v4l2_err (v4l2_dev,"clock provider registration failed\n" ); + goto err_attr; + } + +if (fmd->num_sensors >0 ) { + fmd->subdev_notifier.subdevs = fmd->async_subdevs; + fmd->subdev_notifier.num_subdevs = fmd->num_sensors; + fmd->subdev_notifier.bound = subdev_notifier_bound; + fmd->subdev_notifier.complete = subdev_notifier_complete; + fmd->num_sensors =0 ; + + ret =v4l2_async_notifier_register (&fmd->v4l2_dev, + &fmd->subdev_notifier); +if (ret) + goto err_clk_p; + } -platform_set_drvdata (pdev, fmd); -mutex_unlock (&fmd->media_dev.graph_mutex);return 0 ; -err_unlock: -mutex_unlock (&fmd->media_dev.graph_mutex); +err_clk_p: +fimc_md_unregister_clk_provider (fmd); +err_attr: +device_remove_file (&pdev->dev, &dev_attr_subdev_conf_mode); err_clk:fimc_md_put_clocks (fmd); +err_m_ent:fimc_md_unregister_entities (fmd); -media_device_unregister (&fmd->media_dev); err_md: +media_device_unregister (&fmd->media_dev); +err_v4l2_dev:v4l2_device_unregister (&fmd->v4l2_dev); -fimc_md_unregister_clk_provider (fmd);return ret; }
@@ -1655,12 +1684,15 @@ static int fimc_md_remove(struct platform_device *pdev)return 0 ;fimc_md_unregister_clk_provider (fmd); +v4l2_async_notifier_unregister (&fmd->subdev_notifier); +v4l2_device_unregister (&fmd->v4l2_dev);device_remove_file (&pdev->dev, &dev_attr_subdev_conf_mode);fimc_md_unregister_entities (fmd);fimc_md_pipelines_free (fmd);media_device_unregister (&fmd->media_dev);fimc_md_put_clocks (fmd); +return 0 ; }
drivers/media/platform/exynos4-is/media-dev.h +12 -1
@@ -32,8 +32,9 @@#define PINCTRL_STATE_IDLE "idle" -#define FIMC_MAX_SENSORS 8 +#define FIMC_MAX_SENSORS 4#define FIMC_MAX_CAMCLKS 2 +#define DEFAULT_SENSOR_CLK_FREQ 24000000U/* LCD/ISP Writeback clocks (PIXELASYNCMx) */ enum {
@@ -79,6 +80,7 @@ struct fimc_camclk_info {/** * struct fimc_sensor_info - image data source subdev information * @pdata: sensor's atrributes passed as media device's platform data +* @asd: asynchronous subdev registration data structure * @subdev: image sensor v4l2 subdev * @host: fimc device the sensor is currently linked to *
@@ -86,6 +88,7 @@ struct fimc_camclk_info {*/ struct fimc_sensor_info {struct fimc_source_info pdata; +struct v4l2_async_subdev asd;struct v4l2_subdev *subdev;struct fimc_dev *host; };
@@ -145,6 +148,9 @@ struct fimc_md {int num_clocks; } clk_provider; +struct v4l2_async_notifier subdev_notifier; +struct v4l2_async_subdev *async_subdevs[FIMC_MAX_SENSORS ]; +bool user_subdev_api;spinlock_t slock;struct list_head pipelines;
@@ -162,6 +168,11 @@ static inline struct fimc_md *entity_to_fimc_mdev(struct media_entity *me) container_of(me->parent, struct fimc_md, media_dev); } +static inline struct fimc_md *notifier_to_fimc_md (struct v4l2_async_notifier *n) +{ + return container_of(n, struct fimc_md, subdev_notifier); +} +static inline void fimc_md_graph_lock (struct exynos_video_entity *ve) {mutex_lock (&ve->vdev.entity.parent->graph_mutex);
Signature · 1 line
1.7.9.5
Sylwester Nawrocki
maintainerPATCH v6 9/10 ARM: dts: Add rear camera nodes for Exynos4412 TRATS2 board
In reply to Sylwester NawrockiThis patch enables the rear facing camera (s5c73m3) on TRATS2 board by adding the I2C0 bus controller, s5c73m3 sensor, MIPI CSI-2 receiver and the sensor's voltage regulator supply nodes.
---
Changes since v5:
- none.
Changes since v4:
- removed changes related to s5k6a3 sensor.
--- arch/arm/boot/dts/exynos4412-trats2.dts | 81 +++++++++++++++++++++++++++++-- 1 file changed, 78 insertions(+), 3 deletions(-)
arch/arm/boot/dts/exynos4412-trats2.dts +78 -3
@@ -71,7 +71,33 @@ enable-active-high; }; - /* More to come */ + cam_af_reg: voltage-regulator-2 { + compatible = "regulator-fixed"; + regulator-name = "CAM_AF"; + regulator-min-microvolt = <2800000>; + regulator-max-microvolt = <2800000>; + gpio = <&gpm0 4 0>; + enable-active-high; + }; + + cam_isp_core_reg: voltage-regulator-3 { + compatible = "regulator-fixed"; + regulator-name = "CAM_ISP_CORE_1.2V_EN"; + regulator-min-microvolt = <1200000>; + regulator-max-microvolt = <1200000>; + gpio = <&gpm0 3 0>; + enable-active-high; + regulator-always-on; + }; + + lcd_vdd3_reg: voltage-regulator-4 { + compatible = "regulator-fixed"; + regulator-name = "LCD_VDD_2.2V"; + regulator-min-microvolt = <2200000>; + regulator-max-microvolt = <2200000>; + gpio = <&gpc0 1 0>; + enable-active-high; + }; }; gpio-keys {
@@ -106,6 +132,38 @@ }; }; + i2c_0: i2c at 13860000 { + samsung,i2c-sda-delay = <100>; + samsung,i2c-slave-addr = <0x10>; + samsung,i2c-max-bus-freq = <400000>; + pinctrl-0 = <&i2c0_bus>; + pinctrl-names = "default"; + status = "okay"; + + s5c73m3 at 3c { + compatible = "samsung,s5c73m3"; + reg = <0x3c>; + standby-gpios = <&gpm0 1 1>; /* ISP_STANDBY */ + xshutdown-gpios = <&gpf1 3 1>; /* ISP_RESET */ + vdd-int-supply = <&buck9_reg>; + vddio-cis-supply = <&ldo9_reg>; + vdda-supply = <&ldo17_reg>; + vddio-host-supply = <&ldo18_reg>; + vdd-af-supply = <&cam_af_reg>; + vdd-reg-supply = <&cam_io_reg>; + clock-frequency = <24000000>; + /* CAM_A_CLKOUT */ + clocks = <&camera 0>; + clock-names = "cis_extclk"; + port { + s5c73m3_ep: endpoint { + remote-endpoint = <&csis0_ep>; + data-lanes = <1 2 3 4>; + }; + }; + }; + }; + i2c at 13890000 { samsung,i2c-sda-delay = <100>; samsung,i2c-slave-addr = <0x10>;
@@ -511,8 +569,8 @@ }; }; - camera { - pinctrl-0 = <&cam_port_b_clk_active>; + camera: camera { + pinctrl-0 = <&cam_port_a_clk_active &cam_port_b_clk_active>; pinctrl-names = "default"; status = "okay";
@@ -532,6 +590,23 @@ status = "okay"; }; + csis_0: csis at 11880000 { + status = "okay"; + vddcore-supply = <&ldo8_reg>; + vddio-supply = <&ldo10_reg>; + clock-frequency = <176000000>; + + /* Camera C (3) MIPI CSI-2 (CSIS0) */ + port at 3 { + reg = <3>; + csis0_ep: endpoint { + remote-endpoint = <&s5c73m3_ep>; + data-lanes = <1 2 3 4>; + samsung,csis-hs-settle = <12>; + }; + }; + }; + csis_1: csis at 11890000 { vddcore-supply = <&ldo8_reg>; vddio-supply = <&ldo10_reg>;
Signature · 1 line
1.7.9.5
Sylwester Nawrocki
maintainerPATCH v6 9/10 ARM: dts: Add rear camera nodes for Exynos4412 TRATS2 board
Hi Kukjin,
Quoted 7 lines · Sylwester Nawrocki
This patch enables the rear facing camera (s5c73m3) on TRATS2 board by adding the I2C0 bus controller, s5c73m3 sensor, MIPI CSI-2 receiver and the sensor's voltage regulator supply nodes.
Signed-off-by Andrzej Hajda <[email protected]>Acked-by Kyungmin Park <[email protected]>Signed-off-by Sylwester Nawrocki <[email protected]>
Could still patches 9, 10 be queued for v3.15, or is arm-soc already closed for this merge windiw ?
--
Thanks,
Sylwester
Sylwester Nawrocki
maintainerPATCH v6 10/10 ARM: dts: exynos4: Update camera clk provider and s5k6a3 sensor node
In reply to Sylwester NawrockiRemove unused /camera/clock-controller node and required clock properties to the camera node. This is required for a clock provider that will be referenced by image sensor devices.
Also add required clock related changes to s5k6a3 device node and afvdd regulator supply.
---
Changes since v5:
- none.
Changes since v4:
- added changes at the s5k6a3 device node.
- clock output names adjusted to better match documentation.
--- arch/arm/boot/dts/exynos4.dtsi | 6 ++---- arch/arm/boot/dts/exynos4412-trats2.dts | 5 +++-- 2 files changed, 5 insertions(+), 6 deletions(-)
arch/arm/boot/dts/exynos4.dtsi +2 -4
@@ -109,12 +109,10 @@ status = "disabled"; #address-cells = <1>; #size-cells = <1>; + #clock-cells = <1>; + clock-output-names = "cam_a_clkout", "cam_b_clkout"; ranges; - clock_cam: clock-controller { - #clock-cells = <1>; - }; - fimc_0: fimc at 11800000 { compatible = "samsung,exynos4210-fimc"; reg = <0x11800000 0x1000>;
arch/arm/boot/dts/exynos4412-trats2.dts +3 -2
@@ -647,10 +647,11 @@ reg = <0x10>; svdda-supply = <&cam_io_reg>; svddio-supply = <&ldo19_reg>; + afvdd-supply = <&ldo19_reg>; clock-frequency = <24000000>; /* CAM_B_CLKOUT */ - clocks = <&clock_cam 1>; - clock-names = "mclk"; + clocks = <&camera 1>; + clock-names = "extclk"; samsung,camclk-out = <1>; gpios = <&gpm1 6 0>;
Signature · 1 line
1.7.9.5