Re: Enabling uart 3 in arndale
Please can someone help On 14-Mar-2014, at 1:34 pm, armdev armdev@gmail.com wrote: Hi, We are trying to enable the UART3 on COM18 pins of arndale board. The UART3 RXD and TXD are on pins 2 and 4 which as per the base board specification is connected as XuRXD3 : UART_3_RXD/GPA1[4] : 2 XuTXD3 : UART_3_TXD/GPA1[5] : 4 As per the public reference manual of exynos 5250, there is a register GPACON (0x1140_) Setting GPACON |= 0x0010_ should enable the pins, but I am not able to see any output on UART3. Can you please suggest what is the right procedure -Regards armdev team @FTM -- 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: [U-Boot] Enabling uart 3 in arndale
Hi On Fri, Mar 14, 2014 at 9:04 AM, armdev armdev@gmail.com wrote: Hi, We are trying to enable the UART3 on COM18 pins of arndale board. The UART3 RXD and TXD are on pins 2 and 4 which as per the base board specification is connected as XuRXD3 : UART_3_RXD/GPA1[4] : 2 XuTXD3 : UART_3_TXD/GPA1[5] : 4 As per the public reference manual of exynos 5250, there is a register GPACON (0x1140_) Setting GPACON |= 0x0010_ should enable the pins, but I am not able to see any output on UART3. Can you please suggest what is the right procedure Did you change the board console output ;)? Michael -Regards armdev team @FTM ___ U-Boot mailing list u-b...@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot -- | Michael Nazzareno Trimarchi Amarula Solutions BV | | COO - Founder Cruquiuskade 47 | | +31(0)851119172 Amsterdam 1018 AM NL | | [`as] http://www.amarulasolutions.com | -- 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: [U-Boot] Enabling uart 3 in arndale
Dear Michael, Yep didn’t got any prints. So tried a hack. a) Booted using uart 2 b) set the GPACON c) Copied the config from 0x12c2 to 0x12c3 d)tried to write a char 0x41 (A) on 0x12c20020 0x12c30020, I got output on UART2 but not on UART3. We would like to use both uarts. Have you used UART3, did the boot to UART3 worked for you ? So you see any problem with any of our steps listed in this / pref mail. Thanks armdev team On 16-Mar-2014, at 2:56 pm, Michael Trimarchi mich...@amarulasolutions.com wrote: Hi On Fri, Mar 14, 2014 at 9:04 AM, armdev armdev@gmail.com wrote: Hi, We are trying to enable the UART3 on COM18 pins of arndale board. The UART3 RXD and TXD are on pins 2 and 4 which as per the base board specification is connected as XuRXD3 : UART_3_RXD/GPA1[4] : 2 XuTXD3 : UART_3_TXD/GPA1[5] : 4 As per the public reference manual of exynos 5250, there is a register GPACON (0x1140_) Setting GPACON |= 0x0010_ should enable the pins, but I am not able to see any output on UART3. Can you please suggest what is the right procedure Did you change the board console output ;)? Michael -Regards armdev team @FTM ___ U-Boot mailing list u-b...@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot -- | Michael Nazzareno Trimarchi Amarula Solutions BV | | COO - Founder Cruquiuskade 47 | | +31(0)851119172 Amsterdam 1018 AM NL | | [`as] http://www.amarulasolutions.com | -- 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: [U-Boot] Enabling uart 3 in arndale
Hi On Sun, Mar 16, 2014 at 11:07 AM, armdev armdev@gmail.com wrote: Dear Michael, Yep didn't got any prints. So tried a hack. a) Booted using uart 2 b) set the GPACON c) Copied the config from 0x12c2 to 0x12c3 d)tried to write a char 0x41 (A) on 0x12c20020 0x12c30020, I got output on UART2 but not on UART3. We would like to use both uarts. Have you used UART3, did the boot to UART3 worked for you ? So you see any problem with any of our steps listed in this / pref mail. Are you sure that you don't need to clock uart logic? I don't have such board I'm trying just to help you Michael Thanks armdev team On 16-Mar-2014, at 2:56 pm, Michael Trimarchi mich...@amarulasolutions.com wrote: Hi On Fri, Mar 14, 2014 at 9:04 AM, armdev armdev@gmail.com wrote: Hi, We are trying to enable the UART3 on COM18 pins of arndale board. The UART3 RXD and TXD are on pins 2 and 4 which as per the base board specification is connected as XuRXD3 : UART_3_RXD/GPA1[4] : 2 XuTXD3 : UART_3_TXD/GPA1[5] : 4 As per the public reference manual of exynos 5250, there is a register GPACON (0x1140_) Setting GPACON |= 0x0010_ should enable the pins, but I am not able to see any output on UART3. Can you please suggest what is the right procedure Did you change the board console output ;)? Michael -Regards armdev team @FTM ___ U-Boot mailing list u-b...@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot -- | Michael Nazzareno Trimarchi Amarula Solutions BV | | COO - Founder Cruquiuskade 47 | | +31(0)851119172 Amsterdam 1018 AM NL | | [`as] http://www.amarulasolutions.com | -- | Michael Nazzareno Trimarchi Amarula Solutions BV | | COO - Founder Cruquiuskade 47 | | +31(0)851119172 Amsterdam 1018 AM NL | | [`as] http://www.amarulasolutions.com | -- 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: [U-Boot] Enabling uart 3 in arndale
Dear Michael, Thanks for taking out time to extend help. As per the RefManual the UART is taking the SCLK_UART Clock and as there are 4 channels (4 UARTS). Following is from the public exynos 5250 manual (http://www.samsung.com/global/business/semiconductor/file/product/Exynos_5_Dual_User_Manaul_Public_REV100-0.pdf) Four independent channels with asynchronous and serial input/output ports for general purpose (Channel 0 to 3), and One channel in ISP (ISP-UART Channel 0) … • Each UART contains a Baud-rate generator, a Transmitter, a Receiver and a Control Unit. The Baud-rate generator uses SCLK_UART. -Regards Armdev Team On 16-Mar-2014, at 3:55 pm, Michael Trimarchi mich...@amarulasolutions.com wrote: Hi On Sun, Mar 16, 2014 at 11:07 AM, armdev armdev@gmail.com wrote: Dear Michael, Yep didn't got any prints. So tried a hack. a) Booted using uart 2 b) set the GPACON c) Copied the config from 0x12c2 to 0x12c3 d)tried to write a char 0x41 (A) on 0x12c20020 0x12c30020, I got output on UART2 but not on UART3. We would like to use both uarts. Have you used UART3, did the boot to UART3 worked for you ? So you see any problem with any of our steps listed in this / pref mail. Are you sure that you don't need to clock uart logic? I don't have such board I'm trying just to help you Michael Thanks armdev team On 16-Mar-2014, at 2:56 pm, Michael Trimarchi mich...@amarulasolutions.com wrote: Hi On Fri, Mar 14, 2014 at 9:04 AM, armdev armdev@gmail.com wrote: Hi, We are trying to enable the UART3 on COM18 pins of arndale board. The UART3 RXD and TXD are on pins 2 and 4 which as per the base board specification is connected as XuRXD3 : UART_3_RXD/GPA1[4] : 2 XuTXD3 : UART_3_TXD/GPA1[5] : 4 As per the public reference manual of exynos 5250, there is a register GPACON (0x1140_) Setting GPACON |= 0x0010_ should enable the pins, but I am not able to see any output on UART3. Can you please suggest what is the right procedure Did you change the board console output ;)? Michael -Regards armdev team @FTM ___ U-Boot mailing list u-b...@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot -- | Michael Nazzareno Trimarchi Amarula Solutions BV | | COO - Founder Cruquiuskade 47 | | +31(0)851119172 Amsterdam 1018 AM NL | | [`as] http://www.amarulasolutions.com | -- | Michael Nazzareno Trimarchi Amarula Solutions BV | | COO - Founder Cruquiuskade 47 | | +31(0)851119172 Amsterdam 1018 AM NL | | [`as] http://www.amarulasolutions.com | -- 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: [U-Boot] Enabling uart 3 in arndale
Hi, On 16.03.2014 11:42, armdev wrote: Dear Michael, Thanks for taking out time to extend help. As per the RefManual the UART is taking the SCLK_UART Clock and as there are 4 channels (4 UARTS). Following is from the public exynos 5250 manual (http://www.samsung.com/global/business/semiconductor/file/product/Exynos_5_Dual_User_Manaul_Public_REV100-0.pdf) Four independent channels with asynchronous and serial input/output ports for general purpose (Channel 0 to 3), and One channel in ISP (ISP-UART Channel 0) … • Each UART contains a Baud-rate generator, a Transmitter, a Receiver and a Control Unit. The Baud-rate generator uses SCLK_UART. Each UART block uses independent clock source, i.e. UART0 uses SCLK_UART0 and UART3 uses SCLK_UART3. Do you have MUX_UART3 and DIV_UART3 configured properly? Do you have the IP bus clock (CLK_UART3) ungated? The public manual contains full description of clock tree and clock controller registers, so you should be able to figure this out. 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: Enabling uart 3 in arndale
Hi, On 14.03.2014 09:04, armdev wrote: Hi, We are trying to enable the UART3 on COM18 pins of arndale board. The UART3 RXD and TXD are on pins 2 and 4 which as per the base board specification is connected as XuRXD3 : UART_3_RXD/GPA1[4] : 2 XuTXD3 : UART_3_TXD/GPA1[5] : 4 As per the public reference manual of exynos 5250, there is a register GPACON (0x1140_) Setting GPACON |= 0x0010_ should enable the pins, but I am not able to see any output on UART3. Can you please suggest what is the right procedure The register is GPA1CON and its GPA1CON[4] and [5] bit fields need both to be set to 0x2 - see Pad Control chapter of Exynos5250 public datasheet. Also GPA1PUD should be reconfigured to disable default pull-down on both pins, again you can find details of the register in the datasheet. 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 RE-SEND 1/7] net: sxgbe: add basic framework for Samsung 10Gb ethernet driver
Andrew.an bh74...@samsung.com :
[...]
+struct sxgbe_core_ops {
+ /* MAC core initialization */
+ void (*core_init)(void __iomem *ioaddr);
[...]
+ /* adjust SXGBE speed */
+ void (*set_speed)(void __iomem *ioaddr, unsigned char speed);
+};
This indirection level is never used.
Those are used, can you give more detail?
They are used but they always point to the same set of methods.
Those methods could thus be directly called.
[...]
+/* SXGBE private data structures */
+struct sxgbe_tx_queue {
+ u8 queue_no;
+ unsigned int irq_no;
+ struct sxgbe_priv_data *priv_ptr;
+ struct sxgbe_tx_norm_desc *dma_tx;
You may lay things a bit differently.
can you give more detail?
Bigger fields first, u8 at the end. It will save padding in the struct.
--
Ueimor
--
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: [PATCHv3 5/8] devfreq: exynos4: Use SET_SYSTEM_SLEEP_PM_OPS macro
On Friday, March 14, 2014 6:30 PM, Chanwoo Choi wrote:
This patch use SET_SYSTEM_SLEEP_PM_OPS macro instead of legacy method.
Signed-off-by: Chanwoo Choi cw00.c...@samsung.com
---
drivers/devfreq/exynos/exynos4_bus.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/devfreq/exynos/exynos4_bus.c
b/drivers/devfreq/exynos/exynos4_bus.c
index 60539e8..e5d2c5a 100644
--- a/drivers/devfreq/exynos/exynos4_bus.c
+++ b/drivers/devfreq/exynos/exynos4_bus.c
@@ -1247,6 +1247,7 @@ static int exynos4_busfreq_remove(struct
platform_device *pdev)
return 0;
}
+#ifdef CONFIG_PM_SLEEP
static int exynos4_busfreq_resume(struct device *dev)
{
struct busfreq_data *data = dev_get_drvdata(dev);
@@ -1254,9 +1255,10 @@ static int exynos4_busfreq_resume(struct device *dev)
busfreq_mon_reset(data);
return 0;
}
+#endif
static const struct dev_pm_ops exynos4_busfreq_pm = {
- .resume = exynos4_busfreq_resume,
+ SET_SYSTEM_SLEEP_PM_OPS(NULL, exynos4_busfreq_resume)
Hi Chanwoo Choi,
How about using SIMPLE_DEV_PM_OPS instead of SET_SYSTEM_SLEEP_PM_OPS?
SIMPLE_DEV_PM_OPS is simpler as below.
static SIMPLE_DEV_PM_OPS(exynos4_busfreq_pm, NULL, exynos4_busfreq_resume);
However, if runtime pm functions will be added later,
SIMPLE_DEV_PM_OPS is not necessary.
Best regards,
Jingoo Han
};
static const struct platform_device_id exynos4_busfreq_id[] = {
--
1.8.0
--
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 v11 01/27] iommu/exynos: do not include removed header
On Fri, 14 Mar 2014 17:29:36 +0530, Sachin Kamat wrote: On 14 March 2014 17:19, Cho KyongHo pullip@samsung.com wrote: From: Sachin Kamat [mailto:sachin.ka...@linaro.org] Sent: Friday, March 14, 2014 7:00 PM On 14 March 2014 10:31, Cho KyongHo pullip@samsung.com wrote: Commit 25e9d28d92 (ARM: EXYNOS: remove system mmu initialization from exynos tree) removed arch/arm/mach-exynos/mach/sysmmu.h header without removing remaining use of it from exynos-iommu driver, thus causing a compilation error. This patch fixes the error by removing respective include line from exynos-iommu.c. CC: Tomasz Figa t.f...@samsung.com Signed-off-by: Cho KyongHo pullip@samsung.com --- drivers/iommu/exynos-iommu.c |3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c index 0740189..4876d35 100644 --- a/drivers/iommu/exynos-iommu.c +++ b/drivers/iommu/exynos-iommu.c @@ -12,6 +12,7 @@ #define DEBUG #endif +#include linux/kernel.h This change doesn't look related to the patch subject/description. Yes. But it is simply added without any side-effect. Do you think it should be in a separate patch?. Actually, the added line is a redundant. If it is redundant, then you shouldn't be adding it. If it is required, then please mention about the need in the commit description if not a separate patch. Ok. Thanks for the advice. KyongHo -- 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 v11 11/27] clk: exynos: add gate clock descriptions of System MMU
On Fri, 14 Mar 2014 13:17:26 +0100, Tomasz Figa wrote: Hi KyongHo, On 14.03.2014 06:06, Cho KyongHo wrote: This adds gate clocks of all System MMUs and their master IPs that are not apeared in clk-exynos5250.c and clk-exynos5420.c Also fixes GATE_IP_ACP to 0x18800 and changed GATE_DA to GATE for System MMU clocks in clk-exynos4.c Signed-off-by: Cho KyongHo pullip@samsung.com --- .../devicetree/bindings/clock/exynos5250-clock.txt |3 +++ .../devicetree/bindings/clock/exynos5420-clock.txt |6 +- drivers/clk/samsung/clk-exynos5250.c |5 + drivers/clk/samsung/clk-exynos5420.c | 13 +++-- include/dt-bindings/clock/exynos5250.h |4 include/dt-bindings/clock/exynos5420.h |6 +- 6 files changed, 33 insertions(+), 4 deletions(-) diff --git a/Documentation/devicetree/bindings/clock/exynos5250-clock.txt b/Documentation/devicetree/bindings/clock/exynos5250-clock.txt index 72ce617..67e50ba 100644 --- a/Documentation/devicetree/bindings/clock/exynos5250-clock.txt +++ b/Documentation/devicetree/bindings/clock/exynos5250-clock.txt @@ -162,6 +162,9 @@ clock which they consume. g2d 345 mdma0 346 smmu_mdma0 347 + smmu_tv 348 + smmu_fimd1 349 + smmu_2d 350 This patch should be rebased on top of Andrzej Hajda's patches removing these clock ID listings and reworking dts files to use defined macros. They are present in v3.15-next/dt-clk-exynos branch of linux-samsung tree, but I have asked Kukjin to merge them to his for-next branch, so they could show up in linux-next tree. Thanks for the information. I will also ask Kukjin for the correct base of the patches. KyongHo. -- 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 v11 12/27] ARM: dts: Add description of System MMU of Exynos SoCs
On Fri, 14 Mar 2014 13:20:23 +0100, Tomasz Figa wrote: Hi KyongHo, On 14.03.2014 06:06, Cho KyongHo wrote: This patch adds dts entries for the System MMU devices found on Exynos4 and Exynos5 SoC series and the System MMU binding documentation. CC: Rob Herring robherri...@gmail.com CC: Sylwester Nawrocki s.nawro...@samsung.com Signed-off-by: Cho KyongHo pullip@samsung.com --- .../bindings/iommu/samsung,exynos4210-sysmmu.txt | 86 +++ arch/arm/boot/dts/exynos4.dtsi | 107 arch/arm/boot/dts/exynos4210.dtsi | 23 +- arch/arm/boot/dts/exynos4x12.dtsi | 77 +- arch/arm/boot/dts/exynos5250.dtsi | 266 +++- arch/arm/boot/dts/exynos5420.dtsi | 205 ++- 6 files changed, 758 insertions(+), 6 deletions(-) create mode 100644 Documentation/devicetree/bindings/iommu/samsung,exynos4210-sysmmu.txt diff --git a/Documentation/devicetree/bindings/iommu/samsung,exynos4210-sysmmu.txt b/Documentation/devicetree/bindings/iommu/samsung,exynos4210-sysmmu.txt new file mode 100644 index 000..e4417bb --- /dev/null +++ b/Documentation/devicetree/bindings/iommu/samsung,exynos4210-sysmmu.txt @@ -0,0 +1,86 @@ +Samsung Exynos IOMMU H/W, System MMU (System Memory Management Unit) + +Samsung's Exynos architecture contains System MMUs that enables scattered +physical memory chunks visible as a contiguous region to DMA-capable peripheral +devices like MFC, FIMC, FIMD, GScaler, FIMC-IS and so forth. + +System MMU is an IOMMU and supports identical translation table format to +ARMv7 translation tables with minimum set of page properties including access +permissions, shareability and security protection. In addition, System MMU has +another capabilities like L2 TLB or block-fetch buffers to minimize translation +latency. + +System MMUs are in many to one relation with peripheral devices, i.e. single +peripheral device might have multiple System MMUs (usually one for each bus +master), but one System MMU can handle transactions from only one peripheral +device. The relation between a System MMU and the peripheral device needs to be +defined in device node of the peripheral device. + +MFC in all Exynos SoCs and FIMD, M2M Scalers and G2D in Exynos5420 has 2 System +MMUs. +* MFC has one System MMU on its left and right bus. +* FIMD in Exynos5420 has one System MMU for window 0 and 4, the other system MMU + for window 1, 2 and 3. +* M2M Scalers and G2D in Exynos5420 has one System MMU on the read channel and + the other System MMU on the write channel. +The drivers must consider how to handle those System MMUs. One of the idea is +to implement child devices or sub-devices which are the client devices of the +System MMU. + +Required properties: +- compatible: Should be one of: + samsung,sysmmu-v1 + samsung,sysmmu-v2 + samsung,sysmmu-v3.1 + samsung,sysmmu-v3.2 + samsung,sysmmu-v3.3 + +- reg: A tuple of base address and size of System MMU registers. +- interrupt-parent: The phandle of the interrupt controller of System MMU +- interrupts: An interrupt specifier for interrupt signal of System MMU, + according to the format defined by a particular interrupt + controller. +- clock-names: Should be sysmmu if the System MMU is needed to gate its clock. + Please refer to the following documents: + Documentation/devicetree/bindings/clock/clock-bindings.txt + Documentation/devicetree/bindings/clock/exynos4-clock.txt + Documentation/devicetree/bindings/clock/exynos5250-clock.txt + Documentation/devicetree/bindings/clock/exynos5420-clock.txt + Optional master if the clock to the System MMU is gated by + another gate clock other than sysmmu. The System MMU driver + sets master the parent of sysmmu. + Exynos4 SoCs, there needs no master clockj. + Exynos5 SoCs, some System MMUs must have master clocks. +- clocks: Required if the System MMU is needed to gate its clock. + Please refer to the documents listed above. +- samsung,power-domain: Required if the System MMU is needed to gate its power. + Please refer to the following document: + Documentation/devicetree/bindings/arm/exynos/power_domain.txt +- mmu-masters: A phandle to device nodes representing the master for which + the System MMU can provide a translation. Any additional values + after the phandle will be ignored because a System MMU never + have two or more masters. #stream-id-cells specified in the + master's node will be also ignored. + If more than one phandle is specified, only the first
Re: [PATCH] clk: samsung: fixed compiler warning [-Wpointer-to-int-cast]
On 02/26/2014 11:42 AM, Pankaj Dubey wrote:
When compiled using ARM64 cross compiler, gcc complains as
drivers/clk/samsung/clk.c:293:18:
warning: cast from pointer to integer of different size
[-Wpointer-to-int-cast]
Signed-off-by: Pankaj Dubey pankaj.du...@samsung.com
---
drivers/clk/samsung/clk.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/clk/samsung/clk.c b/drivers/clk/samsung/clk.c
index 91bec3e..59142ba 100644
--- a/drivers/clk/samsung/clk.c
+++ b/drivers/clk/samsung/clk.c
@@ -278,7 +278,7 @@ void __init samsung_clk_of_register_fixed_ext(
for_each_matching_node_and_match(np, clk_matches, match) {
if (of_property_read_u32(np, clock-frequency, freq))
continue;
- fixed_rate_clk[(u32)match-data].fixed_rate = freq;
+ fixed_rate_clk[(unsigned long)match-data].fixed_rate = freq;
}
samsung_clk_register_fixed_rate(fixed_rate_clk, nr_fixed_rate_clk);
}
Gentle Ping.
--
Best Regards,
Pankaj Dubey
--
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: [PATCHv3 5/8] devfreq: exynos4: Use SET_SYSTEM_SLEEP_PM_OPS macro
Hi Jingoo,
On 03/17/2014 09:17 AM, Jingoo Han wrote:
On Friday, March 14, 2014 6:30 PM, Chanwoo Choi wrote:
This patch use SET_SYSTEM_SLEEP_PM_OPS macro instead of legacy method.
Signed-off-by: Chanwoo Choi cw00.c...@samsung.com
---
drivers/devfreq/exynos/exynos4_bus.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/devfreq/exynos/exynos4_bus.c
b/drivers/devfreq/exynos/exynos4_bus.c
index 60539e8..e5d2c5a 100644
--- a/drivers/devfreq/exynos/exynos4_bus.c
+++ b/drivers/devfreq/exynos/exynos4_bus.c
@@ -1247,6 +1247,7 @@ static int exynos4_busfreq_remove(struct
platform_device *pdev)
return 0;
}
+#ifdef CONFIG_PM_SLEEP
static int exynos4_busfreq_resume(struct device *dev)
{
struct busfreq_data *data = dev_get_drvdata(dev);
@@ -1254,9 +1255,10 @@ static int exynos4_busfreq_resume(struct device *dev)
busfreq_mon_reset(data);
return 0;
}
+#endif
static const struct dev_pm_ops exynos4_busfreq_pm = {
-.resume = exynos4_busfreq_resume,
+SET_SYSTEM_SLEEP_PM_OPS(NULL, exynos4_busfreq_resume)
Hi Chanwoo Choi,
How about using SIMPLE_DEV_PM_OPS instead of SET_SYSTEM_SLEEP_PM_OPS?
SIMPLE_DEV_PM_OPS is simpler as below.
static SIMPLE_DEV_PM_OPS(exynos4_busfreq_pm, NULL, exynos4_busfreq_resume);
However, if runtime pm functions will be added later,
SIMPLE_DEV_PM_OPS is not necessary.
OK, I'll use SIMPLE_DEV_PM_OPS on next patchset.
Best Regards,
Chanwoo Choi
--
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: [PATCHv2 0/5] ARM: dts: exynos: Add missing dt data to bring kernel of Exynos4x12
Dear Kukjin, Ping. Please review this patchset. Best Regards, Chanwoo Choi On 03/13/2014 10:57 AM, Chanwoo Choi wrote: Dear Kukjin, On 03/12/2014 08:21 PM, Tomasz Figa wrote: Hi Chanwoo, On 12.03.2014 07:19, Chanwoo Choi wrote: This patch add missing dt data of Exynos4x12 to bring up kernel feature and code clean. This patchset is based on 'v3.15-next/dt-clk-exynos' branch. - git://git.kernel.org/pub/scm/linux/kernel/git/kgene/linux-samsung.git exynos4x12/exynos4412/exynos4212.dtsi - Add ADC (Analog and Digital Converter) to get raw data - Add PMU (Performance Monitoring Unit) for perf event - Add gps_alive power domain to remove power leakage when gps-alive isn't used - Remove duplicate dt data of interrput combiner controller exynos4412-trats.dts - Add ADC dt data with ntc thermistor child to read temperature Changes from v1: - Use clock macro name for Exynos4 instead of constant for ADC - Remove unnecessary description about patch content - Move gps-alive power domain's dt data from exynos4x12.dts to exynos4.dts - Move thermistor dt node outside of ADC dt node and modify node name of thermistor Chanwoo Choi (5): ARM: dts: exynos4x12: Add ADC's dt data to read raw data ARM: dts: exynos4x12: Add PMU dt data to support PMU(Perforamnce Monitoring Unit) ARM: dts: exynos4x12: Add GPS_ALIVE power domain ARM: dts: exynos: Move common dt data for interrupt combiner controller ARM: dts: exynos4412-trats2: Add ADC/themistor dt data to get temperature of SoC/battery arch/arm/boot/dts/exynos4.dtsi | 5 + arch/arm/boot/dts/exynos4212.dtsi | 13 - arch/arm/boot/dts/exynos4412-trats2.dts | 21 + arch/arm/boot/dts/exynos4412.dtsi | 14 -- arch/arm/boot/dts/exynos4x12.dtsi | 26 ++ 5 files changed, 60 insertions(+), 19 deletions(-) Reviewed-by: Tomasz Figa t.f...@samsung.com Please review or comment this patchset. Best Regards, Chanwoo Choi ___ linux-arm-kernel mailing list linux-arm-ker...@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel -- 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: [PATCHv2 0/5] ARM: dts: exynos: Add missing dt data to bring kernel of Exynos4x12
Chanwoo Choi wrote: Dear Kukjin, Ping. Please review this patchset. Looks good to me, will apply tonight. Thanks, Kukjin Best Regards, Chanwoo Choi On 03/13/2014 10:57 AM, Chanwoo Choi wrote: Dear Kukjin, On 03/12/2014 08:21 PM, Tomasz Figa wrote: Hi Chanwoo, On 12.03.2014 07:19, Chanwoo Choi wrote: This patch add missing dt data of Exynos4x12 to bring up kernel feature and code clean. This patchset is based on 'v3.15-next/dt-clk-exynos' branch. - git://git.kernel.org/pub/scm/linux/kernel/git/kgene/linux- samsung.git exynos4x12/exynos4412/exynos4212.dtsi - Add ADC (Analog and Digital Converter) to get raw data - Add PMU (Performance Monitoring Unit) for perf event - Add gps_alive power domain to remove power leakage when gps-alive isn't used - Remove duplicate dt data of interrput combiner controller exynos4412-trats.dts - Add ADC dt data with ntc thermistor child to read temperature Changes from v1: - Use clock macro name for Exynos4 instead of constant for ADC - Remove unnecessary description about patch content - Move gps-alive power domain's dt data from exynos4x12.dts to exynos4.dts - Move thermistor dt node outside of ADC dt node and modify node name of thermistor Chanwoo Choi (5): ARM: dts: exynos4x12: Add ADC's dt data to read raw data ARM: dts: exynos4x12: Add PMU dt data to support PMU(Perforamnce Monitoring Unit) ARM: dts: exynos4x12: Add GPS_ALIVE power domain ARM: dts: exynos: Move common dt data for interrupt combiner controller ARM: dts: exynos4412-trats2: Add ADC/themistor dt data to get temperature of SoC/battery arch/arm/boot/dts/exynos4.dtsi | 5 + arch/arm/boot/dts/exynos4212.dtsi | 13 - arch/arm/boot/dts/exynos4412-trats2.dts | 21 + arch/arm/boot/dts/exynos4412.dtsi | 14 -- arch/arm/boot/dts/exynos4x12.dtsi | 26 ++ 5 files changed, 60 insertions(+), 19 deletions(-) Reviewed-by: Tomasz Figa t.f...@samsung.com Please review or comment this patchset. Best Regards, Chanwoo Choi -- 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: [PATCHv2 0/5] ARM: dts: exynos: Add missing dt data to bring kernel of Exynos4x12
Dear Kukjin, On 03/17/2014 10:44 AM, Kukjin Kim wrote: Chanwoo Choi wrote: Dear Kukjin, Ping. Please review this patchset. Looks good to me, will apply tonight. Thanks for your reply. Best Regards, Chanwoo Choi -- 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: [PATCHv2 0/8] devfreq: exynos4: Support dt and use common ppmu driver
Hi, On 03/14/2014 07:47 PM, Bartlomiej Zolnierkiewicz wrote: On Friday, March 14, 2014 12:14:03 PM Chanwoo Choi wrote: Hi, On 03/14/2014 01:43 AM, Bartlomiej Zolnierkiewicz wrote: Hi, On Thursday, March 13, 2014 05:17:21 PM Chanwoo Choi wrote: This patchset support devicetree and use common ppmu driver instead of individual code of exynos4_bus.c to remove duplicate code. Also this patchset get the resources for busfreq from dt data by using DT helper function. - PPMU register address - PPMU clock - Regulator for INT/MIF block This patchset use SET_SYSTEM_SLEEP_PM_OPS macro intead of legacy method. To remove power-leakage in suspend state, before entering suspend state, disable ppmu clocks. Changes from v1: - Add exynos4_bus.txt documentation for devicetree guide - Fix probe failure if CONFIG_PM_OPP is disabled - Fix typo and resource leak(regulator/clock/memory) when happening probe failure - Add additionally comment for PPMU usage instead of previous PPC - Split separate patch to remove ambiguous of patch Chanwoo Choi (8): devfreq: exynos4: Support devicetree to get device id of Exynos4 SoC devfreq: exynos4: Use common ppmu driver and get ppmu address from dt data devfreq: exynos4: Add ppmu's clock control and code clean about regulator control devfreq: exynos4: Fix bug of resource leak and code clean on probe() devfreq: exynos4: Use SET_SYSTEM_SLEEP_PM_OPS macro devfreq: exynos4: Fix power-leakage of clock on suspend state devfreq: exynos4: Add CONFIG_PM_OPP dependency to fix probe fail devfreq: exynos4: Add busfreq driver for exynos4210/exynos4x12 .../devicetree/bindings/devfreq/exynos4_bus.txt| 49 +++ drivers/devfreq/Kconfig| 1 + drivers/devfreq/exynos/Makefile| 2 +- drivers/devfreq/exynos/exynos4_bus.c | 415 ++--- 4 files changed, 341 insertions(+), 126 deletions(-) create mode 100644 Documentation/devicetree/bindings/devfreq/exynos4_bus.txt Thanks for updating this patchset. There are still some minor issues left though: - patch #4 should be at beginning of the patch series - moving of devfreq_unregister_opp_notifier(dev, data-devfreq) from exynos4_bus_exit() to exynos4_busfreq_remove() should be in patch #4 (which should really be at the beggining of patch series) not #3 - handling of iounmap(data-ppmu[i].hw_base) should be added to exynos4_bus_exit() in patch #2 not #3 - patch #8 summary and description should mention fact that it adds DT binding documentation (not the driver itself) and the patch itself can be slighlty polished OK, I'll re-order the sequence of patchset and modify minior issues about your comment. Also, I'll modify the patch description for patch8. One important note about this patchset not mentioned in the cover letter is that it is improving currently unused driver (because of DT-only mach-exynos conversion the only user was removed in June 2013 and from the reading the code I suspect that even that user hadn't worked previously). As such this patch series should not cause any regressions. I don't understand correct your meaning.I explained DT support on upper patchset description by using DT helper function and I added PPMU descritpion. Also, Each patch include detailed description of patch content. Everything is okay, I just noted that since there are no users of this driver currently (the only user was NURI and it was removed by DT conversion of mach-exynos) it should be okay to merge the patch series quickly once reviewed and acked by the respective maintainers. What is more needed? Users of the driver? ;) Your patchset adds DT support and fixes to the driver but it doesn't add actual users of the driver to arch/arm/boot/dts/ files. Ah, I didn't understand 'users' meanings. Now, clk-exynos4.c driver in mainline don't provide the clocks for PPMU IP. So, I can't add dt node of exynos4_busfreq to exynos4210.dtsi/exynos4x12.dtsi/exynos4210-trats.dts/exynos4412-trats2.dts. First of all, I will add the ppmu clocks to clk-exynos4.c driver and then modify dts file for exynos4_busfreq as your comment. That which add the ppmu clocks is apart from this patch set. Thanks for your comment. Best Regards, Chanwoo Choi -- 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: [PATCHv2 0/8] devfreq: exynos4: Support dt and use common ppmu driver
Hi Tomasz, On 03/15/2014 02:58 AM, Tomasz Figa wrote: Hi Chanwoo, On 13.03.2014 09:17, Chanwoo Choi wrote: This patchset support devicetree and use common ppmu driver instead of individual code of exynos4_bus.c to remove duplicate code. Also this patchset get the resources for busfreq from dt data by using DT helper function. - PPMU register address - PPMU clock - Regulator for INT/MIF block This patchset use SET_SYSTEM_SLEEP_PM_OPS macro intead of legacy method. To remove power-leakage in suspend state, before entering suspend state, disable ppmu clocks. Changes from v1: - Add exynos4_bus.txt documentation for devicetree guide - Fix probe failure if CONFIG_PM_OPP is disabled - Fix typo and resource leak(regulator/clock/memory) when happening probe failure - Add additionally comment for PPMU usage instead of previous PPC - Split separate patch to remove ambiguous of patch Chanwoo Choi (8): devfreq: exynos4: Support devicetree to get device id of Exynos4 SoC devfreq: exynos4: Use common ppmu driver and get ppmu address from dt data devfreq: exynos4: Add ppmu's clock control and code clean about regulator control devfreq: exynos4: Fix bug of resource leak and code clean on probe() devfreq: exynos4: Use SET_SYSTEM_SLEEP_PM_OPS macro devfreq: exynos4: Fix power-leakage of clock on suspend state devfreq: exynos4: Add CONFIG_PM_OPP dependency to fix probe fail devfreq: exynos4: Add busfreq driver for exynos4210/exynos4x12 .../devicetree/bindings/devfreq/exynos4_bus.txt| 49 +++ drivers/devfreq/Kconfig| 1 + drivers/devfreq/exynos/Makefile| 2 +- drivers/devfreq/exynos/exynos4_bus.c | 415 ++--- 4 files changed, 341 insertions(+), 126 deletions(-) create mode 100644 Documentation/devicetree/bindings/devfreq/exynos4_bus.txt I have reviewed this series and there are several comments that I'd like to ask you to address. Please see my replies to particular patches. OK, I'll fix it about your comment. However, this driver, even after applying your series, is still far from a state that would allow it to be enabled. The most important issue is direct access to CMU registers, based on static mapping, which is not allowed on multiplatform kernels and multiplatform-awareness for drivers is currently a must. To allow this driver to be enabled, it needs to be converted to use common clock framework functions to configure all clocks, e.g. clk_set_rate(), clk_set_parent(), etc., without accessing CMU registers directly. Of course as long as the driver is effectively unusable, to keep development, we can proceed with refactoring it step-by-step and your series would be basically the first step, after addressing the review comments. I agree your opinion. When setting frequency of memory bus, this driver access directly to CMU registers. I know it should be modified by using common clk framework as your comment. I'll send patch set about using common clk framework instead of CMU register based on static mapping after finished the review and apply of this patch set as next step. Thanks for your review. Best Regards, Chanwoo Choi -- 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: [PATCHv2 3/8] devfreq: exynos4: Add ppmu's clock control and code clean about regulator control
Hi Tomasz,
On 03/15/2014 02:42 AM, Tomasz Figa wrote:
Hi Chanwoo,
On 13.03.2014 09:17, Chanwoo Choi wrote:
There are not the clock controller of ppmudmc0/1. This patch control the
clock
of ppmudmc0/1 which is used for monitoring memory bus utilization.
Also, this patch code clean about regulator control and free resource
when calling exit/remove function.
For example,
busfreq@106A {
compatible = samsung,exynos4x12-busfreq;
/* Clock for PPMUDMC0/1 */
clocks = clock CLK_PPMUDMC0, clock CLK_PPMUDMC1;
clock-names = ppmudmc0, ppmudmc1;
/* Regulator for MIF/INT block */
vdd_mif-supply = buck1_reg;
vdd_int-supply = buck3_reg;
};
Signed-off-by: Chanwoo Choi cw00.c...@samsung.com
---
drivers/devfreq/exynos/exynos4_bus.c | 114
++-
1 file changed, 100 insertions(+), 14 deletions(-)
diff --git a/drivers/devfreq/exynos/exynos4_bus.c
b/drivers/devfreq/exynos/exynos4_bus.c
index 1a0effa..a2a3a47 100644
--- a/drivers/devfreq/exynos/exynos4_bus.c
+++ b/drivers/devfreq/exynos/exynos4_bus.c
@@ -62,6 +62,11 @@ enum exynos_ppmu_idx {
PPMU_END,
};
+static const char *exynos_ppmu_clk_name[] = {
+[PPMU_DMC0]= ppmudmc0,
+[PPMU_DMC1]= ppmudmc1,
+};
+
#define EX4210_LV_MAXLV_2
#define EX4x12_LV_MAXLV_4
#define EX4210_LV_NUM(LV_2 + 1)
@@ -86,6 +91,7 @@ struct busfreq_data {
struct regulator *vdd_mif; /* Exynos4412/4212 only */
struct busfreq_opp_info curr_oppinfo;
struct exynos_ppmu ppmu[PPMU_END];
+struct clk *clk_ppmu[PPMU_END];
struct notifier_block pm_notifier;
struct mutex lock;
@@ -722,8 +728,26 @@ static int exynos4_bus_get_dev_status(struct device
*dev,
static void exynos4_bus_exit(struct device *dev)
{
struct busfreq_data *data = dev_get_drvdata(dev);
+int i;
+
+/*
+ * Un-map memory map and disable regulator/clocks
+ * to prevent power leakage.
+ */
+regulator_disable(data-vdd_int);
+if (data-type == TYPE_BUSF_EXYNOS4x12)
+regulator_disable(data-vdd_mif);
+
+for (i = 0; i PPMU_END; i++) {
+if (data-clk_ppmu[i])
This check is invalid. Clock pointers must be checked for validity using the
IS_ERR() macro, because NULL is a valid clock pointer value indicating a
dummy clock.
OK, I'll check it by using the IS_ERR() macro as following:
if (IS_ERR(data-clk_ppmu[i]) {
+clk_disable_unprepare(data-clk_ppmu[i]);
+}
-devfreq_unregister_opp_notifier(dev, data-devfreq);
+for (i = 0; i PPMU_END; i++) {
+if (data-ppmu[i].hw_base)
Can this even happen? Is there a PPMU without registers?
+iounmap(data-ppmu[i].hw_base);
+
+}
}
static struct devfreq_dev_profile exynos4_devfreq_profile = {
@@ -987,6 +1011,7 @@ static int exynos4_busfreq_parse_dt(struct busfreq_data
*data)
{
struct device *dev = data-dev;
struct device_node *np = dev-of_node;
+const char **clk_name = exynos_ppmu_clk_name;
int i, ret;
if (!np) {
@@ -1005,8 +1030,70 @@ static int exynos4_busfreq_parse_dt(struct
busfreq_data *data)
}
}
+/*
+ * Get PPMU's clocks to control them. But, if PPMU's clocks
+ * is default 'pass' state, this driver don't need control
+ * PPMU's clock.
+ */
+for (i = 0; i PPMU_END; i++) {
+data-clk_ppmu[i] = devm_clk_get(dev, clk_name[i]);
+if (IS_ERR_OR_NULL(data-clk_ppmu[i])) {
Again, this check is invalid. Only IS_ERR() is the correct way to check
whether returned clock pointer is valid.
ditto.
if (IS_ERR(data-clk_ppmu[i]) {
+dev_warn(dev, Cannot get %s clock\n, clk_name[i]);
+data-clk_ppmu[i] = NULL;
This assignment is wrong. To allow further checking whether the clock was
found the value returned from devm_clk_get() must be retained and then
IS_ERR() used in further code.
However, I believe it should be an error if a clock is not provided. The
driver must make sure that PPMU clocks are ungated before trying to access
them, otherwise the system might hang.
OK, I'll use IS_ERR() macro when checking / handling clock instance of
'data-clk_ppmu[i]'.
And If this driver can't get the clock of ppmu, handel error exception.
+}
+
+ret = clk_prepare_enable(data-clk_ppmu[i]);
The code above allows the clock to be skipped, but this line doesn't check
whether it is valid. Still, I think the clock should be always required.
OK, I'll require clock of ppmu without exception.
+if (ret 0) {
+dev_warn(dev, Cannot enable %s clock\n, clk_name[i]);
+data-clk_ppmu[i] = NULL;
+goto err_clocks;
+}
+}
+
+/* Get regulator to control voltage of int block */
+data-vdd_int = devm_regulator_get(dev, vdd_int);
+if (IS_ERR(data-vdd_int)) {
+
Re: [PATCHv2 6/8] devfreq: exynos4: Fix power-leakage of clock on suspend state
Hi Tomasz, On 03/15/2014 02:52 AM, Tomasz Figa wrote: Hi Chanwoo, On 13.03.2014 09:17, Chanwoo Choi wrote: This patch disable ppmu clocks before entering suspend state to remove power-leakage and enable ppmu clocks on resume function. I don't think there is any need for this, because all the clocks are stopped anyway in SLEEP mode. OK, I'll discard this patch on next patchset. Best Regards, Chanwoo Choi -- 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] drm/exynos: Fix (more) freeing issues in exynos_drm_drv.c
The following commit [0] fixed a use-after-free, but left the subdrv open
in the error path.
[0] commit 6ca605f7c70895a35737435f17ae9cc5e36f1466
drm/exynos: Fix freeing issues in exynos_drm_drv.c
Change-Id: I452e944bf090fb11434d9e34213c890c41c15d73
Signed-off-by: Daniel Kurtz djku...@chromium.org
---
drivers/gpu/drm/exynos/exynos_drm_drv.c | 10 +++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c
b/drivers/gpu/drm/exynos/exynos_drm_drv.c
index 215131a..c204b4e 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
@@ -172,20 +172,24 @@ static int exynos_drm_open(struct drm_device *dev, struct
drm_file *file)
ret = exynos_drm_subdrv_open(dev, file);
if (ret)
- goto out;
+ goto err_file_priv_free;
anon_filp = anon_inode_getfile(exynos_gem, exynos_drm_gem_fops,
NULL, 0);
if (IS_ERR(anon_filp)) {
ret = PTR_ERR(anon_filp);
- goto out;
+ goto err_subdrv_close;
}
anon_filp-f_mode = FMODE_READ | FMODE_WRITE;
file_priv-anon_filp = anon_filp;
return ret;
-out:
+
+err_subdrv_close:
+ exynos_drm_subdrv_close(dev, file);
+
+err_file_priv_free:
kfree(file_priv);
file-driver_priv = NULL;
return ret;
--
1.9.0.279.gdc9e3eb
--
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] drm/exynos: Fix (more) freeing issues in exynos_drm_drv.c
The following commit [0] fixed a use-after-free, but left the subdrv open
in the error path.
[0] commit 6ca605f7c70895a35737435f17ae9cc5e36f1466
drm/exynos: Fix freeing issues in exynos_drm_drv.c
Signed-off-by: Daniel Kurtz djku...@chromium.org
---
Hi, I noticed this when reviewing some recent patches.
I am only able to compile test this patch.
drivers/gpu/drm/exynos/exynos_drm_drv.c | 10 +++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c
b/drivers/gpu/drm/exynos/exynos_drm_drv.c
index 215131a..c204b4e 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
@@ -172,20 +172,24 @@ static int exynos_drm_open(struct drm_device *dev, struct
drm_file *file)
ret = exynos_drm_subdrv_open(dev, file);
if (ret)
- goto out;
+ goto err_file_priv_free;
anon_filp = anon_inode_getfile(exynos_gem, exynos_drm_gem_fops,
NULL, 0);
if (IS_ERR(anon_filp)) {
ret = PTR_ERR(anon_filp);
- goto out;
+ goto err_subdrv_close;
}
anon_filp-f_mode = FMODE_READ | FMODE_WRITE;
file_priv-anon_filp = anon_filp;
return ret;
-out:
+
+err_subdrv_close:
+ exynos_drm_subdrv_close(dev, file);
+
+err_file_priv_free:
kfree(file_priv);
file-driver_priv = NULL;
return ret;
--
1.9.0.279.gdc9e3eb
--
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 RE-SEND 1/7] net: sxgbe: add basic framework for Samsung 10Gb ethernet driver
Francois Romieu rom...@fr.zoreil.com :
[...]
+struct sxgbe_core_ops {
+ /* MAC core initialization */
+ void (*core_init)(void __iomem *ioaddr);
[...]
+ /* adjust SXGBE speed */
+ void (*set_speed)(void __iomem *ioaddr, unsigned char
speed); };
This indirection level is never used.
Those are used, can you give more detail?
They are used but they always point to the same set of methods.
Those methods could thus be directly called.
Yes, those methods can be called directly.
But I think it is acceptable for manageability and extension for future.
[...]
+/* SXGBE private data structures */ struct sxgbe_tx_queue {
+ u8 queue_no;
+ unsigned int irq_no;
+ struct sxgbe_priv_data *priv_ptr;
+ struct sxgbe_tx_norm_desc *dma_tx;
You may lay things a bit differently.
can you give more detail?
Bigger fields first, u8 at the end. It will save padding in the struct.
OK.
--
Ueimor
--
To unsubscribe from this list: send the line unsubscribe netdev in the
body
of a message to majord...@vger.kernel.org More majordomo info at
http://vger.kernel.org/majordomo-info.html
--
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] drm/exynos: Fix (more) freeing issues in exynos_drm_drv.c
Hi Daniel, Thanks for the patch. On 17 March 2014 08:58, Daniel Kurtz djku...@chromium.org wrote: The following commit [0] fixed a use-after-free, but left the subdrv open in the error path. [0] commit 6ca605f7c70895a35737435f17ae9cc5e36f1466 drm/exynos: Fix freeing issues in exynos_drm_drv.c Signed-off-by: Daniel Kurtz djku...@chromium.org Acked-by: Sachin Kamat sachin.ka...@linaro.org -- With warm regards, Sachin -- 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: [PATCHv2 4/8] devfreq: exynos4: Fix bug of resource leak and code clean on probe()
Hi Tomasz,
On 03/15/2014 02:49 AM, Tomasz Figa wrote:
Hi Chanwoo,
On 13.03.2014 09:17, Chanwoo Choi wrote:
This patch fix bug about resource leak when happening probe fail and code
clean
to add debug message.
Signed-off-by: Chanwoo Choi cw00.c...@samsung.com
---
drivers/devfreq/exynos/exynos4_bus.c | 32 ++--
1 file changed, 26 insertions(+), 6 deletions(-)
diff --git a/drivers/devfreq/exynos/exynos4_bus.c
b/drivers/devfreq/exynos/exynos4_bus.c
index a2a3a47..152a3e9 100644
--- a/drivers/devfreq/exynos/exynos4_bus.c
+++ b/drivers/devfreq/exynos/exynos4_bus.c
@@ -1152,8 +1152,11 @@ static int exynos4_busfreq_probe(struct
platform_device *pdev)
dev_err(dev, Cannot determine the device id %d\n, data-type);
err = -EINVAL;
}
-if (err)
+if (err) {
+dev_err(dev, Cannot initialize busfreq table %d\n,
+ data-type);
return err;
+}
rcu_read_lock();
opp = dev_pm_opp_find_freq_floor(dev,
@@ -1176,7 +1179,7 @@ static int exynos4_busfreq_probe(struct
platform_device *pdev)
if (IS_ERR(data-devfreq)) {
dev_err(dev, Failed to add devfreq device\n);
err = PTR_ERR(data-devfreq);
-goto err_opp;
+goto err_devfreq;
}
/*
@@ -1185,18 +1188,35 @@ static int exynos4_busfreq_probe(struct
platform_device *pdev)
*/
busfreq_mon_reset(data);
-devfreq_register_opp_notifier(dev, data-devfreq);
+/* Register opp_notifier for Exynos4 busfreq */
+err = devfreq_register_opp_notifier(dev, data-devfreq);
+if (err 0) {
+dev_err(dev, Failed to register opp notifier\n);
+goto err_notifier_opp;
+}
+/* Register pm_notifier for Exynos4 busfreq */
err = register_pm_notifier(data-pm_notifier);
if (err) {
dev_err(dev, Failed to setup pm notifier\n);
-devfreq_remove_device(data-devfreq);
-return err;
+goto err_notifier_pm;
}
return 0;
-err_opp:
+err_notifier_pm:
+devfreq_unregister_opp_notifier(dev, data-devfreq);
+err_notifier_opp:
+/*
+ * The devfreq_remove_device() would execute finally devfreq-profile
+ * -exit(). To avoid duplicate resource free operation, return directly
+ * before executing resource free below 'err_devfreq' goto statement.
+ */
I'm not quite sure about this. I believe that in this case
devfreq-profile-exit() would be exynos4_bus_exit() and all it does is
devfreq_unregister_opp_notifier(dev, data-devfreq), so all remaining
resources (regulators, clocks, etc.) would get leaked.
This patch execute following sequence to probe exynos4_busfreq.c:
1. Parse dt node to get resource(regulator/clock/memory address).
2. Enable regulator/clock and map memory.
3. Add devfreq device using devfreq_add_device().
The devfreq_add_device() return devfreq instance(data-devfreq).
4. Register opp_notifier using devfreq instance(data-devfreq) which is created
in sequence #3.
Case 1,
If an error happens in sequence #3 for registering devfreq_add_device(),
this case can't execute devfreq-profile-exit() to free resource
because this case has failed to register devfreq-profile to devfreq_list.
So, must goto 'err_devfreq' statement to free resource(regulator/clock/memory).
Case 2,
If an error happens in sequence #4 for registering opp_notifier,
In contrast
this case can execute devfreq-profile-exit() to free resource.
But, After executed devfreq-profile-exit(),
should not execute 'err_devfreq' statement to free resource.
In case, will operate twice of resource.
If my explanation is wrong, please reply your comment.
I believe the correct thing to do would be to remove the .exit() callback
from exynos4_devfreq_profile struct and handle all the clean-up here in error
path.
Best Regards,
Chanwoo Choi
--
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: [PATCHv2 8/8] devfreq: exynos4: Add busfreq driver for exynos4210/exynos4x12
Hi Tomasz, On 03/15/2014 02:35 AM, Tomasz Figa wrote: Hi Chanwoo, Mark, On 14.03.2014 11:56, Chanwoo Choi wrote: Hi Mark, On 03/14/2014 07:35 PM, Mark Rutland wrote: On Fri, Mar 14, 2014 at 07:14:37AM +, Chanwoo Choi wrote: Hi Mark, On 03/14/2014 02:53 AM, Mark Rutland wrote: On Thu, Mar 13, 2014 at 08:17:29AM +, Chanwoo Choi wrote: This patch add busfreq driver for Exynos4210/Exynos4x12 memory interface and bus to support DVFS(Dynamic Voltage Frequency Scaling) according to PPMU counters. PPMU (Performance Profiling Monitorings Units) of Exynos4 SoC provides PPMU counters for DMC(Dynamic Memory Controller) to check memory bus utilization and then busfreq driver adjusts dynamically the operating frequency/voltage by using DEVFREQ Subsystem. Signed-off-by: Chanwoo Choi cw00.c...@samsung.com --- .../devicetree/bindings/devfreq/exynos4_bus.txt| 49 ++ 1 file changed, 49 insertions(+) create mode 100644 Documentation/devicetree/bindings/devfreq/exynos4_bus.txt diff --git a/Documentation/devicetree/bindings/devfreq/exynos4_bus.txt b/Documentation/devicetree/bindings/devfreq/exynos4_bus.txt new file mode 100644 index 000..2a83fcc --- /dev/null +++ b/Documentation/devicetree/bindings/devfreq/exynos4_bus.txt @@ -0,0 +1,49 @@ + +Exynos4210/4x12 busfreq driver +- + +Exynos4210/4x12 Soc busfreq driver with devfreq for Memory bus frequency/voltage +scaling according to PPMU counters of memory controllers + +Required properties: +- compatible: should contain Exynos4 SoC type as follwoing: + - samsung,exynos4x12-busfreq for Exynos4x12 + - samsung,exynos4210-busfreq for Exynos4210 Is there a device called busfreq? What device does this binding describe? I'll add detailed description of busfreq as following: busfreq(bus frequendcy) driver means that busfreq driver control dynamically memory bus frequency/voltage by checking memory bus utilization to optimize power-consumption. When checking memeory bus utilization, exynos4_busfreq driver would use PPMU(Performance Profiling Monitoring Units). This still sounds like a description of the _driver_, not the _device_. The binding should describe the hardware, now the high level abstraction that software is going to build atop of it. It sounds like this is a binding for the DMC PPMU? Is the PPMU a component of the DMC, or is it bolted on the side? PPMU(Performance Profiling Monitoring Unit) is to profile performance event of various IP on Exynos4. Each PPMU provide perforamnce event for each IP. We can check various PPMU as following: PPMU_3D PPMU_ACP PPMU_CAMIF PPMU_CPU PPMU_DMC0 PPMU_DMC1 PPMU_FSYS PPMU_IMAGE PPMU_LCD0 PPMU_LCD1 PPMU_MFC_L PPMU_MFC_R PPMU_TV PPMU_LEFT_BUS PPMU_RIGHT_BUS DMC (Dynamic Memory Controller) control the operation of DRAM in Exynos4 SoC. If we need to get memory bust utilization of DMC, we can get memory bus utilization from PPMU_DMC0/PPMU_DMC1. So, Exynos4's busfreq used two(PPMU_DMC0/PPMU_DMC1) among upper various PPMU list. Well, PPMUs and DMCs are separate hardware blocks found inside Exynos SoCs. Busfreq/devfreq is just a Linux-specific abstraction responsible for collecting data using PPMUs and controlling frequencies and voltages of appropriate power planes, vdd_int responsible for powering DMC0 and DMC1 blocks in this case. I knew already. I'm afraid that the binding you're proposing is unfortunately incorrect, because it represents the software abstraction, not the real hardware. What is exactly incorrect part of this patch? Instead, this should be separated into several independent bindings: - PPMU bindings to list all the PPMU instances present in the SoC and resources they need, - power plane bindings, which define a power plane in which multiple IP blocks might reside, can be monitored by one or more PPMU units and frequency and voltage of which can be configured according to determined performance level. Needed resources will be clocks and regulators to scale and probably also operating points. Then, exynos-busfreq driver should bind to such power planes, parse necessary data from DT (list of PPMUs and IP blocks, clocks, regulators and operating points) and register a devfreq entity. What is 'power plane'? I don't know 'power plane'. If you suggest 'power plane' concept and then merge this concept to mainline, After merged 'power plane' concept, I will apply 'power plane' concept to Exynos4's busfreq driver. I prefer to handle 'power plane' concept on other patchset for Exynos4's busfreq driver. Best Regards, Chanwoo Choi -- 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: [PATCHv2 3/8] devfreq: exynos4: Add ppmu's clock control and code clean about regulator control
Hi Tomasz,
On 03/17/2014 11:51 AM, Chanwoo Choi wrote:
Hi Tomasz,
On 03/15/2014 02:42 AM, Tomasz Figa wrote:
Hi Chanwoo,
On 13.03.2014 09:17, Chanwoo Choi wrote:
There are not the clock controller of ppmudmc0/1. This patch control the
clock
of ppmudmc0/1 which is used for monitoring memory bus utilization.
Also, this patch code clean about regulator control and free resource
when calling exit/remove function.
For example,
busfreq@106A {
compatible = samsung,exynos4x12-busfreq;
/* Clock for PPMUDMC0/1 */
clocks = clock CLK_PPMUDMC0, clock CLK_PPMUDMC1;
clock-names = ppmudmc0, ppmudmc1;
/* Regulator for MIF/INT block */
vdd_mif-supply = buck1_reg;
vdd_int-supply = buck3_reg;
};
Signed-off-by: Chanwoo Choi cw00.c...@samsung.com
---
drivers/devfreq/exynos/exynos4_bus.c | 114
++-
1 file changed, 100 insertions(+), 14 deletions(-)
diff --git a/drivers/devfreq/exynos/exynos4_bus.c
b/drivers/devfreq/exynos/exynos4_bus.c
index 1a0effa..a2a3a47 100644
--- a/drivers/devfreq/exynos/exynos4_bus.c
+++ b/drivers/devfreq/exynos/exynos4_bus.c
@@ -62,6 +62,11 @@ enum exynos_ppmu_idx {
PPMU_END,
};
+static const char *exynos_ppmu_clk_name[] = {
+[PPMU_DMC0]= ppmudmc0,
+[PPMU_DMC1]= ppmudmc1,
+};
+
#define EX4210_LV_MAXLV_2
#define EX4x12_LV_MAXLV_4
#define EX4210_LV_NUM(LV_2 + 1)
@@ -86,6 +91,7 @@ struct busfreq_data {
struct regulator *vdd_mif; /* Exynos4412/4212 only */
struct busfreq_opp_info curr_oppinfo;
struct exynos_ppmu ppmu[PPMU_END];
+struct clk *clk_ppmu[PPMU_END];
struct notifier_block pm_notifier;
struct mutex lock;
@@ -722,8 +728,26 @@ static int exynos4_bus_get_dev_status(struct device
*dev,
static void exynos4_bus_exit(struct device *dev)
{
struct busfreq_data *data = dev_get_drvdata(dev);
+int i;
+
+/*
+ * Un-map memory map and disable regulator/clocks
+ * to prevent power leakage.
+ */
+regulator_disable(data-vdd_int);
+if (data-type == TYPE_BUSF_EXYNOS4x12)
+regulator_disable(data-vdd_mif);
+
+for (i = 0; i PPMU_END; i++) {
+if (data-clk_ppmu[i])
This check is invalid. Clock pointers must be checked for validity using the
IS_ERR() macro, because NULL is a valid clock pointer value indicating a
dummy clock.
OK, I'll check it by using the IS_ERR() macro as following:
I'll modify it as following:
for (i = 0; i PPMU_END; i++) {
if (IS_ERR(data-clk_ppmu[i])
continue;
else
clk_unprepare_disable(data-clk_ppmu[i]);
}
if (IS_ERR(data-clk_ppmu[i]) {
+clk_disable_unprepare(data-clk_ppmu[i]);
+}
-devfreq_unregister_opp_notifier(dev, data-devfreq);
+for (i = 0; i PPMU_END; i++) {
+if (data-ppmu[i].hw_base)
Can this even happen? Is there a PPMU without registers?
OK, I'll always unmap the ppmu address.
+iounmap(data-ppmu[i].hw_base);
+
+}
}
static struct devfreq_dev_profile exynos4_devfreq_profile = {
@@ -987,6 +1011,7 @@ static int exynos4_busfreq_parse_dt(struct
busfreq_data *data)
{
struct device *dev = data-dev;
struct device_node *np = dev-of_node;
+const char **clk_name = exynos_ppmu_clk_name;
int i, ret;
if (!np) {
@@ -1005,8 +1030,70 @@ static int exynos4_busfreq_parse_dt(struct
busfreq_data *data)
}
}
+/*
+ * Get PPMU's clocks to control them. But, if PPMU's clocks
+ * is default 'pass' state, this driver don't need control
+ * PPMU's clock.
+ */
+for (i = 0; i PPMU_END; i++) {
+data-clk_ppmu[i] = devm_clk_get(dev, clk_name[i]);
+if (IS_ERR_OR_NULL(data-clk_ppmu[i])) {
Again, this check is invalid. Only IS_ERR() is the correct way to check
whether returned clock pointer is valid.
ditto.
if (IS_ERR(data-clk_ppmu[i]) {
+dev_warn(dev, Cannot get %s clock\n, clk_name[i]);
+data-clk_ppmu[i] = NULL;
This assignment is wrong. To allow further checking whether the clock was
found the value returned from devm_clk_get() must be retained and then
IS_ERR() used in further code.
However, I believe it should be an error if a clock is not provided. The
driver must make sure that PPMU clocks are ungated before trying to access
them, otherwise the system might hang.
OK, I'll use IS_ERR() macro when checking / handling clock instance of
'data-clk_ppmu[i]'.
And If this driver can't get the clock of ppmu, handel error exception.
+}
+
+ret = clk_prepare_enable(data-clk_ppmu[i]);
The code above allows the clock to be skipped, but this line doesn't check
whether it is valid. Still, I think the clock should be always required.
OK, I'll require clock of ppmu without
RE: [PATCH V2 1/7] net: sxgbe: add basic framework for Samsung 10Gb ethernet driver
Mark Rutland mark.rutl...@arm.com :
On Wed, Mar 12, 2014 at 01:28:00PM +, Byungho An wrote:
From: Siva Reddy siva.kal...@samsung.com
This patch adds support for Samsung 10Gb ethernet driver(sxgbe).
- sxgbe core initialization
- Tx and Rx support
- MDIO support
- ISRs for Tx and Rx
- ifconfig support to driver
Signed-off-by: Siva Reddy Kallam siva.kal...@samsung.com
Signed-off-by: Vipul Pandya vipul.pan...@samsung.com
Signed-off-by: Girish K S ks.g...@samsung.com
Neatening-by: Joe Perches j...@perches.com
Signed-off-by: Byungho An bh74...@samsung.com
---
[...]
diff --git a/Documentation/devicetree/bindings/net/samsung-sxgbe.txt
b/Documentation/devicetree/bindings/net/samsung-sxgbe.txt
new file mode 100644
index 000..f2abf65
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/samsung-sxgbe.txt
Please split the binding into a separate patch from the code (it makes it
far
easier for us DT guys to find that portions relevant to use).
Is there any public documentation?
No.
@@ -0,0 +1,39 @@
+* Samsung 10G Ethernet driver (SXGBE)
+
+Required properties:
+- compatible: Should be samsung,sxgbe-v2.0a
+- reg: Address and length of the register set for the device
+- interrupt-parent: Should be the phandle for the interrupt
+controller
+ that services interrupts for this device
+- interrupts: Should contain the SXGBE interrupts
How many, in which order, what are each of them for?
total = 26, first mandatory interrupt common to all, followed by 8 tx
interrupt (which can vary based on platform), 16 rx interrupts (which can
vary based on platform) and a optional lpi interrupt
+- samsung,burst-mapProgram the possible bursts supported by
sxgbe
+- samsung,fixed-burst Program the DMA to use the fixed burst mode
+- samsung,adv-addr-modeprogram the DMA to use Enhanced address
mode
What are valid values?
Units/types?
Please describe what these actually do.
These will be updated in the binding document as per your review.
+- samsung,force_thresh_dma_modeForce DMA to use the threshold
mode
for
+ both tx and rx
Odd formatting here.
s/_/-/ in property names please.
When and why should this property be set in a dt?
Addressed in the new document
+- samsung,force_sf_dma_modeForce DMA to use the Store and
Forward
+ mode for both tx and rx. This flag is
+ ignored if force_thresh_dma_mode is set.
Likewise, for all points.
OK.
[...]
+/* Clean the tx descriptor as soon as the tx irq is received */
+static void sxgbe_release_tx_desc(struct sxgbe_tx_norm_desc *p) {
+ memset(p, 0, sizeof(struct sxgbe_tx_norm_desc));
You can use sizeof(*p) here.
OK.
[...]
+static int sxgbe_probe_config_dt(struct platform_device *pdev,
+struct sxgbe_plat_data *plat,
+const char **mac) {
+ struct device_node *np = pdev-dev.of_node;
+ struct sxgbe_dma_cfg *dma_cfg;
+ u32 phy_addr;
+
+ if (!np)
+ return -ENODEV;
+
+ *mac = of_get_mac_address(np);
I see that of_get_mac_address returns a *void rather than *char, but it's
always a string of hex digits. Would it make sense to change the
of_get_mac_address prototype to return a char* ?
This implementation is of_ specific, our driver only uses it.
+ plat-interface = of_get_phy_mode(np);
+
+ plat-bus_id = of_alias_get_id(np, ethernet);
+ if (plat-bus_id 0)
+ plat-bus_id = 0;
This wasn't mentioned in the binding.
It doesn't need to be in the binding document, because we get the alias id
by passing the node name. Here ethernet is not a property it is the dt node
name
e.g.
ethernet@x {
};
+
+ of_property_read_u32(np, samsung,phy-addr, plat-phy_addr);
Neither was this.
OK.
+
+ plat-mdio_bus_data = devm_kzalloc(pdev-dev,
+ sizeof(struct
sxgbe_mdio_bus_data),
+ GFP_KERNEL);
+
+ if (of_device_is_compatible(np, samsung,sxgbe-v2.0a))
+ plat-pmt = 1;
Only one compatible string is documented. When would this _not_ be the
case?
Removed as it is unused
[...]
+static int sxgbe_platform_probe(struct platform_device *pdev) {
+ int ret = 0;
+ int loop = 0;
+ int index1, index2;
+ struct resource *res;
+ struct device *dev = pdev-dev;
+ void __iomem *addr = NULL;
+ struct sxgbe_priv_data *priv = NULL;
+ struct sxgbe_plat_data *plat_dat = NULL;
+ const char *mac = NULL;
+ int total_dma_channels = SXGBE_TX_QUEUES + SXGBE_RX_QUEUES;
+
+ /* Get memory resource */
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
