* [PATCH] spi: fsl-spi: use of_iomap() to map parameter ram on CPM1
@ 2015-02-26 16:11 Christophe Leroy
2015-03-03 18:44 ` Mark Brown
0 siblings, 1 reply; 6+ messages in thread
From: Christophe Leroy @ 2015-02-26 16:11 UTC (permalink / raw)
To: Mark Brown; +Cc: linuxppc-dev, linux-kernel, linux-spi
On CPM2, the SPI parameter RAM is dynamically allocated in the dualport RAM
whereas in CPM1, it is statically allocated to a default address with
capability to relocate it somewhere else via the use of CPM micropatch.
The address of the parameter RAM is given by the boot loader and expected
to be mapped via of_iomap()
In the current implementation, in function fsl_spi_cpm_get_pram() there is
a confusion between the SPI_BASE register and the base of the SPI
parameter RAM. Fortunatly, it is working properly with MPC866 and MPC885
because they do set SPI_BASE, but on MPC860 and other old MPC8xx that doesn't
set SPI_BASE, pram_ofs is not properly set.
Also, the parameter RAM is not properly mapped with of_iomap() as it should
but still gets accessible by chance through the full RAM which is mapped
from somewhere else.
This patch applies to the SPI driver the same principle as for the CPM UART:
when the CPM is of type CPM1, we simply do an of_iomap() of the area provided
via the device tree.
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
drivers/spi/spi-fsl-cpm.c | 32 +++++++++++++++-----------------
1 file changed, 15 insertions(+), 17 deletions(-)
diff --git a/drivers/spi/spi-fsl-cpm.c b/drivers/spi/spi-fsl-cpm.c
index e85ab1c..97c28d7 100644
--- a/drivers/spi/spi-fsl-cpm.c
+++ b/drivers/spi/spi-fsl-cpm.c
@@ -264,17 +264,6 @@ static unsigned long fsl_spi_cpm_get_pram(struct mpc8xxx_spi *mspi)
if (mspi->flags & SPI_CPM2) {
pram_ofs = cpm_muram_alloc(SPI_PRAM_SIZE, 64);
out_be16(spi_base, pram_ofs);
- } else {
- struct spi_pram __iomem *pram = spi_base;
- u16 rpbase = in_be16(&pram->rpbase);
-
- /* Microcode relocation patch applied? */
- if (rpbase) {
- pram_ofs = rpbase;
- } else {
- pram_ofs = cpm_muram_alloc(SPI_PRAM_SIZE, 64);
- out_be16(spi_base, pram_ofs);
- }
}
iounmap(spi_base);
@@ -287,7 +276,6 @@ int fsl_spi_cpm_init(struct mpc8xxx_spi *mspi)
struct device_node *np = dev->of_node;
const u32 *iprop;
int size;
- unsigned long pram_ofs;
unsigned long bds_ofs;
if (!(mspi->flags & SPI_CPM_MODE))
@@ -314,8 +302,17 @@ int fsl_spi_cpm_init(struct mpc8xxx_spi *mspi)
}
}
- pram_ofs = fsl_spi_cpm_get_pram(mspi);
- if (IS_ERR_VALUE(pram_ofs)) {
+ if (mspi->flags & SPI_CPM1) {
+ mspi->pram = of_iomap(np, 1);
+ } else {
+ unsigned long pram_ofs = fsl_spi_cpm_get_pram(mspi);
+
+ if (IS_ERR_VALUE(pram_ofs))
+ mspi->pram = NULL;
+ else
+ mspi->pram = cpm_muram_addr(pram_ofs);
+ }
+ if (mspi->pram == NULL) {
dev_err(dev, "can't allocate spi parameter ram\n");
goto err_pram;
}
@@ -341,8 +338,6 @@ int fsl_spi_cpm_init(struct mpc8xxx_spi *mspi)
goto err_dummy_rx;
}
- mspi->pram = cpm_muram_addr(pram_ofs);
-
mspi->tx_bd = cpm_muram_addr(bds_ofs);
mspi->rx_bd = cpm_muram_addr(bds_ofs + sizeof(*mspi->tx_bd));
@@ -370,7 +365,10 @@ err_dummy_rx:
err_dummy_tx:
cpm_muram_free(bds_ofs);
err_bds:
- cpm_muram_free(pram_ofs);
+ if (mspi->flags & SPI_CPM1)
+ iounmap(mspi->pram);
+ else
+ cpm_muram_free(cpm_muram_offset(mspi->pram));
err_pram:
fsl_spi_free_dummy_rx();
return -ENOMEM;
--
2.1.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] spi: fsl-spi: use of_iomap() to map parameter ram on CPM1
2015-02-26 16:11 [PATCH] spi: fsl-spi: use of_iomap() to map parameter ram on CPM1 Christophe Leroy
@ 2015-03-03 18:44 ` Mark Brown
2015-03-04 8:00 ` leroy christophe
0 siblings, 1 reply; 6+ messages in thread
From: Mark Brown @ 2015-03-03 18:44 UTC (permalink / raw)
To: Christophe Leroy; +Cc: linuxppc-dev, linux-kernel, linux-spi
[-- Attachment #1: Type: text/plain, Size: 474 bytes --]
On Thu, Feb 26, 2015 at 05:11:42PM +0100, Christophe Leroy wrote:
> On CPM2, the SPI parameter RAM is dynamically allocated in the dualport RAM
> whereas in CPM1, it is statically allocated to a default address with
> capability to relocate it somewhere else via the use of CPM micropatch.
> The address of the parameter RAM is given by the boot loader and expected
> to be mapped via of_iomap()
Why are we using of_iomap() rather than a generic I/O mapping function
here?
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] spi: fsl-spi: use of_iomap() to map parameter ram on CPM1
2015-03-03 18:44 ` Mark Brown
@ 2015-03-04 8:00 ` leroy christophe
2015-03-06 11:44 ` Mark Brown
0 siblings, 1 reply; 6+ messages in thread
From: leroy christophe @ 2015-03-04 8:00 UTC (permalink / raw)
To: Mark Brown; +Cc: linuxppc-dev, linux-kernel, linux-spi
Le 03/03/2015 19:44, Mark Brown a écrit :
> On Thu, Feb 26, 2015 at 05:11:42PM +0100, Christophe Leroy wrote:
>> On CPM2, the SPI parameter RAM is dynamically allocated in the dualport RAM
>> whereas in CPM1, it is statically allocated to a default address with
>> capability to relocate it somewhere else via the use of CPM micropatch.
>> The address of the parameter RAM is given by the boot loader and expected
>> to be mapped via of_iomap()
> Why are we using of_iomap() rather than a generic I/O mapping function
> here?
Euh ...
because all drivers for powerpc seems to be using of_iomap(), as on
powerpc the HW is described by the bootloader in a OF device tree.
Today, of_iomap() is at least used in FSL SPI, FSL UART, SPI mpc52xx,
UART mpc52xx, i2c-mpc, i2c-cpm, freescale ethernet drivers, etc ....
Is it not correct ?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] spi: fsl-spi: use of_iomap() to map parameter ram on CPM1
2015-03-04 8:00 ` leroy christophe
@ 2015-03-06 11:44 ` Mark Brown
2015-03-12 6:52 ` leroy christophe
0 siblings, 1 reply; 6+ messages in thread
From: Mark Brown @ 2015-03-06 11:44 UTC (permalink / raw)
To: leroy christophe; +Cc: linuxppc-dev, linux-kernel, linux-spi
[-- Attachment #1: Type: text/plain, Size: 694 bytes --]
On Wed, Mar 04, 2015 at 09:00:39AM +0100, leroy christophe wrote:
> Le 03/03/2015 19:44, Mark Brown a écrit :
> >Why are we using of_iomap() rather than a generic I/O mapping function
> >here?
> because all drivers for powerpc seems to be using of_iomap(), as on powerpc
> the HW is described by the bootloader in a OF device tree.
> Today, of_iomap() is at least used in FSL SPI, FSL UART, SPI mpc52xx, UART
> mpc52xx, i2c-mpc, i2c-cpm, freescale ethernet drivers, etc ....
> Is it not correct ?
It's legacy, all that code is really old. Modern code is written in as
architecture and firmware neutral a fashion as possible to make things
more consistent and maintainable.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] spi: fsl-spi: use of_iomap() to map parameter ram on CPM1
2015-03-06 11:44 ` Mark Brown
@ 2015-03-12 6:52 ` leroy christophe
2015-03-18 12:03 ` Mark Brown
0 siblings, 1 reply; 6+ messages in thread
From: leroy christophe @ 2015-03-12 6:52 UTC (permalink / raw)
To: Mark Brown; +Cc: linuxppc-dev, linux-kernel, linux-spi
Le 06/03/2015 12:44, Mark Brown a écrit :
> On Wed, Mar 04, 2015 at 09:00:39AM +0100, leroy christophe wrote:
>> Le 03/03/2015 19:44, Mark Brown a écrit :
>>> Why are we using of_iomap() rather than a generic I/O mapping function
>>> here?
>> because all drivers for powerpc seems to be using of_iomap(), as on powerpc
>> the HW is described by the bootloader in a OF device tree.
>> Today, of_iomap() is at least used in FSL SPI, FSL UART, SPI mpc52xx, UART
>> mpc52xx, i2c-mpc, i2c-cpm, freescale ethernet drivers, etc ....
>> Is it not correct ?
> It's legacy, all that code is really old. Modern code is written in as
> architecture and firmware neutral a fashion as possible to make things
> more consistent and maintainable.
This patch is only a small bug fix.
That driver already contains calls to of_iomap() and other related of_
functions.
Is it worth rewriting the driver for just a small bug fix ?
Christophe
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] spi: fsl-spi: use of_iomap() to map parameter ram on CPM1
2015-03-12 6:52 ` leroy christophe
@ 2015-03-18 12:03 ` Mark Brown
0 siblings, 0 replies; 6+ messages in thread
From: Mark Brown @ 2015-03-18 12:03 UTC (permalink / raw)
To: leroy christophe; +Cc: linuxppc-dev, linux-kernel, linux-spi
[-- Attachment #1: Type: text/plain, Size: 628 bytes --]
On Thu, Mar 12, 2015 at 07:52:53AM +0100, leroy christophe wrote:
> Le 06/03/2015 12:44, Mark Brown a écrit :
> >It's legacy, all that code is really old. Modern code is written in as
> >architecture and firmware neutral a fashion as possible to make things
> >more consistent and maintainable.
> This patch is only a small bug fix.
> That driver already contains calls to of_iomap() and other related of_
> functions.
> Is it worth rewriting the driver for just a small bug fix ?
I don't think you need to fix other usages but I'd rather not introduce
new usages of legacy APIs that just need to be fixed up.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-03-18 12:03 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-02-26 16:11 [PATCH] spi: fsl-spi: use of_iomap() to map parameter ram on CPM1 Christophe Leroy
2015-03-03 18:44 ` Mark Brown
2015-03-04 8:00 ` leroy christophe
2015-03-06 11:44 ` Mark Brown
2015-03-12 6:52 ` leroy christophe
2015-03-18 12:03 ` Mark Brown
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).