* Locking in the clk API
From: Uwe Kleine-König @ 2011-01-12 7:40 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <4D2D1F0D.5040208@codeaurora.org>
Hi Saravana,
On Tue, Jan 11, 2011 at 07:25:01PM -0800, Saravana Kannan wrote:
> On 01/11/2011 06:35 AM, Jeremy Kerr wrote:
> >Hi Paul,
> >
> >>Again, you are approaching it from the angle that an atomic clock is a
> >>special requirement rather than the default behaviour.
> >
> >I'm not considering it a special requirement, but it's still a requirement
> >(that the called function does not sleep).
> >
> >The problem with the inverse logic (clk_enable/clk_enable_sleepable) is that
> >now you've made the caller need to know what kind of clock it has, or might
> >have one day.
>
> I think it's just a matter of how you interpret the name of the API
> in English. It doesn't make the decision making of the developer any
> easier.
>
> Just having a _atomic suffix doesn't mean the driver developer
> doesn't need to know what type of clock it is. They are still making
> the assumption that the enable/disable for that clock can be done
> atomically -- namely an "atomic clock".
But there is a difference to 'one function to rule both sleepable and
atomic clocks'. When calling _atomic on a sleepable clock you get
-ESOMETHING back (and the clock stays off). With a generic clk_enable
you get an oops and so cannot handle the error.
> Similarly, when a driver developer calls the _sleepable APIs in
> their code, for all practical purposes, they are making an
> assumption that the enable/disable for that clock *needs to* (not
> may) sleep.
IMHO this is not right. If the driver developer doesn't care if the
clock sleeps or not (which is the norm I think) he calls the _sleepable
function and if the clock happen to be an atomic one it doesn't hurt
him.
And looking at the usage of the sleeping functions in the gpio API, I'd
bet that at least 50% of the calls to gpio_set_value can/should be
gpio_set_value_cansleep. That's because driver developers don't care or
are not aware of the issue. If it would be gpio_set_value
vs.gpio_set_value_atomic most developers would use the sleeping variant
and the few that should use the _atomic function would notice that when
seeing the corresponding oops.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply
* [PATCH 2/2] ARM i.MX53 enable LOCO board bootup
From: Uwe Kleine-König @ 2011-01-12 7:29 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <AANLkTimODoRu8JSOs5ChOp8i3LUoUP4ZKn=aDHnVU6pL@mail.gmail.com>
On Wed, Jan 12, 2011 at 10:46:24AM +0800, Yong Shen wrote:
> Hi Fabio,
>
> Below is my reply to Baruch, I meant to keep this code here to show
> the reset process clearly.
> >> + if (ret) {
> >> + printk(KERN_ERR"failed to get GPIO_FEC_PHY_RESET: %d\n", ret);
> >> + return;
> >> + }
> >> + gpio_direction_output(SMD_FEC_PHY_RST, 0);
> >> + gpio_set_value(SMD_FEC_PHY_RST, 0);
> >
> > This seems to be redundant. gpio_direction_output() has already set the value
> > to 0.
>
> You are right. But, gpio_set_value is meant to be here to show the
> process of fec reset: first pull low and then pull high. And
> gpio_direction_output here is for direction configuration although it
> has the ability of configure output value.
If you care about it: I'd vote for removing the call to gpio_set_value,
too.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply
* [PATCH v4 08/10] ARM: mxs: add ocotp read function
From: Shawn Guo @ 2011-01-12 6:47 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20110111133137.GS12078@pengutronix.de>
Hi Sascha,
On Tue, Jan 11, 2011 at 02:31:37PM +0100, Sascha Hauer wrote:
> On Thu, Jan 06, 2011 at 03:13:16PM +0800, Shawn Guo wrote:
> > Signed-off-by: Shawn Guo <shawn.guo@freescale.com>
> > ---
> > Changes for v4:
> > - Call cpu_relax() during polling
> >
> > Changes for v2:
> > - Add mutex locking for mxs_read_ocotp()
> > - Use type size_t for count and i
> > - Add comment for clk_enable/disable skipping
> > - Add ERROR bit clearing and polling step
> >
> > arch/arm/mach-mxs/Makefile | 2 +-
> > arch/arm/mach-mxs/include/mach/common.h | 1 +
> > arch/arm/mach-mxs/ocotp.c | 79 +++++++++++++++++++++++++++++++
> > 3 files changed, 81 insertions(+), 1 deletions(-)
> > create mode 100644 arch/arm/mach-mxs/ocotp.c
> >
> > diff --git a/arch/arm/mach-mxs/Makefile b/arch/arm/mach-mxs/Makefile
> > index 39d3f9c..f23ebbd 100644
> > --- a/arch/arm/mach-mxs/Makefile
> > +++ b/arch/arm/mach-mxs/Makefile
> > @@ -1,5 +1,5 @@
> > # Common support
> > -obj-y := clock.o devices.o gpio.o icoll.o iomux.o system.o timer.o
> > +obj-y := clock.o devices.o gpio.o icoll.o iomux.o ocotp.o system.o timer.o
> >
> > obj-$(CONFIG_SOC_IMX23) += clock-mx23.o mm-mx23.o
> > obj-$(CONFIG_SOC_IMX28) += clock-mx28.o mm-mx28.o
> > diff --git a/arch/arm/mach-mxs/include/mach/common.h b/arch/arm/mach-mxs/include/mach/common.h
> > index 59133eb..cf02552 100644
> > --- a/arch/arm/mach-mxs/include/mach/common.h
> > +++ b/arch/arm/mach-mxs/include/mach/common.h
> > @@ -13,6 +13,7 @@
> >
> > struct clk;
> >
> > +extern int mxs_read_ocotp(int offset, int count, u32 *values);
> > extern int mxs_reset_block(void __iomem *);
> > extern void mxs_timer_init(struct clk *, int);
> >
> > diff --git a/arch/arm/mach-mxs/ocotp.c b/arch/arm/mach-mxs/ocotp.c
> > new file mode 100644
> > index 0000000..e2d39aa
> > --- /dev/null
> > +++ b/arch/arm/mach-mxs/ocotp.c
> > @@ -0,0 +1,79 @@
> > +/*
> > + * Copyright 2010 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 as published by
> > + * the Free Software Foundation; either version 2 of the License, or
> > + * (at your option) any later version.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > + * GNU General Public License for more details.
> > + */
> > +
> > +#include <linux/delay.h>
> > +#include <linux/err.h>
> > +#include <linux/mutex.h>
> > +
> > +#include <mach/mxs.h>
> > +
> > +#define BM_OCOTP_CTRL_BUSY (1 << 8)
> > +#define BM_OCOTP_CTRL_ERROR (1 << 9)
> > +#define BM_OCOTP_CTRL_RD_BANK_OPEN (1 << 12)
> > +
> > +static DEFINE_MUTEX(ocotp_mutex);
> > +
> > +int mxs_read_ocotp(unsigned offset, size_t count, u32 *values)
> > +{
> > + void __iomem *ocotp_base = MXS_IO_ADDRESS(MXS_OCOTP_BASE_ADDR);
> > + int timeout = 0x400;
> > + size_t i;
> > +
> > + mutex_lock(&ocotp_mutex);
> > +
> > + /*
> > + * clk_enable(hbus_clk) for ocotp can be skipped
> > + * as it must be on when system is running.
> > + */
> > +
> > + /* try to clear ERROR bit */
> > + __mxs_clrl(BM_OCOTP_CTRL_ERROR, ocotp_base);
>
> This operation does not try to clear the error bit but actually clears
> it...
>
> > +
> > + /* check both BUSY and ERROR cleared */
> > + while ((__raw_readl(ocotp_base) &
> > + (BM_OCOTP_CTRL_BUSY | BM_OCOTP_CTRL_ERROR)) && --timeout)
> > + cpu_relax();
>
> ...which means you do not have to poll the error bit here...
>
> > +
> > + if (unlikely(!timeout))
> > + goto error_unlock;
> > +
> > + /* open OCOTP banks for read */
> > + __mxs_setl(BM_OCOTP_CTRL_RD_BANK_OPEN, ocotp_base);
> > +
> > + /* approximately wait 32 hclk cycles */
> > + udelay(1);
> > +
> > + /* poll BUSY bit becoming cleared */
> > + timeout = 0x400;
> > + while ((__raw_readl(ocotp_base) & BM_OCOTP_CTRL_BUSY) && --timeout)
> > + cpu_relax();
>
> ...which means you can factor out a ocotp_wait_busy function and let the
> code speak instead of the comments.
>
> > +
> > + if (unlikely(!timeout))
> > + goto error_unlock;
> > +
> > + for (i = 0; i < count; i++, offset += 4)
> > + *values++ = __raw_readl(ocotp_base + offset);
>
> The registers in the ocotp are 16 byte aligned. Does it really make
> sense to provide a function allowing to read the gaps between the
> registers?
>
Good catch. The count was added to ease the consecutive otp word
reading, as there is bank open/close cost for otp read. What about
the following changes?
int mxs_read_ocotp(unsigned offset, size_t otp_word_cnt, u32 *values)
{
......
for (i = 0; i < otp_word_cnt; i++, offset += 0x10)
*values++ = __raw_readl(ocotp_base + offset);
......
}
--
Regards,
Shawn
^ permalink raw reply
* [PATCH 2/2] ARM i.MXS: Add auart platform support for i.MX28
From: Shawn Guo @ 2011-01-12 6:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1294758545-9445-3-git-send-email-s.hauer@pengutronix.de>
Hi Sascha,
On Tue, Jan 11, 2011 at 04:09:05PM +0100, Sascha Hauer wrote:
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
> arch/arm/mach-mxs/clock-mx28.c | 5 ++
> arch/arm/mach-mxs/devices-mx28.h | 8 +++
> arch/arm/mach-mxs/devices/Kconfig | 3 +
> arch/arm/mach-mxs/devices/Makefile | 1 +
> arch/arm/mach-mxs/devices/platform-auart.c | 54 +++++++++++++++++++++++
> arch/arm/mach-mxs/include/mach/devices-common.h | 10 ++++
> 6 files changed, 81 insertions(+), 0 deletions(-)
> create mode 100644 arch/arm/mach-mxs/devices/platform-auart.c
>
[...]
> +struct platform_device *__init mxs_add_auart(
> + const struct mxs_auart_data *data)
> +{
> + struct resource res[] = {
> + {
> + .start = data->iobase,
> + .end = data->iobase + SZ_4K - 1,
The io size here is supposed to mean the space for holding the
current defined registers or the memory map size for this block
given by SoC design? If it's the latter, the size should be SZ_8K.
> + .flags = IORESOURCE_MEM,
> + }, {
> + .start = data->irq,
> + .end = data->irq,
> + .flags = IORESOURCE_IRQ,
> + },
> + };
> +
> + return mxs_add_platform_device_dmamask("mxs-auart", data->id,
> + res, ARRAY_SIZE(res), NULL, 0,
> + DMA_BIT_MASK(32));
> +}
> +
--
Regards,
Shawn
^ permalink raw reply
* [RESEND PATCH] serial: samsung: fix device name
From: Kukjin Kim @ 2011-01-12 6:15 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <AANLkTinwwtE5t16zvpsU5eZvEtJedJr=0x_10DcB0U5z@mail.gmail.com>
Darius Augulis wrote:
>
> On Mon, Sep 27, 2010 at 3:18 AM, Ben Dooks <ben-linux@fluff.org> wrote:
> > On 24/09/10 17:37, Paulius Zaleckas wrote:
> >> On 09/24/2010 09:57 AM, Darius Augulis wrote:
> >>> Hi,
> >>>
> >>> On Fri, Sep 24, 2010 at 2:40 AM, Ben Dooks<ben-linux@fluff.org>
?wrote:
> >>>> On 23/09/10 20:15, Darius Augulis wrote:
> >>>>> Swap device and driver names in serial/samsung.c
> >>>>
> >>>> This is far too short, please see the notes below on trying
> >>>> to make this more informative.
> >>>>
> >>>>> Signed-off-by: Darius Augulis<augulis.darius@gmail.com>
> >>>>> ---
> >>>>>
> >>>>> This patch was submitted about 3 months ago, but still not merged.
> >>>>> There was another similar patch from Joonyoung Shim
> >>>>> <jy0922.shim@samsung.com> ?and was discussed here:
> >>>>> http://marc.info/?l=linux-serial&m=127416101222281&w=2.
> >>>>> Joonyoung Shim acked my patch and other people on ARM mailing list
> >>>>> acked it. The maintainer, Ben Dooks, still not responding for
> >>>>> a long time. Another maintainer, Kukjin Kim, refused to merge
> >>>>> it without Ben's review.
> >>>>> I would like to ask somebody pick up this bugfix.
> >>>>
> >>>> I belive last time this was brought up I asked about the affect
> >>>> this has on the userspace. The following issues would be helpful
> >>>> or essential to have noted in the header about the effect of this.
> >>>>
> >>>> - Does it change the /dev name of the device? if so I would thinl
> >>>> ? carefully about applying it, as it would be a change in the way
> >>>> ? that userspace sees the kernel.
> >>>
> >>> It does - now devices are named /dev/s3c2410_serial, and patch changes
> >>> its name to /dev/ttySAC
> >>>
> >>>>
> >>>> - Does it change the kernel output itself? A note on what diffeences
> >>>> ? can be seen in things like dmesg would be helpful.
> >>>
> >>> It does. Serial driver reports device names when probing, so there
> >>> will appear ttySACx instead of
> >>> s3c2410_serialx.
> >>>
> >>>>
> >>>> - Are there any other side effects
> >>>>
> >>>> - Why is this a bug? Maybe the previous points will explain what is
> >>>> ? going on, but if not, then a reasonably concise description of
> >>>> ? what is going on here.
> >>>
> >>> This is bug, because of several points:
> >>>
> >>> 1. Because it contradicts kernel documentation. Please read
> >>> Documentation/arm/Samsung-S3C24XX/Overview.txt line 196.
> >>> This should be enough to apply this patch.
> >>>
> >>> 2. Because s3c2410_serial isn't correct name for serial device node.
> >>> It's name of Samsung serial driver.
> >>>
> >>> 3. Because now almost all userspace systems workaround it by creating
> >>> symlink
> >>> /dev/ttySACx> ?/dev/s3c2410_serialx and only then put some getty on
> >>> created symlink, not on original device.
> >>> Systems which don't create this symlink, fail to boot at all, because
> >>> of wrong console name. Good example is Buildroot.
> >>
> >> I had similar issue with buildroot too, because in /etc/securetty it is
> >> defained as ttySAC0
> >>
> >> 4. For console in kernel boot command line you must enter
console=ttySACx.
> >> Why it sould be different in userspace?
> >>
> >> For me it seems taht this bug was not noticed earlier, because most
> >> embedded
> >> systems were using static device nodes and ttySACx was used in these
cases.
> >
> > As long as these are explained, then I'm reasonably happy for this to be
> > applied, as long as there are no other complaints.
> >
> > However, this is a big change, and I think it should await the next
> > merge window.
>
> I agree that too.
>
Hi Darius and all,
Maybe you remember this, I applied this in my for-next for 38 this merge
window.
If any opinions, please let me know.
Thanks and Happy New year :)
Best regards,
Kgene.
--
Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.
^ permalink raw reply
* Message for testing
From: Guo Shawn-R65073 @ 2011-01-12 6:11 UTC (permalink / raw)
To: linux-arm-kernel
I'm posting this message from Outlook client to see if the mail list can deliver me the message.
Here is the story.
I'm subscribing the list using both my gmail box and freescale mail. The gmail box is working all right. But the freescale one only receives the messages posted by non-freescale mail box. The list never delivers my freescale mail box any messages posted by any @freescale.com either myself or my colleagues. And I'm asking the company IT team to help. He wants to know whether I have this problem or not if I post messages to the list with Outlook client. (The company mail system is a MSFT solution)
Sorry for bothering.
Regards,
Shawn
^ permalink raw reply
* [PATCH 06/16] nuc900fb: don't treat NULL clk as an error
From: Paul Mundt @ 2011-01-12 6:00 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <AANLkTingg+wm8jy4kRL=ubLFrCDXq17jZQkD3v3M4h+Y@mail.gmail.com>
On Wed, Jan 12, 2011 at 09:24:47AM +0800, Wan ZongShun wrote:
> 2011/1/11 Jamie Iles <jamie@jamieiles.com>:
> > clk_get() returns a struct clk cookie to the driver and some platforms
> > may return NULL if they only support a single clock. ??clk_get() has only
> > failed if it returns a ERR_PTR() encoded pointer.
> >
> > Cc: Wan ZongShun <mcuos.com@gmail.com>
> > Cc: linux-fbdev at vger.kernel.org
> > Signed-off-by: Jamie Iles <jamie@jamieiles.com>
>
> Thanks for your patch.
>
> Acked-by: Wan ZongShun <mcuos.com@gmail.com>
>
Applied, thanks.
^ permalink raw reply
* [PATCH 1/2] serial: Add auart driver for i.MX23/28
From: Shawn Guo @ 2011-01-12 5:39 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1294758545-9445-2-git-send-email-s.hauer@pengutronix.de>
On Tue, Jan 11, 2011 at 04:09:04PM +0100, Sascha Hauer wrote:
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
> drivers/serial/Kconfig | 15 +
> drivers/serial/Makefile | 1 +
> drivers/serial/mxs-auart.c | 763 ++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 779 insertions(+), 0 deletions(-)
> create mode 100644 drivers/serial/mxs-auart.c
>
> diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig
> index ec3c214..de37fe5 100644
> --- a/drivers/serial/Kconfig
> +++ b/drivers/serial/Kconfig
> @@ -1657,4 +1657,19 @@ config SERIAL_PCH_UART
> This driver is for PCH(Platform controller Hub) UART of Intel EG20T
> which is an IOH(Input/Output Hub) for x86 embedded processor.
> Enabling PCH_DMA, this PCH UART works as DMA mode.
> +
> +config SERIAL_MXS_AUART
> + depends on ARCH_MXS
> + tristate "i.MXS AUART support"
Can we not use i.MXS? We use i.MX23 and i.MX28 for SoC, MXS for the
arch, but never i.MXS.
> + select SERIAL_CORE
> + help
> + This driver supports the i.MX AUART port.
i.MX is being used to break the naming consistency here.
> +
> +config SERIAL_MXS_AUART_CONSOLE
> + bool "i.MXS AUART console support"
> + depends on SERIAL_MXS_AUART=y
> + select SERIAL_CORE_CONSOLE
> + help
> + Enable a i.MXS AUART port to be the system console.
> +
> endmenu
> diff --git a/drivers/serial/Makefile b/drivers/serial/Makefile
> index 8ea92e9..c855071 100644
> --- a/drivers/serial/Makefile
> +++ b/drivers/serial/Makefile
> @@ -92,3 +92,4 @@ obj-$(CONFIG_SERIAL_MRST_MAX3110) += mrst_max3110.o
> obj-$(CONFIG_SERIAL_MFD_HSU) += mfd.o
> obj-$(CONFIG_SERIAL_IFX6X60) += ifx6x60.o
> obj-$(CONFIG_SERIAL_PCH_UART) += pch_uart.o
> +obj-$(CONFIG_SERIAL_MXS_AUART) += mxs-auart.o
> diff --git a/drivers/serial/mxs-auart.c b/drivers/serial/mxs-auart.c
> new file mode 100644
> index 0000000..dd437ea
> --- /dev/null
> +++ b/drivers/serial/mxs-auart.c
> @@ -0,0 +1,763 @@
> +/*
> + * Application UART driver for hardware found on
> + * Sigmatel STMP37XX/STMP378X and
> + * Freescale i.MX23/28
> + *
> + * Author: dmitry pervushin <dimka@embeddedalley.com>
> + *
> + * Copyright 2010 Sascha Hauer <s.hauer@pengutronix.de>
2011?
> + * Copyright 2008-2010 Freescale Semiconductor, Inc.
> + * Copyright 2008 Embedded Alley Solutions, Inc All Rights Reserved.
> + *
> + * The code contained herein is licensed under the GNU General Public
> + * License. You may obtain a copy of the GNU General Public License
> + * Version 2 or later at the following locations:
> + *
> + * http://www.opensource.org/licenses/gpl-license.html
> + * http://www.gnu.org/copyleft/gpl.html
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/device.h>
> +#include <linux/errno.h>
> +#include <linux/init.h>
> +#include <linux/console.h>
> +#include <linux/interrupt.h>
> +#include <linux/module.h>
> +#include <linux/slab.h>
> +#include <linux/wait.h>
> +#include <linux/tty.h>
> +#include <linux/tty_driver.h>
> +#include <linux/tty_flip.h>
> +#include <linux/serial.h>
> +#include <linux/serial_core.h>
> +#include <linux/platform_device.h>
> +#include <linux/device.h>
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +
> +#include <asm/cacheflush.h>
> +#include <mach/mx28.h>
> +
> +#define MXS_AUART_PORTS 5
> +
> +#define UARTAPP_CTRL0 0x00000000
> +#define UARTAPP_CTRL0_SET 0x00000004
> +#define UARTAPP_CTRL0_CLR 0x00000008
> +#define UARTAPP_CTRL0_TOG 0x0000000c
> +#define UARTAPP_CTRL1 0x00000010
> +#define UARTAPP_CTRL1_SET 0x00000014
> +#define UARTAPP_CTRL1_CLR 0x00000018
> +#define UARTAPP_CTRL1_TOG 0x0000001c
> +#define UARTAPP_CTRL2 0x00000020
> +#define UARTAPP_CTRL2_SET 0x00000024
> +#define UARTAPP_CTRL2_CLR 0x00000028
> +#define UARTAPP_CTRL2_TOG 0x0000002c
> +#define UARTAPP_LINECTRL 0x00000030
> +#define UARTAPP_LINECTRL_SET 0x00000034
> +#define UARTAPP_LINECTRL_CLR 0x00000038
> +#define UARTAPP_LINECTRL_TOG 0x0000003c
> +#define UARTAPP_LINECTRL2 0x00000040
> +#define UARTAPP_LINECTRL2_SET 0x00000044
> +#define UARTAPP_LINECTRL2_CLR 0x00000048
> +#define UARTAPP_LINECTRL2_TOG 0x0000004c
> +#define UARTAPP_INTR 0x00000050
> +#define UARTAPP_INTR_SET 0x00000054
> +#define UARTAPP_INTR_CLR 0x00000058
> +#define UARTAPP_INTR_TOG 0x0000005c
> +#define UARTAPP_DATA 0x00000060
> +#define UARTAPP_STAT 0x00000070
> +#define UARTAPP_DEBUG 0x00000080
> +#define UARTAPP_VERSION 0x00000090
> +#define UARTAPP_AUTOBAUD 0x000000a0
> +
> +#define BM_UARTAPP_CTRL0_SFTRST (1 << 31)
> +#define BM_UARTAPP_CTRL0_CLKGATE (1 << 30)
> +
> +
> +#define BM_UARTAPP_CTRL2_CTSEN (1 << 15)
> +#define BM_UARTAPP_CTRL2_RTS (1 << 11)
> +#define BM_UARTAPP_CTRL2_RXE (1 << 9)
> +#define BM_UARTAPP_CTRL2_TXE (1 << 8)
> +#define BM_UARTAPP_CTRL2_UARTEN (1 << 0)
> +
> +#define BP_UARTAPP_LINECTRL_BAUD_DIVINT 16
> +#define BM_UARTAPP_LINECTRL_BAUD_DIVINT 0xffff0000
> +#define BF_UARTAPP_LINECTRL_BAUD_DIVINT(v) (((v) & 0xffff) << 16)
> +#define BP_UARTAPP_LINECTRL_BAUD_DIVFRAC 8
> +#define BM_UARTAPP_LINECTRL_BAUD_DIVFRAC 0x00003f00
> +#define BF_UARTAPP_LINECTRL_BAUD_DIVFRAC(v) (((v) & 0x3f) << 8)
> +#define BP_UARTAPP_LINECTRL_WLEN 5
> +#define BM_UARTAPP_LINECTRL_WLEN 0x00000060
> +#define BF_UARTAPP_LINECTRL_WLEN(v) (((v) & 0x3) << 5)
> +#define BM_UARTAPP_LINECTRL_FEN (1 << 4)
> +#define BM_UARTAPP_LINECTRL_STP2 (1 << 3)
> +#define BM_UARTAPP_LINECTRL_EPS (1 << 2)
> +#define BM_UARTAPP_LINECTRL_PEN (1 << 1)
> +#define BM_UARTAPP_LINECTRL_BRK (1 << 0)
> +
> +#define BM_UARTAPP_INTR_RTIEN (1 << 22)
> +#define BM_UARTAPP_INTR_TXIEN (1 << 21)
> +#define BM_UARTAPP_INTR_RXIEN (1 << 20)
> +#define BM_UARTAPP_INTR_CTSMIEN (1 << 17)
> +#define BM_UARTAPP_INTR_RTIS (1 << 6)
> +#define BM_UARTAPP_INTR_TXIS (1 << 5)
> +#define BM_UARTAPP_INTR_RXIS (1 << 4)
> +#define BM_UARTAPP_INTR_CTSMIS (1 << 1)
> +
> +#define BM_UARTAPP_STAT_BUSY (1 << 29)
> +#define BM_UARTAPP_STAT_CTS (1 << 28)
> +#define BM_UARTAPP_STAT_TXFE (1 << 27)
> +#define BM_UARTAPP_STAT_TXFF (1 << 25)
> +#define BM_UARTAPP_STAT_RXFE (1 << 24)
> +#define BM_UARTAPP_STAT_OERR (1 << 19)
> +#define BM_UARTAPP_STAT_BERR (1 << 18)
> +#define BM_UARTAPP_STAT_PERR (1 << 17)
> +#define BM_UARTAPP_STAT_FERR (1 << 16)
> +
> +#define MXS_AUART_MAJOR 242
> +#define MXS_AUART_RX_THRESHOLD 16
> +
> +static struct uart_driver auart_driver;
> +
> +struct mxs_auart_port {
> + struct uart_port port;
> +
> + unsigned int flags;
> + unsigned int ctrl;
> +
> + unsigned int irq;
> +
> + struct clk *clk;
> + struct device *dev;
> +};
> +
> +static void mxs_auart_stop_tx(struct uart_port *u);
> +
> +#define to_auart_port(u) container_of(u, struct mxs_auart_port, port)
> +
> +static inline void mxs_auart_tx_chars(struct mxs_auart_port *s)
> +{
> + struct circ_buf *xmit = &s->port.state->xmit;
> +
> + while (!(readl(s->port.membase + UARTAPP_STAT) &
> + BM_UARTAPP_STAT_TXFF)) {
> + if (s->port.x_char) {
> + writel(s->port.x_char,
> + s->port.membase + UARTAPP_DATA);
> + s->port.x_char = 0;
> + continue;
> + }
> + if (!uart_circ_empty(xmit) && !uart_tx_stopped(&s->port)) {
> + writel(xmit->buf[xmit->tail],
> + s->port.membase + UARTAPP_DATA);
> + xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE - 1);
> + if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
> + uart_write_wakeup(&s->port);
> + } else
> + break;
> + }
> + if (uart_circ_empty(&(s->port.state->xmit)))
> + writel(BM_UARTAPP_INTR_TXIEN,
> + s->port.membase + UARTAPP_INTR_CLR);
> + else
> + writel(BM_UARTAPP_INTR_TXIEN,
> + s->port.membase + UARTAPP_INTR_SET);
> +
> + if (uart_tx_stopped(&s->port))
> + mxs_auart_stop_tx(&s->port);
> +}
> +
> +static inline unsigned int
> +mxs_auart_rx_char(struct mxs_auart_port *s, unsigned int stat, u8 c)
> +{
> + int flag;
> +
> + flag = TTY_NORMAL;
> + if (stat & BM_UARTAPP_STAT_BERR) {
> + stat &= ~BM_UARTAPP_STAT_BERR;
> + s->port.icount.brk++;
> + if (uart_handle_break(&s->port))
> + return stat;
> + flag = TTY_BREAK;
> + } else if (stat & BM_UARTAPP_STAT_PERR) {
> + stat &= ~BM_UARTAPP_STAT_PERR;
> + s->port.icount.parity++;
> + flag = TTY_PARITY;
> + } else if (stat & BM_UARTAPP_STAT_FERR) {
> + stat &= ~BM_UARTAPP_STAT_FERR;
> + s->port.icount.frame++;
> + flag = TTY_FRAME;
> + }
> +
> + if (stat & BM_UARTAPP_STAT_OERR)
> + s->port.icount.overrun++;
> +
> + if (uart_handle_sysrq_char(&s->port, c))
> + return stat;
> +
> + uart_insert_char(&s->port, stat, BM_UARTAPP_STAT_OERR, c, flag);
> +
> + return stat;
> +}
> +
> +static void mxs_auart_rx_chars(struct mxs_auart_port *s)
> +{
> + u8 c;
> + struct tty_struct *tty = s->port.state->port.tty;
> + u32 stat = 0;
> +
> + for (;;) {
> + stat = readl(s->port.membase + UARTAPP_STAT);
> + if (stat & BM_UARTAPP_STAT_RXFE)
> + break;
> + c = readl(s->port.membase + UARTAPP_DATA);
> + stat = mxs_auart_rx_char(s, stat, c);
> + writel(stat, s->port.membase + UARTAPP_STAT);
> + }
> +
> + writel(stat, s->port.membase + UARTAPP_STAT);
> + tty_flip_buffer_push(tty);
> +}
> +
> +static int mxs_auart_request_port(struct uart_port *u)
> +{
> + return 0;
> +}
> +
> +static int mxs_auart_verify_port(struct uart_port *u,
> + struct serial_struct *ser)
> +{
> + if (u->type != PORT_UNKNOWN && u->type != PORT_IMX)
> + return -EINVAL;
> + return 0;
> +}
> +
> +static void mxs_auart_config_port(struct uart_port *u, int flags)
> +{
> +}
> +
> +static const char *mxs_auart_type(struct uart_port *u)
> +{
> + struct mxs_auart_port *s = to_auart_port(u);
> +
> + return dev_name(s->dev);
> +}
> +
> +static void mxs_auart_release_port(struct uart_port *u)
> +{
> +}
> +
> +static void mxs_auart_set_mctrl(struct uart_port *u, unsigned mctrl)
> +{
> + struct mxs_auart_port *s = to_auart_port(u);
> +
> + u32 ctrl = readl(u->membase + UARTAPP_CTRL2);
> +
> + ctrl &= ~BM_UARTAPP_CTRL2_RTS;
> + if (mctrl & TIOCM_RTS)
> + ctrl |= BM_UARTAPP_CTRL2_RTS;
> + s->ctrl = mctrl;
> + writel(ctrl, u->membase + UARTAPP_CTRL2);
> +}
> +
> +static u32 mxs_auart_get_mctrl(struct uart_port *u)
> +{
> + struct mxs_auart_port *s = to_auart_port(u);
> + u32 stat = readl(u->membase + UARTAPP_STAT);
> + int ctrl2 = readl(u->membase + UARTAPP_CTRL2);
> + u32 mctrl = s->ctrl;
> +
> + mctrl &= ~TIOCM_CTS;
> + if (stat & BM_UARTAPP_STAT_CTS)
> + mctrl |= TIOCM_CTS;
> +
> + if (ctrl2 & BM_UARTAPP_CTRL2_RTS)
> + mctrl |= TIOCM_RTS;
> +
> + return mctrl;
> +}
> +
> +static void mxs_auart_settermios(struct uart_port *u,
> + struct ktermios *termios,
> + struct ktermios *old)
> +{
> + u32 bm, ctrl, ctrl2, div;
> + unsigned int cflag, baud;
> +
> + cflag = termios->c_cflag;
> +
> + ctrl = BM_UARTAPP_LINECTRL_FEN;
> + ctrl2 = readl(u->membase + UARTAPP_CTRL2);
> +
> + /* byte size */
> + switch (cflag & CSIZE) {
> + case CS5:
> + bm = 0;
> + break;
> + case CS6:
> + bm = 1;
> + break;
> + case CS7:
> + bm = 2;
> + break;
> + case CS8:
> + bm = 3;
> + break;
> + default:
> + return;
> + }
> +
> + ctrl |= BF_UARTAPP_LINECTRL_WLEN(bm);
> +
> + /* parity */
> + if (cflag & PARENB) {
> + ctrl |= BM_UARTAPP_LINECTRL_PEN;
> + if ((cflag & PARODD) == 0)
> + ctrl |= BM_UARTAPP_LINECTRL_EPS;
> + }
> +
> + /* figure out the stop bits requested */
> + if (cflag & CSTOPB)
> + ctrl |= BM_UARTAPP_LINECTRL_STP2;
> +
> + /* figure out the hardware flow control settings */
> + if (cflag & CRTSCTS)
> + ctrl2 |= BM_UARTAPP_CTRL2_CTSEN;
> + else
> + ctrl2 &= ~BM_UARTAPP_CTRL2_CTSEN;
> +
> + /* set baud rate */
> + baud = uart_get_baud_rate(u, termios, old, 0, u->uartclk);
> + div = u->uartclk * 32 / baud;
> + ctrl |= BF_UARTAPP_LINECTRL_BAUD_DIVFRAC(div & 0x3F);
> + ctrl |= BF_UARTAPP_LINECTRL_BAUD_DIVINT(div >> 6);
> +
> + if ((cflag & CREAD) != 0)
> + ctrl2 |= BM_UARTAPP_CTRL2_RXE;
> +
> + writel(ctrl, u->membase + UARTAPP_LINECTRL);
> + writel(ctrl2, u->membase + UARTAPP_CTRL2);
> +}
> +
> +static irqreturn_t mxs_auart_irq_handle(int irq, void *context)
> +{
> + u32 istatus, istat;
> + struct mxs_auart_port *s = context;
> + u32 stat = readl(s->port.membase + UARTAPP_STAT);
> +
> + istatus = istat = readl(s->port.membase + UARTAPP_INTR);
> +
> + if (istat & BM_UARTAPP_INTR_CTSMIS) {
> + uart_handle_cts_change(&s->port, stat & BM_UARTAPP_STAT_CTS);
> + writel(BM_UARTAPP_INTR_CTSMIS,
> + s->port.membase + UARTAPP_INTR_CLR);
> + istat &= ~BM_UARTAPP_INTR_CTSMIS;
> + }
> +
> + if (istat & (BM_UARTAPP_INTR_RTIS | BM_UARTAPP_INTR_RXIS)) {
> + mxs_auart_rx_chars(s);
> + istat &= ~(BM_UARTAPP_INTR_RTIS | BM_UARTAPP_INTR_RXIS);
> + }
> +
> + if (istat & BM_UARTAPP_INTR_TXIS) {
> + mxs_auart_tx_chars(s);
> + istat &= ~BM_UARTAPP_INTR_TXIS;
> + }
> +
> + writel(istatus & (BM_UARTAPP_INTR_RTIS
> + | BM_UARTAPP_INTR_TXIS
> + | BM_UARTAPP_INTR_RXIS
> + | BM_UARTAPP_INTR_CTSMIS),
> + s->port.membase + UARTAPP_INTR_CLR);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static void mxs_auart_reset(struct uart_port *u)
> +{
> + int i;
> + unsigned int reg;
> +
> + writel(BM_UARTAPP_CTRL0_SFTRST,
> + u->membase + UARTAPP_CTRL0_CLR);
> +
> + for (i = 0; i < 10000; i++) {
> + reg = readl(u->membase + UARTAPP_CTRL0);
> + if (!(reg & BM_UARTAPP_CTRL0_SFTRST))
> + break;
> + udelay(3);
> + }
> +
> + writel(BM_UARTAPP_CTRL0_CLKGATE,
> + u->membase + UARTAPP_CTRL0_CLR);
> +}
> +
> +static int mxs_auart_startup(struct uart_port *u)
> +{
> + struct mxs_auart_port *s = to_auart_port(u);
> +
> + clk_enable(s->clk);
> +
> + writel(BM_UARTAPP_CTRL2_UARTEN,
> + s->port.membase + UARTAPP_CTRL2_SET);
> +
> + writel(BM_UARTAPP_INTR_RXIEN | BM_UARTAPP_INTR_RTIEN,
> + s->port.membase + UARTAPP_INTR);
> +
> + writel(BM_UARTAPP_INTR_CTSMIEN,
> + s->port.membase + UARTAPP_INTR_SET);
> +
> + /*
> + * Enable fifo so all four bytes of a DMA word are written to
> + * output (otherwise, only the LSB is written, ie. 1 in 4 bytes)
> + */
> + writel(BM_UARTAPP_LINECTRL_FEN,
> + s->port.membase + UARTAPP_LINECTRL_SET);
> +
> + return 0;
> +}
> +
> +static void mxs_auart_shutdown(struct uart_port *u)
> +{
> + struct mxs_auart_port *s = to_auart_port(u);
> +
> + writel(BM_UARTAPP_CTRL0_SFTRST,
> + s->port.membase + UARTAPP_CTRL0_SET);
> +
> + writel(BM_UARTAPP_INTR_RXIEN | BM_UARTAPP_INTR_RTIEN |
> + BM_UARTAPP_INTR_CTSMIEN,
> + s->port.membase + UARTAPP_INTR_CLR);
> +
> + clk_disable(s->clk);
> +}
> +
> +static unsigned int mxs_auart_tx_empty(struct uart_port *u)
> +{
> + if (readl(u->membase + UARTAPP_STAT) & BM_UARTAPP_STAT_TXFE)
> + return TIOCSER_TEMT;
> + else
> + return 0;
> +}
> +
> +static void mxs_auart_start_tx(struct uart_port *u)
> +{
> + struct mxs_auart_port *s = to_auart_port(u);
> +
> + /* enable transmitter */
> + writel(BM_UARTAPP_CTRL2_TXE, u->membase + UARTAPP_CTRL2_SET);
> +
> + mxs_auart_tx_chars(s);
> +}
> +
> +static void mxs_auart_stop_tx(struct uart_port *u)
> +{
> + writel(BM_UARTAPP_CTRL2_TXE, u->membase + UARTAPP_CTRL2_CLR);
> +}
> +
> +static void mxs_auart_stop_rx(struct uart_port *u)
> +{
> + writel(BM_UARTAPP_CTRL2_RXE, u->membase + UARTAPP_CTRL2_CLR);
> +}
> +
> +static void mxs_auart_break_ctl(struct uart_port *u, int ctl)
> +{
> + if (ctl)
> + writel(BM_UARTAPP_LINECTRL_BRK,
> + u->membase + UARTAPP_LINECTRL_SET);
> + else
> + writel(BM_UARTAPP_LINECTRL_BRK,
> + u->membase + UARTAPP_LINECTRL_CLR);
> +}
> +
> +static void mxs_auart_enable_ms(struct uart_port *port)
> +{
> + /* just empty */
> +}
> +
> +static struct uart_ops mxs_auart_ops = {
> + .tx_empty = mxs_auart_tx_empty,
> + .start_tx = mxs_auart_start_tx,
> + .stop_tx = mxs_auart_stop_tx,
> + .stop_rx = mxs_auart_stop_rx,
> + .enable_ms = mxs_auart_enable_ms,
> + .break_ctl = mxs_auart_break_ctl,
> + .set_mctrl = mxs_auart_set_mctrl,
> + .get_mctrl = mxs_auart_get_mctrl,
> + .startup = mxs_auart_startup,
> + .shutdown = mxs_auart_shutdown,
> + .set_termios = mxs_auart_settermios,
> + .type = mxs_auart_type,
> + .release_port = mxs_auart_release_port,
> + .request_port = mxs_auart_request_port,
> + .config_port = mxs_auart_config_port,
> + .verify_port = mxs_auart_verify_port,
> +};
> +
> +static struct mxs_auart_port *auart_port[MXS_AUART_PORTS];
> +
> +#ifdef CONFIG_SERIAL_MXS_AUART_CONSOLE
> +static void mxs_auart_console_putchar(struct uart_port *port, int ch)
> +{
> + unsigned int status;
> +
> + do {
> + status = readl(port->membase + UARTAPP_STAT);
> + } while (status & BM_UARTAPP_STAT_TXFF);
> + writel(ch, port->membase + UARTAPP_DATA);
> +}
> +
> +static void
> +auart_console_write(struct console *co, const char *str, unsigned int count)
> +{
> + struct mxs_auart_port *s;
> + struct uart_port *port;
> + unsigned int status, old_cr;
> +
> + if (co->index > MXS_AUART_PORTS || co->index < 0)
> + return;
> +
> + s = auart_port[co->index];
> + port = &s->port;
> +
> + clk_enable(s->clk);
> +
> + /* First save the CR then disable the interrupts */
> + old_cr = readl(port->membase + UARTAPP_CTRL2);
> + writel(BM_UARTAPP_CTRL2_UARTEN | BM_UARTAPP_CTRL2_TXE,
> + port->membase + UARTAPP_CTRL2_SET);
> +
> + uart_console_write(port, str, count, mxs_auart_console_putchar);
> +
> + /*
> + * Finally, wait for transmitter to become empty
> + * and restore the TCR
> + */
> + do {
> + status = readl(port->membase + UARTAPP_STAT);
> + } while (status & BM_UARTAPP_STAT_BUSY);
> +
> + writel(old_cr, port->membase + UARTAPP_CTRL2);
> +
> + clk_disable(s->clk);
> +}
> +
> +static void __init
> +auart_console_get_options(struct uart_port *port, int *baud,
> + int *parity, int *bits)
> +{
> + if (readl(port->membase + UARTAPP_CTRL2)
> + & BM_UARTAPP_CTRL2_UARTEN) {
> + unsigned int lcr_h, quot;
> + lcr_h = readl(port->membase + UARTAPP_LINECTRL);
> +
> + *parity = 'n';
> + if (lcr_h & BM_UARTAPP_LINECTRL_PEN) {
> + if (lcr_h & BM_UARTAPP_LINECTRL_EPS)
> + *parity = 'e';
> + else
> + *parity = 'o';
> + }
> +
> + if ((lcr_h & BM_UARTAPP_LINECTRL_WLEN)
> + == BF_UARTAPP_LINECTRL_WLEN(2))
> + *bits = 7;
> + else
> + *bits = 8;
> +
> + quot = (((readl(port->membase + UARTAPP_LINECTRL)
> + & BM_UARTAPP_LINECTRL_BAUD_DIVINT))
> + >> (BP_UARTAPP_LINECTRL_BAUD_DIVINT - 6))
> + | (((readl(port->membase + UARTAPP_LINECTRL)
> + & BM_UARTAPP_LINECTRL_BAUD_DIVFRAC))
> + >> BP_UARTAPP_LINECTRL_BAUD_DIVFRAC);
> + if (quot == 0)
> + quot = 1;
> + *baud = (port->uartclk << 2) / quot;
> + }
> +}
> +
> +static int __init
> +auart_console_setup(struct console *co, char *options)
> +{
> + struct mxs_auart_port *s;
> + int baud = 9600;
> + int bits = 8;
> + int parity = 'n';
> + int flow = 'n';
> + int ret;
> +
> + /*
> + * Check whether an invalid uart number has been specified, and
> + * if so, search for the first available port that does have
> + * console support.
> + */
> + if (co->index == -1 || co->index >= ARRAY_SIZE(auart_port))
> + co->index = 0;
> + s = auart_port[co->index];
> + if (!s)
> + return -ENODEV;
> +
> + clk_enable(s->clk);
> +
> + if (options)
> + uart_parse_options(options, &baud, &parity, &bits, &flow);
> + else
> + auart_console_get_options(&s->port, &baud, &parity, &bits);
> +
> + ret = uart_set_options(&s->port, co, baud, parity, bits, flow);
> +
> + clk_disable(s->clk);
> +
> + return ret;
> +}
> +
> +static struct console auart_console = {
> + .name = "ttyAPP",
> + .write = auart_console_write,
> + .device = uart_console_device,
> + .setup = auart_console_setup,
> + .flags = CON_PRINTBUFFER,
> + .index = -1,
> + .data = &auart_driver,
> +};
> +#endif
> +
> +static struct uart_driver auart_driver = {
> + .owner = THIS_MODULE,
> + .driver_name = "ttyAPP",
> + .dev_name = "ttyAPP",
> + .major = 0,
> + .minor = 0,
> + .nr = MXS_AUART_PORTS,
> +#ifdef CONFIG_SERIAL_MXS_AUART_CONSOLE
> + .cons = &auart_console,
> +#endif
> +};
> +
> +static int __devinit mxs_auart_probe(struct platform_device *pdev)
> +{
> + struct mxs_auart_port *s;
> + u32 version;
> + int ret = 0;
> + struct resource *r;
> +
> + s = kzalloc(sizeof(struct mxs_auart_port), GFP_KERNEL);
> + if (!s) {
> + ret = -ENOMEM;
> + goto out;
> + }
> +
> + s->clk = clk_get(&pdev->dev, NULL);
> + if (IS_ERR(s->clk)) {
> + ret = PTR_ERR(s->clk);
> + goto out_free;
> + }
> +
> + r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (!r) {
> + ret = -ENXIO;
> + goto out_free_clk;
> + }
> +
> + s->port.mapbase = r->start;
> + s->port.membase = ioremap(r->start, resource_size(r));
> + s->port.ops = &mxs_auart_ops;
> + s->port.iotype = UPIO_MEM;
> + s->port.line = pdev->id < 0 ? 0 : pdev->id;
> + s->port.fifosize = 16;
> + s->port.uartclk = clk_get_rate(s->clk);
> + s->port.type = PORT_IMX;
> + s->port.dev = s->dev = get_device(&pdev->dev);
> +
> + s->flags = 0;
> + s->ctrl = 0;
> +
> + s->irq = platform_get_irq(pdev, 0);
> + s->port.irq = s->irq;
> + ret = request_irq(s->irq, mxs_auart_irq_handle, 0, dev_name(&pdev->dev), s);
> + if (ret)
> + goto out_free_clk;
> +
> + platform_set_drvdata(pdev, s);
> +
> + auart_port[pdev->id] = s;
> +
> + mxs_auart_reset(&s->port);
> +
> + ret = uart_add_one_port(&auart_driver, &s->port);
> + if (ret)
> + goto out_free_irq;
> +
> + version = readl(s->port.membase + UARTAPP_VERSION);
> + dev_info(&pdev->dev, "Found APPUART %d.%d.%d\n",
> + (version >> 24) & 0xff,
> + (version >> 16) & 0xff, version & 0xffff);
> +
> + return 0;
> +
> +out_free_irq:
> + auart_port[pdev->id] = NULL;
> + free_irq(s->irq, s);
> +out_free_clk:
> + clk_put(s->clk);
> +out_free:
> + kfree(s);
> +out:
> + return ret;
> +}
> +
> +static int __devexit mxs_auart_remove(struct platform_device *pdev)
> +{
> + struct mxs_auart_port *s = platform_get_drvdata(pdev);
> +
> + uart_remove_one_port(&auart_driver, &s->port);
> +
> + auart_port[pdev->id] = NULL;
> +
> + clk_put(s->clk);
> + free_irq(s->irq, s);
> + kfree(s);
> +
> + return 0;
> +}
> +
> +static struct platform_driver mxs_auart_driver = {
> + .probe = mxs_auart_probe,
> + .remove = __devexit_p(mxs_auart_remove),
> + .driver = {
> + .name = "mxs-auart",
> + .owner = THIS_MODULE,
> + },
> +};
> +
> +static int __init mxs_auart_init(void)
> +{
> + int r;
> +
> + r = uart_register_driver(&auart_driver);
> + if (r)
> + goto out;
> +
> + r = platform_driver_register(&mxs_auart_driver);
> + if (r)
> + goto out_err;
> +
> + return 0;
> +out_err:
> + uart_unregister_driver(&auart_driver);
> +out:
> + return r;
> +}
> +
> +static void __exit mxs_auart_exit(void)
> +{
> + platform_driver_unregister(&mxs_auart_driver);
> + uart_unregister_driver(&auart_driver);
> +}
> +
> +module_init(mxs_auart_init);
> +module_exit(mxs_auart_exit);
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("Freescale MXS application uart driver");
> +MODULE_AUTHOR("Sascha Hauer <s.hauer@pengutronix.de>");
> --
> 1.7.2.3
>
>
--
Regards,
Shawn
^ permalink raw reply
* Add i.MX23/28 auart support
From: Shawn Guo @ 2011-01-12 5:31 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1294758545-9445-1-git-send-email-s.hauer@pengutronix.de>
Hi Sascha,
On Tue, Jan 11, 2011 at 04:09:03PM +0100, Sascha Hauer wrote:
> The following series adds support for the auart found on i.MX23/28
>
> Sascha Hauer (2):
> serial: Add auart driver for i.MX23/28
> ARM i.MXS: Add auart platform support for i.MX28
>
> arch/arm/mach-mxs/clock-mx28.c | 5 +
> arch/arm/mach-mxs/devices-mx28.h | 8 +
> arch/arm/mach-mxs/devices/Kconfig | 3 +
> arch/arm/mach-mxs/devices/Makefile | 1 +
> arch/arm/mach-mxs/devices/platform-auart.c | 54 ++
> arch/arm/mach-mxs/include/mach/devices-common.h | 10 +
> drivers/serial/Kconfig | 15 +
> drivers/serial/Makefile | 1 +
> drivers/serial/mxs-auart.c | 763 +++++++++++++++++++++++
> 9 files changed, 860 insertions(+), 0 deletions(-)
> create mode 100644 arch/arm/mach-mxs/devices/platform-auart.c
> create mode 100644 drivers/serial/mxs-auart.c
>
>
I just fetched the branch imx-for-2.6.38 and tried to apply the patch
set on it, but failed. What commit should I apply against?
--
Regards,
Shawn
^ permalink raw reply
* [PATCH] watchdog: add support for the Synopsys DesignWare WDT
From: viresh kumar @ 2011-01-12 5:16 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1294675181-18273-1-git-send-email-jamie@jamieiles.com>
Jamie,
Sorry for sending comments on V5 !!
I don't know why, but checkpatch used to give few errors which it is
not giving now.
Like:
- Mixing spaces and tabs
- Line over 80 columns.
There are few places in this patch where i have seen these issues, but
checkpatch doesn't
report them at all.
On Mon, Jan 10, 2011 at 9:29 PM, Jamie Iles <jamie@jamieiles.com> wrote:
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index 8a3aa2f..62c607a 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -331,6 +331,15 @@ config IMX2_WDT
> ? ? ? ? ?To compile this driver as a module, choose M here: the
> ? ? ? ? ?module will be called imx2_wdt.
>
> +config DW_WATCHDOG
> + ? ? ? tristate "Synopsys DesignWare watchdog"
> + ? ? ? depends on ARM
Correct me if i am wrong, but can't this peripheral be used for any
other architecture.
We are adding it as a platform device and not amba device.
Might be synopsys give some other interface here, other than AMBA.
> + ? ? ? help
> + ? ? ? ? Say Y here if to include support for the Synopsys DesignWare
> + ? ? ? ? watchdog timer found in many ARM chips.
> + ? ? ? ? To compile this driver as a module, choose M here: the
> + ? ? ? ? module will be called dw_wdt.
> +
> ?# AVR32 Architecture
>
(...)
> diff --git a/drivers/watchdog/dw_wdt.c b/drivers/watchdog/dw_wdt.c
> new file mode 100644
> index 0000000..45fa1fb
> --- /dev/null
> +++ b/drivers/watchdog/dw_wdt.c
> @@ -0,0 +1,405 @@
> +/*
> + * Copyright 2010-2011 Picochip Ltd., Jamie Iles
> + * http://www.picochip.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; either version
> + * 2 of the License, or (at your option) any later version.
> + *
> + * This file implements a driver for the Synopsys DesignWare watchdog device
> + * in the many ARM subsystems. The watchdog has 16 different timeout periods
> + * and these are a function of the input clock frequency.
> + *
> + * The DesignWare watchdog cannot be stopped once it has been started so we
> + * use a software timer to implement a ping that will keep the watchdog alive.
> + * If we receive an expected close for the watchdog then we keep the timer
> + * running, otherwise the timer is stopped and the watchdog will expire.
> + */
> +#define pr_fmt(fmt) "dw_wdt: " fmt
> +
> +#include <linux/bitops.h>
> +#include <linux/clk.h>
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/fs.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/miscdevice.h>
> +#include <linux/module.h>
> +#include <linux/moduleparam.h>
> +#include <linux/pm.h>
> +#include <linux/platform_device.h>
> +#include <linux/spinlock.h>
> +#include <linux/timer.h>
> +#include <linux/uaccess.h>
> +#include <linux/watchdog.h>
> +
can remove this blank line.
> +#include <linux/platform_data/dw_wdt.h>
> +
> +#define WDOG_CONTROL_REG_OFFSET ? ? ? ? ? ? 0x00
> +#define WDOG_CONTROL_REG_WDT_EN_MASK ? ? ? 0x01
> +#define WDOG_TIMEOUT_RANGE_REG_OFFSET ? ? ? 0x04
> +#define WDOG_CURRENT_COUNT_REG_OFFSET ? ? ? 0x08
> +#define WDOG_COUNTER_RESTART_REG_OFFSET ? ? 0x0c
> +#define WDOG_COUNTER_RESTART_KICK_VALUE ? ? ? ? ? ?0x76
I don't know why, but i have seen a lot of comment that people give to
align macro's.
Use tabs instead of spaces between macro name and its value. Also keep
them aligned.
I am not sure if its absolutely necessary.
(...)
> +static int dw_wdt_set_top(unsigned top_s)
> +{
> + ? ? ? int i, top_val = -1;
We can assign top_val = DW_WDT_MAX_TOP, here and
> +
> + ? ? ? /*
> + ? ? ? ?* Iterate over the timeout values until we find the closest match. We
> + ? ? ? ?* always look for >=.
> + ? ? ? ?*/
> + ? ? ? for (i = 0; i <= DW_WDT_MAX_TOP; ++i)
> + ? ? ? ? ? ? ? if (dw_wdt_top_in_seconds(i) >= top_s) {
> + ? ? ? ? ? ? ? ? ? ? ? top_val = i;
> + ? ? ? ? ? ? ? ? ? ? ? break;
> + ? ? ? ? ? ? ? }
> +
> + ? ? ? /*
> + ? ? ? ?* If we didn't find a suitable value, it must have been too large. Go
> + ? ? ? ?* with the biggest that we can.
> + ? ? ? ?*/
> + ? ? ? if (top_val < 0)
> + ? ? ? ? ? ? ? top_val = DW_WDT_MAX_TOP;
remove this check.
> +
> + ? ? ? /* Set the new value in the watchdog. */
> + ? ? ? writel(top_val, dw_wdt.regs + WDOG_TIMEOUT_RANGE_REG_OFFSET);
> +
> + ? ? ? dw_wdt_set_next_heartbeat();
> +
> + ? ? ? return dw_wdt_top_in_seconds(top_val);
> +}
> +
(...)
> +static int __devinit dw_wdt_drv_probe(struct platform_device *pdev)
> +{
> + ? ? ? int ret;
> + ? ? ? struct resource *mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + ? ? ? struct dw_wdt_platform_data *pdata = pdev->dev.platform_data;
> +
> + ? ? ? if (!mem)
> + ? ? ? ? ? ? ? return -EINVAL;
> +
> + ? ? ? if (!devm_request_mem_region(&pdev->dev, mem->start, resource_size(mem),
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?"dw_wdt"))
> + ? ? ? ? ? ? ? return -ENOMEM;
> +
> + ? ? ? dw_wdt.regs = devm_ioremap(&pdev->dev, mem->start,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?resource_size(mem));
this can come in single line.
> + ? ? ? if (!dw_wdt.regs)
> + ? ? ? ? ? ? ? return -ENOMEM;
> +
> + ? ? ? dw_wdt.clk = clk_get(&pdev->dev, NULL);
> + ? ? ? if (IS_ERR(dw_wdt.clk))
> + ? ? ? ? ? ? ? return -ENODEV;
shouldn't we use the error value returned from clk_get here instead of defining
a new value -ENODEV?
> +
> + ? ? ? ret = clk_enable(dw_wdt.clk);
should we call this routine if clk is NULL? Probably we will get error here.
But as we discussed earlier, we should support machines who don't support clk
framework. So, shouldn't we check clk for NULL here and call this
routine only if
we have a valid clk pointer, otherwise continue instead of returning error.
> + ? ? ? if (ret)
> + ? ? ? ? ? ? ? goto out_put_clk;
> +
> + ? ? ? /*
> + ? ? ? ?* The timeout period of the watchdog is derived from the input clock
> + ? ? ? ?* frequency. ?For platforms that don't have a clk for the watchdog,
> + ? ? ? ?* they can specify the WDT clock rate through the clk_rate field of
> + ? ? ? ?* the struct dw_wdt_platform_data platform data.
> + ? ? ? ?*/
> + ? ? ? if (pdata && pdata->clk_rate > 0)
> + ? ? ? ? ? ? ? dw_wdt.clk_rate = pdata->clk_rate;
> + ? ? ? else
> + ? ? ? ? ? ? ? dw_wdt.clk_rate = clk_get_rate(dw_wdt.clk);
> +
I know it doesn't make sense for a platform to have support for clk framework
and pass rate through plat data.
But this code will always take pdata->rate, if it is passed.
Wouldn't it be better if we reverse the sequence.
if (clk)
rate = clk_get_rate(...);
else {
pdata = pdev->dev.platform_data;
if (pdata)
rate = pdata->rate;
}
and then following code
> + ? ? ? if (!dw_wdt.clk_rate) {
> + ? ? ? ? ? ? ? dev_err(&pdev->dev, "no clk rate defined for watchdog, cannot enable\n");
> + ? ? ? ? ? ? ? ret = -EINVAL;
> + ? ? ? ? ? ? ? goto out_disable_clk;
> + ? ? ? }
> +
> + ? ? ? spin_lock_init(&dw_wdt.lock);
> +
> + ? ? ? ret = misc_register(&dw_wdt_miscdev);
> + ? ? ? if (ret)
> + ? ? ? ? ? ? ? goto out_put_clk;
shouldn't it be out_disable_clk??
> +
> + ? ? ? dw_wdt_set_next_heartbeat();
> + ? ? ? setup_timer(&dw_wdt.timer, dw_wdt_ping, 0);
> + ? ? ? mod_timer(&dw_wdt.timer, jiffies + WDT_TIMEOUT);
> +
> + ? ? ? return 0;
> +
> +out_disable_clk:
> + ? ? ? clk_disable(dw_wdt.clk);
> +out_put_clk:
> + ? ? ? clk_put(dw_wdt.clk);
> +
> + ? ? ? return ret;
> +}
> +
> +static int __devexit dw_wdt_drv_remove(struct platform_device *pdev)
> +{
> + ? ? ? misc_deregister(&dw_wdt_miscdev);
> +
> + ? ? ? clk_disable(dw_wdt.clk);
> + ? ? ? clk_put(dw_wdt.clk);
> +
> + ? ? ? return 0;
> +}
> +
> +static struct platform_driver dw_wdt_driver = {
> + ? ? ? .probe ? ? ? ? ?= dw_wdt_drv_probe,
> + ? ? ? .remove ? ? ? ? = __devexit_p(dw_wdt_drv_remove),
> + ? ? ? .driver ? ? ? ? = {
> + ? ? ? ? ? ? ? .name ? = "dw_wdt",
> + ? ? ? ? ? ? ? .owner ?= THIS_MODULE,
> +#ifdef CONFIG_PM
> + ? ? ? ? ? ? ? .pm ? ? = &dw_wdt_pm_ops,
> +#endif /* CONFIG_PM */
> + ? ? ? },
> +};
> +
> +static int __init dw_wdt_watchdog_init(void)
> +{
> + ? ? ? return platform_driver_register(&dw_wdt_driver);
> +}
> +module_init(dw_wdt_watchdog_init);
> +
> +static void __exit dw_wdt_watchdog_exit(void)
> +{
> + ? ? ? platform_driver_unregister(&dw_wdt_driver);
> +}
> +module_exit(dw_wdt_watchdog_exit);
> +
> +MODULE_AUTHOR("Jamie Iles");
> +MODULE_DESCRIPTION("Synopsys DesignWare Watchdog Driver");
> +MODULE_LICENSE("GPL");
> +MODULE_ALIAS_MISCDEV(WATCHDOG_MINOR);
> diff --git a/include/linux/platform_data/dw_wdt.h b/include/linux/platform_data/dw_wdt.h
> new file mode 100644
> index 0000000..0af10ef
> --- /dev/null
> +++ b/include/linux/platform_data/dw_wdt.h
It looks you have created a separate generic folder platform_data.
But generally we put these files directly into existing folders.
I am not quite sure if that's fine or we should avoid this.
^ permalink raw reply
* [PATCH 2/2] ARM i.MXS: Add auart platform support for i.MX28
From: Shawn Guo @ 2011-01-12 5:06 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20110111151725.GT24920@pengutronix.de>
On Tue, Jan 11, 2011 at 04:17:25PM +0100, Uwe Kleine-K?nig wrote:
> Hello Sascha,
>
> On Tue, Jan 11, 2011 at 04:09:05PM +0100, Sascha Hauer wrote:
> > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > ---
> > arch/arm/mach-mxs/clock-mx28.c | 5 ++
> > arch/arm/mach-mxs/devices-mx28.h | 8 +++
> > arch/arm/mach-mxs/devices/Kconfig | 3 +
> > arch/arm/mach-mxs/devices/Makefile | 1 +
> > arch/arm/mach-mxs/devices/platform-auart.c | 54 +++++++++++++++++++++++
> > arch/arm/mach-mxs/include/mach/devices-common.h | 10 ++++
> > 6 files changed, 81 insertions(+), 0 deletions(-)
> > create mode 100644 arch/arm/mach-mxs/devices/platform-auart.c
> >
> > diff --git a/arch/arm/mach-mxs/clock-mx28.c b/arch/arm/mach-mxs/clock-mx28.c
> > index 74e2103..9f65934 100644
> > --- a/arch/arm/mach-mxs/clock-mx28.c
> > +++ b/arch/arm/mach-mxs/clock-mx28.c
> > @@ -603,6 +603,11 @@ _DEFINE_CLOCK(fec_clk, ENET, DISABLE, &hbus_clk);
> >
> > static struct clk_lookup lookups[] = {
> > _REGISTER_CLOCK("mxs-duart.0", NULL, uart_clk)
> > + _REGISTER_CLOCK("mxs-auart.0", NULL, uart_clk)
> > + _REGISTER_CLOCK("mxs-auart.1", NULL, uart_clk)
> > + _REGISTER_CLOCK("mxs-auart.2", NULL, uart_clk)
> > + _REGISTER_CLOCK("mxs-auart.3", NULL, uart_clk)
> > + _REGISTER_CLOCK("mxs-auart.4", NULL, uart_clk)
> > _REGISTER_CLOCK("fec.0", NULL, fec_clk)
> > _REGISTER_CLOCK("rtc", NULL, rtc_clk)
> > _REGISTER_CLOCK("pll2", NULL, pll2_clk)
> > diff --git a/arch/arm/mach-mxs/devices-mx28.h b/arch/arm/mach-mxs/devices-mx28.h
> > index 00b736c..a0b70df 100644
> > --- a/arch/arm/mach-mxs/devices-mx28.h
> > +++ b/arch/arm/mach-mxs/devices-mx28.h
> > @@ -15,6 +15,14 @@ extern const struct mxs_duart_data mx28_duart_data __initconst;
> > #define mx28_add_duart() \
> > mxs_add_duart(&mx28_duart_data)
> >
> > +extern const struct mxs_auart_data mx28_auart_data[] __initconst;
> > +#define mx28_add_auart(id) mxs_add_auart(&mx28_auart_data[id])
> > +#define mx28_add_auart0() mx28_add_auart(0)
> > +#define mx28_add_auart1() mx28_add_auart(1)
> > +#define mx28_add_auart2() mx28_add_auart(2)
> > +#define mx28_add_auart3() mx28_add_auart(3)
> > +#define mx28_add_auart4() mx28_add_auart(4)
> I wouldn't add the mx28_add_auartX macros, only mx28_add_auart. The
> former are a relict of the time when we didn't use an array to get the
> data from.
>
> > +
> > extern const struct mxs_fec_data mx28_fec_data[] __initconst;
> > #define mx28_add_fec(id, pdata) \
> > mxs_add_fec(&mx28_fec_data[id], pdata)
> > diff --git a/arch/arm/mach-mxs/devices/Kconfig b/arch/arm/mach-mxs/devices/Kconfig
> > index a35a2dc..9d16540 100644
> > --- a/arch/arm/mach-mxs/devices/Kconfig
> > +++ b/arch/arm/mach-mxs/devices/Kconfig
> > @@ -1,5 +1,8 @@
> > config MXS_HAVE_PLATFORM_DUART
> > bool
> >
> > +config MXS_HAVE_PLATFORM_AUART
> > + bool
> > +
> This doesn't happen to be an amba device, too?
>
> > config MXS_HAVE_PLATFORM_FEC
> > bool
> > diff --git a/arch/arm/mach-mxs/devices/Makefile b/arch/arm/mach-mxs/devices/Makefile
> > index 4b5266a..b46d18d 100644
> > --- a/arch/arm/mach-mxs/devices/Makefile
> > +++ b/arch/arm/mach-mxs/devices/Makefile
> > @@ -1,2 +1,3 @@
> > obj-$(CONFIG_MXS_HAVE_PLATFORM_DUART) += platform-duart.o
> > +obj-$(CONFIG_MXS_HAVE_PLATFORM_AUART) += platform-auart.o
> > obj-$(CONFIG_MXS_HAVE_PLATFORM_FEC) += platform-fec.o
> this conflicts with the "make duart use amba-pl011 driver" patch.
May I know the reason that the patch is still pending for being
merged? DEBUG_SPINLOCK_SLEEP warning?
[...]
--
Regards,
Shawn
^ permalink raw reply
* [PATCH V2] ARM: SAMSUNG: Add support for clock debugging through debug-fs interface
From: Kukjin Kim @ 2011-01-12 4:56 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <AANLkTinXk5Pefp=1emCRNrtZzsgYDk9LhC3AtOAem0y_@mail.gmail.com>
Amit Daniel Kachhap wrote:
>
> Hi Mr Kim,
>
> Thanks for your comments. Please see the inline comments below.
>
Hi Amit,
I didn't get your updated patch, but it was small things.
So I applied this in my tree for this merge window.
Thanks.
Best regards,
Kgene.
--
Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.
> On 1 December 2010 13:08, Kukjin Kim <kgene.kim@samsung.com> wrote:
> >
> > Amit Daniel Kachhap wrote:
> > >
> > > This patch adds support for clock information exposed to debug-fs
> > interface.
> > >
> > > Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
> > > ---
> > > Code modified for V2 version are,
> > > a)Inserted the debug-fs code inside macro CONFIG_PM_DEBUG as suggested
by
> > > yong.shen at linaro.org.
> >
> > Could you please let me know why?
> Actually this patch is mostly taken from omap architecture, so using
> this macro in addition
> to CONFIG_DEBUG_FS macro. Also it is better to enable all PM debugging
> features with
> one configuration parameter.
>
> >
> > > b)Removed macro CONFIG_PLAT_SAMSUNG and implemented other coding
> standards
> > > as suggested by kgene.kim at samsung.com
> > >
> > > ?arch/arm/plat-samsung/clock.c ? ? ? ? ? ? ?| ? 91
> > > ++++++++++++++++++++++++++++
> > > ?arch/arm/plat-samsung/include/plat/clock.h | ? ?3 +
> > > ?2 files changed, 94 insertions(+), 0 deletions(-)
> > >
> > > diff --git a/arch/arm/plat-samsung/clock.c b/arch/arm/plat-
> samsung/clock.c
> > > index e8d20b0..029a49d 100644
> > > --- a/arch/arm/plat-samsung/clock.c
> > > +++ b/arch/arm/plat-samsung/clock.c
> > > @@ -39,6 +39,9 @@
> > > ?#include <linux/clk.h>
> > > ?#include <linux/spinlock.h>
> > > ?#include <linux/io.h>
> > > +#if defined(CONFIG_DEBUG_FS)
> > > +#include <linux/debugfs.h>
> > > +#endif
> > >
> > > ?#include <mach/hardware.h>
> > > ?#include <asm/irq.h>
> > > @@ -447,3 +450,91 @@ int __init s3c24xx_register_baseclocks(unsigned
long
> > > xtal)
> > > ? ? ? return 0;
> > > ?}
> > >
> > > +#if defined(CONFIG_PM_DEBUG) && defined(CONFIG_DEBUG_FS)
> >
> > Hmm...please let me know the reason...
> same reason as above.
> >
> > > +/* debugfs support to trace clock tree hierarchy and attributes */
> > > +
> > > +static struct dentry *clk_debugfs_root;
> > > +
> > > +static int clk_debugfs_register_one(struct clk *c)
> > > +{
> > > + ? ? int err;
> > > + ? ? struct dentry *d, *child, *child_tmp;
> > > + ? ? struct clk *pa = c->parent;
> > > + ? ? char s[255];
> > > + ? ? char *p = s;
> > > +
> > > + ? ? p += sprintf(p, "%s", c->name);
> > > + ? ? /*Append id field with name also*/
> >
> > No need above comment. If required, please add other comments also.
> ok, will remove this comment in next patch.
> >
> > > + ? ? if (c->id >= 0)
> > > + ? ? ? ? ? ? sprintf(p, ":%d", c->id);
> > > +
> > > + ? ? d = debugfs_create_dir(s, pa ? pa->dent : clk_debugfs_root);
> > > + ? ? if (!d)
> > > + ? ? ? ? ? ? return -ENOMEM;
> > > +
> > > + ? ? c->dent = d;
> > > +
> > > + ? ? d = debugfs_create_u8("usecount", S_IRUGO, c->dent, (u8
> > *)&c->usage);
> > > + ? ? if (!d) {
> > > + ? ? ? ? ? ? err = -ENOMEM;
> > > + ? ? ? ? ? ? goto err_out;
> > > + ? ? }
> >
> > 1 empty line would be helpful to read.
> ok, will update this in next patch.
> >
> > > + ? ? d = debugfs_create_u32("rate", S_IRUGO, c->dent, (u32
*)&c->rate);
> > > + ? ? if (!d) {
> > > + ? ? ? ? ? ? err = -ENOMEM;
> > > + ? ? ? ? ? ? goto err_out;
> > > + ? ? }
> > > + ? ? return 0;
> > > +
> > > +err_out:
> > > + ? ? d = c->dent;
> > > + ? ? list_for_each_entry_safe(child, child_tmp, &d->d_subdirs,
> > d_u.d_child)
> > > + ? ? ? ? ? ? debugfs_remove(child);
> > > + ? ? debugfs_remove(c->dent);
> > > + ? ? return err;
> > > +}
> > > +
> > > +static int clk_debugfs_register(struct clk *c)
> > > +{
> > > + ? ? int err;
> > > + ? ? struct clk *pa = c->parent;
> > > +
> > > + ? ? if (pa && !pa->dent) {
> > > + ? ? ? ? ? ? err = clk_debugfs_register(pa);
> > > + ? ? ? ? ? ? if (err)
> > > + ? ? ? ? ? ? ? ? ? ? return err;
> > > + ? ? }
> > > +
> > > + ? ? if (!c->dent) {
> > > + ? ? ? ? ? ? err = clk_debugfs_register_one(c);
> > > + ? ? ? ? ? ? if (err)
> > > + ? ? ? ? ? ? ? ? ? ? return err;
> > > + ? ? }
> > > + ? ? return 0;
> > > +}
> > > +
> > > +static int __init clk_debugfs_init(void)
> > > +{
> > > + ? ? struct clk *c;
> > > + ? ? struct dentry *d;
> > > + ? ? int err;
> > > +
> > > + ? ? d = debugfs_create_dir("clock", NULL);
> > > + ? ? if (!d)
> > > + ? ? ? ? ? ? return -ENOMEM;
> > > + ? ? clk_debugfs_root = d;
> > > +
> > > + ? ? list_for_each_entry(c, &clocks, list) {
> > > + ? ? ? ? ? ? err = clk_debugfs_register(c);
> > > + ? ? ? ? ? ? if (err)
> > > + ? ? ? ? ? ? ? ? ? ? goto err_out;
> > > + ? ? }
> > > + ? ? return 0;
> > > +err_out:
> > > + ? ? debugfs_remove_recursive(clk_debugfs_root);
> > > + ? ? return err;
> > > +}
> > > +late_initcall(clk_debugfs_init);
> > > +
> > > +#endif /* defined(CONFIG_PM_DEBUG) && defined(CONFIG_DEBUG_FS) */
> > > +
> >
> > No need last one empty line.
> ok, will update this comment in next patch.
> >
> > > diff --git a/arch/arm/plat-samsung/include/plat/clock.h
b/arch/arm/plat-
> > > samsung/include/plat/clock.h
> > > index 0fbcd0e..9a82b88 100644
> > > --- a/arch/arm/plat-samsung/include/plat/clock.h
> > > +++ b/arch/arm/plat-samsung/include/plat/clock.h
> > > @@ -47,6 +47,9 @@ struct clk {
> > >
> > > ? ? ? struct clk_ops ? ? ? ? ?*ops;
> > > ? ? ? int ? ? ? ? ? ? ? ? (*enable)(struct clk *, int enable);
> > > +#if defined(CONFIG_PM_DEBUG) && defined(CONFIG_DEBUG_FS)
> > > + ? ? struct dentry ? ? ? ? ? *dent; ?/* For visible tree hierarchy */
> > > +#endif
> > > ?};
> > >
> > > ?/* other clocks which may be registered by board support */
> > > --
^ permalink raw reply
* Bug: Brownstone breaks in linux-next
From: Mark F. Brown @ 2011-01-12 4:40 UTC (permalink / raw)
To: linux-arm-kernel
Hi Haojian,
Brownstone (ARMv7 mode) does not boot on the latest iteration of
linux-next. It is broken from next-20110107 to next-20110112. I did
not get a chance to investigate further.
^ permalink raw reply
* Locking in the clk API
From: Saravana Kannan @ 2011-01-12 3:25 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <201101112235.43061.jeremy.kerr@canonical.com>
On 01/11/2011 06:35 AM, Jeremy Kerr wrote:
> Hi Paul,
>
>> Again, you are approaching it from the angle that an atomic clock is a
>> special requirement rather than the default behaviour.
>
> I'm not considering it a special requirement, but it's still a requirement
> (that the called function does not sleep).
>
> The problem with the inverse logic (clk_enable/clk_enable_sleepable) is that
> now you've made the caller need to know what kind of clock it has, or might
> have one day.
I think it's just a matter of how you interpret the name of the API in
English. It doesn't make the decision making of the developer any easier.
Just having a _atomic suffix doesn't mean the driver developer doesn't
need to know what type of clock it is. They are still making the
assumption that the enable/disable for that clock can be done atomically
-- namely an "atomic clock".
Similarly, when a driver developer calls the _sleepable APIs in their
code, for all practical purposes, they are making an assumption that the
enable/disable for that clock *needs to* (not may) sleep.
> * For clk_enable/clk_enable_atomic, the decision is: is this call in an
> atomic context?
>
> * For clk_enable/clk_enable_sleepable, the decision is: might the clock code
> have given us a sleeping clock?
Having said the above, I'm slightly leaning towards
clk_enable/disable_atomic since it lines up with the
.suspend/.suspend_noirq functions in pm_ops.
Also, since it's good to reduce the amount of work that needs to be done
atomically, I think it would be good to make a developer explicitly
state they need _atomic functions and make them think about if they
really need to do that.
-Saravana
--
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
^ permalink raw reply
* ARM: relocation out of range (when loading a module)
From: Alexander Holler @ 2011-01-12 3:00 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20110111155930.GH11039@n2100.arm.linux.org.uk>
Hello,
Am 11.01.2011 16:59, schrieb Russell King - ARM Linux:
> On Tue, Jan 11, 2011 at 09:16:38PM +0530, Rabin Vincent wrote:
>> It's possible to hack around this by placing the initramfs at the end of
>> the kernel image rather than at the beginning with the rest of the init
>> data. Something like the below should work, although you should also
>> probably take care of alignment and also have this section freed when
>> the rest of the init data is freed.
>
> You're then running into problems as _sdata.._edata is copied to RAM on
> XIP kernels, and you really don't want to waste time copying the
> initramfs to RAM.
Thanks to all for the provided informations. I'm now seeing some light. ;)
However, looking at the vmlinux.lds.S I think I should better not try to
fix that, I would likely do something wrong because of missing knowledge
about all those sections.
I can offer a patch wich adds a TODO to vmlinux.lds.S, but if someone
else feels the need to fix that, feel free to do so. ;)
Regards,
Alexander
-------------------------------------------------------------------------
From 2ce2934b6a55ac34734781d1a49569d79d0fdcda Mon Sep 17 00:00:00 2001
From: Alexander Holler <holler@ahsoftware.de>
Date: Wed, 12 Jan 2011 02:49:52 +0100
Subject: [PATCH] ARM: Add TODO to move INIT_RAM_FS to another point.
Signed-off-by: Alexander Holler <holler@ahsoftware.de>
---
arch/arm/kernel/vmlinux.lds.S | 6 ++++++
1 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/arch/arm/kernel/vmlinux.lds.S b/arch/arm/kernel/vmlinux.lds.S
index cead889..5da3479 100644
--- a/arch/arm/kernel/vmlinux.lds.S
+++ b/arch/arm/kernel/vmlinux.lds.S
@@ -62,6 +62,12 @@ SECTIONS
INIT_CALLS
CON_INITCALL
SECURITY_INITCALL
+ /*
+ * TODO: The size of INIT_RAM_FS could easily reach a
+ * point (~16MB) when loading modules will fail because
+ * relocations will be out of range. So this place here
+ * isn't the best one.
+ */
INIT_RAM_FS
#ifndef CONFIG_XIP_KERNEL
--
1.7.3.4
-------------------------------------------------------------------------
^ permalink raw reply related
* Locking in the clk API
From: Saravana Kannan @ 2011-01-12 2:56 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20110111091607.GI12552@n2100.arm.linux.org.uk>
On 01/11/2011 01:16 AM, Russell King - ARM Linux wrote:
> On Tue, Jan 11, 2011 at 10:16:42AM +0800, Jeremy Kerr wrote:
>> At present, we can satisfy these with:
>>
>> * clk_enable: may sleep
>
> I object to this as one of the purposes behind the clk API is to allow
> power savings to be made, and unless we can perform clk enable/disable
> from atomic contexts, the best you can do is enable the clock when the
> device is probed and disable it when it's released.
Since dev_pm_ops.suspend is not atomic anymore (am I wrong?), what is
the atomic context that you are having in mind that's related to power
savings? How often do we really need to call clk enable/disable in that
atomic context?
If the system VDD needed to be increased when a clock/core in enabled
(in reality, when a core is enabled), how do we make sure that the
voltage is reduced when the clock/core is turned off? Do we simply punt
the voltage change problem to the driver and say "not our problem"?
I'm not saying that the voltage change should or shouldn't be handled by
the clock driver. But it looks like the same methods used to handle the
voltage change problem could be applied to how we could handle the clk
enable/disable problem.
-Saravana
--
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
^ permalink raw reply
* [PATCH 2/2] ARM i.MX53 enable LOCO board bootup
From: Yong Shen @ 2011-01-12 2:46 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <AANLkTimW54m2f+Z21F+9dLw78WOS24W4yc00D5rztM0C@mail.gmail.com>
Hi Fabio,
Below is my reply to Baruch, I meant to keep this code here to show
the reset process clearly.
>> + if (ret) {
>> + printk(KERN_ERR"failed to get GPIO_FEC_PHY_RESET: %d\n", ret);
>> + return;
>> + }
>> + gpio_direction_output(SMD_FEC_PHY_RST, 0);
>> + gpio_set_value(SMD_FEC_PHY_RST, 0);
>
> This seems to be redundant. gpio_direction_output() has already set the value
> to 0.
You are right. But, gpio_set_value is meant to be here to show the
process of fec reset: first pull low and then pull high. And
gpio_direction_output here is for direction configuration although it
has the ability of configure output value.
thanks
Yong
^ permalink raw reply
* Locking in the clk API
From: Paul Mundt @ 2011-01-12 2:25 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <4D2D09E2.5030305@codeaurora.org>
On Tue, Jan 11, 2011 at 05:54:42PM -0800, Saravana Kannan wrote:
> On 01/11/2011 04:18 AM, Paul Mundt wrote:
> >Again, you are approaching it from the angle that an atomic clock is a
> >special requirement rather than the default behaviour. Sleeping for
> >lookup, addition, and deletion are all quite acceptable, but
> >enable/disable pairs have always been intended to be usable from atomic
> >context. Anyone that doesn't count on that fact is either dealing with
> >special case clocks (PLLs, root clocks, etc.) or simply hasn't bothered
> >implementing any sort of fine grained runtime power management for their
> >platform.
>
> Paul,
>
> I see you repeating this point a couple of times and I'm a bit confused
> how you handle the clock tree/dependencies.
>
> Does your clock driver NOT hide the details of what root clock/PLL a
> branch clock is sourced from? If you do hide the details of the root/PLL
> source, how do you get the branch clk_enable() to be done atomically if
> the root/PLL enables are not possible in atomic context?
>
> Is it simply a matter of your hardware having PLLs and root clocks that
> can be turned on/off quickly?
>
There are a few cases where PLL clocks would benefit from a clk_enable()
that can sleep, but for us these are almost all in the device space. Most
of the SoCs however have fairly straightforward clock topologies where
the root clock in question is an external oscillator that can't be
disabled, and anything chained below that sits behind a PLL divider or
multiplier bank that can likewise be adjusted atomically. The vast
majority of the clocks below that can likewise be trivially
enabled/disabled from atomic context.
In response to your query, no, we do not hide those details. Each on-chip
clock generator is fed by a default oscillator via one input or a board
specific one via another, with a frequency that can vary wildly depending
on the implementation. In general we require the SoC code to register its
clock topology, the board code to work out which source its fed from (and
at what frequency), and then also providing an initial rate propagation
kick down the tree to make sure that all of the core clocks are set up
sensibly and to synchronize hardware/software state.
Having said that, we _do_ have clocks where rate changes are non-trivial
due to it being necessary to synchronize with internal timing circuitry,
but at the moment we don't really deal with those (this is something we'd
have to deal with for dynamically scaling the bus clock for example,
which requires a special watchdog overflow to relock an internal PLL,
which in general we leave fixed).
In general I would be happy to have a sleepable variant of clk_enable(),
it's just making clk_enable() sleepable by default or treating clocks
usable from atomic context as the special case that I take issue with,
particularly when the opposite is the demonstratable norm.
^ permalink raw reply
* [PATCH 08/16] ARM: samsung: serial: don't treat NULL clk as an error
From: Kukjin Kim @ 2011-01-12 2:21 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1294749833-32019-9-git-send-email-jamie@jamieiles.com>
Jamie Iles wrote:
>
> clk_get() returns a struct clk cookie to the driver and some platforms
> may return NULL if they only support a single clock. clk_get() has only
> failed if it returns a ERR_PTR() encoded pointer.
>
> Cc: Kukjin Kim <kgene.kim@samsung.com>
Acked-by: Kukjin Kim <kgene.kim@samsung.com>
Thanks.
Best regards,
Kgene.
--
Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.
> Cc: linux-arm-kernel at lists.infradead.org
> Signed-off-by: Jamie Iles <jamie@jamieiles.com>
> ---
> drivers/serial/samsung.c | 3 ++-
> 1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/serial/samsung.c b/drivers/serial/samsung.c
> index 7ac2bf5..be93d2f 100644
> --- a/drivers/serial/samsung.c
> +++ b/drivers/serial/samsung.c
> @@ -42,6 +42,7 @@
> #include <linux/serial.h>
> #include <linux/delay.h>
> #include <linux/clk.h>
> +#include <linux/err.h>
> #include <linux/cpufreq.h>
>
> #include <asm/irq.h>
> @@ -522,7 +523,7 @@ static int s3c24xx_serial_calcbaud(struct baud_calc
*calc,
> unsigned long rate;
>
> calc->src = clk_get(port->dev, clksrc->name);
> - if (calc->src == NULL || IS_ERR(calc->src))
> + if (IS_ERR(calc->src))
> return 0;
>
> rate = clk_get_rate(calc->src);
> --
1.7.3.4
^ permalink raw reply
* Locking in the clk API
From: Saravana Kannan @ 2011-01-12 1:54 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20110111121816.GB774@linux-sh.org>
On 01/11/2011 04:18 AM, Paul Mundt wrote:
> Again, you are approaching it from the angle that an atomic clock is a
> special requirement rather than the default behaviour. Sleeping for
> lookup, addition, and deletion are all quite acceptable, but
> enable/disable pairs have always been intended to be usable from atomic
> context. Anyone that doesn't count on that fact is either dealing with
> special case clocks (PLLs, root clocks, etc.) or simply hasn't bothered
> implementing any sort of fine grained runtime power management for their
> platform.
Paul,
I see you repeating this point a couple of times and I'm a bit confused
how you handle the clock tree/dependencies.
Does your clock driver NOT hide the details of what root clock/PLL a
branch clock is sourced from? If you do hide the details of the root/PLL
source, how do you get the branch clk_enable() to be done atomically if
the root/PLL enables are not possible in atomic context?
Is it simply a matter of your hardware having PLLs and root clocks that
can be turned on/off quickly?
-Saravana
--
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
^ permalink raw reply
* [PATCH 06/16] nuc900fb: don't treat NULL clk as an error
From: Wan ZongShun @ 2011-01-12 1:24 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1294749833-32019-7-git-send-email-jamie@jamieiles.com>
2011/1/11 Jamie Iles <jamie@jamieiles.com>:
> clk_get() returns a struct clk cookie to the driver and some platforms
> may return NULL if they only support a single clock. ?clk_get() has only
> failed if it returns a ERR_PTR() encoded pointer.
>
> Cc: Wan ZongShun <mcuos.com@gmail.com>
> Cc: linux-fbdev at vger.kernel.org
> Signed-off-by: Jamie Iles <jamie@jamieiles.com>
> ---
> ?drivers/video/nuc900fb.c | ? ?5 +++--
> ?1 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/video/nuc900fb.c b/drivers/video/nuc900fb.c
> index 81687ed..62498bd 100644
> --- a/drivers/video/nuc900fb.c
> +++ b/drivers/video/nuc900fb.c
> @@ -15,6 +15,7 @@
> ?*/
> ?#include <linux/module.h>
> ?#include <linux/kernel.h>
> +#include <linux/err.h>
> ?#include <linux/errno.h>
> ?#include <linux/string.h>
> ?#include <linux/mm.h>
> @@ -597,9 +598,9 @@ static int __devinit nuc900fb_probe(struct platform_device *pdev)
> ? ? ? ?}
>
> ? ? ? ?fbi->clk = clk_get(&pdev->dev, NULL);
> - ? ? ? if (!fbi->clk || IS_ERR(fbi->clk)) {
> + ? ? ? if (IS_ERR(fbi->clk)) {
> ? ? ? ? ? ? ? ?printk(KERN_ERR "nuc900-lcd:failed to get lcd clock source\n");
> - ? ? ? ? ? ? ? ret = -ENOENT;
> + ? ? ? ? ? ? ? ret = PTR_ERR(fbi->clk);
> ? ? ? ? ? ? ? ?goto release_irq;
> ? ? ? ?}
>
Hi,
Thanks for your patch.
Acked-by: Wan ZongShun <mcuos.com@gmail.com>
> --
> 1.7.3.4
>
>
--
*linux-arm-kernel mailing list
mail addr:linux-arm-kernel at lists.infradead.org
you can subscribe by:
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
* linux-arm-NUC900 mailing list
mail addr:NUC900 at googlegroups.com
main web: https://groups.google.com/group/NUC900
you can subscribe it by sending me mail:
mcuos.com at gmail.com
^ permalink raw reply
* [PATCH v6.1 3/3] omap3: beaglexm: fix power on of DVI
From: Tony Lindgren @ 2011-01-12 1:04 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1294791809-28069-1-git-send-email-nm@ti.com>
* Nishanth Menon <nm@ti.com> [110111 16:22]:
> From: Koen Kooi <koen@beagleboard.org>
>
> TFP410 DVI chip is used to provide display out.
> This chip is controlled by 2 lines:
> LDO which supplies the power is controlled over gpio + 2
> and the enable of the chip itself is done over gpio + 1
> NOTE: the LDO is necessary for LED, serial blocks as well.
>
> gpio + 1 was used to sense USB overcurrent in vanilla beagle.
>
> Without this fix, the display would not function as the LDO
> remains shut down.
Thanks, applying.
Tony
^ permalink raw reply
* [PATCH v6.1 3/3] omap3: beaglexm: fix power on of DVI
From: Nishanth Menon @ 2011-01-12 0:23 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <[PATCH v5 0/3] OMAP3: beaglexm: GPIO fixes>
From: Koen Kooi <koen@beagleboard.org>
TFP410 DVI chip is used to provide display out.
This chip is controlled by 2 lines:
LDO which supplies the power is controlled over gpio + 2
and the enable of the chip itself is done over gpio + 1
NOTE: the LDO is necessary for LED, serial blocks as well.
gpio + 1 was used to sense USB overcurrent in vanilla beagle.
Without this fix, the display would not function as the LDO
remains shut down.
[nm at ti.com: split up, added descriptive changelogs]
Signed-off-by: Nishanth Menon <nm@ti.com>
Signed-off-by: Koen Kooi <koen@beagleboard.org>
---
Posting the last of the patches of this series
v6.1: formal submission:
added gpio error checks
v5: http://marc.info/?t=129476616900006&r=1&w=2
arch/arm/mach-omap2/board-omap3beagle.c | 42 ++++++++++++++++++++++++++++--
1 files changed, 39 insertions(+), 3 deletions(-)
diff --git a/arch/arm/mach-omap2/board-omap3beagle.c b/arch/arm/mach-omap2/board-omap3beagle.c
index 673deb9..2ed8040 100644
--- a/arch/arm/mach-omap2/board-omap3beagle.c
+++ b/arch/arm/mach-omap2/board-omap3beagle.c
@@ -273,6 +273,8 @@ static struct gpio_led gpio_leds[];
static int beagle_twl_gpio_setup(struct device *dev,
unsigned gpio, unsigned ngpio)
{
+ int r;
+
if (omap3_beagle_get_rev() == OMAP3BEAGLE_BOARD_XM) {
mmc[0].gpio_wp = -EINVAL;
} else if ((omap3_beagle_get_rev() == OMAP3BEAGLE_BOARD_C1_3) ||
@@ -293,9 +295,16 @@ static int beagle_twl_gpio_setup(struct device *dev,
/* REVISIT: need ehci-omap hooks for external VBUS
* power switch and overcurrent detect
*/
-
- gpio_request(gpio + 1, "EHCI_nOC");
- gpio_direction_input(gpio + 1);
+ if (omap3_beagle_get_rev() != OMAP3BEAGLE_BOARD_XM) {
+ r = gpio_request(gpio + 1, "EHCI_nOC");
+ if (!r) {
+ r = gpio_direction_input(gpio + 1);
+ if (r)
+ gpio_free(gpio + 1);
+ }
+ if (r)
+ pr_err("%s: unable to configure EHCI_nOC\n", __func__);
+ }
/*
* TWL4030_GPIO_MAX + 0 == ledA, EHCI nEN_USB_PWR (out, XM active
@@ -316,6 +325,33 @@ static int beagle_twl_gpio_setup(struct device *dev,
/* TWL4030_GPIO_MAX + 1 == ledB, PMU_STAT (out, active low LED) */
gpio_leds[2].gpio = gpio + TWL4030_GPIO_MAX + 1;
+ /*
+ * gpio + 1 on Xm controls the TFP410's enable line (active low)
+ * gpio + 2 control varies depending on the board rev as follows:
+ * P7/P8 revisions(prototype): Camera EN
+ * A2+ revisions (production): LDO (supplies DVI, serial, led blocks)
+ */
+ if (omap3_beagle_get_rev() == OMAP3BEAGLE_BOARD_XM) {
+ r = gpio_request(gpio + 1, "nDVI_PWR_EN");
+ if (!r) {
+ r = gpio_direction_output(gpio + 1, 0);
+ if (r)
+ gpio_free(gpio + 1);
+ }
+ if (r)
+ pr_err("%s: unable to configure nDVI_PWR_EN\n",
+ __func__);
+ r = gpio_request(gpio + 2, "DVI_LDO_EN");
+ if (!r) {
+ r = gpio_direction_output(gpio + 2, 1);
+ if (r)
+ gpio_free(gpio + 2);
+ }
+ if (r)
+ pr_err("%s: unable to configure DVI_LDO_EN\n",
+ __func__);
+ }
+
return 0;
}
--
1.6.3.3
^ permalink raw reply related
* [PATCH v5 3/3] omap3: beaglexm: fix power on of DVI
From: Nishanth Menon @ 2011-01-12 0:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20110112001509.GA4957@atomide.com>
Tony Lindgren had written, on 01/11/2011 06:15 PM, the following:
> * Nishanth Menon <nm@ti.com> [110111 15:54]:
>> Tony Lindgren had written, on 01/11/2011 05:23 PM, the following:
>> [..]
>>>> -
>>>> - gpio_request(gpio + 1, "EHCI_nOC");
>>>> - gpio_direction_input(gpio + 1);
>>>> + if (omap3_beagle_get_rev() != OMAP3BEAGLE_BOARD_XM) {
>>>> + gpio_request(gpio + 1, "EHCI_nOC");
>>>> + gpio_direction_input(gpio + 1);
>>>> + }
>>> The return value for gpio_request must be checked.
>> Ack.
>> we can go down two paths:
>> a) I can redo this patch as in v6.patch (attached)
>
> Yes let's do that, one comment below though..
>
>> OR
>> b) we take this patch and do another one cleaning the function up -
>> gpio-check.patch
>
> That can be done later.
ok
>
>> + if (omap3_beagle_get_rev() == OMAP3BEAGLE_BOARD_XM) {
>> + r = gpio_request(gpio + 1, "nDVI_PWR_EN");
>> + if (!r) {
>> + r = gpio_direction_output(gpio + 1, 0);
>> + if (r)
>> + gpio_free(gpio + 1);
>> + }
>> + if (r)
>> + pr_err("%s: unable to configure nDVI_PWR_EN\n",
>> + __func__);
>> + r = gpio_request(gpio + 2, "DVI_LDO_EN");
>> + if (!r) {
>> + r = gpio_direction_output(gpio + 2, 1);
>> + if (r)
>> + gpio_free(gpio + 1);
>> + }
>> + if (r)
>> + pr_err("%s: unable to configure DVI_LDO_EN\n",
>> + __func__);
>> + }
>> +
>
> Should the second gpio_free be gpio + 2 instead of gpio + 1?
oops.. yep. will fix and send out a v6 for this alone. thanks for the check.
--
Regards,
Nishanth Menon
^ permalink raw reply
* [PATCH v5 3/3] omap3: beaglexm: fix power on of DVI
From: Tony Lindgren @ 2011-01-12 0:15 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <4D2CEDD7.4020507@ti.com>
* Nishanth Menon <nm@ti.com> [110111 15:54]:
> Tony Lindgren had written, on 01/11/2011 05:23 PM, the following:
> [..]
> >>-
> >>- gpio_request(gpio + 1, "EHCI_nOC");
> >>- gpio_direction_input(gpio + 1);
> >>+ if (omap3_beagle_get_rev() != OMAP3BEAGLE_BOARD_XM) {
> >>+ gpio_request(gpio + 1, "EHCI_nOC");
> >>+ gpio_direction_input(gpio + 1);
> >>+ }
> >
> >The return value for gpio_request must be checked.
> Ack.
> we can go down two paths:
> a) I can redo this patch as in v6.patch (attached)
Yes let's do that, one comment below though..
> OR
> b) we take this patch and do another one cleaning the function up -
> gpio-check.patch
That can be done later.
> + if (omap3_beagle_get_rev() == OMAP3BEAGLE_BOARD_XM) {
> + r = gpio_request(gpio + 1, "nDVI_PWR_EN");
> + if (!r) {
> + r = gpio_direction_output(gpio + 1, 0);
> + if (r)
> + gpio_free(gpio + 1);
> + }
> + if (r)
> + pr_err("%s: unable to configure nDVI_PWR_EN\n",
> + __func__);
> + r = gpio_request(gpio + 2, "DVI_LDO_EN");
> + if (!r) {
> + r = gpio_direction_output(gpio + 2, 1);
> + if (r)
> + gpio_free(gpio + 1);
> + }
> + if (r)
> + pr_err("%s: unable to configure DVI_LDO_EN\n",
> + __func__);
> + }
> +
Should the second gpio_free be gpio + 2 instead of gpio + 1?
Tony
^ 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