[PATCH 1/3] ARM: s3c24xx: add dma_coherent_mask for DMA devices

[PATCH 1/3] ARM: s3c24xx: add dma_coherent_mask for DMA devices

[Vasily Khoruzhick]

Otherwise dma_alloc_coherent() fails.

Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
---
 arch/arm/mach-s3c24xx/common.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/mach-s3c24xx/common.c b/arch/arm/mach-s3c24xx/common.c
index bf50328..7ed0853 100644
--- a/arch/arm/mach-s3c24xx/common.c
+++ b/arch/arm/mach-s3c24xx/common.c
@@ -355,6 +355,7 @@ struct platform_device s3c2410_device_dma = {
 	.resource	= s3c2410_dma_resource,
 	.dev	= {
 		.platform_data	= &s3c2410_dma_platdata,
+		.coherent_dma_mask = ~0,
 	},
 };
 #endif
@@ -452,6 +453,7 @@ struct platform_device s3c2440_device_dma = {
 	.resource	= s3c2410_dma_resource,
 	.dev	= {
 		.platform_data	= &s3c2440_dma_platdata,
+		.coherent_dma_mask = ~0,
 	},
 };
 #endif
-- 
2.3.5

[PATCH 2/3] ARM: s3c24xx: Add DMA resources for SDI and I2S

[Vasily Khoruzhick]

Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
---
 arch/arm/plat-samsung/devs.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm/plat-samsung/devs.c b/arch/arm/plat-samsung/devs.c
