* [PATCH v3 0/9] davinci: Add spi support for da8xx platforms
From: Michael Williamson @ 2011-02-22 13:36 UTC (permalink / raw)
To: linux-arm-kernel
This patch series adds the necessary SPI resources and registration
routines for da850/OMAP-L138/AM18x and da830/OMAP-L137/AM17x devices.
It removes some unused enumerations related to the DMA channels
for the da830 platform as a result of initial reviews of the patch
series.
It moves the event queue number for the SPI DMA control out of
the resources array and into the platform data structure passed to
the davinci spi driver.
It adds on-board SPI FLASH devices for the da830 evm, the da850
evm, and the MityDSP-L138/MityARM-1808 platforms.
These patches are based on work done during testing of davinci SPI driver
submissions incorporated in version 2.6.38 of the kernel, at [1].
This patch series has been tested using a MityDSP-L138 platform,
and the da850 and da830 EVMs.
The patch series is against linux-davinci tree. The davinci spi driver
portion of this patch has been reviewed by the spi list and Acked
by Grant with the agreement to submit through linux-davinci tree.
[1] http://arago-project.org/git/projects/?p=linux-davinci.git;a=shortlog;h=refs/heads/davinci-spi-rewrite
---
Changes since v2:
- combined previously approved / Acked patches into this
patch series per request from maintainer.
- fixed whitespace issue identified by checkpatch
- added function name for warning tracing per comments
Michael Williamson (7):
davinci: remove unused DA830_edma_ch enum
davinci: da8xx: clean up magic numbers in devices-da8xx.c
davinci: spi: move event queue parameter to platform data
davinci: da830: fix driver name for spi clocks
davinci: da850: add spi device clock definitions
davinci: da8xx: add spi resources and registration routine
davinci: add spi devices support for MityDSP-L138/MityARM-1808
platform
Sekhar Nori (2):
davinci: add spi devices support for da850/omap-l138/am18x evm
davinci: add spi devices support for da830/omap-l137/am17x evm
arch/arm/mach-davinci/board-da830-evm.c | 80 +++++++++++++++++++
arch/arm/mach-davinci/board-da850-evm.c | 86 ++++++++++++++++++++
arch/arm/mach-davinci/board-mityomapl138.c | 100 +++++++++++++++++++++++
arch/arm/mach-davinci/da830.c | 4 +-
arch/arm/mach-davinci/da850.c | 16 ++++
arch/arm/mach-davinci/devices-da8xx.c | 119 ++++++++++++++++++++++++++--
arch/arm/mach-davinci/dm355.c | 5 +-
arch/arm/mach-davinci/dm365.c | 5 +-
arch/arm/mach-davinci/include/mach/da8xx.h | 3 +
arch/arm/mach-davinci/include/mach/edma.h | 36 ---------
arch/arm/mach-davinci/include/mach/spi.h | 15 +++-
drivers/spi/davinci_spi.c | 11 +--
12 files changed, 413 insertions(+), 67 deletions(-)
^ permalink raw reply
* [PATCH v4] ARM: mxs: add dma device
From: Shawn Guo @ 2011-02-22 13:36 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1298284306-14710-3-git-send-email-shawn.guo@freescale.com>
Signed-off-by: Shawn Guo <shawn.guo@freescale.com>
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
arch/arm/mach-mxs/clock-mx23.c | 3 +-
arch/arm/mach-mxs/clock-mx28.c | 4 +-
arch/arm/mach-mxs/devices/Makefile | 1 +
arch/arm/mach-mxs/devices/platform-dma.c | 49 ++++++++++++++++++++++++++++++
4 files changed, 54 insertions(+), 3 deletions(-)
create mode 100644 arch/arm/mach-mxs/devices/platform-dma.c
diff --git a/arch/arm/mach-mxs/clock-mx23.c b/arch/arm/mach-mxs/clock-mx23.c
index b1a362e..350b28c 100644
--- a/arch/arm/mach-mxs/clock-mx23.c
+++ b/arch/arm/mach-mxs/clock-mx23.c
@@ -443,7 +443,8 @@ static struct clk_lookup lookups[] = {
/* for amba-pl011 driver */
_REGISTER_CLOCK("duart", NULL, uart_clk)
_REGISTER_CLOCK("rtc", NULL, rtc_clk)
- _REGISTER_CLOCK(NULL, "hclk", hbus_clk)
+ _REGISTER_CLOCK("mxs-dma-apbh", NULL, hbus_clk)
+ _REGISTER_CLOCK("mxs-dma-apbx", NULL, xbus_clk)
_REGISTER_CLOCK(NULL, "usb", usb_clk)
_REGISTER_CLOCK(NULL, "audio", audio_clk)
_REGISTER_CLOCK(NULL, "pwm", pwm_clk)
diff --git a/arch/arm/mach-mxs/clock-mx28.c b/arch/arm/mach-mxs/clock-mx28.c
index c9d7951..a3b4787 100644
--- a/arch/arm/mach-mxs/clock-mx28.c
+++ b/arch/arm/mach-mxs/clock-mx28.c
@@ -617,8 +617,8 @@ static struct clk_lookup lookups[] = {
_REGISTER_CLOCK("fec.0", NULL, fec_clk)
_REGISTER_CLOCK("rtc", NULL, rtc_clk)
_REGISTER_CLOCK("pll2", NULL, pll2_clk)
- _REGISTER_CLOCK(NULL, "hclk", hbus_clk)
- _REGISTER_CLOCK(NULL, "xclk", xbus_clk)
+ _REGISTER_CLOCK("mxs-dma-apbh", NULL, hbus_clk)
+ _REGISTER_CLOCK("mxs-dma-apbx", NULL, xbus_clk)
_REGISTER_CLOCK("flexcan.0", NULL, can0_clk)
_REGISTER_CLOCK("flexcan.1", NULL, can1_clk)
_REGISTER_CLOCK(NULL, "usb0", usb0_clk)
diff --git a/arch/arm/mach-mxs/devices/Makefile b/arch/arm/mach-mxs/devices/Makefile
index a8dc8d5..ca7acc4 100644
--- a/arch/arm/mach-mxs/devices/Makefile
+++ b/arch/arm/mach-mxs/devices/Makefile
@@ -1,4 +1,5 @@
obj-$(CONFIG_MXS_HAVE_AMBA_DUART) += amba-duart.o
obj-$(CONFIG_MXS_HAVE_PLATFORM_AUART) += platform-auart.o
+obj-y += platform-dma.o
obj-$(CONFIG_MXS_HAVE_PLATFORM_FEC) += platform-fec.o
obj-$(CONFIG_MXS_HAVE_PLATFORM_FLEXCAN) += platform-flexcan.o
diff --git a/arch/arm/mach-mxs/devices/platform-dma.c b/arch/arm/mach-mxs/devices/platform-dma.c
new file mode 100644
index 0000000..295c442
--- /dev/null
+++ b/arch/arm/mach-mxs/devices/platform-dma.c
@@ -0,0 +1,49 @@
+/*
+ * Copyright 2011 Freescale Semiconductor, Inc. All Rights Reserved.
+ *
+ * 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.
+ */
+#include <linux/compiler.h>
+#include <linux/err.h>
+#include <linux/init.h>
+
+#include <mach/mx23.h>
+#include <mach/mx28.h>
+#include <mach/devices-common.h>
+
+static struct platform_device *__init mxs_add_dma(const char *devid,
+ resource_size_t base)
+{
+ struct resource res[] = {
+ {
+ .start = base,
+ .end = base + SZ_8K - 1,
+ .flags = IORESOURCE_MEM,
+ }
+ };
+
+ return mxs_add_platform_device_dmamask(devid, -1,
+ res, ARRAY_SIZE(res), NULL, 0,
+ DMA_BIT_MASK(32));
+}
+
+static int __init mxs_add_mxs_dma(void)
+{
+ char *apbh = "mxs-dma-apbh";
+ char *apbx = "mxs-dma-apbx";
+
+ if (cpu_is_mx23()) {
+ mxs_add_dma(apbh, MX23_APBH_DMA_BASE_ADDR);
+ mxs_add_dma(apbx, MX23_APBX_DMA_BASE_ADDR);
+ }
+
+ if (cpu_is_mx28()) {
+ mxs_add_dma(apbh, MX28_APBH_DMA_BASE_ADDR);
+ mxs_add_dma(apbx, MX28_APBX_DMA_BASE_ADDR);
+ }
+
+ return 0;
+}
+arch_initcall(mxs_add_mxs_dma);
--
1.7.1
^ permalink raw reply related
* [PATCH 5/5] arm: mach-mx3: use IMX_GPIO_NR instead of hard-coded values
From: Lothar Waßmann @ 2011-02-22 13:30 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1298379500-27590-6-git-send-email-w.sang@pengutronix.de>
Hi,
> The latter are error-prone because the bank number is one less than one
> would read in the documentation.
>
> Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
> Cc: Eric B?nard <eric@eukrea.com>
> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> ---
> arch/arm/mach-mx3/eukrea_mbimxsd-baseboard.c | 4 ++--
> arch/arm/mach-mx3/mach-cpuimx35.c | 2 +-
> arch/arm/mach-mx3/mach-pcm043.c | 10 +++++-----
> 3 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/arch/arm/mach-mx3/eukrea_mbimxsd-baseboard.c b/arch/arm/mach-mx3/eukrea_mbimxsd-baseboard.c
> index 14a5ffc..8076147 100644
> --- a/arch/arm/mach-mx3/eukrea_mbimxsd-baseboard.c
> +++ b/arch/arm/mach-mx3/eukrea_mbimxsd-baseboard.c
> @@ -165,8 +165,8 @@ static iomux_v3_cfg_t eukrea_mbimxsd_pads[] = {
> MX35_PAD_SD1_DATA3__ESDHC1_DAT3,
> };
>
> -#define GPIO_LED1 (2 * 32 + 29)
> -#define GPIO_SWITCH1 (2 * 32 + 25)
> +#define GPIO_LED1 IMX_GPIO_NR(3, 29)
> +#define GPIO_SWITCH1 IMX_GPIO_NR(3, 25)
> #define GPIO_LCDPWR (4)
>
While you are at it you could also remove the nonsensical () around
the bare number.
Lothar Wa?mann
--
___________________________________________________________
Ka-Ro electronics GmbH | Pascalstra?e 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Gesch?ftsf?hrer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996
www.karo-electronics.de | info at karo-electronics.de
___________________________________________________________
^ permalink raw reply
* [PATCH v2 12/13] da850: pruss SUART platform specific additions.
From: Subhasish Ghosh @ 2011-02-22 13:24 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <4D639C18.9010309@mvista.com>
Ok, will do.
--------------------------------------------------
From: "Sergei Shtylyov" <sshtylyov@mvista.com>
Sent: Tuesday, February 22, 2011 4:50 PM
To: "Subhasish Ghosh" <subhasish@mistralsolutions.com>
Cc: <davinci-linux-open-source@linux.davincidsp.com>;
<sachi@mistralsolutions.com>; "Russell King" <linux@arm.linux.org.uk>;
"Kevin Hilman" <khilman@deeprootsystems.com>; <nsekhar@ti.com>; "open list"
<linux-kernel@vger.kernel.org>; <m-watkins@ti.com>;
<linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH v2 12/13] da850: pruss SUART platform specific
additions.
> Hello.
>
> On 22-02-2011 12:18, Subhasish Ghosh wrote:
>
>> I could not follow your recommendations clearly, are you suggesting this:
>
>> int __init da8xx_register_pruss(struct da8xx_pruss_devices *pruss_device)
>> {
>> #ifdef CONFIG_SERIAL_PRUSS_SUART_MODULE
>> int ret;
>>
>> ret = clk_add_alias(NULL, "da8xx_pruss_uart.1",
>> NULL, &da850_mcasp_device.dev);
>> if (ret < 0)
>> return ret;
>> #endif
>> da8xx_pruss_dev.dev.platform_data = pruss_device;
>> return platform_device_register(&da8xx_pruss_dev);
>> }
>
> Yes. But still better would be to wrap clk_add_alias() into a function
> of its own (defining it empty if CONFIG_SERIAL_PRUSS_SUART_MODULE is not
> defined).
>
> WBR, Sergei
^ permalink raw reply
* [PATCH] ARM: pxa: support 806MHz operating points for PXA31x processors A2 stepping
From: Igor Grinberg @ 2011-02-22 13:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <AANLkTikTHnz+7C-QmXaHwFE0a+pw7U-t2JP11a0qdoyU@mail.gmail.com>
On 02/22/11 12:38, Axel Lin wrote:
> 2011/2/22 Igor Grinberg <grinberg@compulab.co.il>:
>> Another thing (which could be related to the patch) is the package marking.
>> We have PXA3xx SoCs marked A2 stepping, but C624, which means that the
>> highest running frequency should not exceed 624MHz.
> This means not all PXA310 A2 stepping supports 806Mhz.
> Can we differentiate the Cxxx option in software?
Some time ago I spoke with our local Marvell representative... and he said that this
information cannot be retrieved by software.
So this makes me think, that we may be need an option for the platform data
passed from the board (assuming that board should know what SoC is installed).
Or a kernel command line parameter... something much like Eric proposed:
http://www.spinics.net/lists/arm-kernel/msg92900.html
>> What is your PXA3xx A2 marking in respect to Cxxx option?
> Mine is 88AP310-A2-BGK2C806.
nice! :)
--
Regards,
Igor.
^ permalink raw reply
* [PATCH] ARM: pxa: support 806MHz operating points for PXA31x processors A2 stepping
From: Eric Miao @ 2011-02-22 13:19 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <AANLkTikTHnz+7C-QmXaHwFE0a+pw7U-t2JP11a0qdoyU@mail.gmail.com>
>> Another thing (which could be related to the patch) is the package marking.
>> We have PXA3xx SoCs marked A2 stepping, but C624, which means that the
>> highest running frequency should not exceed 624MHz.
>
> This means not all PXA310 A2 stepping supports 806Mhz.
> Can we differentiate the Cxxx option in software?
>
>> What is your PXA3xx A2 marking in respect to Cxxx option?
>
> Mine is 88AP310-A2-BGK2C806.
>
Haojian,
Can you help check on the stepping/markings on these processors
to see which of them supports 806MHz, reliably?
- eric
^ permalink raw reply
* [PATCH 1/6] ARM: tegra: add tegra_gpio_table and tegra_gpio_config
From: Sergei Shtylyov @ 2011-02-22 13:13 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1298354117-19097-2-git-send-email-olof@lixom.net>
Hello.
On 22-02-2011 8:55, Olof Johansson wrote:
> To give one place to setup the pins that are used as GPIOs instead
> of as their pinmuxed functions. Specifying enabled as false explicitly
> disables the gpio mode of that pin (if left on by firmware).
> This should remove the need for calling these from specific drivers and
> thus reduce tegra-specific code from them.
> Signed-off-by: Olof Johansson<olof@lixom.net>
[...]
> diff --git a/arch/arm/mach-tegra/include/mach/gpio.h b/arch/arm/mach-tegra/include/mach/gpio.h
> index e31f486..2369fba 100644
> --- a/arch/arm/mach-tegra/include/mach/gpio.h
> +++ b/arch/arm/mach-tegra/include/mach/gpio.h
> @@ -20,6 +20,7 @@
> #ifndef __MACH_TEGRA_GPIO_H
> #define __MACH_TEGRA_GPIO_H
>
> +#include<linux/init.h>
> #include<mach/irqs.h>
>
> #define TEGRA_NR_GPIOS INT_GPIO_NR
> @@ -47,6 +48,12 @@ static inline int irq_to_gpio(unsigned int irq)
> return -EINVAL;
> }
>
> +struct tegra_gpio_table {
> + int gpio; /* GPIO number */
> + bool enable; /* Enable for GPIO at init? */
> +};
> +
> +void __init tegra_gpio_config(struct tegra_gpio_table *table, int num);
You don't need to annotate the declaration as __init.
WBR, Sergei
^ permalink raw reply
* [PATCH 0/3] OMAP2+ hwmod fixes
From: Rajendra Nayak @ 2011-02-22 13:11 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <alpine.DEB.2.00.1102210901140.22636@utopia.booyaka.com>
Hi Paul,
> -----Original Message-----
> From: Paul Walmsley [mailto:paul at pwsan.com]
> Sent: Monday, February 21, 2011 10:25 PM
> To: Cousson, Benoit; Nayak, Rajendra
> Cc: linux-omap at vger.kernel.org; linux-arm-kernel at lists.infradead.org
> Subject: Re: [PATCH 0/3] OMAP2+ hwmod fixes
>
> Hello Beno?t, Rajendra,
>
> On Fri, 18 Feb 2011, Cousson, Benoit wrote:
>
> > On 2/16/2011 2:43 PM, Nayak, Rajendra wrote:
> > > > -----Original Message-----
> > > > From: Cousson, Benoit [mailto:b-cousson at ti.com]
> > > > Sent: Wednesday, February 16, 2011 6:37 PM
> > > > To: Nayak, Rajendra
> > > > Cc: linux-omap at vger.kernel.org; paul at pwsan.com;
> > > linux-arm-kernel at lists.infradead.org
> > > > Subject: Re: [PATCH 0/3] OMAP2+ hwmod fixes
> > > >
> > > > Hi Rajendra,
> > > >
> > > > On 2/16/2011 1:11 PM, Nayak, Rajendra wrote:
> > > > > This series fixes some hwmod api return values
> > > > > and also adds some state checks.
> > > > > The hwmod iterator functions are made to
> > > > > continue and not break if one of the
> > > > > callback functions ends up with an error.
> > > >
> > > > By doing that, you change the behavior of this function.
> > > > I'm not sure I fully understand why.
> > > > Could you elaborate on the use case?
> > >
> > > Since these functions iterate over all hwmods
> > > calling a common callback function, there might
> > > be cases wherein the callback function for *some*
> > > hwmods might fail. For instance, if you run through
> > > all hwmods and try to clear the context registers
> > > for all of them, for some hwmods which might
> > > not have context registers the callback function
> > > might return a -EINVAL, however that should not
> > > stop you from attempting to clear the context
> > > registers for the rest of the hwmods which have
> > > them and abort the function midway, no?
> > > This is more hypothetical, however the real usecase
> > > that prompted me with this patch was when I
> > > had some wrong state check in _setup function,
> > > and the iterator would stop with the first failure
> > > and not even attempt to setup the rest of the
> > > hwmods.
> >
> > Yeah, but by using that function you implicitly accept that an error
will
> > break the loop, so the function you pass to the iterator should be
written for
> > that. Meaning if you do not want to exit on error you should not
report an
> > error.
> >
> > > > To avoid that behavior in the past, I was just returning
> > > > 0 in case of failure to avoid stopping the iteration.
> > > > It looks like you do not want to stop the iteration but still
> > > > retrieve the error.
> > > > I do not see in this series what you plan to do with the
> > > > error at the end of the iteration.
> > >
> > > Most users of these iterators would just use the non-zero
> > > return value to throw an error/warning out stating there
> > > were failures running through all the callback functions.
> > > That does not change with this patch, and they can still
> > > do it.
> >
> > Except that now, the iterator is not able anymore to stop on error if
needed
> > potentially.
> > My point is that you are changing the behavior of this function
without
> > maintaining the legacy.
> >
> > So maybe creating a new iterator is a better approach.
> > Even is this feature is not used today I have some secret plan for
this
> > behavior in the near future :-)
> >
> > But my initial comment is still valid, if you do not care about the
final
> > error status after the iteration, you'd better not return any error at
the
> > beginning.
> > This was the behavior or the _setup until now.
>
> The original behavior of the iterator was intentional: loops over
> functions like _init_clocks() and _setup() should terminate upon
> encountering an error. This is because the rest of that code currently
> relies on the hwmod/clock data to be accurate in order to work. There
is
> no provision in the code for a hwmod to fail initialization due to data
> errors. The idea was that if the data is inaccurate, the data should be
> fixed first before doing anything else.
The original behavior of the iterators, to terminate upon
encountering an error, seems fine to me. The only problem
I faced was that they fail silently and go undetected, unless
their user catches the return value and WARN's, which I found
was not the case with most users, mainly those of
omap_hwmod_for_each_by_class.
I was thinking of keeping the behaviour of these iterators
same for now and add WARN's in these iterators itself upon
an error, so its seen even if the user fails to catch it.
Regards,
Rajendra
>
> That said, I suppose that it's possible to enhance the code such that
> hwmods could be allowed to fail initialization, and simply brick
> themselves, rather than prevent the rest of the hwmods from
initializing.
> Probably that would need a new _HWMOD_STATE_INIT_FAILED, or something
> similar. If a hwmod would end up in that state, it must not be used by
> any other code, and the code should complain loudly.
>
> The broader issue of whether the iterators should return immediately
upon
> failure (as the current code does), or continue through the rest of the
> list, is a separate one. I'd suggest one of two approaches:
>
> 1. If the rest of the code can be changed to gracefully handle cases
where
> hwmod initialization fails, and if Beno?t agrees, I don't have a problem
> with changing the iterator behavior to ignore failures as you describe.
> Of course, all of the current users of omap_hwmod_for_each*() would need
> to be checked to ensure that this behavior makes sense for them.
>
> ... or ...
>
> 2. The iterators could take an extra parameter that would control the
> behavior upon encountering an error: terminate, or continue. But I am
not
> sure that both cases are needed. Ideas, feedback here are welcome.
>
>
> - Paul
^ permalink raw reply
* [PATCH 5/5] arm: mach-mx3: use IMX_GPIO_NR instead of hard-coded values
From: Wolfram Sang @ 2011-02-22 12:58 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1298379500-27590-1-git-send-email-w.sang@pengutronix.de>
The latter are error-prone because the bank number is one less than one
would read in the documentation.
Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
Cc: Eric B?nard <eric@eukrea.com>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
---
arch/arm/mach-mx3/eukrea_mbimxsd-baseboard.c | 4 ++--
arch/arm/mach-mx3/mach-cpuimx35.c | 2 +-
arch/arm/mach-mx3/mach-pcm043.c | 10 +++++-----
3 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/arch/arm/mach-mx3/eukrea_mbimxsd-baseboard.c b/arch/arm/mach-mx3/eukrea_mbimxsd-baseboard.c
index 14a5ffc..8076147 100644
--- a/arch/arm/mach-mx3/eukrea_mbimxsd-baseboard.c
+++ b/arch/arm/mach-mx3/eukrea_mbimxsd-baseboard.c
@@ -165,8 +165,8 @@ static iomux_v3_cfg_t eukrea_mbimxsd_pads[] = {
MX35_PAD_SD1_DATA3__ESDHC1_DAT3,
};
-#define GPIO_LED1 (2 * 32 + 29)
-#define GPIO_SWITCH1 (2 * 32 + 25)
+#define GPIO_LED1 IMX_GPIO_NR(3, 29)
+#define GPIO_SWITCH1 IMX_GPIO_NR(3, 25)
#define GPIO_LCDPWR (4)
static void eukrea_mbimxsd_lcd_power_set(struct plat_lcd_data *pd,
diff --git a/arch/arm/mach-mx3/mach-cpuimx35.c b/arch/arm/mach-mx3/mach-cpuimx35.c
index 26ae90f..892c3a9 100644
--- a/arch/arm/mach-mx3/mach-cpuimx35.c
+++ b/arch/arm/mach-mx3/mach-cpuimx35.c
@@ -60,7 +60,7 @@ static struct tsc2007_platform_data tsc2007_info = {
.x_plate_ohms = 180,
};
-#define TSC2007_IRQGPIO (2 * 32 + 2)
+#define TSC2007_IRQGPIO IMX_GPIO_NR(3, 2)
static struct i2c_board_info eukrea_cpuimx35_i2c_devices[] = {
{
I2C_BOARD_INFO("pcf8563", 0x51),
diff --git a/arch/arm/mach-mx3/mach-pcm043.c b/arch/arm/mach-mx3/mach-pcm043.c
index 26b686c..51542f7 100644
--- a/arch/arm/mach-mx3/mach-pcm043.c
+++ b/arch/arm/mach-mx3/mach-pcm043.c
@@ -224,12 +224,12 @@ static iomux_v3_cfg_t pcm043_pads[] = {
MX35_PAD_ATA_DATA11__GPIO2_24, /* CardDetect */
};
-#define AC97_GPIO_TXFS (1 * 32 + 31)
-#define AC97_GPIO_TXD (1 * 32 + 28)
-#define AC97_GPIO_RESET (1 * 32 + 0)
+#define AC97_GPIO_TXFS IMX_GPIO_NR(2, 31)
+#define AC97_GPIO_TXD IMX_GPIO_NR(2, 28)
+#define AC97_GPIO_RESET IMX_GPIO_NR(2, 0)
-#define SD1_GPIO_WP (1 * 32 + 23)
-#define SD1_GPIO_CD (1 * 32 + 24)
+#define SD1_GPIO_WP IMX_GPIO_NR(2, 23)
+#define SD1_GPIO_CD IMX_GPIO_NR(2, 24)
static void pcm043_ac97_warm_reset(struct snd_ac97 *ac97)
{
--
1.7.2.3
^ permalink raw reply related
* [PATCH 4/5] arm: mach-mx3: pcm043: add write-protect and card-detect for SD1
From: Wolfram Sang @ 2011-02-22 12:58 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1298379500-27590-1-git-send-email-w.sang@pengutronix.de>
Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
---
arch/arm/mach-mx3/mach-pcm043.c | 13 ++++++++++++-
1 files changed, 12 insertions(+), 1 deletions(-)
diff --git a/arch/arm/mach-mx3/mach-pcm043.c b/arch/arm/mach-mx3/mach-pcm043.c
index bcf83fc..26b686c 100644
--- a/arch/arm/mach-mx3/mach-pcm043.c
+++ b/arch/arm/mach-mx3/mach-pcm043.c
@@ -40,6 +40,7 @@
#include <mach/mx3fb.h>
#include <mach/ulpi.h>
#include <mach/audmux.h>
+#include <mach/esdhc.h>
#include "devices-imx35.h"
#include "devices.h"
@@ -219,12 +220,17 @@ static iomux_v3_cfg_t pcm043_pads[] = {
MX35_PAD_SD1_DATA1__ESDHC1_DAT1,
MX35_PAD_SD1_DATA2__ESDHC1_DAT2,
MX35_PAD_SD1_DATA3__ESDHC1_DAT3,
+ MX35_PAD_ATA_DATA10__GPIO2_23, /* WriteProtect */
+ MX35_PAD_ATA_DATA11__GPIO2_24, /* CardDetect */
};
#define AC97_GPIO_TXFS (1 * 32 + 31)
#define AC97_GPIO_TXD (1 * 32 + 28)
#define AC97_GPIO_RESET (1 * 32 + 0)
+#define SD1_GPIO_WP (1 * 32 + 23)
+#define SD1_GPIO_CD (1 * 32 + 24)
+
static void pcm043_ac97_warm_reset(struct snd_ac97 *ac97)
{
iomux_v3_cfg_t txfs_gpio = MX35_PAD_STXFS4__GPIO2_31;
@@ -307,6 +313,11 @@ pcm037_nand_board_info __initconst = {
.hw_ecc = 1,
};
+static struct esdhc_platform_data sd1_pdata = {
+ .wp_gpio = SD1_GPIO_WP,
+ .cd_gpio = SD1_GPIO_CD,
+};
+
#if defined(CONFIG_USB_ULPI)
static struct mxc_usbh_platform_data otg_pdata __initdata = {
.portsc = MXC_EHCI_MODE_UTMI,
@@ -393,7 +404,7 @@ static void __init mxc_board_init(void)
imx35_add_fsl_usb2_udc(&otg_device_pdata);
imx35_add_flexcan1(NULL);
- imx35_add_sdhci_esdhc_imx(0, NULL);
+ imx35_add_sdhci_esdhc_imx(0, &sd1_pdata);
}
static void __init pcm043_timer_init(void)
--
1.7.2.3
^ permalink raw reply related
* [PATCH 3/5] mmc: sdhci-esdhc-imx: add card detect on custom GPIO
From: Wolfram Sang @ 2011-02-22 12:58 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1298379500-27590-1-git-send-email-w.sang@pengutronix.de>
Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
Tested-by: Marc Reilly <marc@cpdesign.com.au>
---
Change since last version:
* rewrote the warning message to apply to non-existant cd_gpio
arch/arm/plat-mxc/include/mach/esdhc.h | 2 +
drivers/mmc/host/sdhci-esdhc-imx.c | 76 ++++++++++++++++++++++++++++++++
2 files changed, 78 insertions(+), 0 deletions(-)
diff --git a/arch/arm/plat-mxc/include/mach/esdhc.h b/arch/arm/plat-mxc/include/mach/esdhc.h
index dbf6d56..7501d4e 100644
--- a/arch/arm/plat-mxc/include/mach/esdhc.h
+++ b/arch/arm/plat-mxc/include/mach/esdhc.h
@@ -16,9 +16,11 @@
* strongly recommended for i.MX25/35, not needed for other variants
*
* @wp_gpio: gpio for write_protect
+ * @cd_gpio: gpio for card_detect interrupt
*/
struct esdhc_platform_data {
unsigned int wp_gpio;
+ unsigned int cd_gpio;
};
#endif /* __ASM_ARCH_IMX_ESDHC_H */
diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index 49c9801..241eb78 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -32,6 +32,39 @@ static inline void esdhc_clrset_le(struct sdhci_host *host, u32 mask, u32 val, i
writel(((readl(base) & ~(mask << shift)) | (val << shift)), base);
}
+static u32 esdhc_readl_le(struct sdhci_host *host, int reg)
+{
+ /* fake CARD_PRESENT flag on mx25/35 */
+ u32 val = readl(host->ioaddr + reg);
+
+ if (unlikely(reg == SDHCI_PRESENT_STATE)) {
+ struct esdhc_platform_data *boarddata =
+ host->mmc->parent->platform_data;
+
+ if (boarddata && gpio_is_valid(boarddata->cd_gpio)
+ && gpio_get_value(boarddata->cd_gpio))
+ /* no card, if a valid gpio says so... */
+ val &= SDHCI_CARD_PRESENT;
+ else
+ /* ... in all other cases assume card is present */
+ val |= SDHCI_CARD_PRESENT;
+ }
+
+ return val;
+}
+
+static void esdhc_writel_le(struct sdhci_host *host, u32 val, int reg)
+{
+ if (unlikely(reg == SDHCI_INT_ENABLE))
+ /*
+ * these interrupts won't work with a custom card_detect gpio
+ * (only applied to mx25/35)
+ */
+ val &= ~(SDHCI_INT_CARD_REMOVE | SDHCI_INT_CARD_INSERT);
+
+ writel(val, host->ioaddr + reg);
+}
+
static u16 esdhc_readw_le(struct sdhci_host *host, int reg)
{
if (unlikely(reg == SDHCI_HOST_VERSION))
@@ -121,6 +154,14 @@ static struct sdhci_ops sdhci_esdhc_ops = {
.get_min_clock = esdhc_pltfm_get_min_clock,
};
+static irqreturn_t cd_irq(int irq, void *data)
+{
+ struct sdhci_host *sdhost = (struct sdhci_host *)data;
+
+ tasklet_schedule(&sdhost->card_tasklet);
+ return IRQ_HANDLED;
+};
+
static int esdhc_pltfm_init(struct sdhci_host *host, struct sdhci_pltfm_data *pdata)
{
struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
@@ -142,6 +183,8 @@ static int esdhc_pltfm_init(struct sdhci_host *host, struct sdhci_pltfm_data *pd
if (cpu_is_mx25() || cpu_is_mx35()) {
/* Fix errata ENGcm07207 present on i.MX25 and i.MX35 */
host->quirks |= SDHCI_QUIRK_NO_MULTIBLOCK;
+ /* card_detect can't be routed to controller, mark broken */
+ host->quirks |= SDHCI_QUIRK_BROKEN_CARD_DETECTION;
/* write_protect can't be routed to controller, use gpio */
sdhci_esdhc_ops.get_ro = esdhc_pltfm_get_ro;
}
@@ -153,9 +196,35 @@ static int esdhc_pltfm_init(struct sdhci_host *host, struct sdhci_pltfm_data *pd
"no write-protect pin available!\n");
boarddata->wp_gpio = err;
}
+
+ err = gpio_request_one(boarddata->cd_gpio, GPIOF_IN, "ESDHC_CD");
+ if (err) {
+ dev_warn(mmc_dev(host->mmc),
+ "no card-detect pin available!\n");
+ goto no_card_detect_pin;
+ }
+
+ err = request_irq(gpio_to_irq(boarddata->cd_gpio), cd_irq,
+ IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING,
+ mmc_hostname(host->mmc), host);
+ if (err) {
+ dev_warn(mmc_dev(host->mmc), "request irq error\n");
+ goto no_card_detect_irq;
+ }
+
+ sdhci_esdhc_ops.write_l = esdhc_writel_le;
+ sdhci_esdhc_ops.read_l = esdhc_readl_le;
+ /* Now we have a working card_detect again */
+ host->quirks &= ~SDHCI_QUIRK_BROKEN_CARD_DETECTION;
}
return 0;
+
+ no_card_detect_irq:
+ gpio_free(boarddata->cd_gpio);
+ no_card_detect_pin:
+ boarddata->cd_gpio = err;
+ return 0;
}
static void esdhc_pltfm_exit(struct sdhci_host *host)
@@ -166,6 +235,13 @@ static void esdhc_pltfm_exit(struct sdhci_host *host)
if (boarddata && gpio_is_valid(boarddata->wp_gpio))
gpio_free(boarddata->wp_gpio);
+ if (boarddata && gpio_is_valid(boarddata->cd_gpio)) {
+ gpio_free(boarddata->cd_gpio);
+
+ if (!(host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION))
+ free_irq(gpio_to_irq(boarddata->cd_gpio), host);
+ }
+
clk_disable(pltfm_host->clk);
clk_put(pltfm_host->clk);
}
--
1.7.2.3
^ permalink raw reply related
* [PATCH 2/5] mmc: sdhci-esdhc: broken card detection is not a default quirk
From: Wolfram Sang @ 2011-02-22 12:58 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1298379500-27590-1-git-send-email-w.sang@pengutronix.de>
It can be worked around using a GPIO which will be done for i.MX later.
Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
Acked-by: Anton Vorontsov <cbouatmailru@gmail.com>
Tested-by: Marc Reilly <marc@cpdesign.com.au>
---
drivers/mmc/host/sdhci-esdhc-imx.c | 3 ++-
drivers/mmc/host/sdhci-esdhc.h | 1 -
drivers/mmc/host/sdhci-of-esdhc.c | 3 ++-
3 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index 65df00b..49c9801 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -171,7 +171,8 @@ static void esdhc_pltfm_exit(struct sdhci_host *host)
}
struct sdhci_pltfm_data sdhci_esdhc_imx_pdata = {
- .quirks = ESDHC_DEFAULT_QUIRKS | SDHCI_QUIRK_BROKEN_ADMA,
+ .quirks = ESDHC_DEFAULT_QUIRKS | SDHCI_QUIRK_BROKEN_ADMA
+ | SDHCI_QUIRK_BROKEN_CARD_DETECTION,
/* ADMA has issues. Might be fixable */
.ops = &sdhci_esdhc_ops,
.init = esdhc_pltfm_init,
diff --git a/drivers/mmc/host/sdhci-esdhc.h b/drivers/mmc/host/sdhci-esdhc.h
index afaf1bc..c55aae8 100644
--- a/drivers/mmc/host/sdhci-esdhc.h
+++ b/drivers/mmc/host/sdhci-esdhc.h
@@ -19,7 +19,6 @@
*/
#define ESDHC_DEFAULT_QUIRKS (SDHCI_QUIRK_FORCE_BLK_SZ_2048 | \
- SDHCI_QUIRK_BROKEN_CARD_DETECTION | \
SDHCI_QUIRK_NO_BUSY_IRQ | \
SDHCI_QUIRK_NONSTANDARD_CLOCK | \
SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK | \
diff --git a/drivers/mmc/host/sdhci-of-esdhc.c b/drivers/mmc/host/sdhci-of-esdhc.c
index fcd0e1f..08161f6 100644
--- a/drivers/mmc/host/sdhci-of-esdhc.c
+++ b/drivers/mmc/host/sdhci-of-esdhc.c
@@ -73,7 +73,8 @@ static unsigned int esdhc_of_get_min_clock(struct sdhci_host *host)
}
struct sdhci_of_data sdhci_esdhc = {
- .quirks = ESDHC_DEFAULT_QUIRKS,
+ /* card detection could be handled via GPIO */
+ .quirks = ESDHC_DEFAULT_QUIRKS | SDHCI_QUIRK_BROKEN_CARD_DETECTION,
.ops = {
.read_l = sdhci_be32bs_readl,
.read_w = esdhc_readw,
--
1.7.2.3
^ permalink raw reply related
* [PATCH 1/5] mmc: sdhci-esdhc-imx: add support for write protect on custom GPIO
From: Wolfram Sang @ 2011-02-22 12:58 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1298379500-27590-1-git-send-email-w.sang@pengutronix.de>
Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
Tested-by: Marc Reilly <marc@cpdesign.com.au>
---
Change since last version:
* rewrote the warning message to apply to non-existant wp_gpio
arch/arm/plat-mxc/include/mach/esdhc.h | 10 +++++-
drivers/mmc/host/sdhci-esdhc-imx.c | 52 +++++++++++++++++++++++++-------
2 files changed, 50 insertions(+), 12 deletions(-)
diff --git a/arch/arm/plat-mxc/include/mach/esdhc.h b/arch/arm/plat-mxc/include/mach/esdhc.h
index a48a9aa..dbf6d56 100644
--- a/arch/arm/plat-mxc/include/mach/esdhc.h
+++ b/arch/arm/plat-mxc/include/mach/esdhc.h
@@ -10,7 +10,15 @@
#ifndef __ASM_ARCH_IMX_ESDHC_H
#define __ASM_ARCH_IMX_ESDHC_H
+/**
+ * struct esdhc_platform_data - optional platform data for esdhc on i.MX
+ *
+ * strongly recommended for i.MX25/35, not needed for other variants
+ *
+ * @wp_gpio: gpio for write_protect
+ */
+
struct esdhc_platform_data {
- unsigned int wp_gpio; /* write protect pin */
+ unsigned int wp_gpio;
};
#endif /* __ASM_ARCH_IMX_ESDHC_H */
diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index 9b82910..65df00b 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -15,9 +15,11 @@
#include <linux/delay.h>
#include <linux/err.h>
#include <linux/clk.h>
+#include <linux/gpio.h>
#include <linux/mmc/host.h>
#include <linux/mmc/sdhci-pltfm.h>
#include <mach/hardware.h>
+#include <mach/esdhc.h>
#include "sdhci.h"
#include "sdhci-pltfm.h"
#include "sdhci-esdhc.h"
@@ -100,10 +102,31 @@ static unsigned int esdhc_pltfm_get_min_clock(struct sdhci_host *host)
return clk_get_rate(pltfm_host->clk) / 256 / 16;
}
+static unsigned int esdhc_pltfm_get_ro(struct sdhci_host *host)
+{
+ struct esdhc_platform_data *boarddata = host->mmc->parent->platform_data;
+
+ if (boarddata && gpio_is_valid(boarddata->wp_gpio))
+ return gpio_get_value(boarddata->wp_gpio);
+ else
+ return -ENOSYS;
+}
+
+static struct sdhci_ops sdhci_esdhc_ops = {
+ .read_w = esdhc_readw_le,
+ .write_w = esdhc_writew_le,
+ .write_b = esdhc_writeb_le,
+ .set_clock = esdhc_set_clock,
+ .get_max_clock = esdhc_pltfm_get_max_clock,
+ .get_min_clock = esdhc_pltfm_get_min_clock,
+};
+
static int esdhc_pltfm_init(struct sdhci_host *host, struct sdhci_pltfm_data *pdata)
{
struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+ struct esdhc_platform_data *boarddata = host->mmc->parent->platform_data;
struct clk *clk;
+ int err;
clk = clk_get(mmc_dev(host->mmc), NULL);
if (IS_ERR(clk)) {
@@ -116,9 +139,21 @@ static int esdhc_pltfm_init(struct sdhci_host *host, struct sdhci_pltfm_data *pd
if (cpu_is_mx35() || cpu_is_mx51())
host->quirks |= SDHCI_QUIRK_BROKEN_TIMEOUT_VAL;
- /* Fix errata ENGcm07207 which is present on i.MX25 and i.MX35 */
- if (cpu_is_mx25() || cpu_is_mx35())
+ if (cpu_is_mx25() || cpu_is_mx35()) {
+ /* Fix errata ENGcm07207 present on i.MX25 and i.MX35 */
host->quirks |= SDHCI_QUIRK_NO_MULTIBLOCK;
+ /* write_protect can't be routed to controller, use gpio */
+ sdhci_esdhc_ops.get_ro = esdhc_pltfm_get_ro;
+ }
+
+ if (boarddata) {
+ err = gpio_request_one(boarddata->wp_gpio, GPIOF_IN, "ESDHC_WP");
+ if (err) {
+ dev_warn(mmc_dev(host->mmc),
+ "no write-protect pin available!\n");
+ boarddata->wp_gpio = err;
+ }
+ }
return 0;
}
@@ -126,20 +161,15 @@ static int esdhc_pltfm_init(struct sdhci_host *host, struct sdhci_pltfm_data *pd
static void esdhc_pltfm_exit(struct sdhci_host *host)
{
struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+ struct esdhc_platform_data *boarddata = host->mmc->parent->platform_data;
+
+ if (boarddata && gpio_is_valid(boarddata->wp_gpio))
+ gpio_free(boarddata->wp_gpio);
clk_disable(pltfm_host->clk);
clk_put(pltfm_host->clk);
}
-static struct sdhci_ops sdhci_esdhc_ops = {
- .read_w = esdhc_readw_le,
- .write_w = esdhc_writew_le,
- .write_b = esdhc_writeb_le,
- .set_clock = esdhc_set_clock,
- .get_max_clock = esdhc_pltfm_get_max_clock,
- .get_min_clock = esdhc_pltfm_get_min_clock,
-};
-
struct sdhci_pltfm_data sdhci_esdhc_imx_pdata = {
.quirks = ESDHC_DEFAULT_QUIRKS | SDHCI_QUIRK_BROKEN_ADMA,
/* ADMA has issues. Might be fixable */
--
1.7.2.3
^ permalink raw reply related
* [PATCH V2 0/5] sdhci-esdhc-imx: use gpio for write protection and card detection
From: Wolfram Sang @ 2011-02-22 12:58 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
Second version of my series, no major changes:
* rephrased warnings
* collected some tags
* rebased to mmc/mmc-next as of today
Eric wanted to tested these patches soonish. Thanks to Marc for testing the
first three already.
I think the first three patches should go via Chris and the latter two via
Sascha. The last one is more a cleanup-patch, but it depends on the former, so
I included it here.
The branch can be found at:
git://git.pengutronix.de/git/wsa/linux-2.6.git pcm043-mmc
Thanks,
Wolfram
Wolfram Sang (5):
mmc: sdhci-esdhc-imx: add support for write protect on custom GPIO
mmc: sdhci-esdhc: broken card detection is not a default quirk
mmc: sdhci-esdhc-imx: add card detect on custom GPIO
arm: mach-mx3: pcm043: add write-protect and card-detect for SD1
arm: mach-mx3: use IMX_GPIO_NR instead of hard-coded values
arch/arm/mach-mx3/eukrea_mbimxsd-baseboard.c | 4 +-
arch/arm/mach-mx3/mach-cpuimx35.c | 2 +-
arch/arm/mach-mx3/mach-pcm043.c | 19 +++-
arch/arm/plat-mxc/include/mach/esdhc.h | 12 ++-
drivers/mmc/host/sdhci-esdhc-imx.c | 131 +++++++++++++++++++++++---
drivers/mmc/host/sdhci-esdhc.h | 1 -
drivers/mmc/host/sdhci-of-esdhc.c | 3 +-
7 files changed, 148 insertions(+), 22 deletions(-)
--
1.7.2.3
^ permalink raw reply
* [PATCH v2 01/13] mfd: pruss mfd driver.
From: Subhasish Ghosh @ 2011-02-22 12:49 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20110222113310.GD30279@sortiz-mobl>
I am not sure if I understood you correctly, but the current sizeof the
structure da8xx_prusscore_regs is 0x500.
Here is a link to the PRUSS memory map:
http://processors.wiki.ti.com/index.php/PRUSS_Memory_Map
The offset 0x00007000 is the PRU0 reg offset and 0x00007800 is the PRU1 reg
offset.
We cannot have a register file larger than this, but lot of space is left
out, possibly for future development.
--------------------------------------------------
From: "Samuel Ortiz" <sameo@linux.intel.com>
Sent: Tuesday, February 22, 2011 5:03 PM
To: "Wolfgang Grandegger" <wg@grandegger.com>
Cc: "Subhasish Ghosh" <subhasish@mistralsolutions.com>;
<sachi@mistralsolutions.com>;
<davinci-linux-open-source@linux.davincidsp.com>; <nsekhar@ti.com>; "open
list" <linux-kernel@vger.kernel.org>; <m-watkins@ti.com>;
<linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH v2 01/13] mfd: pruss mfd driver.
> On Tue, Feb 22, 2011 at 11:48:51AM +0100, Wolfgang Grandegger wrote:
>> On 02/22/2011 11:31 AM, Samuel Ortiz wrote:
>> > Hi Subhasish,
>> >
>> > On Tue, Feb 22, 2011 at 11:13:38AM +0530, Subhasish Ghosh wrote:
>> >> Thank you for your comments.
>> > No problem.
>> >
>> >>>> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
>> >>>> index fd01836..6c437df 100644
>> >>>> --- a/drivers/mfd/Kconfig
>> >>>> +++ b/drivers/mfd/Kconfig
>> >>>> @@ -81,6 +81,16 @@ config MFD_DM355EVM_MSP
>> >>>> boards. MSP430 firmware manages resets and power sequencing,
>> >>>> inputs from buttons and the IR remote, LEDs, an RTC, and more.
>> >>>>
>> >>>> +config MFD_DA8XX_PRUSS
>> >>>> + tristate "Texas Instruments DA8XX PRUSS support"
>> >>>> + depends on ARCH_DAVINCI && ARCH_DAVINCI_DA850
>> >>> Why are we depending on those ?
>> >>
>> >> SG -- The PRUSS core in only available within DA850 and DA830,
>> >> DA830 support is not yet implemented.
>> > Sure, but if there are no actual code dependencies, I'd like to get rid
>> > of
>> > those depends.
>> >
>> >>>> +u32 pruss_disable(struct device *dev, u8 pruss_num)
>> >>>> +{
>> >>>> + struct da8xx_pruss *pruss = dev_get_drvdata(dev->parent);
>> >>>> + da8xx_prusscore_regs h_pruss;
>> >>>> + u32 temp_reg;
>> >>>> +
>> >>>> + if (pruss_num == DA8XX_PRUCORE_0) {
>> >>>> + /* Disable PRU0 */
>> >>>> + h_pruss = (da8xx_prusscore_regs)
>> >>>> + ((u32) pruss->ioaddr + 0x7000);
>> >>> So it seems you're doing this in several places, and I have a few
>> >>> comments:
>> >>>
>> >>> - You don't need the da8xx_prusscore_regs at all.
>> >>> - Define the register map through a set of #define in your header
>> >>> file.
>> >>> - Use a static routine that takes the core number and returns the
>> >>> register map
>> >>> offset.
>> >>>
>> >>> Then routines like this one will look a lot more readable.
>> >>
>> >> SG -- There are a huge number of PRUSS registers. A lot of them are
>> >> reserved and are expected to change as development on the
>> >> controller is still ongoing.
>> > First of all, from what I read in your patch you're only using the
>> > CONTROL
>> > offset.
>> >
>> >> If we use #defines to plot
>> >> all the registers, then first, there are too many array type
>> >> registers which will need to be duplicated.
>> > What I'm expecting is a small set of defines for the register offsets.
>> > You
>> > have 13 fields in your da8xx_prusscore_regs, you only need to define 13
>> > register offsets.
>> >
>> > So, if you have a:
>> >
>> > static u32 reg_offset(struct device *dev, u8 pru_num)
>> > {
>> > struct da8xx_pruss *pru = dev_get_drvdata(dev->parent);
>> >
>> > switch (pru_num) {
>> > case DA8XX_PRUCORE_0:
>> > return (u32) pru->ioaddr + 0x7000;
>> > case DA8XX_PRUCORE_1:
>> > return (u32) pru->ioaddr + 0x7800;
>> > default:
>> > return 0;
>> > }
>> >
>> >
>> > then routines like pruss_enable (which should return an int, btw) would
>> > look
>> > like:
>> >
>> > int pruss_enable(struct device *dev, u8 pruss_num)
>> > {
>> > u32 offset = reg_offset(dev, pruss_num);
>> >
>> > if (offset == 0)
>> > return -EINVAL;
>> >
>> > __raw_writel(DA8XX_PRUCORE_CONTROL_RESETVAL,
>> > offset + PRU_CORE_CONTROL);
>> >
>> > return 0;
>> > }
>>
>> All registers are memory mapped and could nicely be described by
>> structures (and sub-structures). Therefore we asked to considerer
>> structs, at least for the Pruss SocketCAN drivers.
>>
>> That would result in
>> much much clearer and better readable code. The code above would shrink
>> to:
>>
>> __raw_writel(DA8XX_PRUCORE_CONTROL_RESETVAL,
>> &prucore[pruss_num].control);
> This driver seems to exclusively use the control offset, which is why I
> don't
> see an absolute need for doing this mapping.
> But if both maps are contiguous then doing the mapping would prevent us
> from
> calling reg_offset() and would bring some advantage. I'd then be fine with
> it.
> For now, da8xx_prusscore_regs seems to be larger than the 0x800 interval
> between the 2 maps, so I have no idea if both maps are indeed contiguous.
>
> Cheers,
> Samuel.
>
> --
> Intel Open Source Technology Centre
> http://oss.intel.com/
^ permalink raw reply
* [PATCH v3 3/6] ARM: mxs: dynamically allocate mmc device
From: Shawn Guo @ 2011-02-22 12:39 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20110222083113.GU22310@pengutronix.de>
On Tue, Feb 22, 2011 at 09:31:13AM +0100, Uwe Kleine-K?nig wrote:
> On Mon, Feb 21, 2011 at 06:42:56PM +0800, Shawn Guo wrote:
> > Signed-off-by: Shawn Guo <shawn.guo@freescale.com>
> > ---
> > arch/arm/mach-mxs/clock-mx23.c | 16 +++++
> > arch/arm/mach-mxs/clock-mx28.c | 18 +++++
> > arch/arm/mach-mxs/devices-mx23.h | 4 +
> > arch/arm/mach-mxs/devices-mx28.h | 4 +
> > arch/arm/mach-mxs/devices/Kconfig | 3 +
> > arch/arm/mach-mxs/devices/Makefile | 1 +
> > arch/arm/mach-mxs/devices/platform-mmc.c | 77 +++++++++++++++++++++++
> > arch/arm/mach-mxs/include/mach/devices-common.h | 13 ++++
> > 8 files changed, 136 insertions(+), 0 deletions(-)
> > create mode 100644 arch/arm/mach-mxs/devices/platform-mmc.c
> >
> > diff --git a/arch/arm/mach-mxs/clock-mx23.c b/arch/arm/mach-mxs/clock-mx23.c
> > index c19b69a..a1ed5f4 100644
> > --- a/arch/arm/mach-mxs/clock-mx23.c
> > +++ b/arch/arm/mach-mxs/clock-mx23.c
> > @@ -445,6 +445,7 @@ static struct clk_lookup lookups[] = {
> > _REGISTER_CLOCK("rtc", NULL, rtc_clk)
> > _REGISTER_CLOCK("mxs-dma-apbh", NULL, hbus_clk)
> > _REGISTER_CLOCK("mxs-dma-apbx", NULL, xbus_clk)
> > + _REGISTER_CLOCK("mxs-mmc.0", NULL, ssp_clk)
> > _REGISTER_CLOCK(NULL, "usb", usb_clk)
> > _REGISTER_CLOCK(NULL, "audio", audio_clk)
> > _REGISTER_CLOCK(NULL, "pwm", pwm_clk)
> > @@ -515,6 +516,15 @@ static int clk_misc_init(void)
> > __raw_writel(BM_CLKCTRL_CPU_INTERRUPT_WAIT,
> > CLKCTRL_BASE_ADDR + HW_CLKCTRL_CPU_SET);
> >
> > + /*
> > + * 480 MHz seems too high to be ssp clock source directly,
> > + * so set frac to get a 288 MHz ref_io.
> > + */
> > + reg = __raw_readl(CLKCTRL_BASE_ADDR + HW_CLKCTRL_FRAC);
> > + reg &= ~BM_CLKCTRL_FRAC_IOFRAC;
> > + reg |= 30 << BP_CLKCTRL_FRAC_IOFRAC;
> > + __raw_writel(reg, CLKCTRL_BASE_ADDR + HW_CLKCTRL_FRAC);
> > +
> > return 0;
> > }
> >
> > @@ -522,6 +532,12 @@ int __init mx23_clocks_init(void)
> > {
> > clk_misc_init();
> >
> > + /*
> > + * source ssp clock from ref_io than ref_xtal,
> > + * as ref_xtal only provides 24 MHz as maximum.
> > + */
> > + clk_set_parent(&ssp_clk, &ref_io_clk);
> > +
> > clk_enable(&cpu_clk);
> > clk_enable(&hbus_clk);
> > clk_enable(&xbus_clk);
> > diff --git a/arch/arm/mach-mxs/clock-mx28.c b/arch/arm/mach-mxs/clock-mx28.c
> > index 9f4ee36..aeb7b2c 100644
> > --- a/arch/arm/mach-mxs/clock-mx28.c
> > +++ b/arch/arm/mach-mxs/clock-mx28.c
> > @@ -619,6 +619,8 @@ static struct clk_lookup lookups[] = {
> > _REGISTER_CLOCK("pll2", NULL, pll2_clk)
> > _REGISTER_CLOCK("mxs-dma-apbh", NULL, hbus_clk)
> > _REGISTER_CLOCK("mxs-dma-apbx", NULL, xbus_clk)
> > + _REGISTER_CLOCK("mxs-mmc.0", NULL, ssp0_clk)
> > + _REGISTER_CLOCK("mxs-mmc.1", NULL, ssp1_clk)
> > _REGISTER_CLOCK("flexcan.0", NULL, can0_clk)
> > _REGISTER_CLOCK("flexcan.1", NULL, can1_clk)
> > _REGISTER_CLOCK(NULL, "usb0", usb0_clk)
> > @@ -730,6 +732,15 @@ static int clk_misc_init(void)
> > reg |= BM_CLKCTRL_ENET_CLK_OUT_EN;
> > __raw_writel(reg, CLKCTRL_BASE_ADDR + HW_CLKCTRL_ENET);
> >
> > + /*
> > + * 480 MHz seems too high to be ssp clock source directly,
> > + * so set frac0 to get a 288 MHz ref_io0.
> > + */
> > + reg = __raw_readl(CLKCTRL_BASE_ADDR + HW_CLKCTRL_FRAC0);
> > + reg &= ~BM_CLKCTRL_FRAC0_IO0FRAC;
> > + reg |= 30 << BP_CLKCTRL_FRAC0_IO0FRAC;
> > + __raw_writel(reg, CLKCTRL_BASE_ADDR + HW_CLKCTRL_FRAC0);
> > +
> > return 0;
> > }
> >
> > @@ -737,6 +748,13 @@ int __init mx28_clocks_init(void)
> > {
> > clk_misc_init();
> >
> > + /*
> > + * source ssp clock from ref_io0 than ref_xtal,
> > + * as ref_xtal only provides 24 MHz as maximum.
> > + */
> > + clk_set_parent(&ssp0_clk, &ref_io0_clk);
> > + clk_set_parent(&ssp1_clk, &ref_io0_clk);
> > +
> > clk_enable(&cpu_clk);
> > clk_enable(&hbus_clk);
> > clk_enable(&xbus_clk);
> > diff --git a/arch/arm/mach-mxs/devices-mx23.h b/arch/arm/mach-mxs/devices-mx23.h
> > index 1256788..4419390 100644
> > --- a/arch/arm/mach-mxs/devices-mx23.h
> > +++ b/arch/arm/mach-mxs/devices-mx23.h
> > @@ -14,3 +14,7 @@
> > extern const struct amba_device mx23_duart_device __initconst;
> > #define mx23_add_duart() \
> > mxs_add_duart(&mx23_duart_device)
> > +
> > +extern const struct mxs_mmc_data mx23_mmc_data[] __initconst;
> > +#define mx23_add_mmc(id, pdata) \
> > + mxs_add_mmc(&mx23_mmc_data[id], pdata)
> mxs_mmc please (not sure about mxs_mxs_mmc_data, but it would be
> consistent). At least mx23_add_mxs_mmc and mxs_add_mxs_mmc.
>
> > diff --git a/arch/arm/mach-mxs/devices-mx28.h b/arch/arm/mach-mxs/devices-mx28.h
> > index 3b18304..540639d 100644
> > --- a/arch/arm/mach-mxs/devices-mx28.h
> > +++ b/arch/arm/mach-mxs/devices-mx28.h
> > @@ -32,3 +32,7 @@ extern const struct mxs_flexcan_data mx28_flexcan_data[] __initconst;
> > mxs_add_flexcan(&mx28_flexcan_data[id], pdata)
> > #define mx28_add_flexcan0(pdata) mx28_add_flexcan(0, pdata)
> > #define mx28_add_flexcan1(pdata) mx28_add_flexcan(1, pdata)
> > +
> > +extern const struct mxs_mmc_data mx28_mmc_data[] __initconst;
> > +#define mx28_add_mmc(id, pdata) \
> > + mxs_add_mmc(&mx28_mmc_data[id], pdata)
> This conflicts with my series, but it's trivial enough to let Sascha
> handle that.
>
> > diff --git a/arch/arm/mach-mxs/devices/Kconfig b/arch/arm/mach-mxs/devices/Kconfig
> > index 6c65b67..49307f8 100644
> > --- a/arch/arm/mach-mxs/devices/Kconfig
> > +++ b/arch/arm/mach-mxs/devices/Kconfig
> > @@ -11,3 +11,6 @@ config MXS_HAVE_PLATFORM_FEC
> > config MXS_HAVE_PLATFORM_FLEXCAN
> > select HAVE_CAN_FLEXCAN if CAN
> > bool
> > +
> > +config MXS_HAVE_PLATFORM_MMC
> > + bool
> ditto MXS_HAVE_PLATFORM_MXS_MMC
>
> > diff --git a/arch/arm/mach-mxs/devices/Makefile b/arch/arm/mach-mxs/devices/Makefile
> > index ca7acc4..c0dacfd 100644
> > --- a/arch/arm/mach-mxs/devices/Makefile
> > +++ b/arch/arm/mach-mxs/devices/Makefile
> > @@ -3,3 +3,4 @@ obj-$(CONFIG_MXS_HAVE_PLATFORM_AUART) += platform-auart.o
> > obj-y += platform-dma.o
> > obj-$(CONFIG_MXS_HAVE_PLATFORM_FEC) += platform-fec.o
> > obj-$(CONFIG_MXS_HAVE_PLATFORM_FLEXCAN) += platform-flexcan.o
> > +obj-$(CONFIG_MXS_HAVE_PLATFORM_MMC) += platform-mmc.o
> > diff --git a/arch/arm/mach-mxs/devices/platform-mmc.c b/arch/arm/mach-mxs/devices/platform-mmc.c
> > new file mode 100644
> > index 0000000..868def1
> > --- /dev/null
> > +++ b/arch/arm/mach-mxs/devices/platform-mmc.c
> > @@ -0,0 +1,77 @@
> > +/*
> > + * Copyright (C) 2010 Pengutronix
> > + * Uwe Kleine-Koenig <u.kleine-koenig@pengutronix.de>
> > + *
> > + * Copyright 2011 Freescale Semiconductor, Inc. All Rights Reserved.
> > + *
> > + * 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.
> > + */
> > +
> > +#include <linux/compiler.h>
> > +#include <linux/err.h>
> > +#include <linux/init.h>
> > +
> > +#include <mach/mx23.h>
> > +#include <mach/mx28.h>
> > +#include <mach/devices-common.h>
> > +
> > +#define mxs_mmc_data_entry_single(soc, _id, hwid) \
> > + { \
> > + .id = _id, \
> > + .iobase = soc ## _SSP ## hwid ## _BASE_ADDR, \
> > + .dma = soc ## _DMA_SSP ## hwid, \
> > + .irq_err = soc ## _INT_SSP ## hwid ## _ERROR, \
> > + .irq_dma = soc ## _INT_SSP ## hwid ## _DMA, \
> > + }
> > +
> > +#define mxs_mmc_data_entry(soc, _id, hwid) \
> > + [_id] = mxs_mmc_data_entry_single(soc, _id, hwid)
> > +
> > +
> > +#ifdef CONFIG_SOC_IMX23
> > +const struct mxs_mmc_data mx23_mmc_data[] __initconst = {
> > +#define mx23_mmc_data_entry(_id, hwid) \
> > + mxs_mmc_data_entry(MX23, _id, hwid)
> > + mx23_mmc_data_entry(0, 1),
> > + mx23_mmc_data_entry(1, 2),
> > +};
> hmm, maybe let mxs-mmc.1 use MX23_SSP1_BASE_ADDR?
> BTW, Sascha pointed out that
>
> mxs_mmc_data_entry(MX23, 0, 1),
> mxs_mmc_data_entry(MX23, 1, 2),
>
> is easier than
>
> #define mx23_mmc_data_entry(_id, hwid) \
> mxs_mmc_data_entry(MX23, _id, hwid)
> mx23_mmc_data_entry(0, 1),
> mx23_mmc_data_entry(1, 2),
>
This approach was used in mx23 auart device code which Sascha has
merged.
> though it has to repeat MX23.
>
> > +#endif
> > +
> > +#ifdef CONFIG_SOC_IMX28
> > +const struct mxs_mmc_data mx28_mmc_data[] __initconst = {
> > +#define mx28_mmc_data_entry(_id) \
> > + mxs_mmc_data_entry(MX28, _id, _id)
> > + mx28_mmc_data_entry(0),
> > + mx28_mmc_data_entry(1),
> > +};
> > +#endif
> hmm, so i.MX28 starts at 0, i.MX23 at 1. Well, that's how it is.
>
--
Regards,
Shawn
^ permalink raw reply
* [PATCH 2/2] DM9000B: Fix PHY power for network down/up
From: Sergei Shtylyov @ 2011-02-22 12:36 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <4D62D323.7010403@henry.nestler.mail.gmail.com>
Hello.
On 22-02-2011 0:03, Henry Nestler wrote:
>>> DM9000 revision B needs 1 ms delay after PHY power on (see spec), and PHY
>>> power must on in register
>> Couldn't parse that.
You seem to have missed a word in your patch description.
> This can read in manual DM900B-12-DS-F02 from September 2 2010, Page 14:
> "If this Register 1FH bit 0 is updated from '1' to '0', the all
> Registers can not be accessed within 1ms."
That I've understood.
> The example driver code waits 2 ms.
>>> diff --git a/drivers/net/dm9000.c b/drivers/net/dm9000.c
>>> index 2d4c4fc..5925569 100644
>>> --- a/drivers/net/dm9000.c
>>> +++ b/drivers/net/dm9000.c
>> [...]
>>> @@ -1194,6 +1191,10 @@ dm9000_open(struct net_device *dev)
>>> if (request_irq(dev->irq, dm9000_interrupt, irqflags, dev->name, dev))
>>> return -EAGAIN;
>>>
>>> + /* GPIO0 on pre-activate PHY, Reg 1F is not set by reset */
>>> + iow(db, DM9000_GPR, 0); /* REG_1F bit0 activate phyxcer */
>>> + udelay(1000); /* delay needs by DM9000B */
>> Why not mdelay(1)?
> Because udelay is the base of mdelay.
> See include/linux/delay.h:31
> #define mdelay(n) ... udelay((n)*1000)
And?
WBR, Sergei
^ permalink raw reply
* [PATCH v3 2/2] ARM: mxs: add dma device
From: Shawn Guo @ 2011-02-22 12:34 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20110222075241.GU24426@pengutronix.de>
Hi Sascha,
On Tue, Feb 22, 2011 at 08:52:41AM +0100, Sascha Hauer wrote:
> On Mon, Feb 21, 2011 at 06:31:46PM +0800, Shawn Guo wrote:
>
> Being consistent is one thing, but this should not lead to ifdeffery
> where not necessary. Wouldn't it be much simpler to do the following:
>
> static struct platform_device * __init mxs_add_dma(const char *devid,
> resource_size_t base)
> {
> struct resource res_apbh[] = {
> {
> .start = base,
> .end = base + SZ_8K - 1,
> .flags = IORESOURCE_MEM,
> },
> };
>
> return mxs_add_platform_device_dmamask(devid, -1,
> res, ARRAY_SIZE(res), NULL, 0,
> DMA_BIT_MASK(32));
> }
>
> static int __init mxs_add_mxs_dma(void)
> {
> char *apbh = "mxs-dma-apbh";
> char *apbx = "mxs-dma-apbx";
>
> if (cpu_is_mx23()) {
> mxs_add_dma(apbh, MX23_APBH_DMA_BASE_ADDR);
> mxs_add_dma(apbx, MX23_APBX_DMA_BASE_ADDR);
> }
>
> if (cpu_is_mx28()) {
> mxs_add_dma(apbh, MX28_APBH_DMA_BASE_ADDR);
> mxs_add_dma(apbx, MX28_APBX_DMA_BASE_ADDR);
> }
>
> return 0;
> }
> arch_initcall(mxs_add_mxs_dma);
>
> No ifdef, clear to read and only half the size.
>
Looks sane. Thanks.
--
Regards,
Shawn
^ permalink raw reply
* [PATCH v3 2/2] ARM: mxs: add dma device
From: Shawn Guo @ 2011-02-22 12:27 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20110222080928.GS22310@pengutronix.de>
Hi Uwe,
On Tue, Feb 22, 2011 at 09:09:29AM +0100, Uwe Kleine-K?nig wrote:
> On Mon, Feb 21, 2011 at 06:31:46PM +0800, Shawn Guo wrote:
> > Signed-off-by: Shawn Guo <shawn.guo@freescale.com>
> > ---
> > arch/arm/mach-mxs/clock-mx23.c | 3 +-
> > arch/arm/mach-mxs/clock-mx28.c | 4 +-
> > arch/arm/mach-mxs/devices/Makefile | 1 +
> > arch/arm/mach-mxs/devices/platform-dma.c | 101 ++++++++++++++++++++++++++++++
> > 4 files changed, 106 insertions(+), 3 deletions(-)
> > create mode 100644 arch/arm/mach-mxs/devices/platform-dma.c
> >
> > diff --git a/arch/arm/mach-mxs/clock-mx23.c b/arch/arm/mach-mxs/clock-mx23.c
> > index b1a362e..350b28c 100644
> > --- a/arch/arm/mach-mxs/clock-mx23.c
> > +++ b/arch/arm/mach-mxs/clock-mx23.c
> > @@ -443,7 +443,8 @@ static struct clk_lookup lookups[] = {
> > /* for amba-pl011 driver */
> > _REGISTER_CLOCK("duart", NULL, uart_clk)
> > _REGISTER_CLOCK("rtc", NULL, rtc_clk)
> > - _REGISTER_CLOCK(NULL, "hclk", hbus_clk)
> > + _REGISTER_CLOCK("mxs-dma-apbh", NULL, hbus_clk)
> > + _REGISTER_CLOCK("mxs-dma-apbx", NULL, xbus_clk)
> > _REGISTER_CLOCK(NULL, "usb", usb_clk)
> > _REGISTER_CLOCK(NULL, "audio", audio_clk)
> > _REGISTER_CLOCK(NULL, "pwm", pwm_clk)
> > diff --git a/arch/arm/mach-mxs/clock-mx28.c b/arch/arm/mach-mxs/clock-mx28.c
> > index c9d7951..a3b4787 100644
> > --- a/arch/arm/mach-mxs/clock-mx28.c
> > +++ b/arch/arm/mach-mxs/clock-mx28.c
> > @@ -617,8 +617,8 @@ static struct clk_lookup lookups[] = {
> > _REGISTER_CLOCK("fec.0", NULL, fec_clk)
> > _REGISTER_CLOCK("rtc", NULL, rtc_clk)
> > _REGISTER_CLOCK("pll2", NULL, pll2_clk)
> > - _REGISTER_CLOCK(NULL, "hclk", hbus_clk)
> > - _REGISTER_CLOCK(NULL, "xclk", xbus_clk)
> > + _REGISTER_CLOCK("mxs-dma-apbh", NULL, hbus_clk)
> > + _REGISTER_CLOCK("mxs-dma-apbx", NULL, xbus_clk)
> > _REGISTER_CLOCK("flexcan.0", NULL, can0_clk)
> > _REGISTER_CLOCK("flexcan.1", NULL, can1_clk)
> > _REGISTER_CLOCK(NULL, "usb0", usb0_clk)
> > diff --git a/arch/arm/mach-mxs/devices/Makefile b/arch/arm/mach-mxs/devices/Makefile
> > index a8dc8d5..ca7acc4 100644
> > --- a/arch/arm/mach-mxs/devices/Makefile
> > +++ b/arch/arm/mach-mxs/devices/Makefile
> > @@ -1,4 +1,5 @@
> > obj-$(CONFIG_MXS_HAVE_AMBA_DUART) += amba-duart.o
> > obj-$(CONFIG_MXS_HAVE_PLATFORM_AUART) += platform-auart.o
> > +obj-y += platform-dma.o
> > obj-$(CONFIG_MXS_HAVE_PLATFORM_FEC) += platform-fec.o
> > obj-$(CONFIG_MXS_HAVE_PLATFORM_FLEXCAN) += platform-flexcan.o
> > diff --git a/arch/arm/mach-mxs/devices/platform-dma.c b/arch/arm/mach-mxs/devices/platform-dma.c
> > new file mode 100644
> > index 0000000..ee3220e
> > --- /dev/null
> > +++ b/arch/arm/mach-mxs/devices/platform-dma.c
> I'd prefer to have that called platform-mxs-dma.c to match the driver
> name. (At least that's what you use for the data structs.)
>
If you put this comment a little bit earlier on platform-auart.c which
is the first example that saves the "mxs" from driver name, I would
have platform-mxs-dma.c and platform-mxs-mmc.c from the start.
--
Regards,
Shawn
^ permalink raw reply
* [PATCH v3 5/6] ARM: mxs/mx23evk: add mmc device
From: Shawn Guo @ 2011-02-22 12:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20110222083344.GV22310@pengutronix.de>
Hi Uwe,
On Tue, Feb 22, 2011 at 09:33:44AM +0100, Uwe Kleine-K?nig wrote:
> On Mon, Feb 21, 2011 at 06:42:58PM +0800, Shawn Guo wrote:
> > Signed-off-by: Shawn Guo <shawn.guo@freescale.com>
> > ---
> > arch/arm/mach-mxs/Kconfig | 1 +
> > arch/arm/mach-mxs/mach-mx23evk.c | 47 ++++++++++++++++++++++++++++++++++++++
> > 2 files changed, 48 insertions(+), 0 deletions(-)
> >
> > diff --git a/arch/arm/mach-mxs/Kconfig b/arch/arm/mach-mxs/Kconfig
> > index cd2fbdf..f47fae3 100644
> > --- a/arch/arm/mach-mxs/Kconfig
> > +++ b/arch/arm/mach-mxs/Kconfig
> > @@ -19,6 +19,7 @@ config MACH_MX23EVK
> > bool "Support MX23EVK Platform"
> > select SOC_IMX23
> > select MXS_HAVE_AMBA_DUART
> > + select MXS_HAVE_PLATFORM_MMC
> > default y
> > help
> > Include support for MX23EVK platform. This includes specific
> > diff --git a/arch/arm/mach-mxs/mach-mx23evk.c b/arch/arm/mach-mxs/mach-mx23evk.c
> > index aa06400..21519e3 100644
> > --- a/arch/arm/mach-mxs/mach-mx23evk.c
> > +++ b/arch/arm/mach-mxs/mach-mx23evk.c
> > @@ -26,17 +26,64 @@
> >
> > #include "devices-mx23.h"
> >
> > +#define MX23EVK_MMC0_WRITE_PROTECT MXS_GPIO_NR(1, 30)
> > +#define MX23EVK_MMC0_SLOT_POWER MXS_GPIO_NR(1, 29)
> > +
> > static const iomux_cfg_t mx23evk_pads[] __initconst = {
> > /* duart */
> > MX23_PAD_PWM0__DUART_RX | MXS_PAD_4MA,
> > MX23_PAD_PWM1__DUART_TX | MXS_PAD_4MA,
> > +
> > + /* mmc */
> > + MX23_PAD_SSP1_DATA0__SSP1_DATA0 |
> > + (MXS_PAD_8MA | MXS_PAD_3V3 | MXS_PAD_PULLUP),
> > + MX23_PAD_SSP1_DATA1__SSP1_DATA1 |
> > + (MXS_PAD_8MA | MXS_PAD_3V3 | MXS_PAD_PULLUP),
> > + MX23_PAD_SSP1_DATA2__SSP1_DATA2 |
> > + (MXS_PAD_8MA | MXS_PAD_3V3 | MXS_PAD_PULLUP),
> > + MX23_PAD_SSP1_DATA3__SSP1_DATA3 |
> > + (MXS_PAD_8MA | MXS_PAD_3V3 | MXS_PAD_PULLUP),
> > + MX23_PAD_GPMI_D08__SSP1_DATA4 |
> > + (MXS_PAD_8MA | MXS_PAD_3V3 | MXS_PAD_PULLUP),
> > + MX23_PAD_GPMI_D09__SSP1_DATA5 |
> > + (MXS_PAD_8MA | MXS_PAD_3V3 | MXS_PAD_PULLUP),
> > + MX23_PAD_GPMI_D10__SSP1_DATA6 |
> > + (MXS_PAD_8MA | MXS_PAD_3V3 | MXS_PAD_PULLUP),
> > + MX23_PAD_GPMI_D11__SSP1_DATA7 |
> > + (MXS_PAD_8MA | MXS_PAD_3V3 | MXS_PAD_PULLUP),
> > + MX23_PAD_SSP1_CMD__SSP1_CMD |
> > + (MXS_PAD_8MA | MXS_PAD_3V3 | MXS_PAD_PULLUP),
> > + MX23_PAD_SSP1_DETECT__SSP1_DETECT |
> > + (MXS_PAD_8MA | MXS_PAD_3V3 | MXS_PAD_NOPULL),
> > + MX23_PAD_SSP1_SCK__SSP1_SCK |
> > + (MXS_PAD_8MA | MXS_PAD_3V3 | MXS_PAD_NOPULL),
> > + /* write protect */
> > + MX23_PAD_PWM4__GPIO_1_30 |
> > + (MXS_PAD_4MA | MXS_PAD_3V3 | MXS_PAD_NOPULL),
> > + /* slot power enable */
> > + MX23_PAD_PWM3__GPIO_1_29 |
> > + (MXS_PAD_4MA | MXS_PAD_3V3 | MXS_PAD_NOPULL),
> > +};
> > +
> > +static struct mxs_mmc_platform_data mx23_mmc_pdata __initdata = {
> > + .wp_gpio = MX23EVK_MMC0_WRITE_PROTECT,
> > + .flags = SLOTF_8_BIT_CAPABLE,
> can this be const?
>
Do you care this much to see a new version of the patch?
--
Regards,
Shawn
^ permalink raw reply
* [PATCH v2 5/6] davinci: add spi devices support for da850/omap-l138/am18x evm
From: Michael Williamson @ 2011-02-22 12:13 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <B85A65D85D7EB246BE421B3FB0FBB593024BD48EB5@dbde02.ent.ti.com>
On 2/22/2011 1:37 AM, Nori, Sekhar wrote:
> On Wed, Feb 09, 2011 at 18:41:53, Michael Williamson wrote:
>> From: Sekhar Nori <nsekhar@ti.com>
>>
>> This patch adds the on-board SPI flash device to the
>> DA850/OMAP-L138/AM18x EVM. It also registers the SPI flash
>> device to the MTD subsystem.
>>
>> Based on SPI flash device support for MityDSP-L138F platform.
>>
>> Signed-off-by: Sekhar Nori <nsekhar@ti.com>
>> [michael.williamson at criticallink.com: moved da850_evm_spi1_pdata to devices-da8xx.c]
>> Signed-off-by: Michael Williamson <michael.williamson@criticallink.com>
>> +static void __init da850evm_init_spi1(struct spi_board_info *info, unsigned len)
>> +{
>> + int ret;
>> +
>> + ret = spi_register_board_info(info, len);
>> + if (ret)
>> + pr_warning("failed to register board info : %d\n", ret);
>> +
>> + da8xx_spi_pdata[1].num_chipselect = len;
>> +
>> + ret = da8xx_register_spi(1);
>> + if (ret)
>> + pr_warning("failed to register spi 1 device : %d\n", ret);
>> +}
>
> When reposting can you also modify this to print function name
> with the error message (so it is easy to find what exactly failed).
>
> The same thing needs to be done with DA830 EVM.
>
> Rest of the stuff looks good to me.
>
Sure.
> Thanks,
> Sekhar
^ permalink raw reply
* [PATCH v2 3/6] davinci: da8xx: add spi resources and registration routine
From: Michael Williamson @ 2011-02-22 12:13 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <B85A65D85D7EB246BE421B3FB0FBB593024BD48E7B@dbde02.ent.ti.com>
On 2/22/2011 1:14 AM, Nori, Sekhar wrote:
> Hi Mike,
>
> On Wed, Feb 09, 2011 at 18:41:51, Michael Williamson wrote:
>> Add IO resource structures, platform data, and a registration
>> routine in order to support spi device on DA850/OMAP-L138/AM18x
>> and DA830/OMAP-L137/AM17x platforms.
>>
>> Signed-off-by: Michael Williamson <michael.williamson@criticallink.com>
>
> [...]
>
>> +static struct resource da8xx_spi1_resources[] = {
>> + [0] = {
>> + .start = DA8XX_SPI1_BASE,
>> + .end = DA8XX_SPI1_BASE + SZ_4K -1,
>
> checkpatch reports a warning here. Need to have spaces
> on either side of binary '-'
>
Sorry about that... not sure how I missed that one.
> I tested this series and the two patches it depends on
> using SPI flash present on DA850 and DA830 EVMs (tested
> in DMA mode) and found no issues.
>
Thanks for testing, Sekhar.
> So, with this minor issue fixed, can you please repost
> with my Ack? You can include the two dependent patches
> in the same series so it is easy to pick-up the whole
> bunch.
>
OK.
> Thanks,
> Sekhar
>
^ permalink raw reply
* [PATCH 1/2] PRUSS UIO driver support
From: TK, Pratheesh Gangadhar @ 2011-02-22 12:06 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <4D62C2A0.6060405@mvista.com>
Hi,
> -----Original Message-----
> From: Sergei Shtylyov [mailto:sshtylyov at mvista.com]
> Sent: Tuesday, February 22, 2011 1:23 AM
> To: Hans J. Koch
> Cc: TK, Pratheesh Gangadhar; davinci-linux-open-
> source at linux.davincidsp.com; Arnd Bergmann; Chatterjee, Amit;
> gregkh at suse.de; LKML; Thomas Gleixner; linux-arm-
> kernel at lists.infradead.org
> Subject: Re: [PATCH 1/2] PRUSS UIO driver support
>
> Hello.
>
> Hans J. Koch wrote:
>
> >>> -----Original Message-----
> >>> From: Hans J. Koch [mailto:hjk at hansjkoch.de]
> >>> Sent: Sunday, February 20, 2011 12:04 AM
> >>> To: TK, Pratheesh Gangadhar
> >>> Cc: Thomas Gleixner; Arnd Bergmann; linux-arm-
> kernel at lists.infradead.org;
> >>> davinci-linux-open-source at linux.davincidsp.com; gregkh at suse.de;
> >>> Chatterjee, Amit; Hans J. Koch; LKML
> >>> Subject: Re: [PATCH 1/2] PRUSS UIO driver support
>
> >>> On Sat, Feb 19, 2011 at 09:10:23PM +0530, TK, Pratheesh Gangadhar
> wrote:
> >>>> For my understanding - if the interrupt is not shared and not level
> triggered -
> >>>> is this okay to have empty handler?
>
> >>> Greg already said he won't accept that. And I'm quite sure these
> >>> interrupts
> >>> are level triggered, since that is the default on arch/omap.
>
> >>> E.g. in arch/arm/mach-omap1/irq.c, a loop sets all irqs to level
> triggered
> >>> handling: set_irq_handler(j, handle_level_irq); (line 234)
>
> >> You should be looking at arch/arm/mach-davinci/irq.c
>
> Into arch/arm/mach-davinci/cp_intc.c actually as we're not talking
> about a
> "real" DaVincis here, but about OMAP-L138.
>
> > Hmpf. I alway get lost in the directory structure of TI SoCs...
> > At least I learned that OMAP3 is in the omap2 directory :)
>
> It's probably even more convoluted with DaVinci and friends. :-)
>
> >> and all interrupts except IRQ_TINT1_TINT34 is set to edge trigger mode
> (line 160).
>
> > Is that really needed? Level triggered interrupts should certainly be
> > preferred if possible.
>
> Don't look there, this code is for "real" DaVincis only.
>
Thanks for correcting. Interrupts are configured as edge triggered here as well.
Thanks and Regards,
Pratheesh
^ permalink raw reply
* [PATCH v2 1/2] PRUSS UIO driver support
From: TK, Pratheesh Gangadhar @ 2011-02-22 12:01 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20110221184432.GA2773@local>
Hi,
> -----Original Message-----
> From: Hans J. Koch [mailto:hjk at hansjkoch.de]
> Sent: Tuesday, February 22, 2011 12:15 AM
> To: TK, Pratheesh Gangadhar
> Cc: davinci-linux-open-source at linux.davincidsp.com; hjk at hansjkoch.de;
> gregkh at suse.de; Chatterjee, Amit; linux-kernel at vger.kernel.org; linux-arm-
> kernel at lists.infradead.org
> Subject: Re: [PATCH v2 1/2] PRUSS UIO driver support
>
> On Mon, Feb 21, 2011 at 11:05:14PM +0530, Pratheesh Gangadhar wrote:
> > This patch implements PRUSS (Programmable Real-time Unit Sub System)
> > UIO driver which exports SOC resources associated with PRUSS like
> > I/O, memories and IRQs to user space. PRUSS is dual 32-bit RISC
> > processors which is efficient in performing embedded tasks that
> > require manipulation of packed memory mapped data structures and
> > efficient in handling system events that have tight real time
> > constraints. This driver is currently supported on Texas Instruments
> > DA850, AM18xx and OMAPL1-38 devices.
> > For example, PRUSS runs firmware for real-time critical industrial
> > communication data link layer and communicates with application stack
> > running in user space via shared memory and IRQs.
> >
> > Signed-off-by: Pratheesh Gangadhar <pratheesh@ti.com>
>
> Looks much better now. Just some minor issues, see below.
>
> Thanks,
> Hans
>
> > ---
> > drivers/uio/Kconfig | 9 ++
> > drivers/uio/Makefile | 1 +
> > drivers/uio/uio_pruss.c | 231
> +++++++++++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 241 insertions(+), 0 deletions(-)
> > create mode 100644 drivers/uio/uio_pruss.c
> >
> > diff --git a/drivers/uio/Kconfig b/drivers/uio/Kconfig
> > index bb44079..f92f20d 100644
> > --- a/drivers/uio/Kconfig
> > +++ b/drivers/uio/Kconfig
> > @@ -94,4 +94,13 @@ config UIO_NETX
> > To compile this driver as a module, choose M here; the module
> > will be called uio_netx.
> >
> > +config UIO_PRUSS
> > + tristate "Texas Instruments PRUSS driver"
> > + depends on ARCH_DAVINCI_DA850
> > + help
> > + PRUSS driver for OMAPL138/DA850/AM18XX devices
> > + PRUSS driver requires user space components
>
> It would be nice to mention a link here where these user space components
> are
> available.
Ok, will do.
>
> > + To compile this driver as a module, choose M here: the module
> > + will be called uio_pruss.
> > +
> > endif
> > diff --git a/drivers/uio/Makefile b/drivers/uio/Makefile
> > index 18fd818..d4dd9a5 100644
> > --- a/drivers/uio/Makefile
> > +++ b/drivers/uio/Makefile
> > @@ -6,3 +6,4 @@ obj-$(CONFIG_UIO_AEC) += uio_aec.o
> > obj-$(CONFIG_UIO_SERCOS3) += uio_sercos3.o
> > obj-$(CONFIG_UIO_PCI_GENERIC) += uio_pci_generic.o
> > obj-$(CONFIG_UIO_NETX) += uio_netx.o
> > +obj-$(CONFIG_UIO_PRUSS) += uio_pruss.o
> > diff --git a/drivers/uio/uio_pruss.c b/drivers/uio/uio_pruss.c
> > new file mode 100644
> > index 0000000..b402197
> > --- /dev/null
> > +++ b/drivers/uio/uio_pruss.c
> > @@ -0,0 +1,231 @@
> > +/*
> > + * Programmable Real-Time Unit Sub System (PRUSS) UIO driver
> (uio_pruss)
> > + *
> > + * This driver exports PRUSS host event out interrupts and PRUSS, L3
> RAM,
> > + * and DDR RAM to user space for applications interacting with PRUSS
> firmware
> > + *
> > + * Copyright (C) 2010-11 Texas Instruments Incorporated -
> http://www.ti.com/
> > + *
> > + * This program is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU General Public License as
> > + * published by the Free Software Foundation version 2.
> > + *
> > + * This program is distributed "as is" WITHOUT ANY WARRANTY of any
> > + * kind, whether express or implied; without even the implied warranty
> > + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > + * GNU General Public License for more details.
> > + */
> > +#include <linux/device.h>
> > +#include <linux/module.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/uio_driver.h>
> > +#include <linux/io.h>
> > +#include <linux/clk.h>
> > +#include <linux/dma-mapping.h>
> > +#include <linux/slab.h>
> > +
> > +#define DRV_NAME "pruss"
> > +#define DRV_VERSION "0.50"
> > +
> > +/*
> > + * Host event IRQ numbers from PRUSS - PRUSS can generate upto 8
> interrupt
> > + * events to AINTC of ARM host processor - which can be used for IPC
> b/w PRUSS
> > + * firmware and user space application, async notification from PRU
> firmware
> > + * to user space application
> > + * 3 PRU_EVTOUT0
> > + * 4 PRU_EVTOUT1
> > + * 5 PRU_EVTOUT2
> > + * 6 PRU_EVTOUT3
> > + * 7 PRU_EVTOUT4
> > + * 8 PRU_EVTOUT5
> > + * 9 PRU_EVTOUT6
> > + * 10 PRU_EVTOUT7
> > +*/
> > +
> > +#define MAX_PRUSS_EVTOUT_INSTANCE 8
> > +
> > +#define PRUSS_INTC_HIPIR 0x4900
> > +#define PRUSS_INTC_HIPIR_INTPEND_MASK 0x80000000
> > +#define PRUSS_INTC_HIER 0x5500
> > +
> > +struct clk *pruss_clk;
> > +struct uio_info *info;
> > +void *ddr_virt_addr;
> > +dma_addr_t ddr_phy_addr;
> > +
> > +static irqreturn_t pruss_handler(int irq, struct uio_info *dev_info)
> > +{
> > + void __iomem *int_enable_reg = dev_info->mem[0].internal_addr
> > + + PRUSS_INTC_HIER;
> > + void __iomem *int_status_reg = dev_info->mem[0].internal_addr
> > + + PRUSS_INTC_HIPIR+((irq-1) << 2);
>
> Blank line between variable definitions and code, please.
>
Ok.
> > + /* Is interrupt enabled and active ? */
> > + if (!(ioread32(int_enable_reg) & (1<<(irq-1))) &&
>
> Hmm. I'd prefer blanks around operands, like (1 << (irq - 1))).
> I won't be too picky about that, noticing that checkpatch.pl doesn't
> complain.
> If you want to do me a favor, you can fix it ;-)
>
Ok.
> > + (ioread32(int_status_reg) & PRUSS_INTC_HIPIR_INTPEND_MASK))
> > + return IRQ_NONE;
> > +
> > + /* Disable interrupt */
> > + iowrite32(ioread32(int_enable_reg) & ~(1<<(irq-1)),
>
> If you save the return value of the first ioread32(int_enable_reg) in a
> variable,
> you don't need the second hardware access.
>
Will do.
> > + int_enable_reg);
> > + return IRQ_HANDLED;
> > +}
> > +
> > +static void pruss_cleanup(struct platform_device *dev, struct uio_info
> *info)
> > +{
> > + int count;
>
> Blank line between variable definitions and code, please.
>
Ok.
> > + if (info) {
> > + for (count = 0; count < MAX_PRUSS_EVTOUT_INSTANCE; count++) {
> > + uio_unregister_device(&info[count]);
> > + kfree(info[count].name);
> > + iounmap(info[count].mem[0].internal_addr);
> > +
> > + }
> > + if (ddr_virt_addr)
> > + dma_free_coherent(&dev->dev, info[0].mem[2].size,
> > + info[0].mem[2].internal_addr,
> > + info[0].mem[2].addr);
> > + kfree(info);
> > + }
> > + if (pruss_clk != NULL)
> > + clk_put(pruss_clk);
> > +}
> > +
> > +static int __devinit pruss_probe(struct platform_device *dev)
> > +{
> > + int ret = -ENODEV, count = 0;
> > + struct resource *regs_pruram, *regs_l3ram, *regs_ddr;
> > + char *string;
> > +
> > + /* Power on PRU in case its not done as part of boot-loader */
> > + pruss_clk = clk_get(&dev->dev, "pruss");
> > + if (IS_ERR(pruss_clk)) {
> > + dev_err(&dev->dev, "Failed to get clock\n");
> > + ret = PTR_ERR(pruss_clk);
> > + return ret;
> > + } else {
> > + clk_enable(pruss_clk);
> > + }
> > +
> > + info = kzalloc(sizeof(struct uio_info) * MAX_PRUSS_EVTOUT_INSTANCE,
> > + GFP_KERNEL);
> > + if (info == NULL)
>
> if (!info) looks better.
>
> > + return -ENOMEM;
> > +
> > + regs_pruram = platform_get_resource(dev, IORESOURCE_MEM, 0);
> > + if (!regs_pruram) {
> > + dev_err(&dev->dev, "No memory resource specified\n");
> > + goto out_free;
> > + }
> > +
> > + regs_l3ram = platform_get_resource(dev, IORESOURCE_MEM, 1);
> > + if (!regs_l3ram) {
> > + dev_err(&dev->dev, "No memory resource specified\n");
> > + goto out_free;
> > + }
> > +
> > + regs_ddr = platform_get_resource(dev, IORESOURCE_MEM, 2);
> > + if (!regs_ddr) {
> > + dev_err(&dev->dev, "No memory resource specified\n");
> > + goto out_free;
> > + }
> > + ddr_virt_addr =
> > + dma_alloc_coherent(&dev->dev, regs_ddr->end - regs_ddr->start +
> 1,
> > + &ddr_phy_addr, GFP_KERNEL | GFP_DMA);
> > + if (!ddr_virt_addr) {
> > + dev_err(&dev->dev, "Could not allocate external memory\n");
> > + goto out_free;
> > + }
> > +
> > + for (count = 0; count < MAX_PRUSS_EVTOUT_INSTANCE; count++) {
> > + info[count].mem[0].addr = regs_pruram->start;
> > + info[count].mem[0].size =
> > + regs_pruram->end - regs_pruram->start + 1;
> > + if (!info[count].mem[0].addr ||
> > + !(info[count].mem[0].size - 1)) {
> > + dev_err(&dev->dev, "Invalid memory resource\n");
> > + goto out_free;
> > + }
> > + info[count].mem[0].internal_addr =
> > + ioremap(regs_pruram->start, info[count].mem[0].size);
> > + if (!info[count].mem[0].internal_addr) {
> > + dev_err(&dev->dev,
> > + "Can't remap memory address range\n");
> > + goto out_free;
> > + }
> > + info[count].mem[0].memtype = UIO_MEM_PHYS;
> > +
> > + info[count].mem[1].addr = regs_l3ram->start;
> > + info[count].mem[1].size =
> > + regs_l3ram->end - regs_l3ram->start + 1;
> > + if (!info[count].mem[1].addr ||
> > + !(info[count].mem[1].size - 1)) {
> > + dev_err(&dev->dev, "Invalid memory resource\n");
> > + goto out_free;
> > + }
> > + info[count].mem[1].memtype = UIO_MEM_PHYS;
> > +
> > + info[count].mem[2].internal_addr = ddr_virt_addr;
> > + info[count].mem[2].addr = ddr_phy_addr;
> > + info[count].mem[2].size = regs_ddr->end - regs_ddr->start + 1;
> > + info[count].mem[2].memtype = UIO_MEM_PHYS;
> > +
> > + string = kzalloc(20, GFP_KERNEL);
> > + sprintf(string, "pruss_evt%d", count);
> > + info[count].name = string;
> > + info[count].version = "0.50";
> > +
> > + /* Register PRUSS IRQ lines */
> > + info[count].irq = IRQ_DA8XX_EVTOUT0 + count;
> > +
> > + info[count].irq_flags = 0;
> > + info[count].handler = pruss_handler;
> > +
> > + ret = uio_register_device(&dev->dev, &info[count]);
> > +
> > + if (ret < 0)
> > + goto out_free;
> > + }
> > +
> > + platform_set_drvdata(dev, info);
> > + return 0;
> > +
> > +out_free:
> > + pruss_cleanup(dev, info);
> > + return ret;
> > +}
> > +
> > +static int __devexit pruss_remove(struct platform_device *dev)
> > +{
> > + struct uio_info *info = platform_get_drvdata(dev);
>
> Blank line, please.
>
Ok.
Thanks,
Pratheesh
^ permalink raw reply
* [PATCH v2 1/2] PRUSS UIO driver support
From: TK, Pratheesh Gangadhar @ 2011-02-22 11:58 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <alpine.LFD.2.00.1102211939110.2701@localhost6.localdomain6>
Hi,
> -----Original Message-----
> From: Thomas Gleixner [mailto:tglx at linutronix.de]
> Sent: Tuesday, February 22, 2011 12:24 AM
> To: TK, Pratheesh Gangadhar
> Cc: davinci-linux-open-source at linux.davincidsp.com; hjk at hansjkoch.de;
> gregkh at suse.de; Chatterjee, Amit; linux-kernel at vger.kernel.org; linux-arm-
> kernel at lists.infradead.org
> Subject: Re: [PATCH v2 1/2] PRUSS UIO driver support
>
> On Mon, 21 Feb 2011, Pratheesh Gangadhar wrote:
> > +static irqreturn_t pruss_handler(int irq, struct uio_info *dev_info)
> > +{
> > + void __iomem *int_enable_reg = dev_info->mem[0].internal_addr
> > + + PRUSS_INTC_HIER;
> > + void __iomem *int_status_reg = dev_info->mem[0].internal_addr
> > + + PRUSS_INTC_HIPIR+((irq-1) << 2);
>
> void __iomem *base = dev_info->mem[0].internal_addr;
> void __iomem *int_enable_reg = base + PRUSS_INTC_HIER;
> ....
>
> Makes that readable.
Ok.
>
> > + /* Is interrupt enabled and active ? */
> > + if (!(ioread32(int_enable_reg) & (1<<(irq-1))) &&
> > + (ioread32(int_status_reg) & PRUSS_INTC_HIPIR_INTPEND_MASK))
> > + return IRQ_NONE;
>
> That returns when the interrupt is disabled _AND_ pending. It should
> return when the interrupt is disabled _OR_ not pending.
I should have named it _NOPEND_MASK, basically mask is set when there is no pending interrupt.
> > +
> > + /* Disable interrupt */
> > + iowrite32(ioread32(int_enable_reg) & ~(1<<(irq-1)),
> > + int_enable_reg);
>
> Chosing shorter variable names avoid those line breaks all over the
> place.
>
Ok.
> > + return IRQ_HANDLED;
> > +}
> > +
> > +static void pruss_cleanup(struct platform_device *dev, struct uio_info
> *info)
> > +{
> > + int count;
>
> New line between variables and code please
>
Ok.
> > + if (info) {
>
> This check is silly. pruss_probe() returns right away when it cannot
> allocate info. pruss_remove() is never called when info == NULL
>
Ok.
> > + for (count = 0; count < MAX_PRUSS_EVTOUT_INSTANCE; count++) {
> > + uio_unregister_device(&info[count]);
> > + kfree(info[count].name);
> > + iounmap(info[count].mem[0].internal_addr);
>
> Why do you map/unmap the same physical address 8 times ????
Will fix this.
>
> > + }
> > + if (ddr_virt_addr)
> > + dma_free_coherent(&dev->dev, info[0].mem[2].size,
> > + info[0].mem[2].internal_addr,
> > + info[0].mem[2].addr);
> > + kfree(info);
> > + }
> > + if (pruss_clk != NULL)
>
> Silly check as well.
>
Ok.
> > + clk_put(pruss_clk);
> > +}
> > +
> > +static int __devinit pruss_probe(struct platform_device *dev)
> > +{
> > + int ret = -ENODEV, count = 0;
> > + struct resource *regs_pruram, *regs_l3ram, *regs_ddr;
> > + char *string;
> > +
> > + /* Power on PRU in case its not done as part of boot-loader */
> > + pruss_clk = clk_get(&dev->dev, "pruss");
> > + if (IS_ERR(pruss_clk)) {
> > + dev_err(&dev->dev, "Failed to get clock\n");
> > + ret = PTR_ERR(pruss_clk);
> > + return ret;
> > + } else {
> > + clk_enable(pruss_clk);
> > + }
> > +
> > + info = kzalloc(sizeof(struct uio_info) * MAX_PRUSS_EVTOUT_INSTANCE,
> > + GFP_KERNEL);
> > + if (info == NULL)
>
> if (!info)
>
> > + return -ENOMEM;
>
> Leaves the clock enabled
>
Will fix this.
> > + regs_pruram = platform_get_resource(dev, IORESOURCE_MEM, 0);
> > + if (!regs_pruram) {
> > + dev_err(&dev->dev, "No memory resource specified\n");
> > + goto out_free;
> > + }
> > +
> > + regs_l3ram = platform_get_resource(dev, IORESOURCE_MEM, 1);
> > + if (!regs_l3ram) {
> > + dev_err(&dev->dev, "No memory resource specified\n");
> > + goto out_free;
> > + }
> > +
> > + regs_ddr = platform_get_resource(dev, IORESOURCE_MEM, 2);
> > + if (!regs_ddr) {
> > + dev_err(&dev->dev, "No memory resource specified\n");
> > + goto out_free;
> > + }
> > + ddr_virt_addr =
> > + dma_alloc_coherent(&dev->dev, regs_ddr->end - regs_ddr->start +
> 1,
> > + &ddr_phy_addr, GFP_KERNEL | GFP_DMA);
> > + if (!ddr_virt_addr) {
> > + dev_err(&dev->dev, "Could not allocate external memory\n");
> > + goto out_free;
> > + }
> > +
> > + for (count = 0; count < MAX_PRUSS_EVTOUT_INSTANCE; count++) {
>
> Sigh. Can't you have a pointer struct uio_info *p and do the following.
>
Ok.
> for (cnt = 0; p = info; cnt < MAX_PRUSS_EVTOUT_INSTANCE; cnt++, p++)
> {
>
> p->mem[0] ...
>
> > + info[count].mem[0].addr = regs_pruram->start;
> > + info[count].mem[0].size =
> > + regs_pruram->end - regs_pruram->start + 1;
> > + if (!info[count].mem[0].addr ||
> > + !(info[count].mem[0].size - 1)) {
>
> All you catch is size == 0. So with size == 1 it works ???
I can drop this check as well as normally one won't specify improbable size for mapping memory during configuration.
>
> > + dev_err(&dev->dev, "Invalid memory resource\n");
> > + goto out_free;
> > + }
> > + info[count].mem[0].internal_addr =
> > + ioremap(regs_pruram->start, info[count].mem[0].size);
>
> That's redundant to remap the same address 8 times. That and the check
> above should be done before the loop and the result used in the loop.
>
Ok, will do.
> > + if (!info[count].mem[0].internal_addr) {
> > + dev_err(&dev->dev,
> > + "Can't remap memory address range\n");
> > + goto out_free;
> > + }
> > + info[count].mem[0].memtype = UIO_MEM_PHYS;
> > +
> > + info[count].mem[1].addr = regs_l3ram->start;
> > + info[count].mem[1].size =
> > + regs_l3ram->end - regs_l3ram->start + 1;
> > + if (!info[count].mem[1].addr ||
> > + !(info[count].mem[1].size - 1)) {
> > + dev_err(&dev->dev, "Invalid memory resource\n");
> > + goto out_free;
> > + }
>
> No need to check the same thing over and over.
>
> > + info[count].mem[1].memtype = UIO_MEM_PHYS;
> > +
> > + info[count].mem[2].internal_addr = ddr_virt_addr;
> > + info[count].mem[2].addr = ddr_phy_addr;
> > + info[count].mem[2].size = regs_ddr->end - regs_ddr->start + 1;
> > + info[count].mem[2].memtype = UIO_MEM_PHYS;
> > +
> > + string = kzalloc(20, GFP_KERNEL);
> > + sprintf(string, "pruss_evt%d", count);
> > + info[count].name = string;
>
> kasprintf() please
Ok.
>
> > + info[count].version = "0.50";
> > +
> > + /* Register PRUSS IRQ lines */
> > + info[count].irq = IRQ_DA8XX_EVTOUT0 + count;
> > +
> > + info[count].irq_flags = 0;
>
> Is already zero
>
Ok.
> > + info[count].handler = pruss_handler;
> > +
> > + ret = uio_register_device(&dev->dev, &info[count]);
> > +
> > + if (ret < 0)
> > + goto out_free;
> > + }
> > +
> > + platform_set_drvdata(dev, info);
> > + return 0;
> > +
> > +out_free:
> > + pruss_cleanup(dev, info);
> > + return ret;
> > +}
> > +
> > +static int __devexit pruss_remove(struct platform_device *dev)
> > +{
> > + struct uio_info *info = platform_get_drvdata(dev);
>
> Empty line between variables and code.
>
Ok.
> > + pruss_cleanup(dev, info);
> > + platform_set_drvdata(dev, NULL);
> > + return 0;
>
Thanks,
Pratheesh
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox