Re: [PATCH v2 00/12] Samsung PM consolidation part 2 (multiplatform)
Hi Kukjin, On 06.02.2014 20:12, Tomasz Figa wrote: Current Samsung PM code is heavily unprepared for multiplatform systems. The design implies accessing functions and global variables defined in particular mach- subdirectory from common code in plat-, which is not allowed when building ARCH_MULTIPLATFORM. In addition there is a lot of forced code unification, which makes common function handle any possible quirks of all supported SoCs. In the end this design turned out to not work too well, ending with a lot of empty functions exported from mach-, just because code in common pm.c calls them. Moreover, recent trend of moving lower level suspend/resume code to proper drivers, like pinctrl or clk, made a lot of code there redundant, especially on DT-only platforms like Exynos. This patch series attempts to untie Exynos PM support from the legacy Samsung PM core and make it multiplatform-aware, by isolating truly generic parts of the latter, making them multiplatform-friendly and then reimplementing Exynos PM support in a multiplatform-capable way by using those generic parts. The result is that now PM initialization is started from mach-exynos*-dt, which calls Exynos-specific initialization code that registers platform_suspend_ops, so control flow is basically reversed ending with mach- code calling more generic plat- code if needed. This is limited to Exynos right now, but remaining SoCs could follow in further series. Depends on Samsung PM consolidation part 1 (clocks) series: - http://thread.gmane.org/gmane.linux.kernel.samsung-soc/26816 On Exynos4210-based Trats, Exynos4412-based Trats2 and Exynos5250-based Arndale boards (except suspend/resume, which is broken because of unrelated reasons): Tested-by: Tomasz Figa t.f...@samsung.com Changes since v1 (RFC): - fixed l2x0 resume, - fixed checkpatch complaints (about issues in existing code being moved), - rebased on top of current linux-next, - slightly reordered patches to make the order more logical. Tomasz Figa (12): ARM: EXYNOS: Do not resume l2x0 if not enabled before suspend ARM: SAMSUNG: Add soc_is_s3c2410() helper ARM: SAMSUNG: pm: Save UART DIVSLOT register based on SoC type ARM: SAMSUNG: pm: Use debug_ll_addr() to get UART base address ARM: SAMSUNG: pm: Consolidate PM debug functions ARM: SAMSUNG: pm: Move Samsung PM debug code into separate file ARM: SAMSUNG: Move common save/restore helpers to separate file ARM: SAMSUNG: pm: Move s3c_pm_check_* prototypes to plat/pm-common.h ARM: EXYNOS: Kconfig: Fix abuse of CONFIG_PM ARM: EXYNOS: Remove PM initcalls and useless indirection ARM: EXYNOS: Stop using legacy Samsung PM code ARM: exynos: Allow wake-up using GIC interrupts arch/arm/mach-exynos/Kconfig | 16 +-- arch/arm/mach-exynos/Makefile | 2 +- arch/arm/mach-exynos/common.c | 1 + arch/arm/mach-exynos/common.h | 14 ++ arch/arm/mach-exynos/include/mach/pm-core.h| 75 --- arch/arm/mach-exynos/pm.c | 172 +++-- arch/arm/mach-exynos/regs-pmu.h| 2 + arch/arm/mach-exynos/sleep.S | 85 arch/arm/mach-s3c64xx/pm.c | 1 - arch/arm/mach-s5p64x0/pm.c | 1 - arch/arm/plat-samsung/Makefile | 2 + arch/arm/plat-samsung/include/plat/cpu.h | 6 + arch/arm/plat-samsung/include/plat/pm-common.h | 110 arch/arm/plat-samsung/include/plat/pm.h| 80 +--- arch/arm/plat-samsung/pm-check.c | 2 +- arch/arm/plat-samsung/pm-common.c | 75 +++ arch/arm/plat-samsung/pm-debug.c | 98 ++ arch/arm/plat-samsung/pm.c | 146 - arch/arm/plat-samsung/s5p-sleep.S | 43 --- 19 files changed, 531 insertions(+), 400 deletions(-) delete mode 100644 arch/arm/mach-exynos/include/mach/pm-core.h create mode 100644 arch/arm/mach-exynos/sleep.S create mode 100644 arch/arm/plat-samsung/include/plat/pm-common.h create mode 100644 arch/arm/plat-samsung/pm-common.c create mode 100644 arch/arm/plat-samsung/pm-debug.c What do you think about this series? Best regards, Tomasz -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1] ARM: Exynos: Add generic compatible string
On 21.02.2014 07:08, Sachin Kamat wrote: On 20 February 2014 23:18, Arnd Bergmann a...@arndb.de wrote: On Thursday 20 February 2014 18:34:23 Tomasz Figa wrote: On 20.02.2014 18:00, Arnd Bergmann wrote: Of course nothing stops you from retaining more specific compatible strings. In fact, this is probably the most appropriate solution, because in future you might find out that certain SoCs need some special differentiation, e.g. same ID value on two SoCs. So, to apply this to our case, our Exynos 5250 based Arndale board would be changed from compatible = insignal,arndale, samsung,exynos5260; to compatible = insignal,arndale, samsung,exynos5260, samsung,exynos; Right, this would make sense. Now, the board file will be able to match simply by samsung,exynos compatible string and SoC-specific code in mach-exynos (hopefully none after it gets cleaned up fully) will use soc_is_exynos*() macros (what AFAIK it is already doing at the moment). On principle, I would not take things out of the match list, if that would break booting with old DT file that don't list samsung,exynos as compatible. But for new SoCs that would work. My proposal was about simply adding a fully generic string, without removing the specific ones. For already supported SoCs this is pretty obvious, as existing DTBs would not have this generic string listed. But the specific strings should be also present in DTSes of new SoCs, even if not recognized by the kernel, to make sure that in future any SoC-specific quirks could be easily handled. Yes, that's ideal. Using soc_is_exynos*() too much of course also isn't good. A lot of the things tested for should probably be checked from individual DT properties, but again we have to find the right balance. I wouldn't mind getting rid of the soc_is_exynos*() macros completely, because a) we can't use them in device drivers b) all platform code is supposed to be in drivers c) both rules are enforced for arm64 I fully agree. As I said, after cleaning up mach-exynos/ there should be no more code left using soc_is_*() macros. Ideally, the whole mach-exynos/ should collapse into a single, trivial file, with everything done in dedicated drivers. Right. Now that we have a broader agreement on this, I think we can go ahead with the following steps as an initial approach: 1. Have a common machine file for both exynos4 and 5 files, mach-exynos-dt.c. 2. Introduce a generic compatible string samsung,exynos. 3. Append this to the compatible property list for existing boards. If this plan looks OK, I can send across patches doing this. Looks good. I would also merge common.c with this resulting mach-exynos-dt.c, as it would be the only user of the code there. Best regards, Tomasz -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1] ARM: Exynos: Add generic compatible string
On Friday 21 February 2014 14:18:49 Tomasz Figa wrote: Now that we have a broader agreement on this, I think we can go ahead with the following steps as an initial approach: 1. Have a common machine file for both exynos4 and 5 files, mach-exynos-dt.c. 2. Introduce a generic compatible string samsung,exynos. 3. Append this to the compatible property list for existing boards. If this plan looks OK, I can send across patches doing this. Looks good. I would also merge common.c with this resulting mach-exynos-dt.c, as it would be the only user of the code there. Sounds good. While the naming is not important, I would just call the file 'exynos.c', in line with some of the other platforms we have. Both the 'mach-' and the '-dt' part of the file name are redundant. Alternatively, you could merge it all into common.c. Arnd -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1] ARM: Exynos: Add generic compatible string
On 21.02.2014 15:48, Arnd Bergmann wrote: On Friday 21 February 2014 14:18:49 Tomasz Figa wrote: Now that we have a broader agreement on this, I think we can go ahead with the following steps as an initial approach: 1. Have a common machine file for both exynos4 and 5 files, mach-exynos-dt.c. 2. Introduce a generic compatible string samsung,exynos. 3. Append this to the compatible property list for existing boards. If this plan looks OK, I can send across patches doing this. Looks good. I would also merge common.c with this resulting mach-exynos-dt.c, as it would be the only user of the code there. Sounds good. While the naming is not important, I would just call the file 'exynos.c', in line with some of the other platforms we have. Both the 'mach-' and the '-dt' part of the file name are redundant. Alternatively, you could merge it all into common.c. exynos.c sounds good to me. Best regards, Tomasz -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1] ARM: Exynos: Add generic compatible string
On 21.02.2014 16:21, Tomasz Figa wrote: On 21.02.2014 15:48, Arnd Bergmann wrote: On Friday 21 February 2014 14:18:49 Tomasz Figa wrote: Now that we have a broader agreement on this, I think we can go ahead with the following steps as an initial approach: 1. Have a common machine file for both exynos4 and 5 files, mach-exynos-dt.c. 2. Introduce a generic compatible string samsung,exynos. 3. Append this to the compatible property list for existing boards. If this plan looks OK, I can send across patches doing this. Looks good. I would also merge common.c with this resulting mach-exynos-dt.c, as it would be the only user of the code there. Sounds good. While the naming is not important, I would just call the file 'exynos.c', in line with some of the other platforms we have. Both the 'mach-' and the '-dt' part of the file name are redundant. Alternatively, you could merge it all into common.c. exynos.c sounds good to me. One minor thing. It might be a good idea to base on top of my PM consolidation part 2 series, to avoid merge conflicts: http://thread.gmane.org/gmane.linux.ports.arm.kernel/299340 It should hit Kgene's tree this weekend. Best regards, Tomasz -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 1/2] phy: Select PHY_EXYNOS_MIPI_VIDEO by default for ARCH_EXYNOS
Instead of requiring user to figure out when PHY_EXYNOS_MIPI_VIDEO needs to be selected select it by default for Exynos SoCs. Also enable it when COMPILE_TEST is selected. If required the MIPI CSIS/DPHY driver can be then disabled or compiled in as module. Signed-off-by: Sylwester Nawrocki s.nawro...@samsung.com Acked-by: Kyungmin Park kyungmin.p...@samsung.com --- Changes since first version: - use ARCH_EXYNOS instead of ARCH_EXYNOS4 || ARCH_EXYNOS5 - add dependency on COMPILE_TEST, and ARCH_S5PV210 for PHY_EXYNOS_MIPI_VIDEO --- drivers/phy/Kconfig |5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig index afa2354..ebd455c 100644 --- a/drivers/phy/Kconfig +++ b/drivers/phy/Kconfig @@ -16,8 +16,11 @@ config GENERIC_PHY framework should select this config. config PHY_EXYNOS_MIPI_VIDEO - depends on HAS_IOMEM tristate S5P/EXYNOS SoC series MIPI CSI-2/DSI PHY driver + depends on HAS_IOMEM + depends on ARCH_S5PV210 || ARCH_EXYNOS || COMPILE_TEST + select GENERIC_PHY + default y help Support for MIPI CSI-2 and MIPI DSI DPHY found on Samsung S5P and EXYNOS SoCs. -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 03/10] Documentation: devicetree: Update Samsung FIMC DT binding
On Thu, Feb 20, 2014 at 07:40:30PM +, 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 - there should be no need to require this property by the camera host device binding, a default frequency value can ofen be used; - addition of a requirement of specific order of values in clocks/ clock-names properties, so the first two entry in the clock-names property can be used as parent clock names for the camera master clock provider. It happens all in-kernel dts files list the clock in such order, thus there should be no regression as far as in-kernel dts files are concerned. I'm not sure I follow the reasoning here. Why does this matter? Why can child nodes not get these by name if they have to? Signed-off-by: Sylwester Nawrocki s.nawro...@samsung.com Acked-by: Kyungmin Park kyungmin.p...@samsung.com --- .../devicetree/bindings/media/samsung-fimc.txt | 36 +++- 1 file changed, 28 insertions(+), 8 deletions(-) diff --git a/Documentation/devicetree/bindings/media/samsung-fimc.txt b/Documentation/devicetree/bindings/media/samsung-fimc.txt index 96312f6..1a5820d 100644 --- a/Documentation/devicetree/bindings/media/samsung-fimc.txt +++ b/Documentation/devicetree/bindings/media/samsung-fimc.txt @@ -20,6 +20,7 @@ Required properties: the clock-names property; - clock-names: must contain sclk_cam0, sclk_cam1, pxl_async0, pxl_async1 entries, matching entries in the clocks property. + First two entries must be sclk_cam0, sclk_cam1. I don't think this is a good idea. 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 +33,22 @@ way around. The 'camera' node must include at least one 'fimc' child node. +Optional properties (*: Is that a smiley face? + +- #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, in this order. Parent clock of these clocks are + specified be first two entries of the clock-names property. Do you need this? That's not how clock-names is supposed to work. The clock-names property is for the names of the _input_ clock lines on the device, not the output names on whichever parent clock they came from. Any clock-names property description should define absolutely the set of names. As this does not, NAK. + +(* #clock-cells and clock-output-names are mandatory properties if external +image sensor devices reference 'camera' device node as a clock provider. s/(*/Note:/ + 'fimc' device nodes --- @@ -97,8 +114,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. s/needs to/should/ ? What is this the frequency of? Thanks, Mark. -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 2/2] phy: Select PHY_EXYNOS_DP_VIDEO by default for ARCH_EXYNOS
Instead of requiring user to figure out when PHY_EXYNOS_DP_VIDEO needs to be selected select it by default for Exynos SoCs. Also enable it when COMPILE_TEST is selected. If required the display port PHY driver can be then disabled or compiled in as module. Cc: Jingoo Han jg1@samsung.com Signed-off-by: Sylwester Nawrocki s.nawro...@samsung.com Acked-by: Kyungmin Park kyungmin.p...@samsung.com --- Changes since first version: - use ARCH_EXYNOS instead of ARCH_EXYNOS4 || ARCH_EXYNOS5 - add dependency on COMPILE_TEST --- drivers/phy/Kconfig |2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig index ebd455c..4218c99 100644 --- a/drivers/phy/Kconfig +++ b/drivers/phy/Kconfig @@ -57,6 +57,8 @@ config TWL4030_USB config PHY_EXYNOS_DP_VIDEO tristate EXYNOS SoC series Display Port PHY driver depends on OF + depends on ARCH_EXYNOS || COMPILE_TEST + default y select GENERIC_PHY help Support for Display Port PHY found on Samsung EXYNOS SoCs. -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/2] phy: Select PHY_EXYNOS_DP_VIDEO by default for ARCH_EXYNOS
On Friday 21 February 2014 16:50:02 Sylwester Nawrocki wrote: config PHY_EXYNOS_DP_VIDEO tristate EXYNOS SoC series Display Port PHY driver depends on OF + depends on ARCH_EXYNOS || COMPILE_TEST + default y select GENERIC_PHY help That would turn them on automatically if you enable COMPILE_TEST, which is probably not what you want. How about default ARCH_EXYNOS? Arnd -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 02/10] Documentation: dt: Add DT binding documentation for S5C73M3 camera
On Thu, Feb 20, 2014 at 07:40:29PM +, Sylwester Nawrocki wrote:
This adds DT binding documentation for Samsung S5C73M3 camera sensor
with an embedded ISP.
Signed-off-by: Sylwester Nawrocki s.nawro...@samsung.com
Acked-by: Kyungmin Park kyungmin.p...@samsung.com
---
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
diff --git a/Documentation/devicetree/bindings/media/samsung-s5c73m3.txt
b/Documentation/devicetree/bindings/media/samsung-s5c73m3.txt
new file mode 100644
index 000..4dd3776
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/samsung-s5c73m3.txt
@@ -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.
So this has both an I2C interface and an SPI interface that are used at
the same time?
+
+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;
It might make sense to explicitly link these two nodes somehow, in case
multiple instances appear somewhere. However, that can come later in the
case of a multi-instance device, and isn't necessary now.
+
+For more details see description of the SPI busses bindings
+(../spi/spi-bus.txt) and bindings of a specific bus controller.
+
+Example:
+
+i2c@138A00 {
+ ...
+ s5c73m3@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 = 2400;
+ 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@1392000 {
+ ...
+ s5c73m3_spi: s5c73m3 {
Nit: this should have a 0 unit-address to match the reg.
+ compatible = samsung,s5c73m3;
+ reg = 0;
+
Re: [PATCH v4 07/10] exynos4-is: Add clock provider for the SCLK_CAM clock outputs
On Thu, Feb 20, 2014 at 07:40:34PM +, Sylwester Nawrocki wrote:
This patch adds clock provider so the the SCLK_CAM0/1 output clocks
can be accessed by image sensor devices through the clk API.
Signed-off-by: Sylwester Nawrocki s.nawro...@samsung.com
Acked-by: Kyungmin Park kyungmin.p...@samsung.com
---
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 by 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 | 110
+
drivers/media/platform/exynos4-is/media-dev.h | 19 -
2 files changed, 128 insertions(+), 1 deletion(-)
[...]
+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 ARRAY_SIZE(cp-clks); i++) {
+ struct cam_clk *camclk = cp-camclk[i];
+ struct clk_init_data init;
+
+ ret = of_property_read_string_index(dev-of_node,
+ clock-output-names, i, init.name);
Are there not well-defined names for the clock outputs of the block?
+ if (ret 0)
+ break;
+
+ ret = of_property_read_string_index(dev-of_node,
+ clock-names, i, init.parent_names);
This shouldn't be a parent name. It should be the input line name.
I don't think this makes sense.
Why do you need the name of the parent clock?
+ if (ret 0)
+ break;
+
+ 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);
Are _all_ of the input clock lines available to children in hardware?
The binding and commit message(s) implied only two clocks were, so
what's the point in exporting clocks which aren't available?
Thanks,
Mark.
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/2] phy: Select PHY_EXYNOS_DP_VIDEO by default for ARCH_EXYNOS
On 21/02/14 16:59, Arnd Bergmann wrote: On Friday 21 February 2014 16:50:02 Sylwester Nawrocki wrote: config PHY_EXYNOS_DP_VIDEO tristate EXYNOS SoC series Display Port PHY driver depends on OF + depends on ARCH_EXYNOS || COMPILE_TEST + default y select GENERIC_PHY help That would turn them on automatically if you enable COMPILE_TEST, which is probably not what you want. How about default ARCH_EXYNOS? Hmm, good point, I will modify it that way. For patch 1/2 I guess it should be: default ARCH_S5PV210 || ARCH_EXYNOS or rather default y if ARCH_S5PV210 || ARCH_EXYNOS ? Thanks, Sylwester -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/2] phy: Select PHY_EXYNOS_DP_VIDEO by default for ARCH_EXYNOS
On Friday 21 February 2014 17:25:04 Sylwester Nawrocki wrote: Hmm, good point, I will modify it that way. For patch 1/2 I guess it should be: default ARCH_S5PV210 || ARCH_EXYNOS or rather default y if ARCH_S5PV210 || ARCH_EXYNOS ? Right. I'd probably use the first syntax, but it doesn't really matter. Arnd -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 1/2] phy: Select PHY_EXYNOS_MIPI_VIDEO by default for ARCH_EXYNOS
Instead of requiring user to figure out when PHY_EXYNOS_MIPI_VIDEO needs to be selected select it by default for Exynos SoCs. Also enable it when COMPILE_TEST is selected. If required the MIPI CSIS/DPHY driver can be then disabled or compiled in as module. Signed-off-by: Sylwester Nawrocki s.nawro...@samsung.com Acked-by: Kyungmin Park kyungmin.p...@samsung.com --- Changes since v2: - s/default y/default y if ARCH_S5PV210 || ARCH_EXYNOS Changes since first version: - use ARCH_EXYNOS instead of ARCH_EXYNOS4 || ARCH_EXYNOS5 - add dependency on COMPILE_TEST, and ARCH_S5PV210 for PHY_EXYNOS_MIPI_VIDEO --- drivers/phy/Kconfig |5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig index afa2354..d71be95 100644 --- a/drivers/phy/Kconfig +++ b/drivers/phy/Kconfig @@ -16,8 +16,11 @@ config GENERIC_PHY framework should select this config. config PHY_EXYNOS_MIPI_VIDEO - depends on HAS_IOMEM tristate S5P/EXYNOS SoC series MIPI CSI-2/DSI PHY driver + depends on HAS_IOMEM + depends on ARCH_S5PV210 || ARCH_EXYNOS || COMPILE_TEST + select GENERIC_PHY + default y if ARCH_S5PV210 || ARCH_EXYNOS help Support for MIPI CSI-2 and MIPI DSI DPHY found on Samsung S5P and EXYNOS SoCs. -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 2/2] phy: Select PHY_EXYNOS_DP_VIDEO by default for ARCH_EXYNOS
Instead of requiring user to figure out when PHY_EXYNOS_DP_VIDEO needs to be selected select it by default for Exynos SoCs. Also enable it when COMPILE_TEST is selected. If required the display port PHY driver can be then disabled or compiled in as module. Cc: Jingoo Han jg1@samsung.com Signed-off-by: Sylwester Nawrocki s.nawro...@samsung.com Acked-by: Kyungmin Park kyungmin.p...@samsung.com --- Changes since v2: - s/default y/default ARCH_EXYNOS Changes since first version: - use ARCH_EXYNOS instead of ARCH_EXYNOS4 || ARCH_EXYNOS5 - add dependency on COMPILE_TEST --- drivers/phy/Kconfig |2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig index d71be95..fe1b1f4 100644 --- a/drivers/phy/Kconfig +++ b/drivers/phy/Kconfig @@ -57,6 +57,8 @@ config TWL4030_USB config PHY_EXYNOS_DP_VIDEO tristate EXYNOS SoC series Display Port PHY driver depends on OF + depends on ARCH_EXYNOS || COMPILE_TEST + default ARCH_EXYNOS select GENERIC_PHY help Support for Display Port PHY found on Samsung EXYNOS SoCs. -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1] ARM: EXYNOS: Consolidate CPU init code
Hi Sachin,
On 22.01.2014 12:49, Sachin Kamat wrote:
cpu_table was used to distinguish between different Exynos4 and 5 SoCs
and assign the the initialization and io mapping pointers based on type.
exynos_init is dummy and no longer needed as we do a DT based booting.
By having a common io mapping function we can get rid of the whole
table and avoid populating it for every SoC.
Tested on Exynos4210, 5250 and 5420 based boards.
Signed-off-by: Sachin Kamat sachin.ka...@linaro.org
---
arch/arm/mach-exynos/common.c | 93 -
1 file changed, 17 insertions(+), 76 deletions(-)
On Exynos4412-based Trats2 board:
Tested-by: Tomasz Figa t.f...@samsung.com
However, please see one minor comment inline.
diff --git a/arch/arm/mach-exynos/common.c b/arch/arm/mach-exynos/common.c
index f18be40e5b21..02d0aaa7af59 100644
--- a/arch/arm/mach-exynos/common.c
+++ b/arch/arm/mach-exynos/common.c
@@ -48,55 +48,7 @@
#define L2_AUX_VAL 0x7C470001
#define L2_AUX_MASK 0xC200
-static const char name_exynos4210[] = EXYNOS4210;
-static const char name_exynos4212[] = EXYNOS4212;
-static const char name_exynos4412[] = EXYNOS4412;
-static const char name_exynos5250[] = EXYNOS5250;
-static const char name_exynos5420[] = EXYNOS5420;
-static const char name_exynos5440[] = EXYNOS5440;
-
-static void exynos4_map_io(void);
-static void exynos5_map_io(void);
-static int exynos_init(void);
-
-static struct cpu_table cpu_ids[] __initdata = {
- {
- .idcode = EXYNOS4210_CPU_ID,
- .idmask = EXYNOS4_CPU_MASK,
- .map_io = exynos4_map_io,
- .init = exynos_init,
- .name = name_exynos4210,
- }, {
- .idcode = EXYNOS4212_CPU_ID,
- .idmask = EXYNOS4_CPU_MASK,
- .map_io = exynos4_map_io,
- .init = exynos_init,
- .name = name_exynos4212,
- }, {
- .idcode = EXYNOS4412_CPU_ID,
- .idmask = EXYNOS4_CPU_MASK,
- .map_io = exynos4_map_io,
- .init = exynos_init,
- .name = name_exynos4412,
- }, {
- .idcode = EXYNOS5250_SOC_ID,
- .idmask = EXYNOS5_SOC_MASK,
- .map_io = exynos5_map_io,
- .init = exynos_init,
- .name = name_exynos5250,
- }, {
- .idcode = EXYNOS5420_SOC_ID,
- .idmask = EXYNOS5_SOC_MASK,
- .map_io = exynos5_map_io,
- .init = exynos_init,
- .name = name_exynos5420,
- }, {
- .idcode = EXYNOS5440_SOC_ID,
- .idmask = EXYNOS5_SOC_MASK,
- .init = exynos_init,
- .name = name_exynos5440,
- },
-};
+static void exynos_map_io(void);
cosmetic: Instead of adding forward declaration, the resulting
exynos_map_io() function could be simply merged with exynos_init_io().
Best regards,
Tomasz
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/2] phy: Select PHY_EXYNOS_DP_VIDEO by default for ARCH_EXYNOS
On 21/02/14 17:41, Arnd Bergmann wrote: On Friday 21 February 2014 17:25:04 Sylwester Nawrocki wrote: Hmm, good point, I will modify it that way. For patch 1/2 I guess it should be: default ARCH_S5PV210 || ARCH_EXYNOS or rather default y if ARCH_S5PV210 || ARCH_EXYNOS ? Right. I'd probably use the first syntax, but it doesn't really matter. Oops, I've already sent patch using the second one, as it seemed more common. Thanks, Sylwester -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 02/10] Documentation: dt: Add DT binding documentation for S5C73M3 camera
On 21/02/14 16:42, Mark Rutland wrote:
[...]
+++ b/Documentation/devicetree/bindings/media/samsung-s5c73m3.txt
@@ -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.
So this has both an I2C interface and an SPI interface that are used at
the same time?
Yes, both are needed. AFAIU SPI is added so the firmware upload is faster.
+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;
It might make sense to explicitly link these two nodes somehow, in case
multiple instances appear somewhere. However, that can come later in the
case of a multi-instance device, and isn't necessary now.
I guess a phandle at the I2C slave device node, pointing to the SPI node
and/or the other way around would do. I don't expect these devices to be
used in multiple instances though and would prefer to address that when
necessary.
We could try and create a root node for this device with an interesting
structure, if we wanted to go much into details. But it could get a bit
complicated given the scheme the I2C/SPI bus binding are structured now.
Presumably that's something that could be handled later with a different
compatible string if required.
+For more details see description of the SPI busses bindings
+(../spi/spi-bus.txt) and bindings of a specific bus controller.
+
+Example:
+
+i2c@138A00 {
+...
+s5c73m3@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 = 2400;
+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@1392000 {
+...
+s5c73m3_spi: s5c73m3 {
Nit: this should have a 0 unit-address to match the reg.
OK, I'll correct that.
+compatible = samsung,s5c73m3;
+reg = 0;
+...
+};
+};
Otherwise I don't see anything problematic about the binding.
Acked-by: Mark Rutland mark.rutl...@arm.com
Thanks for the
Re: [PATCH v2 2/2] phy: Select PHY_EXYNOS_DP_VIDEO by default for ARCH_EXYNOS
On Friday 21 February 2014 17:50:44 Sylwester Nawrocki wrote: On 21/02/14 17:41, Arnd Bergmann wrote: On Friday 21 February 2014 17:25:04 Sylwester Nawrocki wrote: Hmm, good point, I will modify it that way. For patch 1/2 I guess it should be: default ARCH_S5PV210 || ARCH_EXYNOS or rather default y if ARCH_S5PV210 || ARCH_EXYNOS ? Right. I'd probably use the first syntax, but it doesn't really matter. Oops, I've already sent patch using the second one, as it seemed more common. As I said, it doesn't matter. Arnd -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