index 83c7d15..62edcab 100644
--- a/arch/arm/plat-samsung/devs.c
+++ b/arch/arm/plat-samsung/devs.c
@@ -564,6 +564,8 @@ void __init s3c_i2c7_set_platdata(struct s3c2410_platform_i2c *pd)
 #ifdef CONFIG_PLAT_S3C24XX
 static struct resource s3c_iis_resource[] = {
 	[0] = DEFINE_RES_MEM(S3C24XX_PA_IIS, S3C24XX_SZ_IIS),
+	[1] = DEFINE_RES_DMA(DMACH_I2S_OUT),
+	[2] = DEFINE_RES_DMA(DMACH_I2S_IN),
 };
 
 struct platform_device s3c_device_iis = {
@@ -863,6 +865,7 @@ struct platform_device s3c_device_rtc = {
 static struct resource s3c_sdi_resource[] = {
 	[0] = DEFINE_RES_MEM(S3C24XX_PA_SDI, S3C24XX_SZ_SDI),
 	[1] = DEFINE_RES_IRQ(IRQ_SDI),
+	[2] = DEFINE_RES_DMA(DMACH_SDI),
 };
 
 struct platform_device s3c_device_sdi = {
-- 
2.3.5

[PATCH 2/3] ARM: s3c24xx: Add DMA resources for SDI and I2S

[Krzysztof Kozlowski]

2015-05-04 3:15 GMT+09:00 Vasily Khoruzhick <anarsoul@gmail.com>:
> Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>

"Add DMA resources for SDI and I2S".,.. what for? why? The commit
message would be appropriate place to put such details.

The patch itself looks good.

Acked-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>

Best regards,
Krzysztof

[PATCH 2/3] ARM: s3c24xx: Add DMA resources for SDI and I2S

[Vasily Khoruzhick]

On Mon, May 4, 2015 at 6:45 AM, Krzysztof Kozlowski
<k.kozlowski@samsung.com> wrote:
> 2015-05-04 3:15 GMT+09:00 Vasily Khoruzhick <anarsoul@gmail.com>:
>> Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
>
> "Add DMA resources for SDI and I2S".,.. what for? why? The commit
> message would be appropriate place to put such details.

Because SDI and I2S drivers need these resources to setup DMA channel properly.
Would you like me to resend the patch with this explanation?

> The patch itself looks good.
>
> Acked-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
>
> Best regards,
> Krzysztof

[PATCH 2/3] ARM: s3c24xx: Add DMA resources for SDI and I2S

[Krzysztof Kozlowski]

2015-05-05 3:10 GMT+09:00 Vasily Khoruzhick <anarsoul@gmail.com>:
> On Mon, May 4, 2015 at 6:45 AM, Krzysztof Kozlowski
> <k.kozlowski@samsung.com> wrote:
>> 2015-05-04 3:15 GMT+09:00 Vasily Khoruzhick <anarsoul@gmail.com>:
>>> Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
>>
>> "Add DMA resources for SDI and I2S".,.. what for? why? The commit
>> message would be appropriate place to put such details.
>
> Because SDI and I2S drivers need these resources to setup DMA channel properly.
> Would you like me to resend the patch with this explanation?

Yes, please resend. The new explanation above is sufficient.

Best regards,
Krzysztof

[PATCH 3/3] ARM: s3c24xx: H1940: Move gpiochip_add call into core_init() callback

[Vasily Khoruzhick]

gpiochip_add() allocates memory, however it's not possible anymore from
machine map_io() callback thus it failed and prevented machine from booting
properly.

Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
---
 arch/arm/mach-s3c24xx/mach-h1940.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-s3c24xx/mach-h1940.c b/arch/arm/mach-s3c24xx/mach-h1940.c
index 86d9ec7..744aa4f 100644
--- a/arch/arm/mach-s3c24xx/mach-h1940.c
+++ b/arch/arm/mach-s3c24xx/mach-h1940.c
@@ -777,9 +777,14 @@ static void __init h1940_map_io(void)
 
 	/* Add latch gpio chip, set latch initial value */
 	h1940_latch_control(0, 0);
-	WARN_ON(gpiochip_add(&h1940_latch_gpiochip));
 }
 
+static __init int h1940_gpiolib_init(void)
+{
+	return gpiochip_add(&h1940_latch_gpiochip);
+}
+core_initcall(h1940_gpiolib_init);
+
 static void __init h1940_init_time(void)
 {
 	s3c2410_init_clocks(12000000);
-- 
2.3.5

[PATCH 3/3] ARM: s3c24xx: H1940: Move gpiochip_add call into core_init() callback

[Krzysztof Kozlowski]

2015-05-04 3:15 GMT+09:00 Vasily Khoruzhick <anarsoul@gmail.com>:
> gpiochip_add() allocates memory, however it's not possible anymore

"...to call it..."? Something is missing in the sentence.

> from
> machine map_io() callback thus it failed and prevented machine from booting
> properly.
>
> Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
> ---
>  arch/arm/mach-s3c24xx/mach-h1940.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/mach-s3c24xx/mach-h1940.c b/arch/arm/mach-s3c24xx/mach-h1940.c
> index 86d9ec7..744aa4f 100644
> --- a/arch/arm/mach-s3c24xx/mach-h1940.c
> +++ b/arch/arm/mach-s3c24xx/mach-h1940.c
> @@ -777,9 +777,14 @@ static void __init h1940_map_io(void)
>
>         /* Add latch gpio chip, set latch initial value */
>         h1940_latch_control(0, 0);
> -       WARN_ON(gpiochip_add(&h1940_latch_gpiochip));
>  }
>
> +static __init int h1940_gpiolib_init(void)
> +{
> +       return gpiochip_add(&h1940_latch_gpiochip);
> +}
> +core_initcall(h1940_gpiolib_init);
> +

arch_initcall() or init_machine() callback seems more appropriate.
What do you think?

Best regards,
Krzysztof

[PATCH 3/3] ARM: s3c24xx: H1940: Move gpiochip_add call into core_init() callback

[Vasily Khoruzhick]

On Mon, May 4, 2015 at 6:23 AM, Krzysztof Kozlowski
<k.kozlowski@samsung.com> wrote:
> 2015-05-04 3:15 GMT+09:00 Vasily Khoruzhick <anarsoul@gmail.com>:
>> gpiochip_add() allocates memory, however it's not possible anymore
>
> "...to call it..."? Something is missing in the sentence.

"it" replaces "memory allocation" here (kzalloc fails)

>> from
>> machine map_io() callback thus it failed and prevented machine from booting
>> properly.
>>
>> Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
>> ---
>>  arch/arm/mach-s3c24xx/mach-h1940.c | 7 ++++++-
>>  1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/mach-s3c24xx/mach-h1940.c b/arch/arm/mach-s3c24xx/mach-h1940.c
>> index 86d9ec7..744aa4f 100644
>> --- a/arch/arm/mach-s3c24xx/mach-h1940.c
>> +++ b/arch/arm/mach-s3c24xx/mach-h1940.c
>> @@ -777,9 +777,14 @@ static void __init h1940_map_io(void)
>>
>>         /* Add latch gpio chip, set latch initial value */
>>         h1940_latch_control(0, 0);
>> -       WARN_ON(gpiochip_add(&h1940_latch_gpiochip));
>>  }
>>
>> +static __init int h1940_gpiolib_init(void)
>> +{
>> +       return gpiochip_add(&h1940_latch_gpiochip);
>> +}
>> +core_initcall(h1940_gpiolib_init);
>> +
>
> arch_initcall() or init_machine() callback seems more appropriate.
> What do you think?

I did the same way as Kukjin Kim did in samsung gpio driver.

> Best regards,
> Krzysztof

[PATCH 3/3] ARM: s3c24xx: H1940: Move gpiochip_add call into core_init() callback

[Krzysztof Kozlowski]

2015-05-05 3:27 GMT+09:00 Vasily Khoruzhick <anarsoul@gmail.com>:
> On Mon, May 4, 2015 at 6:23 AM, Krzysztof Kozlowski
> <k.kozlowski@samsung.com> wrote:
>> 2015-05-04 3:15 GMT+09:00 Vasily Khoruzhick <anarsoul@gmail.com>:
>>> gpiochip_add() allocates memory, however it's not possible anymore
>>
>> "...to call it..."? Something is missing in the sentence.
>
> "it" replaces "memory allocation" here (kzalloc fails)
>
>>> from
>>> machine map_io() callback thus it failed and prevented machine from booting
>>> properly.
>>>
>>> Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
>>> ---
>>>  arch/arm/mach-s3c24xx/mach-h1940.c | 7 ++++++-
>>>  1 file changed, 6 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/arch/arm/mach-s3c24xx/mach-h1940.c b/arch/arm/mach-s3c24xx/mach-h1940.c
>>> index 86d9ec7..744aa4f 100644
>>> --- a/arch/arm/mach-s3c24xx/mach-h1940.c
>>> +++ b/arch/arm/mach-s3c24xx/mach-h1940.c
>>> @@ -777,9 +777,14 @@ static void __init h1940_map_io(void)
>>>
>>>         /* Add latch gpio chip, set latch initial value */
>>>         h1940_latch_control(0, 0);
>>> -       WARN_ON(gpiochip_add(&h1940_latch_gpiochip));
>>>  }
>>>
>>> +static __init int h1940_gpiolib_init(void)
>>> +{
>>> +       return gpiochip_add(&h1940_latch_gpiochip);
>>> +}
>>> +core_initcall(h1940_gpiolib_init);
>>> +
>>
>> arch_initcall() or init_machine() callback seems more appropriate.
>> What do you think?
>
> I did the same way as Kukjin Kim did in samsung gpio driver.

