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
> Signed-off-by: Arun Kumar K
> ---
> .../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 3/3] DMA: PL330: Balance module remove function with probe
On 25 September 2012 18:47, Jassi Brar wrote:
> On Tue, Sep 25, 2012 at 2:27 PM, Inderpal Singh
> 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
>> ---
>> 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 V6 2/2] video: drm: exynos: Add device tree support
2012/9/26 Mark Brown : > On Wed, Sep 26, 2012 at 12:03:44AM +0900, Inki Dae wrote: >> 2012/9/25 Laurent Pinchart : > >> > Aren't DT bindings considered as an ABI, and required to be supported more >> > or >> > less forever ? If you merge this DT binding you'll have to keep supporting >> > it. >> > That's why DT bindings should not be rushed in. > >> is ABI required for DT binding? I know DT binding parses just lcd >> timing data from device tree file so ABI isn't needed. but when it >> comes to DT, I'm novice yet so there may be my missing point. could >> you tell me why DT bindings are considered as an ABI? if there is my >> missing point, will consider it again. > > It's supposed to be possible to ship a DT with a board and then boot any > OS or OS version on the board. If the meaning of the DT keeps changing > then this becomes impossible, you need to keep changing the DT when you > change the thing that parses it (rendering the whole exercise pointless). > thank you for your comments. got it. DT is built as an binary(dtb) and the dtb file should be re-used without any modifications. will keep this patch until the videomode helper will be merged to mainline so that this could be modified based on videomode helper later. > ___ > 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: [PATCH v4] ARM: EXYNOS: Add MFC device tree support
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 > > Signed-off-by: Arun Kumar K > > > Looks good for me. FWIW, I could probably add > > Acked-by: Karol Lewandowski > 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 , 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.html
RE: [PATCH v4 0/3] ARM: Exynos: Add support for MSHC controller
Thomas Abraham wrote: > > This patch series adds Exynos5250 platform support for MSHC controllers. > > Thomas Abraham (3): > ARM: Samsung: Add support for MSHC controller clocks > ARM: Exynos5: Add AUXDATA support for MSHC controllers > ARM: dts: Add nodes for dw_mmc controllers for Samsung Exynos5250 > platforms > > arch/arm/boot/dts/exynos5250-smdk5250.dts | 55 > + > arch/arm/boot/dts/exynos5250.dtsi | 32 + > arch/arm/mach-exynos/clock-exynos5.c | 45 --- > arch/arm/mach-exynos/include/mach/map.h |4 ++ > arch/arm/mach-exynos/mach-exynos5-dt.c|8 > 5 files changed, 115 insertions(+), 29 deletions(-) Applied this series. Thanks. Best regards, Kgene. -- Kukjin Kim , 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.html
RE: [PATCH V2 3/3] ARM: EXYNOS: Add drm-device node to the dtsi file
Leela Krishna Amudala wrote:
>
> This patch adds platform drm-device node to the dtsi file
>
> Signed-off-by: Leela Krishna Amudala
> ---
> arch/arm/boot/dts/exynos5250.dtsi |4
> 1 files changed, 4 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/boot/dts/exynos5250.dtsi
> b/arch/arm/boot/dts/exynos5250.dtsi
> index 6401c94..f0cc06d 100644
> --- a/arch/arm/boot/dts/exynos5250.dtsi
> +++ b/arch/arm/boot/dts/exynos5250.dtsi
> @@ -495,4 +495,8 @@
> reg = <0x1440 0x4>;
> interrupts = <18 5>, <18 4>, <18 6>;
> };
> +
> + drm-device {
> + compatible = "samsung,exynos-drm-device";
> + };
> };
> --
Please check below comments from Thomas Abraham and I agree with his
opinion.
There cannot be a node that represents a virtual device. Device tree should
ideally describe the hardware but the above node does not represent a
hardware device. The creation of the platform device instance for the above
device should handled inside the kernel code.
Thanks.
K-Gene
--
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: [RFC 6/6] ARM: dts: exynos4210: Add platform-specific descriptions for pin controllers
On 09/25/2012 12:35 PM, Tomasz Figa wrote: > On Tuesday 25 of September 2012 12:22:03 Stephen Warren wrote: ... >> BTW, how does the driver know what register addresses to use; I can see >> the base for each pin controller bank is in samsung,pctl-offset, but >> what describes the offset for each of the func, pud, drv, ... registers >> from there? Are the offsets the same for all current Samsung SoCs? > > The offsets are defined as constants in the driver. > > They are the same in all cases, but the "4bit2" bank type of S3C64xx, which > can have up to 16 pins with 4-bit function specifiers, so two registers are > required for function configuration. In this case all the remaining > registers are offset by 0x04. > > I couldn't think about any good solution for this special case, but still, > I haven't been thinking a lot about it, as the driver is targetted at > current Exynos SoCs primarily. I suppose if you always assume that the registers will appear in a specific order, and never have gaps between them, then you can simply always calculate the addresses as e.g.: reg_func = reg_base reg_pud = reg_func + round_up(num_pins / (32 / func_width)) reg_drv = reg_pud + round_up(num_pins / (32 / func_width)) ... Then, there wouldn't ever be any special cases - that calculation would always work. An alternative would be to put each register's address in DT rather than just the base of the register block. It'd certainly be more future-flexible, even if not strictly necessary. -- 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: [RFC 6/6] ARM: dts: exynos4210: Add platform-specific descriptions for pin controllers
On Tuesday 25 of September 2012 12:22:03 Stephen Warren wrote: > On 09/25/2012 11:41 AM, Tomasz Figa wrote: > > On Tuesday 25 of September 2012 10:49:11 Stephen Warren wrote: > >> On 09/25/2012 03:37 AM, Tomasz Figa wrote: > >>> Hi Stephen, > >>> > >>> On Monday 24 of September 2012 17:14:38 Stephen Warren wrote: > On 09/24/2012 03:31 PM, Tomasz Figa wrote: > > On Monday 24 of September 2012 11:42:15 Stephen Warren wrote: > >> On 09/21/2012 01:54 PM, Tomasz Figa wrote: > >>> On Friday 21 of September 2012 12:56:41 Stephen Warren wrote: > On 09/20/2012 02:53 AM, Tomasz Figa wrote: > > The patch "pinctrl: samsung: Parse pin banks from DT" > > introduced > > platform-specific data parsing from DT. > > > > This patch adds all necessary nodes and properties to > > exynos4210 > > device > > tree sources. > > > > +++ b/arch/arm/boot/dts/exynos4210-pinctrl-banks.dtsi > > > > + samsung,pctl-offset = <0x000>; > > + samsung,pin-bank = "gpa0"; > > + samsung,pin-count = <8>; > > + samsung,func-width = <4>; > > + samsung,pud-width = <2>; > > + samsung,drv-width = <2>; > > + samsung,conpdn-width = <2>; > > + samsung,pudpdn-width = <2>; > >> > >> ... > >> > >>> Hmm, could you elaborate on the idea of using mask instead of field > >>> widths? > >> > >> For background: With e.g.: > >> > >> samsung,func-width = <4>; > >> samsung,pud-width = <2>; > >> samsung,drv-width = <2>; > >> > >> How do you know if the layout is: > >> > >> bits:7-4 | 3-2 | 1-0 > >> meaning: func | pud | drv > >> > >> or: > >> > >> bits:7-6 | 5-4 | 3-0 | > >> meaning: drv | pud | func | > >> > >> or: > >> > >> bits:15-12 | 13-8 | 7-6 | 5-3| 2-1 | 0 > >> meaning: func | unused | pud | unused | drv | unused > >> > >> I suppose what you're saying is that for all currently extant Samsung > >> SoCs, there's some rule that defines this; perhaps the fields are > >> always > >> in order MSB to LSB func, pud, drv, and there are never any unused > >> bits > >> between the fields? If so, I suppose that's reasonable, even if it > >> does > >> restrict the binding's ability to support any unanticipated future SoC > >> register layout changes. > > > > I think we have a little misunderstanding here. > > > > All the Samsung SoCs currently available have separate registers for > > particular configuration types. Each register is used to configure all > > pins in a bank. The width field specifies how many bits are used per > > pin, not per configuration type. > > Oh I see. In that case, I guess just having "width" properties is fine, > and I can see how it's much more likely this scheme would be extensible > to any future SoC that sticks with the same overall kind of register > structure. > > It'd be a good idea to describe this explicitly in the binding > documentation. OK. > BTW, how does the driver know what register addresses to use; I can see > the base for each pin controller bank is in samsung,pctl-offset, but > what describes the offset for each of the func, pud, drv, ... registers > from there? Are the offsets the same for all current Samsung SoCs? The offsets are defined as constants in the driver. They are the same in all cases, but the "4bit2" bank type of S3C64xx, which can have up to 16 pins with 4-bit function specifiers, so two registers are required for function configuration. In this case all the remaining registers are offset by 0x04. I couldn't think about any good solution for this special case, but still, I haven't been thinking a lot about it, as the driver is targetted at current Exynos SoCs primarily. 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: [RFC 6/6] ARM: dts: exynos4210: Add platform-specific descriptions for pin controllers
On 09/25/2012 11:41 AM, Tomasz Figa wrote: > On Tuesday 25 of September 2012 10:49:11 Stephen Warren wrote: >> On 09/25/2012 03:37 AM, Tomasz Figa wrote: >>> Hi Stephen, >>> >>> On Monday 24 of September 2012 17:14:38 Stephen Warren wrote: On 09/24/2012 03:31 PM, Tomasz Figa wrote: > On Monday 24 of September 2012 11:42:15 Stephen Warren wrote: >> On 09/21/2012 01:54 PM, Tomasz Figa wrote: >>> On Friday 21 of September 2012 12:56:41 Stephen Warren wrote: On 09/20/2012 02:53 AM, Tomasz Figa wrote: > The patch "pinctrl: samsung: Parse pin banks from DT" introduced > platform-specific data parsing from DT. > > This patch adds all necessary nodes and properties to exynos4210 > device > tree sources. > > +++ b/arch/arm/boot/dts/exynos4210-pinctrl-banks.dtsi > > + samsung,pctl-offset = <0x000>; > + samsung,pin-bank = "gpa0"; > + samsung,pin-count = <8>; > + samsung,func-width = <4>; > + samsung,pud-width = <2>; > + samsung,drv-width = <2>; > + samsung,conpdn-width = <2>; > + samsung,pudpdn-width = <2>; >> >> ... >> >>> Hmm, could you elaborate on the idea of using mask instead of field >>> widths? >> For background: With e.g.: >> >> samsung,func-width = <4>; >> samsung,pud-width = <2>; >> samsung,drv-width = <2>; >> >> How do you know if the layout is: >> >> bits:7-4 | 3-2 | 1-0 >> meaning: func | pud | drv >> >> or: >> >> bits:7-6 | 5-4 | 3-0 | >> meaning: drv | pud | func | >> >> or: >> >> bits:15-12 | 13-8 | 7-6 | 5-3| 2-1 | 0 >> meaning: func | unused | pud | unused | drv | unused >> >> I suppose what you're saying is that for all currently extant Samsung >> SoCs, there's some rule that defines this; perhaps the fields are always >> in order MSB to LSB func, pud, drv, and there are never any unused bits >> between the fields? If so, I suppose that's reasonable, even if it does >> restrict the binding's ability to support any unanticipated future SoC >> register layout changes. > > I think we have a little misunderstanding here. > > All the Samsung SoCs currently available have separate registers for > particular configuration types. Each register is used to configure all pins > in a bank. The width field specifies how many bits are used per pin, not > per configuration type. Oh I see. In that case, I guess just having "width" properties is fine, and I can see how it's much more likely this scheme would be extensible to any future SoC that sticks with the same overall kind of register structure. It'd be a good idea to describe this explicitly in the binding documentation. BTW, how does the driver know what register addresses to use; I can see the base for each pin controller bank is in samsung,pctl-offset, but what describes the offset for each of the func, pud, drv, ... registers from there? Are the offsets the same for all current Samsung SoCs? -- 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: [RFC 6/6] ARM: dts: exynos4210: Add platform-specific descriptions for pin controllers
On Tuesday 25 of September 2012 10:49:11 Stephen Warren wrote: > On 09/25/2012 03:37 AM, Tomasz Figa wrote: > > Hi Stephen, > > > > On Monday 24 of September 2012 17:14:38 Stephen Warren wrote: > >> On 09/24/2012 03:31 PM, Tomasz Figa wrote: > >>> On Monday 24 of September 2012 11:42:15 Stephen Warren wrote: > On 09/21/2012 01:54 PM, Tomasz Figa wrote: > > On Friday 21 of September 2012 12:56:41 Stephen Warren wrote: > >> On 09/20/2012 02:53 AM, Tomasz Figa wrote: > >>> The patch "pinctrl: samsung: Parse pin banks from DT" introduced > >>> platform-specific data parsing from DT. > >>> > >>> This patch adds all necessary nodes and properties to exynos4210 > >>> device > >>> tree sources. > >>> > >>> +++ b/arch/arm/boot/dts/exynos4210-pinctrl-banks.dtsi > >>> > >>> + samsung,pctl-offset = <0x000>; > >>> + samsung,pin-bank = "gpa0"; > >>> + samsung,pin-count = <8>; > >>> + samsung,func-width = <4>; > >>> + samsung,pud-width = <2>; > >>> + samsung,drv-width = <2>; > >>> + samsung,conpdn-width = <2>; > >>> + samsung,pudpdn-width = <2>; > > ... > > > Hmm, could you elaborate on the idea of using mask instead of field > > widths? > For background: With e.g.: > > samsung,func-width = <4>; > samsung,pud-width = <2>; > samsung,drv-width = <2>; > > How do you know if the layout is: > > bits:7-4 | 3-2 | 1-0 > meaning: func | pud | drv > > or: > > bits:7-6 | 5-4 | 3-0 | > meaning: drv | pud | func | > > or: > > bits:15-12 | 13-8 | 7-6 | 5-3| 2-1 | 0 > meaning: func | unused | pud | unused | drv | unused > > I suppose what you're saying is that for all currently extant Samsung > SoCs, there's some rule that defines this; perhaps the fields are always > in order MSB to LSB func, pud, drv, and there are never any unused bits > between the fields? If so, I suppose that's reasonable, even if it does > restrict the binding's ability to support any unanticipated future SoC > register layout changes. I think we have a little misunderstanding here. All the Samsung SoCs currently available have separate registers for particular configuration types. Each register is used to configure all pins in a bank. The width field specifies how many bits are used per pin, not per configuration type. > > I don't see how this could be better and there is an additional > > drawback of having to calculate width and pos from every mask. > > With the DT properties just defining "width", the driver still has to > calculate the pos from every width by adding up the widths of all fields > lower in the register, right? Or, does each field always start at a > hard-coded bit position? > > Anyway, you could completely avoid this question by using masks instead: > > samsung,func-mask = <0xf0>; > samsung,pud-mask = <0xc>; > samsung,drv-mask = <0x3>; > > The mask defines exactly which bits are included in the register field, > so it implicitly defines both the position and width of the field. > > Finding the shift/size is very easy. I believe Tony Lindgren's generic > pinctrl already does this along these lines. Very roughly: > > func_pos = ffs(func_mask); > func_width = ffs(~(func_mask >> func_pos)); Right, this looks fine. > > Anyway, back to your concern, the values that are written to the bit > > fields specified by those bindings are arbitrary SoC-specific values > > anyway, so if, for example, we get a SoC with following register > > layout: > > > > bits:7 | 6 - 4 | 3 | 2 - 0 > > meaning: 0 | func 1 | 0 | func 0 > > > > or > > > > bits:7 - 5 | 4 | 3 - 1 | 0 > > meaning: func 1 | 0 | func 0 | 0 > > > > we can easily define the width as 4 and use appropriate 4-bit function > > values with zeroes on reserved positions. > > The problem with that is that if the datasheet documents "func" values > of 0, 1, 2, 3, whereas your driver expects values that are shifted left > one bit in order to fit into the field, the DT would need to contain 0, > 2, 4, 6. So, the DT values then don't match the documentation, which > would end up being confusing. > > >> I forget, do you actually have multiple different SoCs right now (or > >> in > >> the near future where the HW design is known now for certain even if > >> the > >> chip isn't available) that have different values for all these *-width > >> properties and hence can be represented just using this binding and > >> without altering the driver at all? If so, I suppose the original > >> binding is at least useful (although I would certainly still request > >> to > >> use *-mask instead of *-width properties). > > > > The binding I proposed covers all Samsung SoCs currently available, > > starting from s3c24xx, through s3c64xx (except 4bit2 bank type, with > > two > > function registers), to the whole Exynos series,
Re: [RFC 6/6] ARM: dts: exynos4210: Add platform-specific descriptions for pin controllers
On 09/25/2012 03:37 AM, Tomasz Figa wrote: > Hi Stephen, > > On Monday 24 of September 2012 17:14:38 Stephen Warren wrote: >> On 09/24/2012 03:31 PM, Tomasz Figa wrote: >>> On Monday 24 of September 2012 11:42:15 Stephen Warren wrote: On 09/21/2012 01:54 PM, Tomasz Figa wrote: > On Friday 21 of September 2012 12:56:41 Stephen Warren wrote: >> On 09/20/2012 02:53 AM, Tomasz Figa wrote: >>> The patch "pinctrl: samsung: Parse pin banks from DT" introduced >>> platform-specific data parsing from DT. >>> >>> This patch adds all necessary nodes and properties to exynos4210 >>> device >>> tree sources. >>> >>> +++ b/arch/arm/boot/dts/exynos4210-pinctrl-banks.dtsi >>> >>> + samsung,pctl-offset = <0x000>; >>> + samsung,pin-bank = "gpa0"; >>> + samsung,pin-count = <8>; >>> + samsung,func-width = <4>; >>> + samsung,pud-width = <2>; >>> + samsung,drv-width = <2>; >>> + samsung,conpdn-width = <2>; >>> + samsung,pudpdn-width = <2>; ... > Hmm, could you elaborate on the idea of using mask instead of field widths? For background: With e.g.: samsung,func-width = <4>; samsung,pud-width = <2>; samsung,drv-width = <2>; How do you know if the layout is: bits:7-4 | 3-2 | 1-0 meaning: func | pud | drv or: bits:7-6 | 5-4 | 3-0 | meaning: drv | pud | func | or: bits:15-12 | 13-8 | 7-6 | 5-3| 2-1 | 0 meaning: func | unused | pud | unused | drv | unused I suppose what you're saying is that for all currently extant Samsung SoCs, there's some rule that defines this; perhaps the fields are always in order MSB to LSB func, pud, drv, and there are never any unused bits between the fields? If so, I suppose that's reasonable, even if it does restrict the binding's ability to support any unanticipated future SoC register layout changes. > I don't see how this could be better and there is an additional drawback of > having to calculate width and pos from every mask. With the DT properties just defining "width", the driver still has to calculate the pos from every width by adding up the widths of all fields lower in the register, right? Or, does each field always start at a hard-coded bit position? Anyway, you could completely avoid this question by using masks instead: samsung,func-mask = <0xf0>; samsung,pud-mask = <0xc>; samsung,drv-mask = <0x3>; The mask defines exactly which bits are included in the register field, so it implicitly defines both the position and width of the field. Finding the shift/size is very easy. I believe Tony Lindgren's generic pinctrl already does this along these lines. Very roughly: func_pos = ffs(func_mask); func_width = ffs(~(func_mask >> func_pos)); > Anyway, back to your concern, the values that are written to the bit fields > specified by those bindings are arbitrary SoC-specific values anyway, so > if, for example, we get a SoC with following register layout: > > bits:7 | 6 - 4 | 3 | 2 - 0 > meaning: 0 | func 1 | 0 | func 0 > > or > > bits:7 - 5 | 4 | 3 - 1 | 0 > meaning: func 1 | 0 | func 0 | 0 > > we can easily define the width as 4 and use appropriate 4-bit function > values with zeroes on reserved positions. The problem with that is that if the datasheet documents "func" values of 0, 1, 2, 3, whereas your driver expects values that are shifted left one bit in order to fit into the field, the DT would need to contain 0, 2, 4, 6. So, the DT values then don't match the documentation, which would end up being confusing. >> I forget, do you actually have multiple different SoCs right now (or in >> the near future where the HW design is known now for certain even if the >> chip isn't available) that have different values for all these *-width >> properties and hence can be represented just using this binding and >> without altering the driver at all? If so, I suppose the original >> binding is at least useful (although I would certainly still request to >> use *-mask instead of *-width properties). > > The binding I proposed covers all Samsung SoCs currently available, > starting from s3c24xx, through s3c64xx (except 4bit2 bank type, with two > function registers), to the whole Exynos series, including latest Exynos5. OK, the HW is nice and consistent then. In that case, the binding is probably reasonable. Hopefully the HW designers are aware they shouldn't randomly break the uniformity! -- 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 2/2] video: drm: exynos: Add device tree support
On Wed, Sep 26, 2012 at 12:03:44AM +0900, Inki Dae wrote: > 2012/9/25 Laurent Pinchart : > > Aren't DT bindings considered as an ABI, and required to be supported more > > or > > less forever ? If you merge this DT binding you'll have to keep supporting > > it. > > That's why DT bindings should not be rushed in. > is ABI required for DT binding? I know DT binding parses just lcd > timing data from device tree file so ABI isn't needed. but when it > comes to DT, I'm novice yet so there may be my missing point. could > you tell me why DT bindings are considered as an ABI? if there is my > missing point, will consider it again. It's supposed to be possible to ship a DT with a board and then boot any OS or OS version on the board. If the meaning of the DT keeps changing then this becomes impossible, you need to keep changing the DT when you change the thing that parses it (rendering the whole exercise pointless). -- 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 2/3] DMA: PL330: Change allocation method to properly free DMA descriptors
On 25 September 2012 18:39, Jassi Brar wrote:
> On Tue, Sep 25, 2012 at 2:27 PM, Inderpal Singh
> wrote:
>> In probe, memory for multiple DMA descriptors were being allocated at once
>> and then it was being split and added into DMA pool one by one. The address
>> of this memory allocation is not being saved anywhere. To free this memory,
>> the address is required. Initially the first node of the pool will be
>> pointed by this address but as we use this pool the descs will shuffle and
>> hence we will lose the track of the address.
>>
>> This patch does following:
>>
>> 1. Allocates DMA descs one by one and then adds them to pool so that all
>>descs can be fetched and memory freed one by one. This way runtime
>>added descs can also be freed.
>> 2. Free DMA descs in case of error in probe and in module's remove function
>>
>> Signed-off-by: Inderpal Singh
>> ---
>> drivers/dma/pl330.c | 28 +---
>> 1 file changed, 21 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
>> index 10c6b6a..04d83e6 100644
>> --- a/drivers/dma/pl330.c
>> +++ b/drivers/dma/pl330.c
>> @@ -2541,20 +2541,19 @@ static int add_desc(struct dma_pl330_dmac *pdmac,
>> gfp_t flg, int count)
>> if (!pdmac)
>> return 0;
>>
>> - desc = kmalloc(count * sizeof(*desc), flg);
>> - if (!desc)
>> - return 0;
>> -
>> spin_lock_irqsave(&pdmac->pool_lock, flags);
>>
>> for (i = 0; i < count; i++) {
>> - _init_desc(&desc[i]);
>> - list_add_tail(&desc[i].node, &pdmac->desc_pool);
>> + desc = kmalloc(sizeof(*desc), flg);
>> + if (!desc)
>> + break;
>> + _init_desc(desc);
>> + list_add_tail(&desc->node, &pdmac->desc_pool);
>> }
>>
>> spin_unlock_irqrestore(&pdmac->pool_lock, flags);
>>
>> - return count;
>> + return i;
>> }
>>
> OK, but the kmalloc shouldn't be done with irqs disabled. Please
> protect only the list_add_tail()
>
Yes, I missed it. I will update and send it again.
Thanks,
Inder
> thanks.
>
>> static struct dma_pl330_desc *
>> @@ -2857,6 +2856,7 @@ pl330_probe(struct amba_device *adev, const struct
>> amba_id *id)
>> struct dma_pl330_platdata *pdat;
>> struct dma_pl330_dmac *pdmac;
>> struct dma_pl330_chan *pch;
>> + struct dma_pl330_desc *desc;
>> struct pl330_info *pi;
>> struct dma_device *pd;
>> struct resource *res;
>> @@ -2978,6 +2978,12 @@ pl330_probe(struct amba_device *adev, const struct
>> amba_id *id)
>> probe_err5:
>> kfree(pdmac->peripherals);
>> probe_err4:
>> + while (!list_empty(&pdmac->desc_pool)) {
>> + desc = list_entry(pdmac->desc_pool.next,
>> + struct dma_pl330_desc, node);
>> + list_del(&desc->node);
>> + kfree(desc);
>> + }
>> pl330_del(pi);
>> probe_err3:
>> free_irq(irq, pi);
>> @@ -2994,6 +3000,7 @@ static int __devexit pl330_remove(struct amba_device
>> *adev)
>> {
>> struct dma_pl330_dmac *pdmac = amba_get_drvdata(adev);
>> struct dma_pl330_chan *pch, *_p;
>> + struct dma_pl330_desc *desc;
>> struct pl330_info *pi;
>> struct resource *res;
>> int irq;
>> @@ -3015,6 +3022,13 @@ static int __devexit pl330_remove(struct amba_device
>> *adev)
>> pl330_free_chan_resources(&pch->chan);
>> }
>>
>> + while (!list_empty(&pdmac->desc_pool)) {
>> + desc = list_entry(pdmac->desc_pool.next,
>> + struct dma_pl330_desc, node);
>> + list_del(&desc->node);
>> + kfree(desc);
>> + }
>> +
>> pi = &pdmac->pif;
>>
>> pl330_del(pi);
>> --
>> 1.7.9.5
>>
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] DMA: PL330: Free memory allocated for peripheral channels
On 25 September 2012 18:17, Jassi Brar wrote:
> On Tue, Sep 25, 2012 at 2:27 PM, Inderpal Singh
> wrote:
>> The allocated memory for peripheral channels is not being freed upon
>> failure in probe and in module's remove funtion. It will lead to memory
>> leakage. Hence free the allocated memory.
>>
>> Signed-off-by: Inderpal Singh
>> ---
>> drivers/dma/pl330.c |5 -
>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
>> index 2ebd4cd..10c6b6a 100644
>> --- a/drivers/dma/pl330.c
>> +++ b/drivers/dma/pl330.c
>> @@ -2962,7 +2962,7 @@ pl330_probe(struct amba_device *adev, const struct
>> amba_id *id)
>> ret = dma_async_device_register(pd);
>> if (ret) {
>> dev_err(&adev->dev, "unable to register DMAC\n");
>> - goto probe_err4;
>> + goto probe_err5;
>> }
>>
> Sorry this patch seems malformed. Against which tree did you prepare it ?
>
This patch depends on "61c6e7531d3b66b3 DMA: PL330: Check the
pointer returned by kzalloc" which is on vinod's slave-dma's "fixes"
branch. So I merged slave-dma's "next" and "fixes" branches. Now after
merging, build error occurs due to some conflict so I had to apply the
patch sent by Sachin at [1]
Same had been mentioned in the cover letter.
[1] http://permalink.gmane.org/gmane.linux.kernel.next/24274
Thanks,
Inder
> -jassi
>
>
>
>> dev_info(&adev->dev,
>> @@ -2975,6 +2975,8 @@ pl330_probe(struct amba_device *adev, const struct
>> amba_id *id)
>>
>> return 0;
>>
>> +probe_err5:
>> + kfree(pdmac->peripherals);
>> probe_err4:
>> pl330_del(pi);
>> probe_err3:
>> @@ -3025,6 +3027,7 @@ static int __devexit pl330_remove(struct amba_device
>> *adev)
>> res = &adev->res;
>> release_mem_region(res->start, resource_size(res));
>>
>> + kfree(pdmac->peripherals);
>> kfree(pdmac);
>>
>> return 0;
>> --
>> 1.7.9.5
>>
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V6 2/2] video: drm: exynos: Add device tree support
2012/9/25 Laurent Pinchart : > On Monday 24 September 2012 21:35:46 Inki Dae wrote: >> 2012/9/22 Stephen Warren : >> > On 09/21/2012 01:22 AM, Inki Dae wrote: >> >> 2012/9/21 Stephen Warren : >> >>> On 09/21/2012 05:22 AM, Leela Krishna Amudala wrote: >> This patch adds device tree based discovery support for exynos DRM-FIMD >> driver which includes driver modification to handle platform data in >> both the cases with DT and non-DT, Also adds the documentation for >> bindings. >> >> diff --git a/Documentation/devicetree/bindings/drm/exynos/fimd.txt >> b/Documentation/devicetree/bindings/drm/exynos/fimd.txt>>> >> >>> ... >> >>> >> + - samsung,fimd-display: This property should specify the phandle of >> the >> + display device node which holds the video interface timing with the >> + below mentioned properties. >> + >> + - lcd-htiming: Specifies the horizontal timing for the overlay. The >> + horizontal timing includes four parameters in the following >> order. >> + >> + - horizontal back porch (in number of lcd clocks) >> + - horizontal front porch (in number of lcd clocks) >> + - hsync pulse width (in number of lcd clocks) >> + - Display panels X resolution. >> + >> + - lcd-vtiming: Specifies the vertical timing for the overlay. The >> + vertical timing includes four parameters in the following order. >> + >> + - vertical back porch (in number of lcd lines) >> + - vertical front porch (in number of lcd lines) >> + - vsync pulse width (in number of lcd clocks) >> + - Display panels Y resolution. >> >>> >> >>> Should this not use the new videomode timings that are under discussion >> >>> at: >> >>> >> >>> http://lists.freedesktop.org/archives/dri-devel/2012-July/024875.html >> >> >> >> ok, I agree with you. then the videomode helper is going to be merged >> >> to mainline(3.6)? if so, this patch should be reworked based on the >> >> videomode helper. >> > >> > I think the videomode helpers would be merged for 3.7 at the very >> > earliest; 3.6 is cooked already. Given there are still some comments on >> > the binding, perhaps it won't happen until 3.8, but it'd be best to ask >> > on that thread so that people more directly involved with the status can >> > answer. >> >> as I mentioned before, it's better to use videomode helper instead but >> for this, we should wait for that the videomode helper are merged to >> mainline so I think it's better to merge it as is and then modify it >> for videomode helper to be used later. > > Aren't DT bindings considered as an ABI, and required to be supported more or > less forever ? If you merge this DT binding you'll have to keep supporting it. > That's why DT bindings should not be rushed in. > is ABI required for DT binding? I know DT binding parses just lcd timing data from device tree file so ABI isn't needed. but when it comes to DT, I'm novice yet so there may be my missing point. could you tell me why DT bindings are considered as an ABI? if there is my missing point, will consider it again. Thanks, Inki Dae > -- > Regards, > > Laurent Pinchart > > ___ > dri-devel mailing list > dri-de...@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel -- 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 5:48 PM, Rob Herring wrote:
> On 09/25/2012 06:23 AM, Praveen Paneri wrote:
>> Hi Rob,
>>
>> On Mon, Sep 24, 2012 at 6:34 PM, Rob Herring 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
Acked-by: Heiko Stuebner
---
.../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.
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
Re: [PATCH 3/3] DMA: PL330: Balance module remove function with probe
On Tue, Sep 25, 2012 at 2:27 PM, Inderpal Singh
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
> ---
> 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.
-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 2/3] DMA: PL330: Change allocation method to properly free DMA descriptors
On Tue, Sep 25, 2012 at 2:27 PM, Inderpal Singh
wrote:
> In probe, memory for multiple DMA descriptors were being allocated at once
> and then it was being split and added into DMA pool one by one. The address
> of this memory allocation is not being saved anywhere. To free this memory,
> the address is required. Initially the first node of the pool will be
> pointed by this address but as we use this pool the descs will shuffle and
> hence we will lose the track of the address.
>
> This patch does following:
>
> 1. Allocates DMA descs one by one and then adds them to pool so that all
>descs can be fetched and memory freed one by one. This way runtime
>added descs can also be freed.
> 2. Free DMA descs in case of error in probe and in module's remove function
>
> Signed-off-by: Inderpal Singh
> ---
> drivers/dma/pl330.c | 28 +---
> 1 file changed, 21 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
> index 10c6b6a..04d83e6 100644
> --- a/drivers/dma/pl330.c
> +++ b/drivers/dma/pl330.c
> @@ -2541,20 +2541,19 @@ static int add_desc(struct dma_pl330_dmac *pdmac,
> gfp_t flg, int count)
> if (!pdmac)
> return 0;
>
> - desc = kmalloc(count * sizeof(*desc), flg);
> - if (!desc)
> - return 0;
> -
> spin_lock_irqsave(&pdmac->pool_lock, flags);
>
> for (i = 0; i < count; i++) {
> - _init_desc(&desc[i]);
> - list_add_tail(&desc[i].node, &pdmac->desc_pool);
> + desc = kmalloc(sizeof(*desc), flg);
> + if (!desc)
> + break;
> + _init_desc(desc);
> + list_add_tail(&desc->node, &pdmac->desc_pool);
> }
>
> spin_unlock_irqrestore(&pdmac->pool_lock, flags);
>
> - return count;
> + return i;
> }
>
OK, but the kmalloc shouldn't be done with irqs disabled. Please
protect only the list_add_tail()
thanks.
> static struct dma_pl330_desc *
> @@ -2857,6 +2856,7 @@ pl330_probe(struct amba_device *adev, const struct
> amba_id *id)
> struct dma_pl330_platdata *pdat;
> struct dma_pl330_dmac *pdmac;
> struct dma_pl330_chan *pch;
> + struct dma_pl330_desc *desc;
> struct pl330_info *pi;
> struct dma_device *pd;
> struct resource *res;
> @@ -2978,6 +2978,12 @@ pl330_probe(struct amba_device *adev, const struct
> amba_id *id)
> probe_err5:
> kfree(pdmac->peripherals);
> probe_err4:
> + while (!list_empty(&pdmac->desc_pool)) {
> + desc = list_entry(pdmac->desc_pool.next,
> + struct dma_pl330_desc, node);
> + list_del(&desc->node);
> + kfree(desc);
> + }
> pl330_del(pi);
> probe_err3:
> free_irq(irq, pi);
> @@ -2994,6 +3000,7 @@ static int __devexit pl330_remove(struct amba_device
> *adev)
> {
> struct dma_pl330_dmac *pdmac = amba_get_drvdata(adev);
> struct dma_pl330_chan *pch, *_p;
> + struct dma_pl330_desc *desc;
> struct pl330_info *pi;
> struct resource *res;
> int irq;
> @@ -3015,6 +3022,13 @@ static int __devexit pl330_remove(struct amba_device
> *adev)
> pl330_free_chan_resources(&pch->chan);
> }
>
> + while (!list_empty(&pdmac->desc_pool)) {
> + desc = list_entry(pdmac->desc_pool.next,
> + struct dma_pl330_desc, node);
> + list_del(&desc->node);
> + kfree(desc);
> + }
> +
> pi = &pdmac->pif;
>
> pl330_del(pi);
> --
> 1.7.9.5
>
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V6 2/2] video: drm: exynos: Add device tree support
On Monday 24 September 2012 21:35:46 Inki Dae wrote: > 2012/9/22 Stephen Warren : > > On 09/21/2012 01:22 AM, Inki Dae wrote: > >> 2012/9/21 Stephen Warren : > >>> On 09/21/2012 05:22 AM, Leela Krishna Amudala wrote: > This patch adds device tree based discovery support for exynos DRM-FIMD > driver which includes driver modification to handle platform data in > both the cases with DT and non-DT, Also adds the documentation for > bindings. > > diff --git a/Documentation/devicetree/bindings/drm/exynos/fimd.txt > b/Documentation/devicetree/bindings/drm/exynos/fimd.txt>>> > >>> ... > >>> > + - samsung,fimd-display: This property should specify the phandle of > the > + display device node which holds the video interface timing with the > + below mentioned properties. > + > + - lcd-htiming: Specifies the horizontal timing for the overlay. The > + horizontal timing includes four parameters in the following > order. > + > + - horizontal back porch (in number of lcd clocks) > + - horizontal front porch (in number of lcd clocks) > + - hsync pulse width (in number of lcd clocks) > + - Display panels X resolution. > + > + - lcd-vtiming: Specifies the vertical timing for the overlay. The > + vertical timing includes four parameters in the following order. > + > + - vertical back porch (in number of lcd lines) > + - vertical front porch (in number of lcd lines) > + - vsync pulse width (in number of lcd clocks) > + - Display panels Y resolution. > >>> > >>> Should this not use the new videomode timings that are under discussion > >>> at: > >>> > >>> http://lists.freedesktop.org/archives/dri-devel/2012-July/024875.html > >> > >> ok, I agree with you. then the videomode helper is going to be merged > >> to mainline(3.6)? if so, this patch should be reworked based on the > >> videomode helper. > > > > I think the videomode helpers would be merged for 3.7 at the very > > earliest; 3.6 is cooked already. Given there are still some comments on > > the binding, perhaps it won't happen until 3.8, but it'd be best to ask > > on that thread so that people more directly involved with the status can > > answer. > > as I mentioned before, it's better to use videomode helper instead but > for this, we should wait for that the videomode helper are merged to > mainline so I think it's better to merge it as is and then modify it > for videomode helper to be used later. Aren't DT bindings considered as an ABI, and required to be supported more or less forever ? If you merge this DT binding you'll have to keep supporting it. That's why DT bindings should not be rushed in. -- Regards, Laurent Pinchart -- 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/3] DMA: PL330: Free memory allocated for peripheral channels
On Tue, Sep 25, 2012 at 2:27 PM, Inderpal Singh
wrote:
> The allocated memory for peripheral channels is not being freed upon
> failure in probe and in module's remove funtion. It will lead to memory
> leakage. Hence free the allocated memory.
>
> Signed-off-by: Inderpal Singh
> ---
> drivers/dma/pl330.c |5 -
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
> index 2ebd4cd..10c6b6a 100644
> --- a/drivers/dma/pl330.c
> +++ b/drivers/dma/pl330.c
> @@ -2962,7 +2962,7 @@ pl330_probe(struct amba_device *adev, const struct
> amba_id *id)
> ret = dma_async_device_register(pd);
> if (ret) {
> dev_err(&adev->dev, "unable to register DMAC\n");
> - goto probe_err4;
> + goto probe_err5;
> }
>
Sorry this patch seems malformed. Against which tree did you prepare it ?
-jassi
> dev_info(&adev->dev,
> @@ -2975,6 +2975,8 @@ pl330_probe(struct amba_device *adev, const struct
> amba_id *id)
>
> return 0;
>
> +probe_err5:
> + kfree(pdmac->peripherals);
> probe_err4:
> pl330_del(pi);
> probe_err3:
> @@ -3025,6 +3027,7 @@ static int __devexit pl330_remove(struct amba_device
> *adev)
> res = &adev->res;
> release_mem_region(res->start, resource_size(res));
>
> + kfree(pdmac->peripherals);
> kfree(pdmac);
>
> return 0;
> --
> 1.7.9.5
>
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 1/5] usb: phy: samsung: Introducing usb phy driver for hsotg
On 09/25/2012 06:23 AM, Praveen Paneri wrote: > Hi Rob, > > On Mon, Sep 24, 2012 at 6:34 PM, Rob Herring 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 >>> Acked-by: Heiko Stuebner >>> --- >>> .../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. Rob -- 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 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 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 >> Acked-by: Heiko Stuebner >> --- >> .../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 >> + * >> + * 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 >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> + >> +/* 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 (0x2 << 0) >> +#define S3C
Re: [PATCH v6 1/5] usb: phy: samsung: Introducing usb phy driver for hsotg
Hi Rob, On Mon, Sep 24, 2012 at 6:34 PM, Rob Herring 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 >> Acked-by: Heiko Stuebner >> --- >> .../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. Praveen > > Rob > >> 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 >> + * >> + * 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 >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> + >> +/* 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
Re: [RFC 6/6] ARM: dts: exynos4210: Add platform-specific descriptions for pin controllers
Hi Stephen,
On Monday 24 of September 2012 17:14:38 Stephen Warren wrote:
> On 09/24/2012 03:31 PM, Tomasz Figa wrote:
> > On Monday 24 of September 2012 11:42:15 Stephen Warren wrote:
> >> On 09/21/2012 01:54 PM, Tomasz Figa wrote:
> >>> On Friday 21 of September 2012 12:56:41 Stephen Warren wrote:
> On 09/20/2012 02:53 AM, Tomasz Figa wrote:
> > The patch "pinctrl: samsung: Parse pin banks from DT" introduced
> > platform-specific data parsing from DT.
> >
> > This patch adds all necessary nodes and properties to exynos4210
> > device
> > tree sources.
> >
> > +++ b/arch/arm/boot/dts/exynos4210-pinctrl-banks.dtsi
> >
> > + samsung,pctl-offset = <0x000>;
> > + samsung,pin-bank = "gpa0";
> > + samsung,pin-count = <8>;
> > + samsung,func-width = <4>;
> > + samsung,pud-width = <2>;
> > + samsung,drv-width = <2>;
> > + samsung,conpdn-width = <2>;
> > + samsung,pudpdn-width = <2>;
>
> The properties above represent the width of the fields. Must all
> fields
> always be packed together into say the LSB of the registers? What if
> there are gaps between the fields in a future SoC variant, or the
> order
> of the fields in the register changes? I think you want to add
> either
> a
> samsung,func-bit/samsung,func-position property for each of the
> fields,
> or change from samsung,func-width=<4> to samsung,field-mask=<0xf>.
> IIRC,
> the generic pinctrl binding used a mask for this purpose.
>
> What if a future SoC variant adds more fields to the register? At
> that
> point, you'd need to edit the driver anyway in order to define a new
> DT
> property to represent the new field. Perhaps instead of having an
> explicit named property per field in the register, you want a simple
> list of fields:
>
> samsung,pin-property-names = "func", "pud", "drv", "conpdn",
> "pudpdn";
> samsung,pin-propert-masks = <0xf 0x30 0xc0 0x300 0xc00>;
>
> That would allow a completely arbitrary number of fields and layouts
> to
> be described.
>
> I wonder if for absolute generality you want a samsung,pin-stride
> property to represent the difference in register address per pin. I
> assume that's hard-coded as 4 right now.
> >>>
> >>> Hmm, considering so many different possible changes, maybe a more
> >>> conservative solution would be better, like reducing the amount of
> >>> information held in DT to bank type, e.g.
> >>>
> >>> samsung,bank-type = "exynos4";
> >>>
> >>> or maybe
> >>>
> >>> compatible = "samsung,exynos4-pin-bank*;
> >>>
> >>> and then define supported bank types in the driver. SoC-specific data
> >>> would remain in DT, i.e. pctl-offset, pin-bank, pin-count,
> >>> eint-offset, etc.
> >>
> >> Yes, removing much of the data from DT and putting it into a tiny
> >> table
> >> in the driver makes sense to me in this case.
> >
> > A hybrid solution came to my mind, define bank types in device tree
> > once
> >
> > and then reference them in banks. Something like:
> > pinctrl-bank-types {
> >
> > bank_off: bank-off {
> >
> > samsung,func-width = <4>;
> > samsung,pud-width = <2>;
> > samsung,drv-width = <2>;
> > samsung,conpdn-width = <2>;
> > samsung,pudpdn-width = <2>;
> >
> > };
> >
> > bank_alive: bank-alive {
> >
> > samsung,func-width = <4>;
> > samsung,pud-width = <2>;
> > samsung,drv-width = <2>;
> >
> > };
> >
> > };
> >
> > /* ... */
> >
> > pinctrl@12345678 {
> >
> > /* ... */
> > gpa0: gpa0 {
> >
> > /* ... */
> > samsung,bank-type = <&bank_off>;
> > /* ... */
> >
> > };
> > /* ... */
> >
> > };
> >
> > This would add the possibility to define new banks types quickly, but
> > would not add too much overhead.
> >
> > What do you think?
>
> That does solve the issue of parsing those same values over and over,
> but at the cost of making the binding a lot more conceptually complex.
>
> However, it doesn't solve any of the extensibility issues I raised such
> as whether pos+size or mask would be a better representation, what about
> if the fields end up being in different (separate) registers on newer
> SoCs, etc.
Hmm, could you elaborate on the idea of using mask instead of field widths?
I don't see how this could be better and ther
[PATCH 1/3] DMA: PL330: Free memory allocated for peripheral channels
The allocated memory for peripheral channels is not being freed upon
failure in probe and in module's remove funtion. It will lead to memory
leakage. Hence free the allocated memory.
Signed-off-by: Inderpal Singh
---
drivers/dma/pl330.c |5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
index 2ebd4cd..10c6b6a 100644
--- a/drivers/dma/pl330.c
+++ b/drivers/dma/pl330.c
@@ -2962,7 +2962,7 @@ pl330_probe(struct amba_device *adev, const struct
amba_id *id)
ret = dma_async_device_register(pd);
if (ret) {
dev_err(&adev->dev, "unable to register DMAC\n");
- goto probe_err4;
+ goto probe_err5;
}
dev_info(&adev->dev,
@@ -2975,6 +2975,8 @@ pl330_probe(struct amba_device *adev, const struct
amba_id *id)
return 0;
+probe_err5:
+ kfree(pdmac->peripherals);
probe_err4:
pl330_del(pi);
probe_err3:
@@ -3025,6 +3027,7 @@ static int __devexit pl330_remove(struct amba_device
*adev)
res = &adev->res;
release_mem_region(res->start, resource_size(res));
+ kfree(pdmac->peripherals);
kfree(pdmac);
return 0;
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/3] DMA: PL330: Balance module remove function with probe
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
---
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);
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/3] DMA: PL330: Fix mem leaks and balance probe/remove
The first 2 patches of this series fix memory leaks because the memory allocated for peripheral channels and DMA descriptors were not getting freed. The third patch balances the module's remove function. This patchset is based on slave-dma tree's "next" branch merged with "fixes" branch and applied patch at [1] to fix the build error. [1] http://permalink.gmane.org/gmane.linux.kernel.next/24274 Inderpal Singh (3): DMA: PL330: Free memory allocated for peripheral channels DMA: PL330: Change allocation method to properly free DMA descriptors DMA: PL330: Balance module remove function with probe drivers/dma/pl330.c | 37 - 1 file changed, 24 insertions(+), 13 deletions(-) -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/3] DMA: PL330: Change allocation method to properly free DMA descriptors
In probe, memory for multiple DMA descriptors were being allocated at once
and then it was being split and added into DMA pool one by one. The address
of this memory allocation is not being saved anywhere. To free this memory,
the address is required. Initially the first node of the pool will be
pointed by this address but as we use this pool the descs will shuffle and
hence we will lose the track of the address.
This patch does following:
1. Allocates DMA descs one by one and then adds them to pool so that all
descs can be fetched and memory freed one by one. This way runtime
added descs can also be freed.
2. Free DMA descs in case of error in probe and in module's remove function
Signed-off-by: Inderpal Singh
---
drivers/dma/pl330.c | 28 +---
1 file changed, 21 insertions(+), 7 deletions(-)
diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
index 10c6b6a..04d83e6 100644
--- a/drivers/dma/pl330.c
+++ b/drivers/dma/pl330.c
@@ -2541,20 +2541,19 @@ static int add_desc(struct dma_pl330_dmac *pdmac, gfp_t
flg, int count)
if (!pdmac)
return 0;
- desc = kmalloc(count * sizeof(*desc), flg);
- if (!desc)
- return 0;
-
spin_lock_irqsave(&pdmac->pool_lock, flags);
for (i = 0; i < count; i++) {
- _init_desc(&desc[i]);
- list_add_tail(&desc[i].node, &pdmac->desc_pool);
+ desc = kmalloc(sizeof(*desc), flg);
+ if (!desc)
+ break;
+ _init_desc(desc);
+ list_add_tail(&desc->node, &pdmac->desc_pool);
}
spin_unlock_irqrestore(&pdmac->pool_lock, flags);
- return count;
+ return i;
}
static struct dma_pl330_desc *
@@ -2857,6 +2856,7 @@ pl330_probe(struct amba_device *adev, const struct
amba_id *id)
struct dma_pl330_platdata *pdat;
struct dma_pl330_dmac *pdmac;
struct dma_pl330_chan *pch;
+ struct dma_pl330_desc *desc;
struct pl330_info *pi;
struct dma_device *pd;
struct resource *res;
@@ -2978,6 +2978,12 @@ pl330_probe(struct amba_device *adev, const struct
amba_id *id)
probe_err5:
kfree(pdmac->peripherals);
probe_err4:
+ while (!list_empty(&pdmac->desc_pool)) {
+ desc = list_entry(pdmac->desc_pool.next,
+ struct dma_pl330_desc, node);
+ list_del(&desc->node);
+ kfree(desc);
+ }
pl330_del(pi);
probe_err3:
free_irq(irq, pi);
@@ -2994,6 +3000,7 @@ static int __devexit pl330_remove(struct amba_device
*adev)
{
struct dma_pl330_dmac *pdmac = amba_get_drvdata(adev);
struct dma_pl330_chan *pch, *_p;
+ struct dma_pl330_desc *desc;
struct pl330_info *pi;
struct resource *res;
int irq;
@@ -3015,6 +3022,13 @@ static int __devexit pl330_remove(struct amba_device
*adev)
pl330_free_chan_resources(&pch->chan);
}
+ while (!list_empty(&pdmac->desc_pool)) {
+ desc = list_entry(pdmac->desc_pool.next,
+ struct dma_pl330_desc, node);
+ list_del(&desc->node);
+ kfree(desc);
+ }
+
pi = &pdmac->pif;
pl330_del(pi);
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
