* [PATCH 1/2] ARM imx53: add pwm devices support
From: Eric Miao @ 2011-03-01 8:21 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20110301081043.GS22310@pengutronix.de>
>> ?config IMX_HAVE_PLATFORM_MXC_PWM
>> ? ? ? bool
>> + ? ? default y if ARCH_MX21 || ARCH_MX25 || SOC_IMX27 || SOC_IMX51 || SOC_IMX53
> If you do this, can you please use SOC_IMX21 and SOC_IMX25? ?Or maybe
> even def_bool y?
>
Or the other way, in config SOC_IMX53, select IMX_HAVE_PLATFORM_MXC_PWM?
^ permalink raw reply
* [RFC] device.h: add device_set_platdata routine
From: viresh kumar @ 2011-03-01 8:27 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20110301075953.GR22310@pengutronix.de>
On 03/01/2011 01:29 PM, Uwe Kleine-K?nig wrote:
> [added gregkh and lkml to Cc:]
>
thanks.
> On Tue, Mar 01, 2011 at 10:03:20AM +0530, Viresh Kumar wrote:
>> device.h supports device_get_platdata but doesn't support device_set_platdata.
>> This routine is required by platforms in which device structure is declared
>> in a machine specific file and platform data comes from board specific file.
>>
>> This will be used by SPEAr patches sent in separate patch series.
>>
>> Signed-off-by: Viresh Kumar <viresh.kumar@st.com>
>> ---
>> include/linux/device.h | 5 +++++
>> 1 files changed, 5 insertions(+), 0 deletions(-)
>>
>> diff --git a/include/linux/device.h b/include/linux/device.h
>> index 1bf5cf0..6ce0f20 100644
>> --- a/include/linux/device.h
>> +++ b/include/linux/device.h
>> @@ -576,6 +576,11 @@ static inline void *dev_get_platdata(const struct device *dev)
>> return dev->platform_data;
>> }
>>
>> +static inline void dev_set_platdata(struct device *dev, void *platdata)
>> +{
>> + dev->platform_data = platdata;
>> +}
>> +
> Note that dev->platform_data was designed to hold dynamically allocated
> memory, at least it's kfreed in platform_device_release. And note there
> is platform_device_add_data that kmemdups its argument into
> pdev->dev.platform_data.
>
Ok. So we should use platform_device_add_data instead and mark our platform
data's struct as __init. So that it doesn't consume any memory after
this routine is done??
> Compared to your dev_set_platdata platform_device_add_data only works
> for platform_devices, don't know if it's worth to change that.
>
Currently i need this for platform devs only. So its good enough for me.
--
viresh
^ permalink raw reply
* [PATCH] plat-mxc: Provide irq_chip name for GPIO IRQs
From: Wolfram Sang @ 2011-03-01 8:34 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <201103010838.37197.alexander.stein@systec-electronic.com>
Hi,
> > static struct irq_chip gpio_irq_chip = {
> > + .name = "GPIO",
I'd suggest a more specific name, e.g. "i.MX GPIO"? You can then also
add my
Acked-by: Wolfram Sang <w.sang@pengutronix.de>
Regards,
Wolfram
--
Pengutronix e.K. | Wolfram Sang |
Industrial Linux Solutions | http://www.pengutronix.de/ |
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20110301/dacba0d6/attachment.sig>
^ permalink raw reply
* [RFC PATCH] ARM: Use generic BUG() handler
From: Russell King - ARM Linux @ 2011-03-01 8:49 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1298939263-16421-1-git-send-email-sjg@chromium.org>
On Mon, Feb 28, 2011 at 04:27:43PM -0800, Simon Glass wrote:
> + asm volatile("1:\t.word %c3\n" \
> + ".pushsection __bug_table,\"a\"\n" \
> + "2:\t.word 1b, %c0\n" \
> + "\t.hword %c1, 0\n" \
> + "\t.org 2b+%c2\n" \
%c doesn't work on lots of versions of gcc, which is why we can't use
the generic bug support. There's no way to reliably generate constants
without many compiler versions spitting out a '#' before them.
^ permalink raw reply
* [RFC PATCH] ARM: Use generic BUG() handler
From: Russell King - ARM Linux @ 2011-03-01 8:59 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20110301084949.GA16733@n2100.arm.linux.org.uk>
On Tue, Mar 01, 2011 at 08:49:49AM +0000, Russell King - ARM Linux wrote:
> On Mon, Feb 28, 2011 at 04:27:43PM -0800, Simon Glass wrote:
> > + asm volatile("1:\t.word %c3\n" \
> > + ".pushsection __bug_table,\"a\"\n" \
> > + "2:\t.word 1b, %c0\n" \
> > + "\t.hword %c1, 0\n" \
> > + "\t.org 2b+%c2\n" \
>
> %c doesn't work on lots of versions of gcc, which is why we can't use
> the generic bug support. There's no way to reliably generate constants
> without many compiler versions spitting out a '#' before them.
gcc 4.3.2:
asm(".word %c0" : : "i" (0));
produces:
.word #0
which gas chokes on:
/tmp/cc2hGOHd.s:12: Error: bad expression
/tmp/cc2hGOHd.s:12: Error: junk at end of line, first unrecognized character is `0'
So what this means is that it's impossible to generate constants in
assembly with GCC targetting ARM without having them prefixed by '#',
which in turn makes it impossible to use the generic BUG support.
I reported this bug to gcc folk many years ago. I've no idea which
version it has been fixed in or if it's even been fixed.
^ permalink raw reply
* [PATCH 1/2] ARM imx53: add pwm devices support
From: Uwe Kleine-König @ 2011-03-01 9:04 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <AANLkTin5SCtSZfFhErb9baWpMioDWk=UA15cOddb1B=R@mail.gmail.com>
On Tue, Mar 01, 2011 at 04:21:37PM +0800, Eric Miao wrote:
> >> ?config IMX_HAVE_PLATFORM_MXC_PWM
> >> ? ? ? bool
> >> + ? ? default y if ARCH_MX21 || ARCH_MX25 || SOC_IMX27 || SOC_IMX51 || SOC_IMX53
> > If you do this, can you please use SOC_IMX21 and SOC_IMX25? ?Or maybe
> > even def_bool y?
> >
>
> Or the other way, in config SOC_IMX53, select IMX_HAVE_PLATFORM_MXC_PWM?
Yeah, that's how most of the other IMX_HAVE_PLATFORM_XYZ work.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply
* [RFC] device.h: add device_set_platdata routine
From: Uwe Kleine-König @ 2011-03-01 9:05 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <4D6CADE5.6000206@st.com>
Hi Viresh,
> > On Tue, Mar 01, 2011 at 10:03:20AM +0530, Viresh Kumar wrote:
> >> device.h supports device_get_platdata but doesn't support device_set_platdata.
> >> This routine is required by platforms in which device structure is declared
> >> in a machine specific file and platform data comes from board specific file.
> >>
> >> This will be used by SPEAr patches sent in separate patch series.
> >>
> >> Signed-off-by: Viresh Kumar <viresh.kumar@st.com>
> >> ---
> >> include/linux/device.h | 5 +++++
> >> 1 files changed, 5 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/include/linux/device.h b/include/linux/device.h
> >> index 1bf5cf0..6ce0f20 100644
> >> --- a/include/linux/device.h
> >> +++ b/include/linux/device.h
> >> @@ -576,6 +576,11 @@ static inline void *dev_get_platdata(const struct device *dev)
> >> return dev->platform_data;
> >> }
> >>
> >> +static inline void dev_set_platdata(struct device *dev, void *platdata)
> >> +{
> >> + dev->platform_data = platdata;
> >> +}
> >> +
> > Note that dev->platform_data was designed to hold dynamically allocated
> > memory, at least it's kfreed in platform_device_release. And note there
> > is platform_device_add_data that kmemdups its argument into
> > pdev->dev.platform_data.
> >
>
> Ok. So we should use platform_device_add_data instead and mark our platform
> data's struct as __init. So that it doesn't consume any memory after
> this routine is done??
Yeah, either __initdata or still better const + __initconst if possible.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply
* [PATCH 1/2] ARM imx53: add pwm devices support
From: Jason Chen @ 2011-03-01 9:08 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20110301081043.GS22310@pengutronix.de>
hi, Uwe,
Thanks for your comments.
2011/3/1 Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>:
> On Tue, Mar 01, 2011 at 02:30:17PM +0800, Jason Chen wrote:
>> Signed-off-by: Jason Chen <b02280@freescale.com>
>> ---
>> ?arch/arm/mach-mx5/clock-mx51-mx53.c ? ? ? ? ?| ? ?2 ++
>> ?arch/arm/mach-mx5/devices-imx53.h ? ? ? ? ? ?| ? ?4 ++++
>> ?arch/arm/mach-mx5/devices.c ? ? ? ? ? ? ? ? ?| ? 10 ++++++++++
>> ?arch/arm/mach-mx5/devices.h ? ? ? ? ? ? ? ? ?| ? ?2 ++
>> ?arch/arm/plat-mxc/devices/Kconfig ? ? ? ? ? ?| ? ?1 +
>> ?arch/arm/plat-mxc/devices/platform-mxc_pwm.c | ? ?9 +++++++++
>> ?arch/arm/plat-mxc/pwm.c ? ? ? ? ? ? ? ? ? ? ?| ? ?3 ++-
>> ?7 files changed, 30 insertions(+), 1 deletions(-)
>>
>> diff --git a/arch/arm/mach-mx5/clock-mx51-mx53.c b/arch/arm/mach-mx5/clock-mx51-mx53.c
>> index 8164b1d..e18807b 100644
>> --- a/arch/arm/mach-mx5/clock-mx51-mx53.c
>> +++ b/arch/arm/mach-mx5/clock-mx51-mx53.c
>> @@ -1338,6 +1338,8 @@ static struct clk_lookup mx53_lookups[] = {
>> ? ? ? _REGISTER_CLOCK("imx53-cspi.0", NULL, cspi_clk)
>> ? ? ? _REGISTER_CLOCK("imx2-wdt.0", NULL, dummy_clk)
>> ? ? ? _REGISTER_CLOCK("imx2-wdt.1", NULL, dummy_clk)
>> + ? ? _REGISTER_CLOCK("mxc_pwm.0", "pwm", pwm1_clk)
>> + ? ? _REGISTER_CLOCK("mxc_pwm.1", "pwm", pwm2_clk)
> Do you really need "pwm"? ?I think NULL should do the job.
yes, I will remove it.
>
>> ?};
>>
>> ?static void clk_tree_init(void)
>> diff --git a/arch/arm/mach-mx5/devices-imx53.h b/arch/arm/mach-mx5/devices-imx53.h
>> index 9251008..5a1d6c9 100644
>> --- a/arch/arm/mach-mx5/devices-imx53.h
>> +++ b/arch/arm/mach-mx5/devices-imx53.h
>> @@ -33,3 +33,7 @@ extern const struct imx_spi_imx_data imx53_ecspi_data[] __initconst;
>> ?extern const struct imx_imx2_wdt_data imx53_imx2_wdt_data[] __initconst;
>> ?#define imx53_add_imx2_wdt(id, pdata) ? ? ? ?\
>> ? ? ? imx_add_imx2_wdt(&imx53_imx2_wdt_data[id])
>> +
>> +extern const struct imx_mxc_pwm_data imx53_mxc_pwm_data[] __initconst;
>> +#define imx53_add_mxc_pwm(id) ? ? ? ?\
>> + ? ? imx_add_mxc_pwm(&imx53_mxc_pwm_data[id])
>> diff --git a/arch/arm/mach-mx5/devices.c b/arch/arm/mach-mx5/devices.c
>> index 153ada5..74aec2b 100644
>> --- a/arch/arm/mach-mx5/devices.c
>> +++ b/arch/arm/mach-mx5/devices.c
>> @@ -120,6 +120,16 @@ struct platform_device mxc_usbh2_device = {
>> ? ? ? },
>> ?};
>>
>> +struct platform_device mxc_pwm1_backlight_device = {
>> + ? ? .name = "pwm-backlight",
>> + ? ? .id = 0,
>> +};
>> +
>> +struct platform_device mxc_pwm2_backlight_device = {
>> + ? ? .name = "pwm-backlight",
>> + ? ? .id = 1,
>> +};
>> +
> IMHO these should go into a seperate patch and preferably dynamically
> allocated.
ok, I will seperate this.
>
>> ?static struct mxc_gpio_port mxc_gpio_ports[] = {
>> ? ? ? {
>> ? ? ? ? ? ? ? .chip.label = "gpio-0",
>> diff --git a/arch/arm/mach-mx5/devices.h b/arch/arm/mach-mx5/devices.h
>> index 55a5129..a74cd97 100644
>> --- a/arch/arm/mach-mx5/devices.h
>> +++ b/arch/arm/mach-mx5/devices.h
>> @@ -3,3 +3,5 @@ extern struct platform_device mxc_usbh1_device;
>> ?extern struct platform_device mxc_usbh2_device;
>> ?extern struct platform_device mxc_usbdr_udc_device;
>> ?extern struct platform_device mxc_hsi2c_device;
>> +extern struct platform_device mxc_pwm1_backlight_device;
>> +extern struct platform_device mxc_pwm2_backlight_device;
>> diff --git a/arch/arm/plat-mxc/devices/Kconfig b/arch/arm/plat-mxc/devices/Kconfig
>> index b9ab1d5..aee9f4b 100644
>> --- a/arch/arm/plat-mxc/devices/Kconfig
>> +++ b/arch/arm/plat-mxc/devices/Kconfig
>> @@ -58,6 +58,7 @@ config IMX_HAVE_PLATFORM_MXC_NAND
>>
>> ?config IMX_HAVE_PLATFORM_MXC_PWM
>> ? ? ? bool
>> + ? ? default y if ARCH_MX21 || ARCH_MX25 || SOC_IMX27 || SOC_IMX51 || SOC_IMX53
> If you do this, can you please use SOC_IMX21 and SOC_IMX25? ?Or maybe
> even def_bool y?
>
sure
>> ?config IMX_HAVE_PLATFORM_MXC_RNGA
>> ? ? ? bool
>> diff --git a/arch/arm/plat-mxc/devices/platform-mxc_pwm.c b/arch/arm/plat-mxc/devices/platform-mxc_pwm.c
>> index b0c4ae2..18cfd07 100644
>> --- a/arch/arm/plat-mxc/devices/platform-mxc_pwm.c
>> +++ b/arch/arm/plat-mxc/devices/platform-mxc_pwm.c
>> @@ -49,6 +49,15 @@ const struct imx_mxc_pwm_data imx51_mxc_pwm_data[] __initconst = {
>> ?};
>> ?#endif /* ifdef CONFIG_SOC_IMX51 */
>>
>> +#ifdef CONFIG_SOC_IMX53
>> +const struct imx_mxc_pwm_data imx53_mxc_pwm_data[] __initconst = {
>> +#define imx53_mxc_pwm_data_entry(_id, _hwid) ? ? ? ? ? ? ? ? ? ? ? ? \
>> + ? ? imx_mxc_pwm_data_entry(MX53, _id, _hwid, SZ_16K)
>> + ? ? imx53_mxc_pwm_data_entry(0, 1),
>> + ? ? imx53_mxc_pwm_data_entry(1, 2),
>> +};
>> +#endif /* ifdef CONFIG_SOC_IMX53 */
>> +
>> ?struct platform_device *__init imx_add_mxc_pwm(
>> ? ? ? ? ? ? ? const struct imx_mxc_pwm_data *data)
>> ?{
>> diff --git a/arch/arm/plat-mxc/pwm.c b/arch/arm/plat-mxc/pwm.c
>> index 7a61ef8..61dd8fb 100644
>> --- a/arch/arm/plat-mxc/pwm.c
>> +++ b/arch/arm/plat-mxc/pwm.c
>> @@ -57,7 +57,8 @@ int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns)
>> ? ? ? if (pwm == NULL || period_ns == 0 || duty_ns > period_ns)
>> ? ? ? ? ? ? ? return -EINVAL;
>>
>> - ? ? if (cpu_is_mx27() || cpu_is_mx3() || cpu_is_mx25() || cpu_is_mx51()) {
>> + ? ? if (cpu_is_mx27() || cpu_is_mx3() || cpu_is_mx25() || cpu_is_mx51() ||
>> + ? ? ? ? ? ? cpu_is_mx53()) {
>> ? ? ? ? ? ? ? unsigned long long c;
>> ? ? ? ? ? ? ? unsigned long period_cycles, duty_cycles, prescale;
>> ? ? ? ? ? ? ? u32 cr;
>
> Best regards
> Uwe
>
> --
> Pengutronix e.K. ? ? ? ? ? ? ? ? ? ? ? ? ? | Uwe Kleine-K?nig ? ? ? ? ? ?|
> Industrial Linux Solutions ? ? ? ? ? ? ? ? | http://www.pengutronix.de/ ?|
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
^ permalink raw reply
* [RFC PATCH] ARM: Use generic BUG() handler
From: Mikael Pettersson @ 2011-03-01 9:12 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20110301085911.GB16733@n2100.arm.linux.org.uk>
Russell King - ARM Linux writes:
> On Tue, Mar 01, 2011 at 08:49:49AM +0000, Russell King - ARM Linux wrote:
> > On Mon, Feb 28, 2011 at 04:27:43PM -0800, Simon Glass wrote:
> > > + asm volatile("1:\t.word %c3\n" \
> > > + ".pushsection __bug_table,\"a\"\n" \
> > > + "2:\t.word 1b, %c0\n" \
> > > + "\t.hword %c1, 0\n" \
> > > + "\t.org 2b+%c2\n" \
> >
> > %c doesn't work on lots of versions of gcc, which is why we can't use
> > the generic bug support. There's no way to reliably generate constants
> > without many compiler versions spitting out a '#' before them.
>
> gcc 4.3.2:
>
> asm(".word %c0" : : "i" (0));
>
> produces:
>
> .word #0
>
> which gas chokes on:
>
> /tmp/cc2hGOHd.s:12: Error: bad expression
> /tmp/cc2hGOHd.s:12: Error: junk at end of line, first unrecognized character is `0'
>
> So what this means is that it's impossible to generate constants in
> assembly with GCC targetting ARM without having them prefixed by '#',
> which in turn makes it impossible to use the generic BUG support.
>
> I reported this bug to gcc folk many years ago. I've no idea which
> version it has been fixed in or if it's even been fixed.
What's the gcc bugzilla bug number?
^ permalink raw reply
* [RFC] device.h: add device_set_platdata routine
From: viresh kumar @ 2011-03-01 9:13 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20110301090518.GV22310@pengutronix.de>
On 03/01/2011 02:35 PM, Uwe Kleine-K?nig wrote:
>> > Ok. So we should use platform_device_add_data instead and mark our platform
>> > data's struct as __init. So that it doesn't consume any memory after
>> > this routine is done??
> Yeah, either __initdata or still better const + __initconst if possible.
Ok. Thanks for your help.
--
viresh
^ permalink raw reply
* [PATCH 3/8] Add a mfd IPUv3 driver
From: Sascha Hauer @ 2011-03-01 9:15 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <201102281811.45277.arnd@arndb.de>
Hi Arnd,
On Mon, Feb 28, 2011 at 06:11:45PM +0100, Arnd Bergmann wrote:
> Hi Sascha,
>
> I've had a brief look around the driver. It looks reasonable in general,
> but the division into subdrivers feels a bit ad-hoc. If all the components
> are built into a single module, I don't see the need for separating the
> data by functional units by file. It seems simple enough to turn this
> into a layered driver with multiple sub-devices each derived from a
> platform_device on its own.
Ok, will do.
>
> On Monday 28 February 2011, Sascha Hauer wrote:
> > arch/arm/plat-mxc/include/mach/ipu-v3.h | 49 +++
> > drivers/video/Kconfig | 2 +
> > drivers/video/Makefile | 1 +
> > drivers/video/imx-ipu-v3/Kconfig | 10 +
> > drivers/video/imx-ipu-v3/Makefile | 3 +
> > drivers/video/imx-ipu-v3/ipu-common.c | 666 +++++++++++++++++++++++++++++++
> > drivers/video/imx-ipu-v3/ipu-cpmem.c | 612 ++++++++++++++++++++++++++++
> > drivers/video/imx-ipu-v3/ipu-dc.c | 364 +++++++++++++++++
> > drivers/video/imx-ipu-v3/ipu-di.c | 550 +++++++++++++++++++++++++
> > drivers/video/imx-ipu-v3/ipu-dmfc.c | 355 ++++++++++++++++
> > drivers/video/imx-ipu-v3/ipu-dp.c | 476 ++++++++++++++++++++++
> > drivers/video/imx-ipu-v3/ipu-prv.h | 216 ++++++++++
> > include/video/imx-ipu-v3.h | 219 ++++++++++
>
> I wonder if this is something that would fit better in drivers/gpu instead
> of drivers/video. We recently discussed the benefits of KMS vs fb drivers,
> and I think it would be good to be prepared for making this a KMS driver
> in the long run, even if the immediate target has to be a simple frame buffer
> driver.
When turning this into a kms driver moving the source code will be the
smallest problem I guess.
I have no preference where to put this code. First it was in
drivers/mfd/ and it felt wrong because there seems to be too much code
in it a mfd maintainer shouldn't be bothered with. drivers/video/ seems
to be wrong because this code will probably support cameras which belong
to drivers/media/video/. So if there's consensus on drivers/gpu/ I will
happily put it there.
What directory do you suggest? drivers/gpu/ or some subdirectory
(drm/vga)?
>
> > +#include "ipu-prv.h"
> > +
> > +static struct clk *ipu_clk;
> > +static struct device *ipu_dev;
> > +
> > +static DEFINE_SPINLOCK(ipu_lock);
> > +static DEFINE_MUTEX(ipu_channel_lock);
> > +void __iomem *ipu_cm_reg;
> > +void __iomem *ipu_idmac_reg;
> > +
> > +static int ipu_use_count;
> > +
> > +static struct ipu_channel channels[64];
>
> Keeping these global limits you to just one ipu, and makes
> understanding the code a little harder. How about moving
> these variables into a struct ipu_data (or similar) that
> is referenced from the platform_device?
>
> > +EXPORT_SYMBOL(ipu_idmac_put);
>
> Why not EXPORT_SYMBOL_GPL?
>
> > +static LIST_HEAD(ipu_irq_handlers);
> > +
> > +static void ipu_irq_update_irq_mask(void)
> > +{
> > + struct ipu_irq_handler *handler;
> > + int i;
> > +
> > + DECLARE_IPU_IRQ_BITMAP(irqs);
> > +
> > + bitmap_zero(irqs, IPU_IRQ_COUNT);
> > +
> > + list_for_each_entry(handler, &ipu_irq_handlers, list)
> > + bitmap_or(irqs, irqs, handler->ipu_irqs, IPU_IRQ_COUNT);
> > +
> > + for (i = 0; i < BITS_TO_LONGS(IPU_IRQ_COUNT); i++)
> > + ipu_cm_write(irqs[i], IPU_INT_CTRL(i + 1));
> > +}
> > +
> > +int ipu_irq_add_handler(struct ipu_irq_handler *ipuirq)
> > +{
> > + unsigned long flags;
> > +
> > + spin_lock_irqsave(&ipu_lock, flags);
> > +
> > + list_add_tail(&ipuirq->list, &ipu_irq_handlers);
> > + ipu_irq_update_irq_mask();
> > +
> > + spin_unlock_irqrestore(&ipu_lock, flags);
> > + return 0;
> > +}
> > +EXPORT_SYMBOL(ipu_irq_add_handler);
>
> The interrupt logic needs some comments. What are you trying to achieve here?
The IPU has 463 status bits which can trigger an interrupt. Most
of them are associated to channels, some are general interrupts. My
problem is that I don't know how the interrupts will be used by drivers
later. Most drivers will probably use only one interrupt but others
will be interested in a larger group of interrupts. So I decided to
use a bitmap allowing each driver to register for groups of event
it is interested in.
>
> > +int ipu_wait_for_interrupt(int interrupt, int timeout_ms)
> > +{
> > + struct ipu_irq_handler handler;
> > + DECLARE_COMPLETION_ONSTACK(completion);
> > + int ret;
> > +
> > + bitmap_zero(handler.ipu_irqs, IPU_IRQ_COUNT);
> > + bitmap_set(handler.ipu_irqs, interrupt, 1);
> > +
> > + ipu_cm_write(1 << (interrupt % 32), IPU_INT_STAT(interrupt / 32 + 1));
> > +
> > + handler.handler = ipu_completion_handler;
> > + handler.context = &completion;
> > + ipu_irq_add_handler(&handler);
> > +
> > + ret = wait_for_completion_timeout(&completion,
> > + msecs_to_jiffies(timeout_ms));
> > +
> > + ipu_irq_remove_handler(&handler);
> > +
> > + if (ret > 0)
> > + ret = 0;
> > + else
> > + ret = -ETIMEDOUT;
> > +
> > + return ret;
> > +}
> > +EXPORT_SYMBOL(ipu_wait_for_interrupt);
>
> If I understand this correctly, this is a very complicated way to implement
> something that could be done with a single wait_event_timeout() and
> wake_up_interruptible_all() from the irq handler.
I added this as a convenience function for a common usecase: wait until
a frame is done or wait until some FIFOs are drained.
I think I can switch this into wait_event_timeout(), but lets first see
if my bitmap-irq implementation survives the review.
>
> > +static irqreturn_t ipu_irq_handler(int irq, void *desc)
> > +{
> > + DECLARE_IPU_IRQ_BITMAP(status);
> > + struct ipu_irq_handler *handler;
> > + int i;
> > +
> > + for (i = 0; i < BITS_TO_LONGS(IPU_IRQ_COUNT); i++) {
> > + status[i] = ipu_cm_read(IPU_INT_STAT(i + 1));
> > + ipu_cm_write(status[i], IPU_INT_STAT(i + 1));
> > + }
> > +
> > + list_for_each_entry(handler, &ipu_irq_handlers, list) {
> > + DECLARE_IPU_IRQ_BITMAP(tmp);
> > + if (bitmap_and(tmp, status, handler->ipu_irqs, IPU_IRQ_COUNT))
> > + handler->handler(tmp, handler->context);
> > + }
> > +
> > + return IRQ_HANDLED;
> > +}
>
> This needs to take spin_lock_irqsave before walking the ipu_irq_handlers
> list, in order to prevent another CPU from modifying the list
> while you are in the handler.
ok.
>
> > +static int ipu_reset(void)
> > +{
> > + int timeout = 10000;
> > + u32 val;
> > +
> > + /* hard reset the IPU */
> > + val = readl(MX51_IO_ADDRESS(MX51_SRC_BASE_ADDR));
> > + val |= 1 << 3;
> > + writel(val, MX51_IO_ADDRESS(MX51_SRC_BASE_ADDR));
> > +
> > + ipu_cm_write(0x807FFFFF, IPU_MEM_RST);
> > +
> > + while (ipu_cm_read(IPU_MEM_RST) & 0x80000000) {
> > + if (!timeout--)
> > + return -ETIME;
> > + udelay(100);
> > + }
>
> You have a timeout of over one second with udelay, which
> is not acceptable on many systems. AFAICT, the function
> can sleep, so you can replace udelay with msleep(1), and
> you should use a better logic to determine the end of the
> loop:
>
> unsigned long timeout = jiffies + msecs_to_jiffies(1000);
>
> while (ipu_cm_read(IPU_MEM_RST) & 0x80000000) {
> if (time_after(jiffies, timeout))
> return -ETIME;
> msleep(1);
> }
ok.
>
> > +static u32 *ipu_cpmem_base;
> > +static struct device *ipu_dev;
> > +
> > +struct ipu_ch_param_word {
> > + u32 data[5];
> > + u32 res[3];
> > +};
> > +
> > +struct ipu_ch_param {
> > + struct ipu_ch_param_word word[2];
> > +};
>
> Same comment as for the previous file
> > +
> > +static void __iomem *ipu_dc_reg;
> > +static void __iomem *ipu_dc_tmpl_reg;
> > +static struct device *ipu_dev;
> > +
> > +struct ipu_dc {
> > + unsigned int di; /* The display interface number assigned to this dc channel */
> > + unsigned int channel_offset;
> > +};
> > +
> > +static struct ipu_dc dc_channels[10];
>
> And here again.
>
> > +static void ipu_dc_link_event(int chan, int event, int addr, int priority)
> > +{
> > + u32 reg;
> > +
> > + reg = __raw_readl(DC_RL_CH(chan, event));
> > + reg &= ~(0xFFFF << (16 * (event & 0x1)));
> > + reg |= ((addr << 8) | priority) << (16 * (event & 0x1));
> > + __raw_writel(reg, DC_RL_CH(chan, event));
> > +}
>
> Better use readl/writel instead of __raw_readl/__raw_writel, throughout the
> code.
>
> > +int ipu_di_init(struct platform_device *pdev, int id, unsigned long base,
> > + u32 module, struct clk *ipu_clk);
> > +void ipu_di_exit(struct platform_device *pdev, int id);
> > +
> > +int ipu_dmfc_init(struct platform_device *pdev, unsigned long base,
> > + struct clk *ipu_clk);
> > +void ipu_dmfc_exit(struct platform_device *pdev);
> > +
> > +int ipu_dp_init(struct platform_device *pdev, unsigned long base);
> > +void ipu_dp_exit(struct platform_device *pdev);
> > +
> > +int ipu_dc_init(struct platform_device *pdev, unsigned long base,
> > + unsigned long template_base);
> > +void ipu_dc_exit(struct platform_device *pdev);
> > +
> > +int ipu_cpmem_init(struct platform_device *pdev, unsigned long base);
> > +void ipu_cpmem_exit(struct platform_device *pdev);
>
> If you make the main driver an mfd device, the sub-drivers could become
> real platform drivers, which can structure the layering in a more modular
> way.
> E.g. instead of a single module init function, each subdriver can be
> a module by itself and look at the resources associated with the
> platform device it matches.
ok.
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply
* [PATCH 1/3] ARM: S5P: Add s5p_timer support for HRT
From: Sylwester Nawrocki @ 2011-03-01 9:16 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20110226142413.GC3640@n2100.arm.linux.org.uk>
Hello,
On 02/26/2011 03:24 PM, Russell King - ARM Linux wrote:
> On Sat, Feb 26, 2011 at 11:45:55AM +0900, Sangbeom Kim wrote:
>> +static irqreturn_t s5p_clock_event_isr(int irq, void *dev_id)
>> +{
>> + struct clock_event_device *evt = &time_event_device;
>
> struct clock_event_device *evt = dev_id;
>
>> +
>> + evt->event_handler(evt);
>> +
>> + return IRQ_HANDLED;
>> +}
>> +
>> +static struct irqaction s5p_clock_event_irq = {
>> + .name = "s5p_time_irq",
>> + .flags = IRQF_DISABLED | IRQF_TIMER | IRQF_IRQPOLL,
>> + .handler = s5p_clock_event_isr,
>
> .dev_id = &time_event_device,
>
>> +};
> ...
>> +static void __init s5p_timer_resources(void)
>> +{
>> + struct platform_device tmpdev;
>> +
>> + tmpdev.dev.bus = &platform_bus_type;
>> +
>> + timerclk = clk_get(NULL, "timers");
>> + if (IS_ERR(timerclk))
>> + panic("failed to get timers clock for system timer");
>> +
>> + clk_enable(timerclk);
>> +
>> + tmpdev.id = timer_source.event_id;
>
> So Samsung clock stuff is still being idiotic. Will this ever be fixed?
>
Are there any serious difficulties preventing converting Samsung platforms
to clkdev? I don't seem to be aware of any.
Shortcomings of current clk API appeared few times already, and this one
being a disgraceful example.
>> + tin_event = clk_get(&tmpdev.dev, "pwm-tin");
>> + if (IS_ERR(tin_event)) {
>> + clk_put(tin_event);
>
> Don't clk_put errors.
>
>> + panic("failed to get pwm-tin2 clock for system timer");
>> + }
>> +
>> + tdiv_event = clk_get(&tmpdev.dev, "pwm-tdiv");
>> + if (IS_ERR(tdiv_event)) {
>> + clk_put(tdiv_event);
>
> Ditto.
>
>> + panic("failed to get pwm-tdiv2 clock for system timer");
>> + }
>> +
>> + clk_enable(tin_event);
>> +
>> + tmpdev.id = timer_source.source_id;
>> + tin_source = clk_get(&tmpdev.dev, "pwm-tin");
>> + if (IS_ERR(tin_source)) {
>> + clk_put(tin_source);
>
> Ditto.
>
>> + panic("failed to get pwm-tin4 clock for system timer");
>> + }
>> +
>> + tdiv_source = clk_get(&tmpdev.dev, "pwm-tdiv");
>> + if (IS_ERR(tdiv_source)) {
>> + clk_put(tdiv_source);
>
> Ditto.
--
Sylwester Nawrocki
Samsung Poland R&D Center
^ permalink raw reply
* [PATCH] arm: mxs: add irq_chip-name for GPIO IRQs
From: Wolfram Sang @ 2011-03-01 9:21 UTC (permalink / raw)
To: linux-arm-kernel
Reported-by: Alexander Stein <alexander.stein@systec-electronic.com>
Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
---
arch/arm/mach-mxs/gpio.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/arch/arm/mach-mxs/gpio.c b/arch/arm/mach-mxs/gpio.c
index 64848fa..5120ab5 100644
--- a/arch/arm/mach-mxs/gpio.c
+++ b/arch/arm/mach-mxs/gpio.c
@@ -182,6 +182,7 @@ static int mxs_gpio_set_wake_irq(u32 irq, u32 enable)
}
static struct irq_chip gpio_irq_chip = {
+ .name = "mxs gpio",
.ack = mxs_gpio_ack_irq,
.mask = mxs_gpio_mask_irq,
.unmask = mxs_gpio_unmask_irq,
--
1.7.2.3
^ permalink raw reply related
* [PATCH v2] plat-mxc: Provide irq_chip name for GPIO IRQs
From: Alexander Stein @ 2011-03-01 9:23 UTC (permalink / raw)
To: linux-arm-kernel
Acked-by: Wolfram Sang <w.sang@pengutronix.de>
Signed-off-by: Alexander Stein <alexander.stein@systec-electronic.com>
---
Changes in v2:
* Changed name to a more specific name
arch/arm/plat-mxc/gpio.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/arch/arm/plat-mxc/gpio.c b/arch/arm/plat-mxc/gpio.c
index d17b3c9..1d7d6bf 100644
--- a/arch/arm/plat-mxc/gpio.c
+++ b/arch/arm/plat-mxc/gpio.c
@@ -233,6 +233,7 @@ static int gpio_set_wake_irq(struct irq_data *d, u32 enable)
}
static struct irq_chip gpio_irq_chip = {
+ .name = "i.MX GPIO",
.irq_ack = gpio_ack_irq,
.irq_mask = gpio_mask_irq,
.irq_unmask = gpio_unmask_irq,
--
1.7.3.4
^ permalink raw reply related
* [PATCH 1/2] ARM imx53: add pwm devices support
From: Jason Chen @ 2011-03-01 9:38 UTC (permalink / raw)
To: linux-arm-kernel
Signed-off-by: Jason Chen <b02280@freescale.com>
---
arch/arm/mach-mx5/clock-mx51-mx53.c | 2 ++
arch/arm/mach-mx5/devices-imx53.h | 4 ++++
arch/arm/plat-mxc/devices/platform-mxc_pwm.c | 9 +++++++++
arch/arm/plat-mxc/pwm.c | 3 ++-
4 files changed, 17 insertions(+), 1 deletions(-)
diff --git a/arch/arm/mach-mx5/clock-mx51-mx53.c b/arch/arm/mach-mx5/clock-mx51-mx53.c
index 8164b1d..8a2b8de 100644
--- a/arch/arm/mach-mx5/clock-mx51-mx53.c
+++ b/arch/arm/mach-mx5/clock-mx51-mx53.c
@@ -1338,6 +1338,8 @@ static struct clk_lookup mx53_lookups[] = {
_REGISTER_CLOCK("imx53-cspi.0", NULL, cspi_clk)
_REGISTER_CLOCK("imx2-wdt.0", NULL, dummy_clk)
_REGISTER_CLOCK("imx2-wdt.1", NULL, dummy_clk)
+ _REGISTER_CLOCK("mxc_pwm.0", NULL, pwm1_clk)
+ _REGISTER_CLOCK("mxc_pwm.1", NULL, pwm2_clk)
};
static void clk_tree_init(void)
diff --git a/arch/arm/mach-mx5/devices-imx53.h b/arch/arm/mach-mx5/devices-imx53.h
index 9251008..5a1d6c9 100644
--- a/arch/arm/mach-mx5/devices-imx53.h
+++ b/arch/arm/mach-mx5/devices-imx53.h
@@ -33,3 +33,7 @@ extern const struct imx_spi_imx_data imx53_ecspi_data[] __initconst;
extern const struct imx_imx2_wdt_data imx53_imx2_wdt_data[] __initconst;
#define imx53_add_imx2_wdt(id, pdata) \
imx_add_imx2_wdt(&imx53_imx2_wdt_data[id])
+
+extern const struct imx_mxc_pwm_data imx53_mxc_pwm_data[] __initconst;
+#define imx53_add_mxc_pwm(id) \
+ imx_add_mxc_pwm(&imx53_mxc_pwm_data[id])
diff --git a/arch/arm/plat-mxc/devices/platform-mxc_pwm.c b/arch/arm/plat-mxc/devices/platform-mxc_pwm.c
index b0c4ae2..18cfd07 100644
--- a/arch/arm/plat-mxc/devices/platform-mxc_pwm.c
+++ b/arch/arm/plat-mxc/devices/platform-mxc_pwm.c
@@ -49,6 +49,15 @@ const struct imx_mxc_pwm_data imx51_mxc_pwm_data[] __initconst = {
};
#endif /* ifdef CONFIG_SOC_IMX51 */
+#ifdef CONFIG_SOC_IMX53
+const struct imx_mxc_pwm_data imx53_mxc_pwm_data[] __initconst = {
+#define imx53_mxc_pwm_data_entry(_id, _hwid) \
+ imx_mxc_pwm_data_entry(MX53, _id, _hwid, SZ_16K)
+ imx53_mxc_pwm_data_entry(0, 1),
+ imx53_mxc_pwm_data_entry(1, 2),
+};
+#endif /* ifdef CONFIG_SOC_IMX53 */
+
struct platform_device *__init imx_add_mxc_pwm(
const struct imx_mxc_pwm_data *data)
{
diff --git a/arch/arm/plat-mxc/pwm.c b/arch/arm/plat-mxc/pwm.c
index 7a61ef8..61dd8fb 100644
--- a/arch/arm/plat-mxc/pwm.c
+++ b/arch/arm/plat-mxc/pwm.c
@@ -57,7 +57,8 @@ int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns)
if (pwm == NULL || period_ns == 0 || duty_ns > period_ns)
return -EINVAL;
- if (cpu_is_mx27() || cpu_is_mx3() || cpu_is_mx25() || cpu_is_mx51()) {
+ if (cpu_is_mx27() || cpu_is_mx3() || cpu_is_mx25() || cpu_is_mx51() ||
+ cpu_is_mx53()) {
unsigned long long c;
unsigned long period_cycles, duty_cycles, prescale;
u32 cr;
--
1.7.1
^ permalink raw reply related
* [PATCH 2/2] ARM MX53_LOCO: add pwm backlight device
From: Jason Chen @ 2011-03-01 9:38 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1298972303-1010-1-git-send-email-b02280@freescale.com>
Signed-off-by: Jason Chen <b02280@freescale.com>
---
arch/arm/mach-mx5/Kconfig | 1 +
arch/arm/mach-mx5/board-mx53_loco.c | 11 +++++++++++
arch/arm/mach-mx5/devices-imx53.h | 4 ++++
3 files changed, 16 insertions(+), 0 deletions(-)
diff --git a/arch/arm/mach-mx5/Kconfig b/arch/arm/mach-mx5/Kconfig
index f065a0d..baf5223 100644
--- a/arch/arm/mach-mx5/Kconfig
+++ b/arch/arm/mach-mx5/Kconfig
@@ -162,6 +162,7 @@ config MACH_MX53_LOCO
select IMX_HAVE_PLATFORM_IMX2_WDT
select IMX_HAVE_PLATFORM_IMX_I2C
select IMX_HAVE_PLATFORM_IMX_UART
+ select IMX_HAVE_PLATFORM_MXC_PWM
help
Include support for MX53 LOCO platform. This includes specific
configurations for the board and its peripherals.
diff --git a/arch/arm/mach-mx5/board-mx53_loco.c b/arch/arm/mach-mx5/board-mx53_loco.c
index 160899e..3b0b200 100644
--- a/arch/arm/mach-mx5/board-mx53_loco.c
+++ b/arch/arm/mach-mx5/board-mx53_loco.c
@@ -23,6 +23,7 @@
#include <linux/fec.h>
#include <linux/delay.h>
#include <linux/gpio.h>
+#include <linux/pwm_backlight.h>
#include <mach/common.h>
#include <mach/hardware.h>
@@ -203,6 +204,13 @@ static const struct imxi2c_platform_data mx53_loco_i2c_data __initconst = {
.bitrate = 100000,
};
+static struct platform_pwm_backlight_data loco_pwm_backlight_data = {
+ .pwm_id = 1,
+ .max_brightness = 255,
+ .dft_brightness = 128,
+ .pwm_period_ns = 50000,
+};
+
static void __init mx53_loco_board_init(void)
{
mxc_iomux_v3_setup_multiple_pads(mx53_loco_pads,
@@ -213,6 +221,9 @@ static void __init mx53_loco_board_init(void)
imx53_add_imx2_wdt(0, NULL);
imx53_add_imx_i2c(0, &mx53_loco_i2c_data);
imx53_add_imx_i2c(1, &mx53_loco_i2c_data);
+
+ imx53_add_mxc_pwm(1);
+ imx53_add_mxc_pwm_backlight(0, &loco_pwm_backlight_data);
}
static void __init mx53_loco_timer_init(void)
diff --git a/arch/arm/mach-mx5/devices-imx53.h b/arch/arm/mach-mx5/devices-imx53.h
index 5a1d6c9..e9c5661 100644
--- a/arch/arm/mach-mx5/devices-imx53.h
+++ b/arch/arm/mach-mx5/devices-imx53.h
@@ -37,3 +37,7 @@ extern const struct imx_imx2_wdt_data imx53_imx2_wdt_data[] __initconst;
extern const struct imx_mxc_pwm_data imx53_mxc_pwm_data[] __initconst;
#define imx53_add_mxc_pwm(id) \
imx_add_mxc_pwm(&imx53_mxc_pwm_data[id])
+
+#define imx53_add_mxc_pwm_backlight(id, pdata) \
+ imx_add_platform_device("pwm-backlight", id, NULL, \
+ 0, pdata, sizeof(*pdata));
--
1.7.1
^ permalink raw reply related
* [PATCH 3/8] Add a mfd IPUv3 driver
From: Sascha Hauer @ 2011-03-01 9:39 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <alpine.LFD.2.00.1102281858240.2701@localhost6.localdomain6>
On Mon, Feb 28, 2011 at 07:33:05PM +0100, Thomas Gleixner wrote:
> On Mon, 28 Feb 2011, Sascha Hauer wrote:
> > +
> > +static int ipu_use_count;
> > +
> > +static struct ipu_channel channels[64];
> > +
> > +struct ipu_channel *ipu_idmac_get(unsigned num)
> > +{
> > + struct ipu_channel *channel;
> > +
> > + dev_dbg(ipu_dev, "%s %d\n", __func__, num);
> > +
> > + if (num > 63)
>
> >= ARRAY_SIZE(channels) or a sensible define please
>
> > + return ERR_PTR(-ENODEV);
> > +
> > + mutex_lock(&ipu_channel_lock);
> > +
> > + channel = &channels[num];
> > +
> > + if (channel->busy) {
> > + channel = ERR_PTR(-EBUSY);
> > + goto out;
> > + }
> > +
> > + channel->busy = 1;
> > + channel->num = num;
> > +
> > +out:
> > + mutex_unlock(&ipu_channel_lock);
> > +
> > + return channel;
> > +}
> > +EXPORT_SYMBOL(ipu_idmac_get);
>
> EXPORT_SYMBOL_GPL all over the place
>
> > +void ipu_idmac_put(struct ipu_channel *channel)
> > +{
> > + dev_dbg(ipu_dev, "%s %d\n", __func__, channel->num);
>
> Do we really need this debug stuff in all these functions ?
Reading this comment I expected tons of dev_dbg in the driver. The one
you mentioned above (plus the corresponding one in ipu_idmac_get) are
indeed not particularly useful, but do you think there is still too much
debug code left?
>
> > + mutex_lock(&ipu_channel_lock);
> > +
> > + channel->busy = 0;
> > +
> > + mutex_unlock(&ipu_channel_lock);
> > +}
> > +EXPORT_SYMBOL(ipu_idmac_put);
> > +
>
> Also exported functions want a proper kerneldoc comment.
>
> > +void ipu_idmac_set_double_buffer(struct ipu_channel *channel, bool doublebuffer)
>
> > +static LIST_HEAD(ipu_irq_handlers);
> > +
> > +static void ipu_irq_update_irq_mask(void)
> > +{
> > + struct ipu_irq_handler *handler;
> > + int i;
> > +
> > + DECLARE_IPU_IRQ_BITMAP(irqs);
>
> Why the hell do we need this? It's a bog standard bitmap, right ?
It's defined as:
#define DECLARE_IPU_IRQ_BITMAP(name) DECLARE_BITMAP(name, IPU_IRQ_COUNT)
So yes, it's a standard bitmask. It can be used in client drivers
aswell. Where's the problem of adding a define for this so that client
drivers do not have to care about the size of the bitmap?
>
> > + bitmap_zero(irqs, IPU_IRQ_COUNT);
> > +
> > + list_for_each_entry(handler, &ipu_irq_handlers, list)
> > + bitmap_or(irqs, irqs, handler->ipu_irqs, IPU_IRQ_COUNT);
> > +
> > + for (i = 0; i < BITS_TO_LONGS(IPU_IRQ_COUNT); i++)
> > + ipu_cm_write(irqs[i], IPU_INT_CTRL(i + 1));
> > +}
>
> > +static void ipu_completion_handler(unsigned long *bitmask, void *context)
> > +{
> > + struct completion *completion = context;
> > +
> > + complete(completion);
> > +}
> > +
> > +int ipu_wait_for_interrupt(int interrupt, int timeout_ms)
> > +{
> > + struct ipu_irq_handler handler;
> > + DECLARE_COMPLETION_ONSTACK(completion);
> > + int ret;
> > +
> > + bitmap_zero(handler.ipu_irqs, IPU_IRQ_COUNT);
> > + bitmap_set(handler.ipu_irqs, interrupt, 1);
> > +
> > + ipu_cm_write(1 << (interrupt % 32), IPU_INT_STAT(interrupt / 32 + 1));
> > +
> > + handler.handler = ipu_completion_handler;
> > + handler.context = &completion;
> > + ipu_irq_add_handler(&handler);
> > +
> > + ret = wait_for_completion_timeout(&completion,
> > + msecs_to_jiffies(timeout_ms));
> > +
> > + ipu_irq_remove_handler(&handler);
> > +
> > + if (ret > 0)
> > + ret = 0;
> > + else
> > + ret = -ETIMEDOUT;
> > +
> > + return ret;
>
> return ret > 0 ? 0 : -ETIMEDOUT;
>
> perhaps ?
ok.
>
>
> > +}
> > +EXPORT_SYMBOL(ipu_wait_for_interrupt);
> > +
> > +static irqreturn_t ipu_irq_handler(int irq, void *desc)
> > +{
> > + DECLARE_IPU_IRQ_BITMAP(status);
> > + struct ipu_irq_handler *handler;
> > + int i;
> > +
> > + for (i = 0; i < BITS_TO_LONGS(IPU_IRQ_COUNT); i++) {
> > + status[i] = ipu_cm_read(IPU_INT_STAT(i + 1));
> > + ipu_cm_write(status[i], IPU_INT_STAT(i + 1));
> > + }
> > +
> > + list_for_each_entry(handler, &ipu_irq_handlers, list) {
> > + DECLARE_IPU_IRQ_BITMAP(tmp);
> > + if (bitmap_and(tmp, status, handler->ipu_irqs, IPU_IRQ_COUNT))
> > + handler->handler(tmp, handler->context);
> > + }
>
> And what protects the list walk? Just the fact that this is a UP
> machine?
Will fix.
>
> > +void ipu_put(void)
> > +{
> > + mutex_lock(&ipu_channel_lock);
> > +
> > + ipu_use_count--;
> > +
> > + if (ipu_use_count == 0)
> > + clk_disable(ipu_clk);
> > +
> > + if (ipu_use_count < 0) {
> > + dev_err(ipu_dev, "ipu use count < 0\n");
>
> This wants to be a WARN_ON(ipu_use_count < 0) so you get some
> information which code is calling this.
yes
>
> > + ipu_use_count = 0;
> > + }
> > +
> > + mutex_unlock(&ipu_channel_lock);
> > +}
>
> > +static int __devinit ipu_probe(struct platform_device *pdev)
> > +{
> > + struct resource *res;
> > + unsigned long ipu_base;
> > + int ret, irq1, irq2;
> > +
> > + /* There can be only one */
> > + if (ipu_dev)
> > + return -EBUSY;
> > +
> > + spin_lock_init(&ipu_lock);
> > +
> > + ipu_dev = &pdev->dev;
> > +
> > + irq1 = platform_get_irq(pdev, 0);
> > + irq2 = platform_get_irq(pdev, 1);
> > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > +
> > + if (!res || irq1 < 0 || irq2 < 0)
> > + return -ENODEV;
> > +
> > + ipu_base = res->start;
> > +
> > + ipu_cm_reg = ioremap(ipu_base + IPU_CM_REG_BASE, PAGE_SIZE);
> > + if (!ipu_cm_reg) {
> > + ret = -ENOMEM;
> > + goto failed_ioremap1;
> > + }
> > +
> > + ipu_idmac_reg = ioremap(ipu_base + IPU_IDMAC_REG_BASE, PAGE_SIZE);
> > + if (!ipu_idmac_reg) {
> > + ret = -ENOMEM;
> > + goto failed_ioremap2;
> > + }
> > +
> > + ipu_clk = clk_get(&pdev->dev, "ipu");
> > + if (IS_ERR(ipu_clk)) {
> > + ret = PTR_ERR(ipu_clk);
> > + dev_err(&pdev->dev, "clk_get failed with %d", ret);
> > + goto failed_clk_get;
> > + }
> > +
> > + ipu_get();
> > +
> > + ret = request_irq(irq1, ipu_irq_handler, IRQF_DISABLED, pdev->name,
> > + &pdev->dev);
>
> s/IRQF_DISABLED/0/ We run all handlers with interrupts disabled
> nowadays.
ok.
>
> > + ret = ipu_submodules_init(pdev, ipu_base, ipu_clk);
> > + if (ret)
> > + goto failed_submodules_init;
> > +
> > + /* Set sync refresh channels as high priority */
> > + ipu_idmac_write(0x18800000, IDMAC_CHA_PRI(0));
>
> Hmm, this random prio setting here is odd.
This is 1:1 from the Freescale Kernel and I never thought about it. We
can remove it and see what happens. Maybe then some day we'll learn
*why* this is done.
>
> > + ret = ipu_add_client_devices(pdev);
> > + if (ret) {
> > + dev_err(&pdev->dev, "adding client devices failed with %d\n", ret);
> > + goto failed_add_clients;
> > + }
>
> White space damage.
>
> > + ret = ipu_wait_for_interrupt(irq, 50);
> > + if (ret)
> > + return;
> > +
> > + /* Wait for DC triple buffer to empty */
> > + if (dc_channels[dc_chan].di == 0)
> > + while ((__raw_readl(DC_STAT) & 0x00000002)
> > + != 0x00000002) {
> > + msleep(2);
> > + timeout -= 2;
> > + if (timeout <= 0)
> > + break;
>
> So we poll stuff which is updated from some other function ?
We poll the DC_STAT register here which is updated from the hardware.
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply
* [PATCH v6 1/1] PRUSS UIO driver support
From: Thomas Gleixner @ 2011-03-01 9:51 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1298926895-5294-2-git-send-email-pratheesh@ti.com>
On Tue, 1 Mar 2011, Pratheesh Gangadhar wrote:
> +
> +static spinlock_t lock;
static DEFINE_SPINLOCK(lock);
> +static struct clk *pruss_clk;
> +static struct uio_info *info;
> +static dma_addr_t sram_paddr, ddr_paddr;
> +static void *prussio_vaddr, *sram_vaddr, *ddr_vaddr;
> +
> +static irqreturn_t pruss_handler(int irq, struct uio_info *dev_info)
> +{
> + unsigned long flags;
> + int val, intr_mask = (1 << (irq - 1));
> + void __iomem *base = dev_info->mem[0].internal_addr;
> + void __iomem *intren_reg = base + PINTC_HIER;
> + void __iomem *intrstat_reg = base + PINTC_HIPIR + ((irq - 1) << 2);
> +
> + spin_lock_irqsave(&lock, flags);
spin_lock() is enough as we run handlers with interrupts disabled.
> + val = ioread32(intren_reg);
> + /* Is interrupt enabled and active ? */
> + if (!(val & intr_mask) && (ioread32(intrstat_reg) & HIPIR_NOPEND)) {
> + spin_unlock_irqrestore(&lock, flags);
> + return IRQ_NONE;
> + }
> +
> + /* Disable interrupt */
> + iowrite32((val & ~intr_mask), intren_reg);
> + spin_unlock_irqrestore(&lock, flags);
> + return IRQ_HANDLED;
> +}
So now you still have not solved the problem of user space enabling an
interrupt again. That's racy as well and you can solve it by providing
an uio->irq_control function which handles the interrupt enable
register under the lock as well.
Thanks,
tglx
^ permalink raw reply
* Linux support for Samsung S3C2510A Processor
From: Madhavi Manchala @ 2011-03-01 9:53 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20110228133256.GB22646@trinity.fluff.org>
On Mon, Feb 28, 2011 at 7:02 PM, Ben Dooks <ben@trinity.fluff.org> wrote:
> On Mon, Feb 28, 2011 at 06:52:15PM +0530, Madhavi Manchala wrote:
>> Dear All,
>>
>> Does Linux kernel supports the Samsung S3C2510A processor? Why am I
>> asking this question is, when I looked into ?the arch/arm/ directory I
>> did not find anything which is related to Samsung S3C2510A processor.
>> There are other directories like mach-s3c24xx and mach-s3c64xx
>> directories, but not mach-s3c25xx.
>>
>> Any ideas / hints will be appreciated.
>
> I think this is a no-mmu version, and there is not a lot of ARM no-mmu
> support at the moment.
Dear Ben,
Thank you very much for the information.
I think, if I want to port the Linux on to my board, which is based on
Samsung s3c2510a processor, then I need to develop the
arch/arm/mach-s3c2510 sources, which are currently not available in
the kernel source tree.
I saw that you only developed the sources for Samsung s3c24xx
processor in arch/arm/ of the Linux sources.
So, please suggest me with some good links / points for writing the
sources for Samsung s3c2510a processor under arch/arm. At least, how
can I start? Any templates?
Thank you very much for your suggestions.
Thanks and Regards,
Madhavi M.
> --
> Ben Dooks, ben at fluff.org, http://www.fluff.org/ben/
>
> Large Hadron Colada: A large Pina Colada that makes the universe disappear.
>
>
^ permalink raw reply
* [PATCH v6 1/1] PRUSS UIO driver support
From: Thomas Gleixner @ 2011-03-01 9:53 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <B85A65D85D7EB246BE421B3FB0FBB593024BF3601C@dbde02.ent.ti.com>
On Tue, 1 Mar 2011, TK, Pratheesh Gangadhar wrote:
> > On Tue, Mar 01, 2011 at 02:31:35AM +0530, Pratheesh Gangadhar wrote:
> > > +
> > > + /* Register PRUSS IRQ lines */
> > > + p->irq = IRQ_DA8XX_EVTOUT0 + cnt;
> > > + p->handler = pruss_handler;
> > > +
> > > + ret = uio_register_device(&dev->dev, p);
> > > +
> > > + if (ret < 0)
> > > + goto out_free;
> > > + }
> > > +
> > > + spin_lock_init(&lock);
> >
> > That's too late. uio_register_device() enables the irq, and your spin_lock
> > is not ready at that time.
>
> This is ok in this context as "modprobe uio_pruss" is pre-requisite for
> running PRUSS firmware and without firmware running PRUSS won't
> generate interrupts. Actually PRUSS INTC is not setup till we start
> user application.
No, it's not. If you enable interrupts you have to be prepared for
getting one whether the device mask is enabled or not. It's that
simple.
Thanks,
tglx
^ permalink raw reply
* [PATCH V4 1/4] ARM: imx53_loco: add esdhc device support
From: Richard Zhu @ 2011-03-01 9:58 UTC (permalink / raw)
To: linux-arm-kernel
Signed-off-by: Richard Zhao <richard.zhao@freescale.com>
Acked-by: Wolfram Sang <w.sang@pengutronix.de>
---
arch/arm/mach-mx5/Kconfig | 1 +
arch/arm/mach-mx5/board-mx53_loco.c | 2 ++
2 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/arch/arm/mach-mx5/Kconfig b/arch/arm/mach-mx5/Kconfig
index f065a0d..a72c833 100644
--- a/arch/arm/mach-mx5/Kconfig
+++ b/arch/arm/mach-mx5/Kconfig
@@ -162,6 +162,7 @@ config MACH_MX53_LOCO
select IMX_HAVE_PLATFORM_IMX2_WDT
select IMX_HAVE_PLATFORM_IMX_I2C
select IMX_HAVE_PLATFORM_IMX_UART
+ select IMX_HAVE_PLATFORM_SDHCI_ESDHC_IMX
help
Include support for MX53 LOCO platform. This includes specific
configurations for the board and its peripherals.
diff --git a/arch/arm/mach-mx5/board-mx53_loco.c b/arch/arm/mach-mx5/board-mx53_loco.c
index 160899e..0a18f8d 100644
--- a/arch/arm/mach-mx5/board-mx53_loco.c
+++ b/arch/arm/mach-mx5/board-mx53_loco.c
@@ -213,6 +213,8 @@ static void __init mx53_loco_board_init(void)
imx53_add_imx2_wdt(0, NULL);
imx53_add_imx_i2c(0, &mx53_loco_i2c_data);
imx53_add_imx_i2c(1, &mx53_loco_i2c_data);
+ imx53_add_sdhci_esdhc_imx(0, NULL);
+ imx53_add_sdhci_esdhc_imx(2, NULL);
}
static void __init mx53_loco_timer_init(void)
--
1.7.1
^ permalink raw reply related
* [PATCH V4 2/4] ARM: imx51/53: add sdhc3/4 clock
From: Richard Zhu @ 2011-03-01 9:58 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1298973500-2858-1-git-send-email-Hong-Xing.Zhu@freescale.com>
Signed-off-by: Richard Zhao <richard.zhao@freescale.com>
---
arch/arm/mach-mx5/clock-mx51-mx53.c | 140 ++++++++++++++++++++++++++++++++++-
arch/arm/mach-mx5/crm_regs.h | 7 ++
2 files changed, 146 insertions(+), 1 deletions(-)
diff --git a/arch/arm/mach-mx5/clock-mx51-mx53.c b/arch/arm/mach-mx5/clock-mx51-mx53.c
index 8164b1d..652ace4 100644
--- a/arch/arm/mach-mx5/clock-mx51-mx53.c
+++ b/arch/arm/mach-mx5/clock-mx51-mx53.c
@@ -42,6 +42,9 @@ static struct clk usboh3_clk;
static struct clk emi_fast_clk;
static struct clk ipu_clk;
static struct clk mipi_hsc1_clk;
+static struct clk esdhc1_clk;
+static struct clk esdhc2_clk;
+static struct clk esdhc3_mx53_clk;
#define MAX_DPLL_WAIT_TRIES 1000 /* 1000 * udelay(1) = 1ms */
@@ -1143,10 +1146,80 @@ CLK_GET_RATE(esdhc1, 1, ESDHC1_MSHC1)
CLK_SET_PARENT(esdhc1, 1, ESDHC1_MSHC1)
CLK_SET_RATE(esdhc1, 1, ESDHC1_MSHC1)
+/* mx51 specific */
CLK_GET_RATE(esdhc2, 1, ESDHC2_MSHC2)
CLK_SET_PARENT(esdhc2, 1, ESDHC2_MSHC2)
CLK_SET_RATE(esdhc2, 1, ESDHC2_MSHC2)
+static int clk_esdhc3_set_parent(struct clk *clk, struct clk *parent)
+{
+ u32 reg;
+
+ reg = __raw_readl(MXC_CCM_CSCMR1);
+ if (parent == &esdhc1_clk)
+ reg &= ~MXC_CCM_CSCMR1_ESDHC3_CLK_SEL;
+ else if (parent == &esdhc2_clk)
+ reg |= MXC_CCM_CSCMR1_ESDHC3_CLK_SEL;
+ else
+ return -EINVAL;
+ __raw_writel(reg, MXC_CCM_CSCMR1);
+
+ return 0;
+}
+
+static int clk_esdhc4_set_parent(struct clk *clk, struct clk *parent)
+{
+ u32 reg;
+
+ reg = __raw_readl(MXC_CCM_CSCMR1);
+ if (parent == &esdhc1_clk)
+ reg &= ~MXC_CCM_CSCMR1_ESDHC4_CLK_SEL;
+ else if (parent == &esdhc2_clk)
+ reg |= MXC_CCM_CSCMR1_ESDHC4_CLK_SEL;
+ else
+ return -EINVAL;
+ __raw_writel(reg, MXC_CCM_CSCMR1);
+
+ return 0;
+}
+
+/* mx53 specific */
+static int clk_esdhc2_mx53_set_parent(struct clk *clk, struct clk *parent)
+{
+ u32 reg;
+
+ reg = __raw_readl(MXC_CCM_CSCMR1);
+ if (parent == &esdhc1_clk)
+ reg &= ~MXC_CCM_CSCMR1_ESDHC2_MSHC2_MX53_CLK_SEL;
+ else if (parent == &esdhc3_mx53_clk)
+ reg |= MXC_CCM_CSCMR1_ESDHC2_MSHC2_MX53_CLK_SEL;
+ else
+ return -EINVAL;
+ __raw_writel(reg, MXC_CCM_CSCMR1);
+
+ return 0;
+}
+
+CLK_GET_RATE(esdhc3_mx53, 1, ESDHC3_MX53)
+CLK_SET_PARENT(esdhc3_mx53, 1, ESDHC3_MX53)
+CLK_SET_RATE(esdhc3_mx53, 1, ESDHC3_MX53)
+
+static int clk_esdhc4_mx53_set_parent(struct clk *clk, struct clk *parent)
+{
+ u32 reg;
+
+ reg = __raw_readl(MXC_CCM_CSCMR1);
+ if (parent == &esdhc1_clk)
+ reg &= ~MXC_CCM_CSCMR1_ESDHC4_CLK_SEL;
+ else if (parent == &esdhc3_mx53_clk)
+ reg |= MXC_CCM_CSCMR1_ESDHC4_CLK_SEL;
+ else
+ return -EINVAL;
+ __raw_writel(reg, MXC_CCM_CSCMR1);
+
+ return 0;
+}
+
#define DEFINE_CLOCK_FULL(name, i, er, es, gr, sr, e, d, p, s) \
static struct clk name = { \
.id = i, \
@@ -1251,9 +1324,62 @@ DEFINE_CLOCK_MAX(esdhc1_clk, 0, MXC_CCM_CCGR3, MXC_CCM_CCGRx_CG1_OFFSET,
clk_esdhc1, &pll2_sw_clk, &esdhc1_ipg_clk);
DEFINE_CLOCK_FULL(esdhc2_ipg_clk, 1, MXC_CCM_CCGR3, MXC_CCM_CCGRx_CG2_OFFSET,
NULL, NULL, _clk_max_enable, _clk_max_disable, &ipg_clk, NULL);
+DEFINE_CLOCK_FULL(esdhc3_ipg_clk, 2, MXC_CCM_CCGR3, MXC_CCM_CCGRx_CG4_OFFSET,
+ NULL, NULL, _clk_max_enable, _clk_max_disable, &ipg_clk, NULL);
+DEFINE_CLOCK_FULL(esdhc4_ipg_clk, 3, MXC_CCM_CCGR3, MXC_CCM_CCGRx_CG6_OFFSET,
+ NULL, NULL, _clk_max_enable, _clk_max_disable, &ipg_clk, NULL);
+
+/* mx51 specific */
DEFINE_CLOCK_MAX(esdhc2_clk, 1, MXC_CCM_CCGR3, MXC_CCM_CCGRx_CG3_OFFSET,
clk_esdhc2, &pll2_sw_clk, &esdhc2_ipg_clk);
+static struct clk esdhc3_clk = {
+ .id = 2,
+ .parent = &esdhc1_clk,
+ .set_parent = clk_esdhc3_set_parent,
+ .enable_reg = MXC_CCM_CCGR3,
+ .enable_shift = MXC_CCM_CCGRx_CG5_OFFSET,
+ .enable = _clk_max_enable,
+ .disable = _clk_max_disable,
+ .secondary = &esdhc3_ipg_clk,
+};
+static struct clk esdhc4_clk = {
+ .id = 3,
+ .parent = &esdhc1_clk,
+ .set_parent = clk_esdhc4_set_parent,
+ .enable_reg = MXC_CCM_CCGR3,
+ .enable_shift = MXC_CCM_CCGRx_CG7_OFFSET,
+ .enable = _clk_max_enable,
+ .disable = _clk_max_disable,
+ .secondary = &esdhc4_ipg_clk,
+};
+
+/* mx53 specific */
+static struct clk esdhc2_mx53_clk = {
+ .id = 2,
+ .parent = &esdhc1_clk,
+ .set_parent = clk_esdhc2_mx53_set_parent,
+ .enable_reg = MXC_CCM_CCGR3,
+ .enable_shift = MXC_CCM_CCGRx_CG3_OFFSET,
+ .enable = _clk_max_enable,
+ .disable = _clk_max_disable,
+ .secondary = &esdhc3_ipg_clk,
+};
+
+DEFINE_CLOCK_MAX(esdhc3_mx53_clk, 2, MXC_CCM_CCGR3, MXC_CCM_CCGRx_CG5_OFFSET,
+ clk_esdhc3_mx53, &pll2_sw_clk, &esdhc2_ipg_clk);
+
+static struct clk esdhc4_mx53_clk = {
+ .id = 3,
+ .parent = &esdhc1_clk,
+ .set_parent = clk_esdhc4_mx53_set_parent,
+ .enable_reg = MXC_CCM_CCGR3,
+ .enable_shift = MXC_CCM_CCGRx_CG7_OFFSET,
+ .enable = _clk_max_enable,
+ .disable = _clk_max_disable,
+ .secondary = &esdhc4_ipg_clk,
+};
+
DEFINE_CLOCK(mipi_esc_clk, 0, MXC_CCM_CCGR4, MXC_CCM_CCGRx_CG5_OFFSET, NULL, NULL, NULL, &pll2_sw_clk);
DEFINE_CLOCK(mipi_hsc2_clk, 0, MXC_CCM_CCGR4, MXC_CCM_CCGRx_CG4_OFFSET, NULL, NULL, &mipi_esc_clk, &pll2_sw_clk);
DEFINE_CLOCK(mipi_hsc1_clk, 0, MXC_CCM_CCGR4, MXC_CCM_CCGRx_CG3_OFFSET, NULL, NULL, &mipi_hsc2_clk, &pll2_sw_clk);
@@ -1312,6 +1438,8 @@ static struct clk_lookup mx51_lookups[] = {
_REGISTER_CLOCK("imx51-cspi.0", NULL, cspi_clk)
_REGISTER_CLOCK("sdhci-esdhc-imx.0", NULL, esdhc1_clk)
_REGISTER_CLOCK("sdhci-esdhc-imx.1", NULL, esdhc2_clk)
+ _REGISTER_CLOCK("sdhci-esdhc-imx.2", NULL, esdhc3_clk)
+ _REGISTER_CLOCK("sdhci-esdhc-imx.3", NULL, esdhc4_clk)
_REGISTER_CLOCK(NULL, "cpu_clk", cpu_clk)
_REGISTER_CLOCK(NULL, "iim_clk", iim_clk)
_REGISTER_CLOCK("imx2-wdt.0", NULL, dummy_clk)
@@ -1332,7 +1460,9 @@ static struct clk_lookup mx53_lookups[] = {
_REGISTER_CLOCK("imx-i2c.0", NULL, i2c1_clk)
_REGISTER_CLOCK("imx-i2c.1", NULL, i2c2_clk)
_REGISTER_CLOCK("sdhci-esdhc-imx.0", NULL, esdhc1_clk)
- _REGISTER_CLOCK("sdhci-esdhc-imx.1", NULL, esdhc2_clk)
+ _REGISTER_CLOCK("sdhci-esdhc-imx.1", NULL, esdhc2_mx53_clk)
+ _REGISTER_CLOCK("sdhci-esdhc-imx.2", NULL, esdhc3_mx53_clk)
+ _REGISTER_CLOCK("sdhci-esdhc-imx.3", NULL, esdhc4_mx53_clk)
_REGISTER_CLOCK("imx53-ecspi.0", NULL, ecspi1_clk)
_REGISTER_CLOCK("imx53-ecspi.1", NULL, ecspi2_clk)
_REGISTER_CLOCK("imx53-cspi.0", NULL, cspi_clk)
@@ -1425,6 +1555,14 @@ int __init mx53_clocks_init(unsigned long ckil, unsigned long osc,
mx53_revision();
clk_disable(&iim_clk);
+ /* Set SDHC parents to be PLL2 */
+ clk_set_parent(&esdhc1_clk, &pll2_sw_clk);
+ clk_set_parent(&esdhc3_mx53_clk, &pll2_sw_clk);
+
+ /* set SDHC root clock as 200MHZ*/
+ clk_set_rate(&esdhc1_clk, 200000000);
+ clk_set_rate(&esdhc3_mx53_clk, 200000000);
+
/* System timer */
mxc_timer_init(&gpt_clk, MX53_IO_ADDRESS(MX53_GPT1_BASE_ADDR),
MX53_INT_GPT);
diff --git a/arch/arm/mach-mx5/crm_regs.h b/arch/arm/mach-mx5/crm_regs.h
index b462c22..87c0c58 100644
--- a/arch/arm/mach-mx5/crm_regs.h
+++ b/arch/arm/mach-mx5/crm_regs.h
@@ -217,9 +217,12 @@
#define MXC_CCM_CSCMR1_ESDHC1_MSHC1_CLK_SEL_OFFSET (20)
#define MXC_CCM_CSCMR1_ESDHC1_MSHC1_CLK_SEL_MASK (0x3 << 20)
#define MXC_CCM_CSCMR1_ESDHC3_CLK_SEL (0x1 << 19)
+#define MXC_CCM_CSCMR1_ESDHC2_MSHC2_MX53_CLK_SEL (0x1 << 19)
#define MXC_CCM_CSCMR1_ESDHC4_CLK_SEL (0x1 << 18)
#define MXC_CCM_CSCMR1_ESDHC2_MSHC2_CLK_SEL_OFFSET (16)
#define MXC_CCM_CSCMR1_ESDHC2_MSHC2_CLK_SEL_MASK (0x3 << 16)
+#define MXC_CCM_CSCMR1_ESDHC3_MX53_CLK_SEL_OFFSET (16)
+#define MXC_CCM_CSCMR1_ESDHC3_MX53_CLK_SEL_MASK (0x3 << 16)
#define MXC_CCM_CSCMR1_SSI1_CLK_SEL_OFFSET (14)
#define MXC_CCM_CSCMR1_SSI1_CLK_SEL_MASK (0x3 << 14)
#define MXC_CCM_CSCMR1_SSI2_CLK_SEL_OFFSET (12)
@@ -271,6 +274,10 @@
#define MXC_CCM_CSCDR1_ESDHC2_MSHC2_CLK_PRED_MASK (0x7 << 22)
#define MXC_CCM_CSCDR1_ESDHC2_MSHC2_CLK_PODF_OFFSET (19)
#define MXC_CCM_CSCDR1_ESDHC2_MSHC2_CLK_PODF_MASK (0x7 << 19)
+#define MXC_CCM_CSCDR1_ESDHC3_MX53_CLK_PRED_OFFSET (22)
+#define MXC_CCM_CSCDR1_ESDHC3_MX53_CLK_PRED_MASK (0x7 << 22)
+#define MXC_CCM_CSCDR1_ESDHC3_MX53_CLK_PODF_OFFSET (19)
+#define MXC_CCM_CSCDR1_ESDHC3_MX53_CLK_PODF_MASK (0x7 << 19)
#define MXC_CCM_CSCDR1_ESDHC1_MSHC1_CLK_PRED_OFFSET (16)
#define MXC_CCM_CSCDR1_ESDHC1_MSHC1_CLK_PRED_MASK (0x7 << 16)
#define MXC_CCM_CSCDR1_PGC_CLK_PODF_OFFSET (14)
--
1.7.1
^ permalink raw reply related
* [PATCH V4 3/4] mmc: sdhci-esdhc: remove SDHCI_QUIRK_NO_CARD_NO_RESET from ESDHC_DEFAULT_QUIRKS
From: Richard Zhu @ 2011-03-01 9:58 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1298973500-2858-1-git-send-email-Hong-Xing.Zhu@freescale.com>
sdhci-esdhc-imx does not need SDHCI_QUIRK_NO_CARD_NO_RESET. Make it OF-specific.
Signed-off-by: Richard Zhu <Hong-Xing.Zhu@freescale.com>
Tested-by: Wolfram Sang <w.sang@pengutronix.de>
---
drivers/mmc/host/sdhci-esdhc.h | 3 +--
drivers/mmc/host/sdhci-of-esdhc.c | 2 +-
2 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/mmc/host/sdhci-esdhc.h b/drivers/mmc/host/sdhci-esdhc.h
index afaf1bc..303cde0 100644
--- a/drivers/mmc/host/sdhci-esdhc.h
+++ b/drivers/mmc/host/sdhci-esdhc.h
@@ -24,8 +24,7 @@
SDHCI_QUIRK_NONSTANDARD_CLOCK | \
SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK | \
SDHCI_QUIRK_PIO_NEEDS_DELAY | \
- SDHCI_QUIRK_RESTORE_IRQS_AFTER_RESET | \
- SDHCI_QUIRK_NO_CARD_NO_RESET)
+ SDHCI_QUIRK_RESTORE_IRQS_AFTER_RESET)
#define ESDHC_SYSTEM_CONTROL 0x2c
#define ESDHC_CLOCK_MASK 0x0000fff0
diff --git a/drivers/mmc/host/sdhci-of-esdhc.c b/drivers/mmc/host/sdhci-of-esdhc.c
index fcd0e1f..6337607 100644
--- a/drivers/mmc/host/sdhci-of-esdhc.c
+++ b/drivers/mmc/host/sdhci-of-esdhc.c
@@ -73,7 +73,7 @@ static unsigned int esdhc_of_get_min_clock(struct sdhci_host *host)
}
struct sdhci_of_data sdhci_esdhc = {
- .quirks = ESDHC_DEFAULT_QUIRKS,
+ .quirks = ESDHC_DEFAULT_QUIRKS | SDHCI_QUIRK_NO_CARD_NO_RESET,
.ops = {
.read_l = sdhci_be32bs_readl,
.read_w = esdhc_readw,
--
1.7.1
^ permalink raw reply related
* [PATCH V4 4/4] mmc: sdhci-esdhc: enable esdhc on imx53
From: Richard Zhu @ 2011-03-01 9:58 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1298973500-2858-1-git-send-email-Hong-Xing.Zhu@freescale.com>
Fix the NO INT in the Multi-BLK IO in SD/MMC, and
Multi-BLK read in SDIO
The CMDTYPE of the CMD register(offset 0xE) should be set to
"11" when the STOP CMD12 is issued on imx53 to abort one
open ended multi-blk IO. Otherwise one the TC INT wouldn't
be generated.
In exact block transfer, the controller doesn't complete the
operations automatically as required at the end of the
transfer and remains on hold if the abort command is not sent.
As a result, the TC flag is not asserted and SW received timeout
exeception. set bit1 of Vendor Spec registor to fix it
Signed-off-by: Richard Zhu <Hong-Xing.Zhu@freescale.com>
Signed-off-by: Richard Zhao <richard.zhao@freescale.com>
---
drivers/mmc/host/sdhci-esdhc-imx.c | 76 ++++++++++++++++++++++++++++++++++-
drivers/mmc/host/sdhci-pltfm.h | 2 +-
2 files changed, 74 insertions(+), 4 deletions(-)
diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index 9b82910..5ea4b5f 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -15,13 +15,42 @@
#include <linux/delay.h>
#include <linux/err.h>
#include <linux/clk.h>
+#include <linux/slab.h>
#include <linux/mmc/host.h>
#include <linux/mmc/sdhci-pltfm.h>
+#include <linux/mmc/mmc.h>
+#include <linux/mmc/sdio.h>
#include <mach/hardware.h>
#include "sdhci.h"
#include "sdhci-pltfm.h"
#include "sdhci-esdhc.h"
+/* Abort type definition in the command register */
+#define SDHCI_CMD_ABORTCMD 0xC0
+/* VENDOR SPEC register */
+#define SDHCI_VENDOR_SPEC 0xC0
+ #define SDHCI_VENDOR_SPEC_SDIO_QUIRK 0x00000002
+
+/*
+ * The CMDTYPE of the CMD register(offset 0xE) should be set to
+ * "11" when the STOP CMD12 is issued on imx53 to abort one
+ * open ended multi-blk IO. Otherwise the TC INT wouldn't
+ * be generated.
+ * In exact block transfer, the controller doesn't complete the
+ * operations automatically as required at the end of the
+ * transfer and remains on hold if the abort command is not sent.
+ * As a result, the TC flag is not asserted and SW received timeout
+ * exeception. Bit1 of Vendor Spec registor is used to fix it.
+ */
+#define IMX_MULTIBLK_NO_INT (1 << 0)
+
+struct pltfm_imx_data {
+ int flags;
+ u32 mod_val;
+};
+
+static struct sdhci_ops sdhci_esdhc_ops;
+
static inline void esdhc_clrset_le(struct sdhci_host *host, u32 mask, u32 val, int reg)
{
void __iomem *base = host->ioaddr + (reg & ~0x3);
@@ -38,20 +67,49 @@ static u16 esdhc_readw_le(struct sdhci_host *host, int reg)
return readw(host->ioaddr + reg);
}
+static void esdhc_writel_le(struct sdhci_host *host, u32 val, int reg)
+{
+ switch (reg) {
+ case SDHCI_INT_STATUS:
+ if (val & SDHCI_INT_DATA_END) {
+ u32 v;
+ v = readl(host->ioaddr + SDHCI_VENDOR_SPEC);
+ v &= ~SDHCI_VENDOR_SPEC_SDIO_QUIRK;
+ writel(v, host->ioaddr + SDHCI_VENDOR_SPEC);
+ }
+ break;
+ }
+ writel(val, host->ioaddr + reg);
+}
+
static void esdhc_writew_le(struct sdhci_host *host, u16 val, int reg)
{
struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+ struct pltfm_imx_data *imx_data =
+ (struct pltfm_imx_data *)pltfm_host->priv;
switch (reg) {
case SDHCI_TRANSFER_MODE:
+ if ((host->cmd->opcode == SD_IO_RW_EXTENDED)
+ && (host->cmd->data->blocks > 1)
+ && (host->cmd->data->flags & MMC_DATA_READ)
+ && (imx_data->flags & IMX_MULTIBLK_NO_INT)) {
+ u32 v;
+ v = readl(host->ioaddr + SDHCI_VENDOR_SPEC);
+ v |= SDHCI_VENDOR_SPEC_SDIO_QUIRK;
+ writel(v, host->ioaddr + SDHCI_VENDOR_SPEC);
+ }
/*
* Postpone this write, we must do it together with a
* command write that is down below.
*/
- pltfm_host->scratchpad = val;
+ imx_data->mod_val = val;
return;
case SDHCI_COMMAND:
- writel(val << 16 | pltfm_host->scratchpad,
+ if ((host->cmd->opcode == MMC_STOP_TRANSMISSION)
+ && (imx_data->flags & IMX_MULTIBLK_NO_INT))
+ val |= SDHCI_CMD_ABORTCMD;
+ writel(val << 16 | imx_data->mod_val,
host->ioaddr + SDHCI_TRANSFER_MODE);
return;
case SDHCI_BLOCK_SIZE:
@@ -104,6 +162,10 @@ static int esdhc_pltfm_init(struct sdhci_host *host, struct sdhci_pltfm_data *pd
{
struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
struct clk *clk;
+ struct pltfm_imx_data *imx_data;
+
+ imx_data = kzalloc(sizeof(struct pltfm_imx_data), GFP_KERNEL);
+ pltfm_host->priv = (void *)imx_data;
clk = clk_get(mmc_dev(host->mmc), NULL);
if (IS_ERR(clk)) {
@@ -113,22 +175,30 @@ static int esdhc_pltfm_init(struct sdhci_host *host, struct sdhci_pltfm_data *pd
clk_enable(clk);
pltfm_host->clk = clk;
- if (cpu_is_mx35() || cpu_is_mx51())
+ if (!cpu_is_mx25())
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())
host->quirks |= SDHCI_QUIRK_NO_MULTIBLOCK;
+ if (!(cpu_is_mx25() || cpu_is_mx35() || cpu_is_mx51())) {
+ imx_data->flags |= IMX_MULTIBLK_NO_INT;
+ sdhci_esdhc_ops.write_l = esdhc_writel_le;
+ }
+
return 0;
}
static void esdhc_pltfm_exit(struct sdhci_host *host)
{
struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+ struct pltfm_imx_data *imx_data =
+ (struct pltfm_imx_data *)pltfm_host->priv;
clk_disable(pltfm_host->clk);
clk_put(pltfm_host->clk);
+ kfree(imx_data);
}
static struct sdhci_ops sdhci_esdhc_ops = {
diff --git a/drivers/mmc/host/sdhci-pltfm.h b/drivers/mmc/host/sdhci-pltfm.h
index ea2e44d..2b37016 100644
--- a/drivers/mmc/host/sdhci-pltfm.h
+++ b/drivers/mmc/host/sdhci-pltfm.h
@@ -17,7 +17,7 @@
struct sdhci_pltfm_host {
struct clk *clk;
- u32 scratchpad; /* to handle quirks across io-accessor calls */
+ void *priv; /* to handle quirks across io-accessor calls */
};
extern struct sdhci_pltfm_data sdhci_cns3xxx_pdata;
--
1.7.1
^ permalink raw reply related
* [PATCH 3/8] Add a mfd IPUv3 driver
From: Thomas Gleixner @ 2011-03-01 10:00 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20110301093956.GL29521@pengutronix.de>
On Tue, 1 Mar 2011, Sascha Hauer wrote:
> On Mon, Feb 28, 2011 at 07:33:05PM +0100, Thomas Gleixner wrote:
> > > +void ipu_idmac_put(struct ipu_channel *channel)
> > > +{
> > > + dev_dbg(ipu_dev, "%s %d\n", __func__, channel->num);
> >
> > Do we really need this debug stuff in all these functions ?
>
> Reading this comment I expected tons of dev_dbg in the driver. The one
> you mentioned above (plus the corresponding one in ipu_idmac_get) are
> indeed not particularly useful, but do you think there is still too much
> debug code left?
Well, I don't see a point in having useless debug stuff around.
> > > + DECLARE_IPU_IRQ_BITMAP(irqs);
> >
> > Why the hell do we need this? It's a bog standard bitmap, right ?
>
> It's defined as:
>
> #define DECLARE_IPU_IRQ_BITMAP(name) DECLARE_BITMAP(name, IPU_IRQ_COUNT)
>
> So yes, it's a standard bitmask. It can be used in client drivers
> aswell. Where's the problem of adding a define for this so that client
> drivers do not have to care about the size of the bitmap?
That's nonsense. You have to know about the size of the bitmap for any
operation on it. Or are you going to provide wrappers around
bitmap_zero() and all other possible bitmap_* functions as well?
> >
> > > + bitmap_zero(irqs, IPU_IRQ_COUNT);
> > > + ret = ipu_submodules_init(pdev, ipu_base, ipu_clk);
> > > + if (ret)
> > > + goto failed_submodules_init;
> > > +
> > > + /* Set sync refresh channels as high priority */
> > > + ipu_idmac_write(0x18800000, IDMAC_CHA_PRI(0));
> >
> > Hmm, this random prio setting here is odd.
>
> This is 1:1 from the Freescale Kernel and I never thought about it. We
> can remove it and see what happens. Maybe then some day we'll learn
> *why* this is done.
Well, the point is to move that to the init function which deals with
IDMAC and not have it at some random place in the code.
> > > + /* Wait for DC triple buffer to empty */
> > > + if (dc_channels[dc_chan].di == 0)
> > > + while ((__raw_readl(DC_STAT) & 0x00000002)
> > > + != 0x00000002) {
> > > + msleep(2);
> > > + timeout -= 2;
> > > + if (timeout <= 0)
> > > + break;
> >
> > So we poll stuff which is updated from some other function ?
>
> We poll the DC_STAT register here which is updated from the hardware.
And there is no interrupt for this ?
Thanks,
tglx
^ 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