But that was almost  years ago and actually this is not a core code
but arch code. Anyway I think this shouldn't matter so I have no
strong preference.

Best regards,
Krzysztof

[PATCH 1/3] ARM: s3c24xx: add dma_coherent_mask for DMA devices

[Krzysztof Kozlowski]

2015-05-04 3:15 GMT+09:00 Vasily Khoruzhick <anarsoul@gmail.com>:
> Otherwise dma_alloc_coherent() fails.

Could you say few more words about the failure?
Also the patch looks like a candidate for CC-stable along with Fixes tag.

>
> Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
> ---
>  arch/arm/mach-s3c24xx/common.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/arch/arm/mach-s3c24xx/common.c b/arch/arm/mach-s3c24xx/common.c
> index bf50328..7ed0853 100644
> --- a/arch/arm/mach-s3c24xx/common.c
> +++ b/arch/arm/mach-s3c24xx/common.c
> @@ -355,6 +355,7 @@ struct platform_device s3c2410_device_dma = {
>         .resource       = s3c2410_dma_resource,
>         .dev    = {
>                 .platform_data  = &s3c2410_dma_platdata,
> +               .coherent_dma_mask = ~0,

DMA_BIT_MASK(32)?

>         },
>  };
>  #endif
> @@ -452,6 +453,7 @@ struct platform_device s3c2440_device_dma = {
>         .resource       = s3c2410_dma_resource,
>         .dev    = {
>                 .platform_data  = &s3c2440_dma_platdata,
> +               .coherent_dma_mask = ~0,

Ditto.

Best regards,
Krzysztof

[PATCH 1/3] ARM: s3c24xx: add dma_coherent_mask for DMA devices

[Vasily Khoruzhick]

On Mon, May 4, 2015 at 6:42 AM, Krzysztof Kozlowski
<k.kozlowski@samsung.com> wrote:
> 2015-05-04 3:15 GMT+09:00 Vasily Khoruzhick <anarsoul@gmail.com>:
>> Otherwise dma_alloc_coherent() fails.
>
> Could you say few more words about the failure?
> Also the patch looks like a candidate for CC-stable along with Fixes tag.

Hm, any call of dma_alloc_coherent() will fail for s3c24xx-dmac
device. For example, asoc dmaengine driver. What exactly do
you want me to precise?

>>
>> Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
>> ---
>>  arch/arm/mach-s3c24xx/common.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/arch/arm/mach-s3c24xx/common.c b/arch/arm/mach-s3c24xx/common.c
>> index bf50328..7ed0853 100644
>> --- a/arch/arm/mach-s3c24xx/common.c
>> +++ b/arch/arm/mach-s3c24xx/common.c
>> @@ -355,6 +355,7 @@ struct platform_device s3c2410_device_dma = {
>>         .resource       = s3c2410_dma_resource,
>>         .dev    = {
>>                 .platform_data  = &s3c2410_dma_platdata,
>> +               .coherent_dma_mask = ~0,
>
> DMA_BIT_MASK(32)?
>
>>         },
>>  };
>>  #endif
>> @@ -452,6 +453,7 @@ struct platform_device s3c2440_device_dma = {
>>         .resource       = s3c2410_dma_resource,
>>         .dev    = {
>>                 .platform_data  = &s3c2440_dma_platdata,
>> +               .coherent_dma_mask = ~0,
>
> Ditto.
>
> Best regards,
> Krzysztof

[PATCH 1/3] ARM: s3c24xx: add dma_coherent_mask for DMA devices

[Krzysztof Kozlowski]

2015-05-05 2:54 GMT+09:00 Vasily Khoruzhick <anarsoul@gmail.com>:
> On Mon, May 4, 2015 at 6:42 AM, Krzysztof Kozlowski
> <k.kozlowski@samsung.com> wrote:
>> 2015-05-04 3:15 GMT+09:00 Vasily Khoruzhick <anarsoul@gmail.com>:
>>> Otherwise dma_alloc_coherent() fails.
>>
>> Could you say few more words about the failure?
>> Also the patch looks like a candidate for CC-stable along with Fixes tag.
>
> Hm, any call of dma_alloc_coherent() will fail for s3c24xx-dmac
> device. For example, asoc dmaengine driver. What exactly do
> you want me to precise?

Anything more than just "fails" would be welcome. Especially for
backporting to stable.

Best regards,
Krzysztof