* [PATCH] spi_mpc8xxx: issue with using definition of pram in Device Tree
@ 2010-09-16 7:05 christophe leroy
2010-09-24 7:10 ` Grant Likely
0 siblings, 1 reply; 6+ messages in thread
From: christophe leroy @ 2010-09-16 7:05 UTC (permalink / raw)
To: David Brownell, Grant Likely, spi-devel-general, linux-kernel,
linuxppc-dev
This patch applies to 2.6.34.7 and 2.6.35.4
It fixes an issue during the probe for CPM1 with definition of parameter ram from DTS
Signed-off-by: christophe leroy <christophe.leroy@c-s.fr>
diff -urN b/drivers/spi/spi_mpc8xxx.c c/drivers/spi/spi_mpc8xxx.c
--- b/drivers/spi/spi_mpc8xxx.c 2010-09-08 16:43:50.000000000 +0200
+++ c/drivers/spi/spi_mpc8xxx.c 2010-09-08 16:44:03.000000000 +0200
@@ -822,7 +822,7 @@
if (!iprop || size != sizeof(*iprop) * 4)
return -ENOMEM;
- spi_base_ofs = cpm_muram_alloc_fixed(iprop[2], 2);
+ spi_base_ofs = iprop[2];
if (IS_ERR_VALUE(spi_base_ofs))
return -ENOMEM;
@@ -844,7 +844,6 @@
return spi_base_ofs;
}
- cpm_muram_free(spi_base_ofs);
return pram_ofs;
}
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] spi_mpc8xxx: issue with using definition of pram in Device Tree
2010-09-16 7:05 [PATCH] spi_mpc8xxx: issue with using definition of pram in Device Tree christophe leroy
@ 2010-09-24 7:10 ` Grant Likely
2010-09-24 7:20 ` LEROY Christophe
2010-09-24 15:07 ` Scott Wood
0 siblings, 2 replies; 6+ messages in thread
From: Grant Likely @ 2010-09-24 7:10 UTC (permalink / raw)
To: christophe leroy
Cc: David Brownell, linux-kernel, spi-devel-general, Anton Vorontsov,
linuxppc-dev
On Thu, Sep 16, 2010 at 09:05:03AM +0200, christophe leroy wrote:
> This patch applies to 2.6.34.7 and 2.6.35.4
> It fixes an issue during the probe for CPM1 with definition of parameter ram from DTS
>
> Signed-off-by: christophe leroy <christophe.leroy@c-s.fr>
I'm sorry, I don't understand the fix from the given description.
What is the problem, and why is cpm_muram_alloc_fixed() the wrong
thing to call on CPM1? Does CPM2 still need it?
g.
>
> diff -urN b/drivers/spi/spi_mpc8xxx.c c/drivers/spi/spi_mpc8xxx.c
> --- b/drivers/spi/spi_mpc8xxx.c 2010-09-08 16:43:50.000000000 +0200
> +++ c/drivers/spi/spi_mpc8xxx.c 2010-09-08 16:44:03.000000000 +0200
> @@ -822,7 +822,7 @@
> if (!iprop || size != sizeof(*iprop) * 4)
> return -ENOMEM;
>
> - spi_base_ofs = cpm_muram_alloc_fixed(iprop[2], 2);
> + spi_base_ofs = iprop[2];
> if (IS_ERR_VALUE(spi_base_ofs))
> return -ENOMEM;
>
> @@ -844,7 +844,6 @@
> return spi_base_ofs;
> }
>
> - cpm_muram_free(spi_base_ofs);
> return pram_ofs;
> }
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] spi_mpc8xxx: issue with using definition of pram in Device Tree
2010-09-24 7:10 ` Grant Likely
@ 2010-09-24 7:20 ` LEROY Christophe
2010-09-24 7:57 ` Anton Vorontsov
2010-09-24 15:07 ` Scott Wood
1 sibling, 1 reply; 6+ messages in thread
From: LEROY Christophe @ 2010-09-24 7:20 UTC (permalink / raw)
To: Grant Likely
Cc: David Brownell, linux-kernel, spi-devel-general, Anton Vorontsov,
linuxppc-dev
[-- Attachment #1: Type: text/plain, Size: 1618 bytes --]
Hello,
The issue is that cpm_muram_alloc_fixed() allocates memory from the
general purpose muram area (from 0x0 to 0x1bff).
Here we need to return a pointer to the parameter RAM, which is located
somewhere starting at 0x1c00. It is not a dynamic allocation that is
required here but only to point on the correct location in the parameter
RAM.
For the CPM2, I don't know. I'm working with a MPC866.
Attached is a previous discussion on the subject where I explain a bit
more in details the issue.
Regards
C. Leroy
Le 24/09/2010 09:10, Grant Likely a écrit :
> On Thu, Sep 16, 2010 at 09:05:03AM +0200, christophe leroy wrote:
>> This patch applies to 2.6.34.7 and 2.6.35.4
>> It fixes an issue during the probe for CPM1 with definition of parameter ram from DTS
>>
>> Signed-off-by: christophe leroy<christophe.leroy@c-s.fr>
> I'm sorry, I don't understand the fix from the given description.
> What is the problem, and why is cpm_muram_alloc_fixed() the wrong
> thing to call on CPM1? Does CPM2 still need it?
>
> g.
>
>> diff -urN b/drivers/spi/spi_mpc8xxx.c c/drivers/spi/spi_mpc8xxx.c
>> --- b/drivers/spi/spi_mpc8xxx.c 2010-09-08 16:43:50.000000000 +0200
>> +++ c/drivers/spi/spi_mpc8xxx.c 2010-09-08 16:44:03.000000000 +0200
>> @@ -822,7 +822,7 @@
>> if (!iprop || size != sizeof(*iprop) * 4)
>> return -ENOMEM;
>>
>> - spi_base_ofs = cpm_muram_alloc_fixed(iprop[2], 2);
>> + spi_base_ofs = iprop[2];
>> if (IS_ERR_VALUE(spi_base_ofs))
>> return -ENOMEM;
>>
>> @@ -844,7 +844,6 @@
>> return spi_base_ofs;
>> }
>>
>> - cpm_muram_free(spi_base_ofs);
>> return pram_ofs;
>> }
[-- Attachment #2: Message joint --]
[-- Type: message/rfc822, Size: 6373 bytes --]
From: Scott Wood <scottwood@freescale.com>
To: LEROY Christophe <christophe.leroy@c-s.fr>
Cc: Kumar Gala <kumar.gala@freescale.com>, LinuxPPC-dev <linuxppc-dev@lists.ozlabs.org>
Subject: Re: Small issue at init with spi_mpc8xxx.c with CPM1
Date: Tue, 7 Sep 2010 15:00:38 -0500
Message-ID: <20100907150038.57a7b065@schlenkerla.am.freescale.net>
On Tue, 7 Sep 2010 11:17:17 +0200
LEROY Christophe <christophe.leroy@c-s.fr> wrote:
>
> Dear Kumar,
>
> I have a small issue in the init of spi_mpc8xxx.c with MPC866 (CPM1)
>
> Unlike cpm_uart that maps the parameter ram directly using
> of_iomap(np,1), spi_mpc8xxx.c uses cpm_muram_alloc_fixed().
>
> This has two impacts in the .dts file:
> * The driver must be declared with pram at 1d80 instead of 3d80 whereas
> it is not a child of muram@2000 but a child of cpm@9c0
> * muram@2000/data@0 must be declared with reg = <0x0 0x2000> whereas
> is should be reg=<0x0 0x1c00> to avoid cpm_muram_alloc() to allocate
> space from parameters ram.
>
> Maybe I misunderstood something ?
Don't make the device tree lie, fix the driver instead.
The allocator should not be given any chunks of muram that are
dedicated to a fixed purpose -- it might hand it out to something else
before you reserve it. I don't think that cpm_muram_alloc_fixed() has
any legitimate use at all.
-Scott
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] spi_mpc8xxx: issue with using definition of pram in Device Tree
2010-09-24 7:20 ` LEROY Christophe
@ 2010-09-24 7:57 ` Anton Vorontsov
2010-09-24 15:12 ` Scott Wood
0 siblings, 1 reply; 6+ messages in thread
From: Anton Vorontsov @ 2010-09-24 7:57 UTC (permalink / raw)
To: LEROY Christophe
Cc: David Brownell, linux-kernel, spi-devel-general, linuxppc-dev
Hello,
On Fri, Sep 24, 2010 at 09:20:27AM +0200, LEROY Christophe wrote:
> The issue is that cpm_muram_alloc_fixed() allocates memory from the
> general purpose muram area (from 0x0 to 0x1bff).
> Here we need to return a pointer to the parameter RAM, which is
> located somewhere starting at 0x1c00. It is not a dynamic allocation
> that is required here but only to point on the correct location in
> the parameter RAM.
>
> For the CPM2, I don't know. I'm working with a MPC866.
>
> Attached is a previous discussion on the subject where I explain a
> bit more in details the issue.
The patch looks OK, I think.
Doesn't explain why that worked on MPC8272 (CPM2) and MPC8560
(also CPM2) machines though. But here's my guess (I no longer
have these boards to test it):
On 8272 I used this node:
+ spi@4c0 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ compatible = "fsl,cpm2-spi", "fsl,spi";
+ reg = <0x11a80 0x40 0x89fc 0x2>;
On that SOC there are two muram data regions 0x0..0x2000 and
0x9000..0x9100. Note that we actually don't want "data" regions,
and the only reason why that worked is that sysdev/cpm_common.c
maps muram(0)..muram(max).
Thanks,
--
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] spi_mpc8xxx: issue with using definition of pram in Device Tree
2010-09-24 7:10 ` Grant Likely
2010-09-24 7:20 ` LEROY Christophe
@ 2010-09-24 15:07 ` Scott Wood
1 sibling, 0 replies; 6+ messages in thread
From: Scott Wood @ 2010-09-24 15:07 UTC (permalink / raw)
To: Grant Likely
Cc: christophe leroy, David Brownell, linux-kernel, spi-devel-general,
Anton Vorontsov, linuxppc-dev
On Fri, 24 Sep 2010 01:10:06 -0600
Grant Likely <grant.likely@secretlab.ca> wrote:
> On Thu, Sep 16, 2010 at 09:05:03AM +0200, christophe leroy wrote:
> > This patch applies to 2.6.34.7 and 2.6.35.4
> > It fixes an issue during the probe for CPM1 with definition of parameter ram from DTS
> >
> > Signed-off-by: christophe leroy <christophe.leroy@c-s.fr>
>
> I'm sorry, I don't understand the fix from the given description.
> What is the problem, and why is cpm_muram_alloc_fixed() the wrong
> thing to call on CPM1? Does CPM2 still need it?
I don't see how cpm_muram_alloc_fixed() can be used safely at all. If
you need a fixed address, it shouldn't be part of the general
allocation pool, or something else might get it first.
-Scott
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] spi_mpc8xxx: issue with using definition of pram in Device Tree
2010-09-24 7:57 ` Anton Vorontsov
@ 2010-09-24 15:12 ` Scott Wood
0 siblings, 0 replies; 6+ messages in thread
From: Scott Wood @ 2010-09-24 15:12 UTC (permalink / raw)
To: Anton Vorontsov
Cc: LEROY Christophe, spi-devel-general, David Brownell, linuxppc-dev,
linux-kernel
On Fri, 24 Sep 2010 11:57:40 +0400
Anton Vorontsov <cbouatmailru@gmail.com> wrote:
> Doesn't explain why that worked on MPC8272 (CPM2) and MPC8560
> (also CPM2) machines though. But here's my guess (I no longer
> have these boards to test it):
>
> On 8272 I used this node:
>
> + spi@4c0 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + compatible = "fsl,cpm2-spi", "fsl,spi";
> + reg = <0x11a80 0x40 0x89fc 0x2>;
>
> On that SOC there are two muram data regions 0x0..0x2000 and
> 0x9000..0x9100. Note that we actually don't want "data" regions,
> and the only reason why that worked is that sysdev/cpm_common.c
> maps muram(0)..muram(max).
Wouldn't it still fail the rh_alloc_fixed call?
-Scott
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-09-24 15:12 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-09-16 7:05 [PATCH] spi_mpc8xxx: issue with using definition of pram in Device Tree christophe leroy
2010-09-24 7:10 ` Grant Likely
2010-09-24 7:20 ` LEROY Christophe
2010-09-24 7:57 ` Anton Vorontsov
2010-09-24 15:12 ` Scott Wood
2010-09-24 15:07 ` Scott Wood
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).