Re: [PATCH 3/3] DMA: PL330: Balance module remove function with probe
On 25 September 2012 18:47, Jassi Brar jassisinghb...@gmail.com wrote: On Tue, Sep 25, 2012 at 2:27 PM, Inderpal Singh inderpal.si...@linaro.org wrote: Since peripheral channel resources are not being allocated at probe, no need to flush the channels and free the resources in remove function. Signed-off-by: Inderpal Singh inderpal.si...@linaro.org --- drivers/dma/pl330.c |8 +--- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c index 04d83e6..6f06080 100644 --- a/drivers/dma/pl330.c +++ b/drivers/dma/pl330.c @@ -3012,16 +3012,10 @@ static int __devexit pl330_remove(struct amba_device *adev) /* Idle the DMAC */ list_for_each_entry_safe(pch, _p, pdmac-ddma.channels, - chan.device_node) { - + chan.device_node) /* Remove the channel */ list_del(pch-chan.device_node); - /* Flush the channel */ - pl330_control(pch-chan, DMA_TERMINATE_ALL, 0); - pl330_free_chan_resources(pch-chan); - } - while (!list_empty(pdmac-desc_pool)) { desc = list_entry(pdmac-desc_pool.next, struct dma_pl330_desc, node); I am not sure about this patch. The DMA_TERMINATE_ALL is only called by the client and if the pl330 module is forced unloaded while some client is queued, we have to manually do DMA_TERMINATE_ALL. A better option could be to simply fail pl330_remove if some client is queued on the DMAC. If we fail pl330_remove while some client is queued, the force unload will fail and the force unload will lose its purpose. How about conditionally DMA_TERMINATE_ALL and free resources like below ? @@ -3017,9 +3017,11 @@ static int __devexit pl330_remove(struct amba_device *adev) /* Remove the channel */ list_del(pch-chan.device_node); - /* Flush the channel */ - pl330_control(pch-chan, DMA_TERMINATE_ALL, 0); - pl330_free_chan_resources(pch-chan); + if (pch-chan.client_count != 0) { + /* Flush the channel */ + pl330_control(pch-chan, DMA_TERMINATE_ALL, 0); + pl330_free_chan_resources(pch-chan); + } } while (!list_empty(pdmac-desc_pool)) { Thanks, Inder -jassi -- 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] ARM: EXYNOS: Add MFC device tree support
Hi Arun, A little nitpick inline. On Saturday 22 of September 2012 23:37:17 Arun Kumar K wrote: This patch adds device tree entry for MFC v6 in the Exynos5 SoC. Makes the required changes in the clock files and adds MFC to the DT device list. Signed-off-by: Naveen Krishna Chatradhi ch.nav...@samsung.com Signed-off-by: Arun Kumar K arun...@samsung.com --- .../devicetree/bindings/media/s5p-mfc.txt | 27 ++ arch/arm/boot/dts/exynos5250-smdk5250.dts |7 arch/arm/boot/dts/exynos5250.dtsi |6 +++ arch/arm/mach-exynos/Kconfig |1 + arch/arm/mach-exynos/clock-exynos5.c |2 +- arch/arm/mach-exynos/common.c | 37 arch/arm/mach-exynos/common.h | 10 + arch/arm/mach-exynos/mach-exynos5-dt.c | 16 8 files changed, 105 insertions(+), 1 deletions(-) create mode 100644 Documentation/devicetree/bindings/media/s5p-mfc.txt diff --git a/Documentation/devicetree/bindings/media/s5p-mfc.txt b/Documentation/devicetree/bindings/media/s5p-mfc.txt new file mode 100644 index 000..f95e775 --- /dev/null +++ b/Documentation/devicetree/bindings/media/s5p-mfc.txt @@ -0,0 +1,27 @@ +* Samsung Multi Format Codec (MFC) + +Multi Format Codec (MFC) is the IP present in Samsung SoCs which +supports high resolution decoding and encoding functionalities. +The MFC device driver is a v4l2 driver which can encode/decode +video raw/elementary streams and has support for all popular +video codecs. + +Required properties: + - compatible : value should be either one among the following + (a) samsung,mfc-v5 for MFC v5 present in Exynos4 SoCs + (b) samsung,mfc-v6 for MFC v6 present in Exynos5 SoCs + + - reg : Physical base address of the IP registers and length of memory + mapped region. + + - interrupts : MFC interrupt number to the CPU. + + - samsung,mfc-r : Base address of the first memory bank used by MFC + for DMA contiguous memory allocation. + + - samsung,mfc-r-size : Size of the first memory bank. + + - samsung,mfc-l : Base address of the second memory bank used by MFC + for DMA contiguous memory allocation. + + - samsung,mfc-l-size : Size of the second memory bank. Maybe the base address and size could be merged into one property with two values, as it is done with the reg property? What do you think? diff --git a/arch/arm/boot/dts/exynos5250-smdk5250.dts b/arch/arm/boot/dts/exynos5250-smdk5250.dts index 8a5e348..99890ec 100644 --- a/arch/arm/boot/dts/exynos5250-smdk5250.dts +++ b/arch/arm/boot/dts/exynos5250-smdk5250.dts @@ -109,4 +109,11 @@ spi_2: spi@12d4 { status = disabled; }; + + codec@1100 { + samsung,mfc-r = 0x4300; + samsung,mfc-r-size = 8388608; + samsung,mfc-l = 0x5100; + samsung,mfc-l-size = 8388608; It would look like this: samsung-mfc-r = 0x4300 0x80; samsung-mfc-l = 0x5100 0x80; Best regards, Tomasz Figa -- 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 v6 1/5] usb: phy: samsung: Introducing usb phy driver for hsotg
Hi, On Tue, Sep 25, 2012 at 6:47 PM, ABRAHAM, KISHON VIJAY kis...@ti.com wrote: Hi, On Tue, Sep 25, 2012 at 5:48 PM, Rob Herring robherri...@gmail.com wrote: On 09/25/2012 06:23 AM, Praveen Paneri wrote: Hi Rob, On Mon, Sep 24, 2012 at 6:34 PM, Rob Herring robherri...@gmail.com wrote: On 09/17/2012 07:54 AM, Praveen Paneri wrote: This driver uses usb_phy interface to interact with s3c-hsotg. Supports phy_init and phy_shutdown functions to enable/disable phy. Tested with smdk6410 and smdkv310. More SoCs can be brought under later. Signed-off-by: Praveen Paneri p.pan...@samsung.com Acked-by: Heiko Stuebner he...@sntech.de --- .../devicetree/bindings/usb/samsung-usbphy.txt |9 + drivers/usb/phy/Kconfig|8 + drivers/usb/phy/Makefile |1 + drivers/usb/phy/samsung-usbphy.c | 360 include/linux/platform_data/samsung-usbphy.h | 27 ++ 5 files changed, 405 insertions(+), 0 deletions(-) create mode 100644 Documentation/devicetree/bindings/usb/samsung-usbphy.txt create mode 100644 drivers/usb/phy/samsung-usbphy.c create mode 100644 include/linux/platform_data/samsung-usbphy.h diff --git a/Documentation/devicetree/bindings/usb/samsung-usbphy.txt b/Documentation/devicetree/bindings/usb/samsung-usbphy.txt new file mode 100644 index 000..fefd9c8 --- /dev/null +++ b/Documentation/devicetree/bindings/usb/samsung-usbphy.txt @@ -0,0 +1,9 @@ +* Samsung's usb phy transceiver + +The Samsung's phy transceiver is used for controlling usb otg phy for +s3c-hsotg usb device controller. + +Required properties: +- compatible : should be samsung,exynos4210-usbphy +- reg : base physical address of the phy registers and length of memory mapped + region. What's missing here is describing the connection of phys to host controllers. We've got several people adding usb phy bindings and need to define them in a common way. yes! it just covers the generic binding. I will update it accordingly as the generic phy framework takes its final shape. That sounds like the wrong way to define a binding... Figuring out how to describe the h/w should not be dependent on changes in the kernel. Bindings are an ABI and should not be evolving. But since Kishon is getting the generic bindings ready, I can use them when those are ready. So do we need to hold the merge of this patch until then? Thanks, Praveen There can be multiple ways to define the binding. For e.g. We discussed few ways of binding the phys to the controller controller { phy0 = phandle1_name; phy1 = phandle2_name; } phy0 and phy1 are any name given to obtain a reference to the phy and the controller should send the phandle name like get_phy_by_phandle(phy0);. Then we thought of standardizing that name. and then finally we settled on something like this controller { phy = phandle0_name, phandle1_name; } so that controller can obtain a reference to the PHY using *of_phy_get(struct device *dev, const char *phandle, u8 index)* Thanks Kishon -- 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 -- 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 v6 1/5] usb: phy: samsung: Introducing usb phy driver for hsotg
On Tue, Sep 25, 2012 at 4:59 PM, Marc Kleine-Budde m...@pengutronix.de wrote: On 09/24/2012 11:38 AM, Praveen Paneri wrote: Hi Kishon, Felipe, Any further comments on these patches? Can they be merged now? One nitpick inline. Marc Thanks, Praveen On Mon, Sep 17, 2012 at 6:24 PM, Praveen Paneri p.pan...@samsung.com wrote: This driver uses usb_phy interface to interact with s3c-hsotg. Supports phy_init and phy_shutdown functions to enable/disable phy. Tested with smdk6410 and smdkv310. More SoCs can be brought under later. Signed-off-by: Praveen Paneri p.pan...@samsung.com Acked-by: Heiko Stuebner he...@sntech.de --- .../devicetree/bindings/usb/samsung-usbphy.txt |9 + drivers/usb/phy/Kconfig|8 + drivers/usb/phy/Makefile |1 + drivers/usb/phy/samsung-usbphy.c | 360 include/linux/platform_data/samsung-usbphy.h | 27 ++ 5 files changed, 405 insertions(+), 0 deletions(-) create mode 100644 Documentation/devicetree/bindings/usb/samsung-usbphy.txt create mode 100644 drivers/usb/phy/samsung-usbphy.c create mode 100644 include/linux/platform_data/samsung-usbphy.h diff --git a/Documentation/devicetree/bindings/usb/samsung-usbphy.txt b/Documentation/devicetree/bindings/usb/samsung-usbphy.txt new file mode 100644 index 000..fefd9c8 --- /dev/null +++ b/Documentation/devicetree/bindings/usb/samsung-usbphy.txt @@ -0,0 +1,9 @@ +* Samsung's usb phy transceiver + +The Samsung's phy transceiver is used for controlling usb otg phy for +s3c-hsotg usb device controller. + +Required properties: +- compatible : should be samsung,exynos4210-usbphy +- reg : base physical address of the phy registers and length of memory mapped + region. diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig index 63c339b..313685f 100644 --- a/drivers/usb/phy/Kconfig +++ b/drivers/usb/phy/Kconfig @@ -32,3 +32,11 @@ config MV_U3D_PHY help Enable this to support Marvell USB 3.0 phy controller for Marvell SoC. + +config SAMSUNG_USBPHY + bool Samsung USB PHY controller Driver + depends on USB_S3C_HSOTG + select USB_OTG_UTILS + help + Enable this to support Samsung USB phy controller for samsung + SoCs. diff --git a/drivers/usb/phy/Makefile b/drivers/usb/phy/Makefile index b069f29..55dcfc1 100644 --- a/drivers/usb/phy/Makefile +++ b/drivers/usb/phy/Makefile @@ -8,3 +8,4 @@ obj-$(CONFIG_OMAP_USB2) += omap-usb2.o obj-$(CONFIG_USB_ISP1301) += isp1301.o obj-$(CONFIG_MV_U3D_PHY) += mv_u3d_phy.o obj-$(CONFIG_USB_EHCI_TEGRA) += tegra_usb_phy.o +obj-$(CONFIG_SAMSUNG_USBPHY) += samsung-usbphy.o diff --git a/drivers/usb/phy/samsung-usbphy.c b/drivers/usb/phy/samsung-usbphy.c new file mode 100644 index 000..95ec4d0 --- /dev/null +++ b/drivers/usb/phy/samsung-usbphy.c @@ -0,0 +1,360 @@ +/* linux/drivers/usb/phy/samsung-usbphy.c + * + * Copyright (c) 2012 Samsung Electronics Co., Ltd. + * http://www.samsung.com + * + * Author: Praveen Paneri p.pan...@samsung.com + * + * Samsung USB2.0 High-speed OTG transceiver, talks to S3C HS OTG controller + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include linux/module.h +#include linux/platform_device.h +#include linux/clk.h +#include linux/delay.h +#include linux/err.h +#include linux/io.h +#include linux/of.h +#include linux/usb/otg.h +#include linux/platform_data/samsung-usbphy.h + +/* Register definitions */ + +#define S3C_PHYPWR (0x00) + +#define S3C_PHYPWR_NORMAL_MASK (0x19 0) +#define S3C_PHYPWR_OTG_DISABLE (1 4) +#define S3C_PHYPWR_ANALOG_POWERDOWN(1 3) +#define S3C_PHYPWR_FORCE_SUSPEND (1 1) +/* For Exynos4 */ +#define EXYNOS4_PHYPWR_NORMAL_MASK (0x39 0) +#define EXYNOS4_PHYPWR_SLEEP (1 5) + +#define S3C_PHYCLK (0x04) + +#define S3C_PHYCLK_MODE_SERIAL (1 6) +#define S3C_PHYCLK_EXT_OSC (1 5) +#define S3C_PHYCLK_COMMON_ON_N (1 4) +#define S3C_PHYCLK_ID_PULL (1 2) +#define S3C_PHYCLK_CLKSEL_MASK (0x3 0) +#define S3C_PHYCLK_CLKSEL_SHIFT(0) +#define S3C_PHYCLK_CLKSEL_48M (0x0 0) +#define S3C_PHYCLK_CLKSEL_12M
Re: [PATCH 3/3] DMA: PL330: Balance module remove function with probe
On Wed, Sep 26, 2012 at 12:11 PM, Inderpal Singh inderpal.si...@linaro.org wrote: How about conditionally DMA_TERMINATE_ALL and free resources like below ? @@ -3017,9 +3017,11 @@ static int __devexit pl330_remove(struct amba_device *adev) /* Remove the channel */ list_del(pch-chan.device_node); - /* Flush the channel */ - pl330_control(pch-chan, DMA_TERMINATE_ALL, 0); - pl330_free_chan_resources(pch-chan); + if (pch-chan.client_count != 0) { + /* Flush the channel */ + pl330_control(pch-chan, DMA_TERMINATE_ALL, 0); + pl330_free_chan_resources(pch-chan); + } } It is perfectly safe to flush the channels that have client_count == 0 as well. Which is already the case. -- 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 3/3] DMA: PL330: Balance module remove function with probe
On 26 September 2012 15:02, Jassi Brar jassisinghb...@gmail.com wrote: On Wed, Sep 26, 2012 at 12:11 PM, Inderpal Singh inderpal.si...@linaro.org wrote: How about conditionally DMA_TERMINATE_ALL and free resources like below ? @@ -3017,9 +3017,11 @@ static int __devexit pl330_remove(struct amba_device *adev) /* Remove the channel */ list_del(pch-chan.device_node); - /* Flush the channel */ - pl330_control(pch-chan, DMA_TERMINATE_ALL, 0); - pl330_free_chan_resources(pch-chan); + if (pch-chan.client_count != 0) { + /* Flush the channel */ + pl330_control(pch-chan, DMA_TERMINATE_ALL, 0); + pl330_free_chan_resources(pch-chan); + } } It is perfectly safe to flush the channels that have client_count == 0 as well. Which is already the case. As per my understanding, if client_count ==0, It may mean following: 1. This channel was never allocated to any client. Hence alloc_chan_resources was not done for it. So, no need to flush and free resources if it was never allocated at the first place. If we free_chan_resources, it will not be balanced against alloc_chan_resources. Scenario: insmod and then rmmod. Or, 2. The channel was allocated to some client, alloc_chan_resource was done, the work for the client is completed and free_chan_resources was performed. Now since resources have already been freed, no need to flush and free_chan_resources again in remove. The whole idea of this patch is to balance alloc_chan_resources and free_chan_resources. Thanks, Inder -- 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] ARM: EXYNOS: Add MFC device tree support
Hi Kukjin, As suggested by you, the function exynos_fdt_find_mfc_mem() can be moved to plat-samsung. I feel plat-samsung/s5p-dev-mfc.c would be a good place. Hope it is ok? Also I will wrap it with #ifdef CONFIG_OF for non-dt mode. I will post the updated patch with the suggested changes. Regards Arun On Wed, Sep 26, 2012 at 7:42 AM, Kukjin Kim kgene@samsung.com wrote: Karol Lewandowski wrote: On 09/22/2012 08:07 PM, Arun Kumar K wrote: This patch adds device tree entry for MFC v6 in the Exynos5 SoC. Makes the required changes in the clock files and adds MFC to the DT device list. Signed-off-by: Naveen Krishna Chatradhi ch.nav...@samsung.com Signed-off-by: Arun Kumar K arun...@samsung.com Looks good for me. FWIW, I could probably add Acked-by: Karol Lewandowski k.lewando...@samsung.com Happens following build error with exynos4_defconfig because of non-DT ? arch/arm/mach-exynos/common.c: In function 'exynos_fdt_find_mfc_mem': arch/arm/mach-exynos/common.c:1058: error: implicit declaration of function 'of_flat_dt_is_compatible' arch/arm/mach-exynos/common.c:1061: error: implicit declaration of function 'of_get_flat_dt_prop' arch/arm/mach-exynos/common.c:1061: warning: assignment makes pointer from integer without a cast arch/arm/mach-exynos/common.c:1064: error: implicit declaration of function 'of_read_ulong' arch/arm/mach-exynos/common.c:1066: warning: assignment makes pointer from integer without a cast arch/arm/mach-exynos/common.c:1071: warning: assignment makes pointer from integer without a cast arch/arm/mach-exynos/common.c:1076: warning: assignment makes pointer from integer without a cast make[1]: *** [arch/arm/mach-exynos/common.o] Error 1 And I think, firstly we could move the function exynos_fdt_find_mfc_mem() into plat-samsung/ for using on other Samsung stuff such as s5pv210 and need to enclose '#ifdef CONFIG_OF for non-DT? Thanks. Best regards, Kgene. -- Kukjin Kim kgene@samsung.com, Senior Engineer, SW Solution Development Team, Samsung Electronics Co., Ltd. -- 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.htmlN‹§²æìr¸›yúèšØb²X¬¶Ç§vØ^–)Þº{.nÇ+‰·¥Š{±±©¬ºx,¡È§¶›¡Ü¨}©ž²Æ zÚj:+v‰¨¾«‘êçzZ+€Ê+zf£¢·hšˆ§~††Ûiÿûàz¹®w¥¢¸?™¨èÚ¢)ߢf
Re: [PATCH v4] ARM: EXYNOS: Add MFC device tree support
Hi Tomasz, Maybe the base address and size could be merged into one property with two values, as it is done with the reg property? What do you think? + codec@1100 { + samsung,mfc-r = 0x4300; + samsung,mfc-r-size = 8388608; + samsung,mfc-l = 0x5100; + samsung,mfc-l-size = 8388608; It would look like this: samsung-mfc-r = 0x4300 0x80; samsung-mfc-l = 0x5100 0x80; Thanks for the suggestion. It sure looks better this way. I will include this change and post updated patch. Regards ArunN‹§²æìr¸›yúèšØb²X¬¶Ç§vØ^–)Þº{.nÇ+‰·¥Š{±±©¬ºx,¡È§¶›¡Ü¨}©ž²Æ zÚj:+v‰¨¾«‘êçzZ+€Ê+zf£¢·hšˆ§~††Ûiÿûàz¹®w¥¢¸?™¨èÚ¢)ߢf
RE: [GIT PULL 4/4] Samsung DT for v3.7
Kukjin Kim wrote: On 09/21/12 15:26, Kukjin Kim wrote: Hi Arnd, Olof This is for updating Samsung DT for v3.7. But now this makes many conflicts with pinctrl dt because of moving file so I'm sorting them out to avoid useless conflicts. I'll re-send this soon again. This is jut for your information. I created following branch which is merged on top of Samsung pinctrl and board branches already are pulled into arm-soc. This includes moving common exynos4 dt for supporting other exynos4 SoCs easily and adding exynos43210-trats board support and some updates. Please pull from: git://git.kernel.org/pub/scm/linux/kernel/git/kgene/linux-samsung.git next/dt-samsung-new If any problems, please kindly let me know. Hi Arnd and Olof, Maybe, you missed this one? Note, this will make conflicts with Rob's change, move all dtb targets. Thanks. Best regards, Kgene. -- Kukjin Kim kgene@samsung.com, Senior Engineer, SW Solution Development Team, Samsung Electronics Co., Ltd. The following changes since commit 5698bd757d55b1bb87edd1a9744ab09c142abfc2: Linux 3.6-rc6 (2012-09-16 14:58:51 -0700) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/kgene/linux-samsung.git next/dt-samsung Kukjin Kim (1): ARM: dts: use uart2 for console on smdkv310 and smdk5250 Tomasz Figa (6): ARM: dts: Move parts common to EXYNOS4 from exynos4210.dtsi to exynos4.dtsi ARM: EXYNOS: Use exynos4 prefix instead of exynos4210 on exynos4-dt ARM: dts: Assume status of all optional nodes as disabled for exynos4 ARM: dts: Specify address and size cells for i2c controllers for EXYNOS4 ARM: EXYNOS: Add OF compatibility lookups for EXYNOS4 i2c adapters ARM: dts: Add basic dts file for Samsung Trats board Tushar Behera (1): ARM: dts: Enable on-board keys as wakeup source for exynos4210-origen arch/arm/boot/dts/exynos4.dtsi | 248 + arch/arm/boot/dts/exynos4210-origen.dts | 63 +--- arch/arm/boot/dts/exynos4210-smdkv310.dts | 54 +-- arch/arm/boot/dts/exynos4210-trats.dts | 237 +++ arch/arm/boot/dts/exynos4210.dtsi | 181 +- arch/arm/boot/dts/exynos5250-smdk5250.dts | 2 +- arch/arm/mach-exynos/Makefile.boot | 2 +- arch/arm/mach-exynos/mach-exynos4-dt.c | 32 +++- 8 files changed, 523 insertions(+), 296 deletions(-) create mode 100644 arch/arm/boot/dts/exynos4.dtsi create mode 100644 arch/arm/boot/dts/exynos4210-trats.dts -- 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 1/2] ARM: SAMSUNG: Insert bitmap_gpio_int member in samsung_gpio_chip
When a device uses GPIO interrupt, its driver assumes that GPIO should be INPUT mode. However, GPIO of SAMSUNG SoC is sepated to INPUT mode and INTERRUPT mode. They are set by 0x0 and 0xF in GPIO control register. If the register is set to INPUT mode, the interrupt never occur. Therefore, it's necessary to set INTERRUPT mode instead of INPUT mode when the pin is used for GPIO interrupt. This patch inserts the bitmap_gpio_int member in struct samsung_ gpio_chip in order to represent use of GPIO interrupt for each pin and sets the related bit when s5p_register_gpio_interrupt function is called. Signed-off-by: Eunki Kim eunki_...@samsung.com Cc: Grant Likely grant.lik...@secretlab.ca Cc: Linus Walleij linus.wall...@linaro.org --- arch/arm/plat-samsung/include/plat/gpio-core.h |2 ++ arch/arm/plat-samsung/s5p-irq-gpioint.c|8 ++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/arch/arm/plat-samsung/include/plat/gpio-core.h b/arch/arm/plat-samsung/include/plat/gpio-core.h index 1fe6917..dfd8b7a 100644 --- a/arch/arm/plat-samsung/include/plat/gpio-core.h +++ b/arch/arm/plat-samsung/include/plat/gpio-core.h @@ -48,6 +48,7 @@ struct samsung_gpio_cfg; * @config: special function and pull-resistor control information. * @lock: Lock for exclusive access to this gpio bank. * @pm_save: Save information for suspend/resume support. + * @bitmap_gpio_int: Bitmap for representing GPIO interrupt or not. * * This wrapper provides the necessary information for the Samsung * specific gpios being registered with gpiolib. @@ -71,6 +72,7 @@ struct samsung_gpio_chip { #ifdef CONFIG_PM u32 pm_save[4]; #endif + u32 bitmap_gpio_int; }; static inline struct samsung_gpio_chip *to_samsung_gpio(struct gpio_chip *gpc) diff --git a/arch/arm/plat-samsung/s5p-irq-gpioint.c b/arch/arm/plat-samsung/s5p-irq-gpioint.c index f9431fe..d981c61 100644 --- a/arch/arm/plat-samsung/s5p-irq-gpioint.c +++ b/arch/arm/plat-samsung/s5p-irq-gpioint.c @@ -185,7 +185,7 @@ int __init s5p_register_gpio_interrupt(int pin) /* check if the group has been already registered */ if (my_chip-irq_base) - return my_chip-irq_base + offset; + goto success; /* register gpio group */ ret = s5p_gpioint_add(my_chip); @@ -193,9 +193,13 @@ int __init s5p_register_gpio_interrupt(int pin) my_chip-chip.to_irq = samsung_gpiolib_to_irq; printk(KERN_INFO Registered interrupt support for gpio group %d.\n, group); - return my_chip-irq_base + offset; + goto success; } return ret; +success: + my_chip-bitmap_gpio_int |= BIT(offset); + + return my_chip-irq_base + offset; } int __init s5p_register_gpioint_bank(int chain_irq, int start, int nr_groups) -- 1.7.1 -- 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 2/2] gpio: samsung: Fix input mode setting function for GPIO int
This patch makes GPIO pin to INTERRUPT mode or INPUT mode according to bitmap_gpio_int. When the related bit of bitmap_gpio_int is set, it makes GPIO pin to INTERRUPT mode instrad of INPUT mode in the samsung_gpiolib_4bit_input function. Signed-off-by: Eunki Kim eunki_...@samsung.com Cc: Kukjin Kim kgene@samsung.com --- drivers/gpio/gpio-samsung.c |7 ++- 1 files changed, 6 insertions(+), 1 deletions(-) diff --git a/drivers/gpio/gpio-samsung.c b/drivers/gpio/gpio-samsung.c index ba126cc..19caef4 100644 --- a/drivers/gpio/gpio-samsung.c +++ b/drivers/gpio/gpio-samsung.c @@ -596,7 +596,10 @@ static int samsung_gpiolib_4bit_input(struct gpio_chip *chip, unsigned long con; con = __raw_readl(base + GPIOCON_OFF); - con = ~(0xf con_4bit_shift(offset)); + if (ourchip-bitmap_gpio_int BIT(offset)) + con |= 0xf con_4bit_shift(offset); + else + con = ~(0xf con_4bit_shift(offset)); __raw_writel(con, base + GPIOCON_OFF); gpio_dbg(%s: %p: CON now %08lx\n, __func__, base, con); @@ -1018,6 +1021,8 @@ static void __init samsung_gpiolib_add_4bit_chips(struct samsung_gpio_chip *chip if ((base != NULL) (chip-base == NULL)) chip-base = base + ((i) * 0x20); + chip-bitmap_gpio_int = 0; + samsung_gpiolib_add(chip); } } -- 1.7.1 -- 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 3/3] DMA: PL330: Balance module remove function with probe
On 26 September 2012 22:19, Jassi Brar jassisinghb...@gmail.com wrote: On Wed, Sep 26, 2012 at 4:25 PM, Inderpal Singh inderpal.si...@linaro.org wrote: On 26 September 2012 15:02, Jassi Brar jassisinghb...@gmail.com wrote: On Wed, Sep 26, 2012 at 12:11 PM, Inderpal Singh inderpal.si...@linaro.org wrote: How about conditionally DMA_TERMINATE_ALL and free resources like below ? @@ -3017,9 +3017,11 @@ static int __devexit pl330_remove(struct amba_device *adev) /* Remove the channel */ list_del(pch-chan.device_node); - /* Flush the channel */ - pl330_control(pch-chan, DMA_TERMINATE_ALL, 0); - pl330_free_chan_resources(pch-chan); + if (pch-chan.client_count != 0) { + /* Flush the channel */ + pl330_control(pch-chan, DMA_TERMINATE_ALL, 0); + pl330_free_chan_resources(pch-chan); + } } It is perfectly safe to flush the channels that have client_count == 0 as well. Which is already the case. As per my understanding, if client_count ==0, It may mean following: 1. This channel was never allocated to any client. Hence alloc_chan_resources was not done for it. So, no need to flush and free resources if it was never allocated at the first place. If we free_chan_resources, it will not be balanced against alloc_chan_resources. Scenario: insmod and then rmmod. Or, 2. The channel was allocated to some client, alloc_chan_resource was done, the work for the client is completed and free_chan_resources was performed. Now since resources have already been freed, no need to flush and free_chan_resources again in remove. The whole idea of this patch is to balance alloc_chan_resources and free_chan_resources. Do you see any issue with channels that have zero client_count ? As I explained in my previous mail, If the client_count is zero, either the alloc_chan_resources is not done(or failed) for that channel or the free_chan_resource have already been done. Please refer below code from dmaengine.c static void dma_chan_put(struct dma_chan *chan) { if (!chan-client_count) return; /* this channel failed alloc_chan_resources */ chan-client_count--; module_put(dma_chan_to_owner(chan)); if (chan-client_count == 0) chan-device-device_free_chan_resources(chan); } As you mentioned channel flush is needed if some client is queued and force unload happens. I am not against flushing and freeing channels. I am __only__ suggesting to flush and free channels for which alloc_chan_resource was successful, which can be decided by chan-client_count as being done in above function. Don't you think free_chan_resource should be done __only if__ alloc_chan_resource was successful ? Thanks, Inder AFAIU there are already enough checks in the path to weed out unused channels, so let us please not uglify the code by adding new unnecessary checks. thanks. -- 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/2] ARM: SAMSUNG: Insert bitmap_gpio_int member in samsung_gpio_chip
On 09/27/2012 12:55 PM, Eunki Kim wrote: When a device uses GPIO interrupt, its driver assumes that GPIO should be INPUT mode. However, GPIO of SAMSUNG SoC is sepated to INPUT mode and INTERRUPT mode. They are set by 0x0 and 0xF in GPIO control register. If the register is set to INPUT mode, the interrupt never occur. Therefore, it's necessary to set INTERRUPT mode instead of INPUT mode when the pin is used for GPIO interrupt. If for this, already the patch was posted. http://lists.infradead.org/pipermail/linux-arm-kernel/2011-November/074387.html Thanks. This patch inserts the bitmap_gpio_int member in struct samsung_ gpio_chip in order to represent use of GPIO interrupt for each pin and sets the related bit when s5p_register_gpio_interrupt function is called. Signed-off-by: Eunki Kim eunki_...@samsung.com Cc: Grant Likely grant.lik...@secretlab.ca Cc: Linus Walleij linus.wall...@linaro.org --- arch/arm/plat-samsung/include/plat/gpio-core.h |2 ++ arch/arm/plat-samsung/s5p-irq-gpioint.c|8 ++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/arch/arm/plat-samsung/include/plat/gpio-core.h b/arch/arm/plat-samsung/include/plat/gpio-core.h index 1fe6917..dfd8b7a 100644 --- a/arch/arm/plat-samsung/include/plat/gpio-core.h +++ b/arch/arm/plat-samsung/include/plat/gpio-core.h @@ -48,6 +48,7 @@ struct samsung_gpio_cfg; * @config: special function and pull-resistor control information. * @lock: Lock for exclusive access to this gpio bank. * @pm_save: Save information for suspend/resume support. + * @bitmap_gpio_int: Bitmap for representing GPIO interrupt or not. * * This wrapper provides the necessary information for the Samsung * specific gpios being registered with gpiolib. @@ -71,6 +72,7 @@ struct samsung_gpio_chip { #ifdef CONFIG_PM u32 pm_save[4]; #endif + u32 bitmap_gpio_int; }; static inline struct samsung_gpio_chip *to_samsung_gpio(struct gpio_chip *gpc) diff --git a/arch/arm/plat-samsung/s5p-irq-gpioint.c b/arch/arm/plat-samsung/s5p-irq-gpioint.c index f9431fe..d981c61 100644 --- a/arch/arm/plat-samsung/s5p-irq-gpioint.c +++ b/arch/arm/plat-samsung/s5p-irq-gpioint.c @@ -185,7 +185,7 @@ int __init s5p_register_gpio_interrupt(int pin) /* check if the group has been already registered */ if (my_chip-irq_base) - return my_chip-irq_base + offset; + goto success; /* register gpio group */ ret = s5p_gpioint_add(my_chip); @@ -193,9 +193,13 @@ int __init s5p_register_gpio_interrupt(int pin) my_chip-chip.to_irq = samsung_gpiolib_to_irq; printk(KERN_INFO Registered interrupt support for gpio group %d.\n, group); - return my_chip-irq_base + offset; + goto success; } return ret; +success: + my_chip-bitmap_gpio_int |= BIT(offset); + + return my_chip-irq_base + offset; } int __init s5p_register_gpioint_bank(int chain_irq, int start, int nr_groups) -- 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 3/3] DMA: PL330: Balance module remove function with probe
On 27 September 2012 10:35, Jassi Brar jassisinghb...@gmail.com wrote: On Thu, Sep 27, 2012 at 9:43 AM, Inderpal Singh inderpal.si...@linaro.org wrote: Don't you think free_chan_resource should be done __only if__ alloc_chan_resource was successful ? No, I don't think so. Thanks. Thanks for quick response. Please elaborate more on this as I find it against the general rule and against the dmaengine implementation which checks on the same condition before proceeding for free_chan_resouces in dma_chan_put function. Thanks, Inder -- 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/2] ARM: SAMSUNG: Insert bitmap_gpio_int member in samsung_gpio_chip
-Original Message- From: Joonyoung Shim [mailto:jy0922.s...@samsung.com] Sent: Thursday, September 27, 2012 1:50 PM To: Eunki Kim Cc: linux-arm-ker...@lists.infradead.org; linux-samsung-soc@vger.kernel.org; grant.lik...@secretlab.ca; linus.wall...@linaro.org; kgene@samsung.com Subject: Re: [PATCH 1/2] ARM: SAMSUNG: Insert bitmap_gpio_int member in samsung_gpio_chip On 09/27/2012 12:55 PM, Eunki Kim wrote: When a device uses GPIO interrupt, its driver assumes that GPIO should be INPUT mode. However, GPIO of SAMSUNG SoC is sepated to INPUT mode and INTERRUPT mode. They are set by 0x0 and 0xF in GPIO control register. If the register is set to INPUT mode, the interrupt never occur. Therefore, it's necessary to set INTERRUPT mode instead of INPUT mode when the pin is used for GPIO interrupt. If for this, already the patch was posted. http://lists.infradead.org/pipermail/linux-arm-kernel/2011-November/074387.html Thanks. The old posted patch and my new patches solve the same problem. However, the old patch fixes s5p_gpioint_set_type function which is irq_set_type function of GPIO int. The objective of irq_set_type function is setting IRQ type as rising, falling, level and etc. It's not setting GPIO pin configuration. The new patches solve the problem root. It monitors use of GPIO int for each pin. If then, it sets INTERRUPT mode instead of INPUT mode when gpio_direction_input function is called. As you know, the objective of gpio_direction_input function is setting the GPIO configuration. This patch inserts the bitmap_gpio_int member in struct samsung_ gpio_chip in order to represent use of GPIO interrupt for each pin and sets the related bit when s5p_register_gpio_interrupt function is called. Signed-off-by: Eunki Kim eunki_...@samsung.com Cc: Grant Likely grant.lik...@secretlab.ca Cc: Linus Walleij linus.wall...@linaro.org --- arch/arm/plat-samsung/include/plat/gpio-core.h |2 ++ arch/arm/plat-samsung/s5p-irq-gpioint.c|8 ++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/arch/arm/plat-samsung/include/plat/gpio-core.h b/arch/arm/plat-samsung/include/plat/gpio-core.h index 1fe6917..dfd8b7a 100644 --- a/arch/arm/plat-samsung/include/plat/gpio-core.h +++ b/arch/arm/plat-samsung/include/plat/gpio-core.h @@ -48,6 +48,7 @@ struct samsung_gpio_cfg; * @config: special function and pull-resistor control information. * @lock: Lock for exclusive access to this gpio bank. * @pm_save: Save information for suspend/resume support. + * @bitmap_gpio_int: Bitmap for representing GPIO interrupt or not. * * This wrapper provides the necessary information for the Samsung * specific gpios being registered with gpiolib. @@ -71,6 +72,7 @@ struct samsung_gpio_chip { #ifdef CONFIG_PM u32 pm_save[4]; #endif + u32 bitmap_gpio_int; }; static inline struct samsung_gpio_chip *to_samsung_gpio(struct gpio_chip *gpc) diff --git a/arch/arm/plat-samsung/s5p-irq-gpioint.c b/arch/arm/plat-samsung/s5p-irq-gpioint.c index f9431fe..d981c61 100644 --- a/arch/arm/plat-samsung/s5p-irq-gpioint.c +++ b/arch/arm/plat-samsung/s5p-irq-gpioint.c @@ -185,7 +185,7 @@ int __init s5p_register_gpio_interrupt(int pin) /* check if the group has been already registered */ if (my_chip-irq_base) - return my_chip-irq_base + offset; + goto success; /* register gpio group */ ret = s5p_gpioint_add(my_chip); @@ -193,9 +193,13 @@ int __init s5p_register_gpio_interrupt(int pin) my_chip-chip.to_irq = samsung_gpiolib_to_irq; printk(KERN_INFO Registered interrupt support for gpio group %d.\n, group); - return my_chip-irq_base + offset; + goto success; } return ret; +success: + my_chip-bitmap_gpio_int |= BIT(offset); + + return my_chip-irq_base + offset; } int __init s5p_register_gpioint_bank(int chain_irq, int start, int nr_groups) -- 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