* [PATCH] ARM: kprobes: only patch instructions on one CPU
From: Tixy @ 2011-10-13 9:07 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1318492515.2265.51.camel@computer2>
On Thu, 2011-10-13 at 08:55 +0100, Tixy wrote:
> On Wed, 2011-10-12 at 21:42 +0530, Rabin Vincent wrote:
> > The text patching needs to be done only once, instead of once on
> each
> > CPU. The other CPUs will busy wait inside the stop machine code
> until
> > the patching is done.
>
> Where in the stop machine code do the other CPU's busy wait?
>
> How I read the code is that __stop_cpus() calls queue_stop_cpus_work()
> which queues the work on each of the specified CPU's work list and
> wakes
> that CPU's stopper thread to process it. __stop_cpus() then calls
> wait_for_completion() to wait for these CPUs to finish the work.
>
> I don't see how the execution path of CPUs not specified in cpumask is
> interrupted in any way.
>
As I said in my reply to the other mail, I had missed the fact that it
is stop_machine_cpu_stop() which is used to call our function, and this
synchronises all cores. Therefore, this patch is correct, assuming that
a flush_icache_range executed on one core also flushes I-caches on other
cores. (I'm a bit doubtfull of this as I beleive that at least
ARM11MPCore requires this to be managed in software and I can't find any
code that handles this.)
--
Tixy
>
> >
> > Cc: Jon Medhurst <tixy@yxit.co.uk>
> > Cc: Nicolas Pitre <nicolas.pitre@linaro.org>
> > Signed-off-by: Rabin Vincent <rabin@rab.in>
> > ---
> > arch/arm/kernel/kprobes.c | 4 ++--
> > 1 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/arm/kernel/kprobes.c b/arch/arm/kernel/kprobes.c
> > index 129c116..e9f95300 100644
> > --- a/arch/arm/kernel/kprobes.c
> > +++ b/arch/arm/kernel/kprobes.c
> > @@ -127,7 +127,7 @@ void __kprobes arch_arm_kprobe(struct kprobe *p)
> > flush_insns(addr, sizeof(u16));
> > } else if (addr & 2) {
> > /* A 32-bit instruction spanning two words needs
> special care */
> > - stop_machine(set_t32_breakpoint, (void *)addr,
> &cpu_online_map);
> > + stop_machine(set_t32_breakpoint, (void *)addr, NULL);
> > } else {
> > /* Word aligned 32-bit instruction can be written
> atomically */
> > u32 bkp = KPROBE_THUMB32_BREAKPOINT_INSTRUCTION;
> > @@ -190,7 +190,7 @@ int __kprobes __arch_disarm_kprobe(void *p)
> >
> > void __kprobes arch_disarm_kprobe(struct kprobe *p)
> > {
> > - stop_machine(__arch_disarm_kprobe, p, &cpu_online_map);
> > + stop_machine(__arch_disarm_kprobe, p, NULL);
> > }
> >
> > void __kprobes arch_remove_kprobe(struct kprobe *p)
>
>
>
^ permalink raw reply
* [PATCH 1/5 v4] ARM Realview PCIX map include file changes
From: Colin Tuckley @ 2011-10-13 9:09 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20111012172007.GA21648@n2100.arm.linux.org.uk>
> -----Original Message-----
> From: Russell King - ARM Linux [mailto:linux at arm.linux.org.uk]
> Subject: Re: [PATCH 1/5 v4] ARM Realview PCIX map include file changes
> On Mon, Oct 10, 2011 at 10:27:08AM +0100, Colin Tuckley wrote:
> > Ah, I wasn't aware of that. However when I include asm/io.h and
> remove the
>
> The general rule is that linux/io.h includes asm/io.h, which in turn
> includes mach/io.h includes and nothing should include either asm/io.h
> nor mach/io.h directly - if it does, things probably break - which is
> desirable behaviour because that indicates that the include is wrong.
Right sounds very sensible :)
> > A quick check shows that the piece of code you quote above does not
> exist in
> > my (3.1.0-rc9) source tree.
>
> It's not in -rc9, but will be during the next merge window - and is in
> linux-next.
Ah, understood. Do you have any other concerns about this patch? If not then
I'll submit it to your patch system with the IO_SPACE_LIMIT stuff fixed.
Colin
^ permalink raw reply
* [RFC 1/5] ARM: dev_archdata: add private iommu extension
From: Ohad Ben-Cohen @ 2011-10-13 9:15 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAK=Wgba6xxD2DBJhM--AcTPBoNRiOZ_6f8+j3bfaCqZMO8A4+w@mail.gmail.com>
On Wed, Oct 5, 2011 at 11:26 AM, Ohad Ben-Cohen <ohad@wizery.com> wrote:
> Hi Russell,
>
> On Sun, Sep 25, 2011 at 1:58 PM, Ohad Ben-Cohen <ohad@wizery.com> wrote:
>> Add a private iommu pointer to the ARM-specific arch data in the
>> device struct, which will be used to attach iommu-specific data
>> to devices which require iommu support.
>
> Can you please take a look and ack/nack ?
gentle reminder
^ permalink raw reply
* [PATCH 3/9] ARM: SPMP8000: Add clk support
From: Russell King - ARM Linux @ 2011-10-13 9:38 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1318178172-7965-4-git-send-email-zoss@devai.org>
On Sun, Oct 09, 2011 at 06:36:06PM +0200, Zoltan Devai wrote:
> Signed-off-by: Zoltan Devai <zoss@devai.org>
> ---
> arch/arm/mach-spmp8000/clkdev.c | 586 +++++++++++++++++++++++++++
> arch/arm/mach-spmp8000/clock.c | 155 +++++++
> arch/arm/mach-spmp8000/include/mach/clock.h | 37 ++
> 3 files changed, 778 insertions(+), 0 deletions(-)
> create mode 100644 arch/arm/mach-spmp8000/clkdev.c
> create mode 100644 arch/arm/mach-spmp8000/clock.c
> create mode 100644 arch/arm/mach-spmp8000/include/mach/clock.h
>
> diff --git a/arch/arm/mach-spmp8000/clkdev.c b/arch/arm/mach-spmp8000/clkdev.c
> new file mode 100644
> index 0000000..c492d20
> --- /dev/null
> +++ b/arch/arm/mach-spmp8000/clkdev.c
> @@ -0,0 +1,586 @@
> +/*
> + * SPMP8000 machines clock support
> + *
> + * Copyright (C) 2011 Zoltan Devai <zoss@devai.org>
> + *
> + * This file is licensed under the terms of the GNU General Public
> + * License version 2. This program is licensed "as is" without any
> + * warranty of any kind, whether express or implied.
> + */
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/io.h>
> +#include <linux/err.h>
> +#include <linux/spinlock.h>
> +#include <linux/clk.h>
> +#include <linux/clkdev.h>
> +#include <mach/clock.h>
> +#include <mach/scu.h>
I find the use of 'scu' for something which isn't the SMP Coherence Unit
rather confusing - and I suspect you will too if/when spmp8000 has SMP
support.
> +static int divider_set_clock(struct clk *clk, int on)
> +{
> + u32 divider;
> +
> + if (!(clk->flags & CLK_DIVIDER_WITH_ENABLE))
> + return -EINVAL;
> +
> + divider = readl(REG_SCU_BASE(clk->scu) + clk->dividerreg);
> + if (on)
> + divider |= DIVIDER_ENABLE_BIT;
> + else
> + divider = 0;
> + writel(divider, REG_SCU_BASE(clk->scu) + clk->dividerreg);
When you enable, you preserve the other bits in the register. When you
disable, you zero the whole register. That looks rather odd - either
you want to preserve the other bits or you don't.
> +
> + return 0;
> +}
> +
> +static void divider_enable_clock(struct clk *clk)
> +{
> + divider_set_clock(clk, 1);
> +}
> +
> +static void divider_disable_clock(struct clk *clk)
> +{
> + divider_set_clock(clk, 0);
> +}
afaics, this is the only place divider_set_clock() is called from - which
returns an int which is never checked. Is there any point to it
returning an int?
> +
> +static unsigned long divider_get_rate(struct clk *clk)
> +{
> + u32 divider;
> + unsigned long parent_rate = clk_get_rate(clk->parent);
> +
> + if (!parent_rate) {
> + clk->rate = 0;
> + return clk->rate;
> + }
> +
> + divider = readl(REG_SCU_BASE(clk->scu) + clk->dividerreg);
> + if ((clk->flags & CLK_DIVIDER_WITH_ENABLE) &&
> + !(divider & DIVIDER_ENABLE_BIT)) {
> + clk->rate = 0UL;
> + return clk->rate;
You really should return the rate which you're going to get when the
clock is enabled here, not zero because it happens to be disabled at
the current point in time.
> + }
> +
> + clk->rate = parent_rate / ((divider & clk->divmask) + 1);
> +
> + return clk->rate;
> +}
> +
> +static int divider_set_rate(struct clk *clk, unsigned long rate)
> +{
> + unsigned long parent_rate = clk_get_rate(clk->parent);
> + u32 divider, divider_old;
> +
> + if (unlikely(!parent_rate || rate > parent_rate)) {
> + clk->rate = 0;
> + pr_debug("parent rate not sufficient\n");
> + return -EINVAL;
> + }
You should set to the maximum (parent rate) in that case.
> +
> + divider = (parent_rate / rate) - 1;
> +
> + if (divider > clk->divmask) {
> + pr_debug("input clock too high\n");
> + return -EINVAL;
> + };
Set to the minimum rate here. The trailing ';' is not required.
> +
> + divider_old = readl(REG_SCU_BASE(clk->scu) + clk->dividerreg);
> + writel(0, REG_SCU_BASE(clk->scu) + clk->dividerreg);
> + writel(divider, REG_SCU_BASE(clk->scu) + clk->dividerreg);
> +
> + /* Re-enable clock if it was enabled before */
> + if (divider_old & DIVIDER_ENABLE_BIT)
> + writel(divider | DIVIDER_ENABLE_BIT,
> + REG_SCU_BASE(clk->scu) + clk->dividerreg);
> +
> + clk->rate = parent_rate / (divider + 1);
> +
> + return 0;
> +}
...
> +static void __clk_disable(struct clk *clk)
> +{
> + BUG_ON(clk->refcount == 0);
> +
> + if (!(--clk->refcount) && clk->disable) {
> + clk->disable(clk);
> + if (clk->parent)
> + __clk_disable(clk->parent);
> + }
> +}
What if the clock has a parent but no disable function?
> +
> +static int __clk_enable(struct clk *clk)
> +{
> + int ret = 0;
> +
> + if (clk->refcount++ == 0 && clk->enable) {
> + if (clk->parent)
> + ret = __clk_enable(clk->parent);
> + if (ret)
> + return ret;
> + else
> + clk->enable(clk);
What if the clock has a parent but no enable function?
^ permalink raw reply
* [PATCH V4 3/3] ARM: SAMSUNG: Add lookup of sdhci-s3c clocks using generic names
From: Sylwester Nawrocki @ 2011-10-13 9:40 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20111013085521.GJ21648@n2100.arm.linux.org.uk>
On 10/13/2011 10:55 AM, Russell King - ARM Linux wrote:
> On Thu, Oct 13, 2011 at 09:55:04AM +0200, Sylwester Nawrocki wrote:
>> Yes, it allowed to add clkdev support with minimal changes required.
>>
>> But let's see what happens when the works on common struct clk for all
>> architectures are more or less completed and we convert Samsung platforms
>> to use it;)
>
> The common struct clk shouldn't have much to do with the clkdev part of
> it - clkdev just provides a method to associate a struct clk with the
> drivers request. The common struct clk doesn't change that.
Indeed. I was just wondering where we end up with the arch struct clk attribute
which is specific to clkdev processes.
It could just go into the hardware-specific data, although it doesn't sound
like a perfectly right approach.
^ permalink raw reply
* [PATCH 4/9] ARM: SPMP8000: Add ADC driver
From: Russell King - ARM Linux @ 2011-10-13 9:47 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20111010114407.GD3607@opensource.wolfsonmicro.com>
On Mon, Oct 10, 2011 at 12:44:08PM +0100, Mark Brown wrote:
> On Mon, Oct 10, 2011 at 01:42:30PM +0200, Zoltan Devai wrote:
>
> > Where should the adc driver go in this case ?
>
> At the minute it seems to me that arch/arm is as good a place as any
> really - currently this code is getting dumped wherever the main device
> is.
No it isn't - we want drivers out of arch/arm (it's already been a topic
of flame for Linus, so it's something that we should try _really_ hard
to avoid.)
As this is clearly a device driver (it has a device driver struct, it
relies upon a struct device, etc) then it needs to live outside of
arch/arm/ - and I think Arnd's suggestion of drivers/adc is probably
the right place for it to move to (even though its probably the first
to create the directory.) More stuff can come along in the future,
and then its all together to start creating a common API in there.
The other thing I'd say is that things like the callback definitions
and driver data should not be in some arch/arm/ include file - if
it's private to the ADC, it should go in drivers/adc too. If not,
then somewhere in include/linux (maybe include/linux/adc) is a better
place.
What we *do* need to avoid is having the spmp8000 ADC callback function
data structure, the foo ADC callback function data structure, the bar
ADC callback function data structure and so forth. We need to think
*NOW* about defining a common ADC callback structure so that we don't
spawn a new problem which'll take effort to solve.
^ permalink raw reply
* [PATCH 9/9] ARM: SPMP8000: Add Kconfig and Makefile entries to build the machine
From: Russell King - ARM Linux @ 2011-10-13 9:53 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1318178172-7965-10-git-send-email-zoss@devai.org>
On Sun, Oct 09, 2011 at 06:36:12PM +0200, Zoltan Devai wrote:
> +config ARCH_SPMP8000
> + bool "Sunplus SPMP8000"
> + select CPU_ARM926T
> + select ARM_VIC
> + select GENERIC_IRQ_CHIP
> + select MULTI_IRQ_HANDLER
> + select CLKSRC_MMIO
> + select GENERIC_CLOCKEVENTS
> + select ARCH_WANT_OPTIONAL_GPIOLIB
> + select USE_OF
> + select CLKDEV_LOOKUP
> + select HAVE_PWM
> + select ARCH_HAS_CPUFREQ
Please arrange these select statements in alphabetical order - it helps
avoid the 'add to the end' conflict problem.
^ permalink raw reply
* [PATCH 8/9] ARM: SPMP8000: Add support for the Letcool board
From: Russell King - ARM Linux @ 2011-10-13 9:54 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1318178172-7965-9-git-send-email-zoss@devai.org>
On Sun, Oct 09, 2011 at 06:36:11PM +0200, Zoltan Devai wrote:
> +#include <linux/init.h>
> +#include <linux/io.h>
> +#include <linux/platform_device.h>
> +#include <linux/pwm_backlight.h>
> +#include <linux/input.h>
> +#include <linux/basic_mmio_gpio.h>
> +#include <linux/gpio_keys.h>
> +#include <linux/clk.h>
> +#include <linux/of.h>
> +#include <linux/of_platform.h>
> +#include <asm/hardware/vic.h>
> +#include <asm/mach/arch.h>
> +#include <asm/mach-types.h>
> +
> +#include <mach/core.h>
> +#include <mach/gpio.h>
In addition to Arnd's comments - please use linux/gpio.h rather than
mach/gpio.h or asm/gpio.h throughout your patches. This helps to make
them future-proof.
^ permalink raw reply
* [PATCH] UART: add CSR SiRFprimaII SoC on-chip uart drivers
From: Alan Cox @ 2011-10-13 9:55 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1318487293-6963-1-git-send-email-Barry.Song@csr.com>
Looks basically ok but somewhat outdated for the tty layer. In
particular it needs to be aware of the refcounting on tty objects and
of the fact we allow arbitary baud rate handling
> +static struct sirfsoc_baudrate_to_regv baudrate_to_regv[] = {
const
> +static unsigned int
> +sirfsoc_uart_pio_rx_chars(struct uart_port *port, unsigned int
> max_rx_count) +{
> + unsigned int ch, rx_count = 0;
> + int temp;
> + struct tty_struct *tty = port->state->port.tty;
tty = tty_port_tty_get(&port->state->port);
[the newer tty code is refcounting, also tty can be NULL here and needs
h handling accordingly]
> + while (!(rd_regl(port, SIRFUART_RX_FIFO_STATUS) &
> +
> SIRFUART_FIFOEMPTY_MASK(port))) {
> + temp =
> tty_buffer_request_room(port->state->port.tty, 1);
> + if (unlikely(temp == 0)) {
> + port->icount.buf_overrun++;
> + break;
> + }
You don't need to do this - just uart_insert_char. If we run out of mid
layer buffering it's not a port overrun as such (ie a fifo exceeded)
it's a system wide memory crunch and something pretty serious and
bigger is going on.
> + port->icount.rx += rx_count;
> + tty_flip_buffer_push(tty);
[Do these only if tty != NULL obviously)
and
tty_kref_put(tty);
> +static void sirfsoc_uart_set_termios(struct uart_port *port,
> + struct ktermios *termios,
> + struct ktermios *old)
> +{
If you don't support CMSPAR then clear the bit in the passed termios.
> + for (ic = 0; ic < SIRFUART_BAUD_RATE_SUPPORT_NR; ic++)
> + if (baud_rate == baudrate_to_regv[ic].baud_rate)
> + clk_div_reg = baudrate_to_regv[ic].reg_val;
> + if (clk_div_reg == 0)
> + pr_err("SiRF UART: Cannot set Baud Rate (9600 ~
> 4000000).\n");
The baud rate is not guaranteed to be one the Bxxxxx values, you should
be computing it not using a table.
Also the pr_err means any user can fill the logs with junk.
The correct behaviour here is
compute the best timing for the baud rate requested
compute the actual baud rate obtained
then report it back to the caller
if (tty_termios_baud_rate(termios))
tty_termios_encode_baud_rate(termios, baud. baud);
The above assuming you set the same input and output rate
> +static struct console sirfsoc_uart_console = {
> + .name = "ttyS",
ttyS is 8250 devces. Pick a new name for your own.
>
> + .dev_name = SIRFSOC_UART_NAME,
> + .major = SIRFSOC_UART_MAJOR,
> + .minor = SIRFSOC_UART_MINOR,
Use dynamic allocations
> +#define SIRFSOC_UART_NAME "ttyS"
> +#define SIRFSOC_UART_MAJOR TTY_MAJOR
> +#define SIRFSOC_UART_MINOR 64
These values belong to an existing inerface, don't reuse the names like
that, and please use dynamic allocation for the numbers
> +#define uart_tx_port_tty_invalid(port) \
> + (((port)->state == NULL) || ((port)->state->port.tty ==
> NULL))
This has no locking so little meaning - what guarantees it doesn't
change under or after the call. I suspect you want to be checking and
referencing the tty in one shot as in the example I gave above for the
rx fix.
^ permalink raw reply
* [PATCH 1/2] [ARM] mach-types: Re-add apf9328
From: Russell King - ARM Linux @ 2011-10-13 9:59 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20111013090609.GT577@pengutronix.de>
On Thu, Oct 13, 2011 at 11:06:09AM +0200, Uwe Kleine-K?nig wrote:
> On Sat, Oct 08, 2011 at 11:07:22PM +0100, Russell King - ARM Linux wrote:
> > On Sat, Oct 08, 2011 at 09:26:21PM +0200, Wolfram Sang wrote:
> > > On Sat, Oct 08, 2011 at 05:19:48PM +0200, Gwenhael Goavec-Merou wrote:
> > > >
> > > > Signed-off-by: Gwenhael Goavec-Merou <gwenhael.goavec-merou@armadeus.com>
> > >
> > > I think this file is maintained by Russell, yet I want to ack that the entry
> > > below is missing for building mx1_defconfig. Found that, too.
> >
> > The thing is - read the comments at the top of the file. There's hints
> > there as to what will happen if you patch the file. The hint is
> > 'automatically generated'. So when I next update the file, any patches
> > done to the file will be wiped out.
> Currently the id is still missing in your for-next branch and it is
> marked as "mainlined" on http://www.arm.linux.org.uk/developer/machines/
> (906).
>
> That means that the next update should include the id, right?
Not if it's a buggered entry - which it doesn't appear to be. The problem
is that now that I've stopped manually editing out the buggered entries,
favouring doing it automatically, any update to the file ends up breaking
the damned thing for someone.
It's rather too late in the cycle for me to commit an update now - I
suspect doing so will upset a lot of people.
^ permalink raw reply
* [PATCH V2 1/2] ARM: EXYNOS4: Add SPI support
From: Russell King - ARM Linux @ 2011-10-13 10:02 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1318234084-19960-2-git-send-email-padma.v@samsung.com>
On Mon, Oct 10, 2011 at 01:38:04PM +0530, Padmavathi Venna wrote:
> +void __init exynos4_spi_set_info(int cntrlr, int src_clk_nr, int num_cs)
> +{
> + struct s3c64xx_spi_info *pd;
> +
> + /* Reject invalid configuration */
> + if (!num_cs || src_clk_nr < 0
> + || src_clk_nr > EXYNOS4_SPI_SRCCLK_SCLK) {
> + printk(KERN_ERR "%s: Invalid SPI configuration\n", __func__);
Let's start using pr_err() rather than printk(KERN_ERR...) in ARM code
please.
^ permalink raw reply
* [PATCH 1/2] drivers: create a pin control subsystem v9
From: Linus Walleij @ 2011-10-13 10:04 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAPTzV16bKaHxoyWEv1H8HGyY_CZhY9orXAuWA-wa0yb2NZ5UcA@mail.gmail.com>
On Thu, Oct 13, 2011 at 2:55 AM, Chanho Park <parkch98@gmail.com> wrote:
> Some gpio-ranges doesn't match with pin numbers.
> For example, gpio_range_b starts gpio 48.
> However, a pin base number of gpio_range_b is 96. It isn't same with gpio base.
> A pinctrl driver must know this pin_space-gpio_range mappings.
The GPIO pin space is one global space, and the only thing
the struct pinctrl_gpio_range does is to say that this pin range
is handled by this chip. You can register several ranges to the
same pin controller.
Then the pin control driver gets called like this:
pin_request(pctldev, pin, gpiostr, true, range);
ops->gpio_request_enable(pctldev, gpio_range, pin);
In this case you know that pin is an index into the range
supplied, note that we always get the range into the driver.
These ranges are defined by the driver as well, and it has
an .id field. Thus the driver shall add/subtract whatever offset
it needs to map that GPIO pin into a proper pin number,
there can be so many strange ways of doing this that the
pin control framework is not dealing with it.
So I think this should be done by the driver, and a clever
way is to use the .id field of the range as index to offset
arrays etc.
> +int pinctrl_get_device_gpio_range(unsigned gpio,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? struct pinctrl_dev **outdev,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? struct pinctrl_gpio_range **outrange)
> +{
> + ? ? ? struct pinctrl_dev *pctldev = NULL;
> +
> + ? ? ? /* Loop over the pin controllers */
> + ? ? ? mutex_lock(&pinctrldev_list_mutex);
> + ? ? ? list_for_each_entry(pctldev, &pinctrldev_list, node) {
> + ? ? ? ? ? ? ? struct pinctrl_gpio_range *range;
> +
> + ? ? ? ? ? ? ? range = pinctrl_match_gpio_range(pctldev, gpio);
> + ? ? ? ? ? ? ? if (range != NULL) {
> + ? ? ? ? ? ? ? ? ? ? ? *outdev = pctldev;
> + ? ? ? ? ? ? ? ? ? ? ? *outrange = range;
>
> missing mutex_unlock
Argh!
Thanks, fixed it.
Yours,
Linus Walleij
^ permalink raw reply
* [RESEND][PATCH V2 0/2] Add a common macro for creating struct clk_lookup entries
From: Russell King - ARM Linux @ 2011-10-13 10:20 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <057301cc880c$cc7f1f50$657d5df0$%kim@samsung.com>
On Tue, Oct 11, 2011 at 08:56:23PM +0900, Kukjin Kim wrote:
> Russell,
> Maybe as you know, this is required on upcoming Samsung stuff. Actually it's
> just change/consolidate on other stuff but add on Samsung stuff. So if
> you're ok, how about sending this to upstream via arm-soc or your tree in
> this merge window?
I think taking the first patch is the right thing before the merge
window opens, because that's the one which everything else will depend
upon.
The second patch probably needs splitting up between the various SoCs
and either submitted to their maintainers or to Arnd after the merge
window has closed.
I suspect that we're looking at one of:
a) a matter of days to the merge window opening - though the timing of
it (9 days before the kernel summit) doesn't look good, unless it's
going to be a short window.
b) -rc9 will persist for the next fortnight
c) we'll see the first 3.x-rc10 kernel.
^ permalink raw reply
* [RESEND][PATCH V2 1/2] ARM: CLKDEV: Add Common Macro for clk_lookup
From: Russell King - ARM Linux @ 2011-10-13 10:20 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1318333530-7023-2-git-send-email-padma.v@samsung.com>
On Tue, Oct 11, 2011 at 05:15:29PM +0530, Padmavathi Venna wrote:
> From: Padmavathi Venna <padma.v@samsung.com>
>
> Added a standardized macro CLKDEV_INIT which can used across all
> the platforms to support clkdev
>
> Suggested by: Russell King <rmk+kernel@arm.linux.org.uk>
Should be Suggested-by: (with a '-' rather than a ' ').
Other than that, it's fine. Please submit to the patch system, thanks.
^ permalink raw reply
* [PATCH v5 00/10] Per SoC descriptor
From: Russell King - ARM Linux @ 2011-10-13 10:32 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1318255342-17451-1-git-send-email-marc.zyngier@arm.com>
On Mon, Oct 10, 2011 at 03:02:12PM +0100, Marc Zyngier wrote:
> arch/arm/include/asm/mach/arch.h | 2 +
> arch/arm/include/asm/smp.h | 15 -----
> arch/arm/include/asm/soc.h | 64 ++++++++++++++++++++
> arch/arm/kernel/setup.c | 8 +++
> arch/arm/kernel/smp.c | 73 ++++++++++++++++++++++-
> arch/arm/mach-exynos4/core.h | 14 ++++
> arch/arm/mach-exynos4/cpu.c | 8 +++
> arch/arm/mach-exynos4/hotplug.c | 8 ++-
> arch/arm/mach-exynos4/mach-armlex4210.c | 3 +
> arch/arm/mach-exynos4/mach-nuri.c | 3 +
> arch/arm/mach-exynos4/mach-origen.c | 3 +
> arch/arm/mach-exynos4/mach-smdk4x12.c | 4 +
> arch/arm/mach-exynos4/mach-smdkv310.c | 4 +
> arch/arm/mach-exynos4/mach-universal_c210.c | 3 +
> arch/arm/mach-exynos4/platsmp.c | 25 +++++++-
> arch/arm/mach-msm/Makefile | 2 +-
> arch/arm/mach-msm/board-msm8960.c | 3 +
> arch/arm/mach-msm/board-msm8x60.c | 7 ++
> arch/arm/mach-msm/core.c | 17 +++++
> arch/arm/mach-msm/core.h | 9 +++
> arch/arm/mach-msm/hotplug.c | 8 ++-
> arch/arm/mach-msm/platsmp.c | 24 ++++++-
> arch/arm/mach-omap2/board-4430sdp.c | 1 +
> arch/arm/mach-omap2/board-omap4panda.c | 1 +
> arch/arm/mach-omap2/include/mach/omap4-common.h | 14 ++++
> arch/arm/mach-omap2/omap-hotplug.c | 6 +-
> arch/arm/mach-omap2/omap-smp.c | 24 ++++++-
> arch/arm/mach-omap2/omap4-common.c | 7 ++
> arch/arm/mach-realview/core.c | 7 ++
> arch/arm/mach-realview/core.h | 9 +++
> arch/arm/mach-realview/hotplug.c | 6 +-
> arch/arm/mach-realview/platsmp.c | 24 ++++++-
> arch/arm/mach-realview/realview_eb.c | 1 +
> arch/arm/mach-realview/realview_pb1176.c | 1 +
> arch/arm/mach-realview/realview_pb11mp.c | 1 +
> arch/arm/mach-realview/realview_pba8.c | 1 +
> arch/arm/mach-realview/realview_pbx.c | 1 +
> arch/arm/mach-shmobile/Makefile | 2 +-
> arch/arm/mach-shmobile/board-ag5evm.c | 1 +
> arch/arm/mach-shmobile/hotplug.c | 8 ++-
> arch/arm/mach-shmobile/include/mach/common.h | 7 +-
> arch/arm/mach-shmobile/include/mach/sh73a0.h | 6 ++
> arch/arm/mach-shmobile/platsmp.c | 68 ---------------------
> arch/arm/mach-shmobile/setup-sh73a0.c | 6 ++
> arch/arm/mach-shmobile/smp-sh73a0.c | 35 ++++++++++-
> arch/arm/mach-tegra/board-dt.c | 2 +
> arch/arm/mach-tegra/board-harmony.c | 2 +
> arch/arm/mach-tegra/board-paz00.c | 2 +
> arch/arm/mach-tegra/board-seaboard.c | 4 +
> arch/arm/mach-tegra/board-trimslice.c | 2 +
> arch/arm/mach-tegra/common.c | 8 +++
> arch/arm/mach-tegra/common.h | 11 ++++
> arch/arm/mach-tegra/hotplug.c | 6 +-
> arch/arm/mach-tegra/platsmp.c | 26 +++++++-
> arch/arm/mach-ux500/board-mop500.c | 3 +
> arch/arm/mach-ux500/board-u5500.c | 1 +
> arch/arm/mach-ux500/hotplug.c | 8 ++-
> arch/arm/mach-ux500/include/mach/setup.h | 9 +++
> arch/arm/mach-ux500/platsmp.c | 23 ++++++-
> arch/arm/mach-vexpress/core.h | 10 +++
> arch/arm/mach-vexpress/hotplug.c | 6 +-
> arch/arm/mach-vexpress/platsmp.c | 24 ++++++-
> arch/arm/mach-vexpress/v2m.c | 9 +++
> arch/arm/plat-versatile/include/plat/platsmp.h | 14 ++++
> arch/arm/plat-versatile/platsmp.c | 4 +-
> 65 files changed, 569 insertions(+), 149 deletions(-)
This isn't particularly appealing for something that's supposed to
be a clean up - it's introducing 400+ new lines of code. It looks like
most of this comes from the platform stuff rather than core stuff.
Can't we do any better with this? We really need things to be going
in the other direction.
^ permalink raw reply
* [PATCH 1/2] drivers: create a pin control subsystem v9
From: Linus Walleij @ 2011-10-13 10:39 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20111013031819.GA15829@ponder.secretlab.ca>
On Thu, Oct 13, 2011 at 5:18 AM, Grant Likely <grant.likely@secretlab.ca> wrote:
> Nit: put the changelog above the s-o-b lines so it will appear in the
> linux commit log.
OK I'll collect all of them and stash them into it for v10
>> +The GPIO drivers may want to perform operations of various types on the same
>> +physical pins that are also registered as GPIO pins.
>
> ...also registered as PINMUX pins?
Yes.
>> ?source "drivers/ptp/Kconfig"
>>
>> +# pinctrl before gpio - gpio drivers may need it
>
> This shouldn't actually be an ordering constraint. ?It might be a
> temporary restriction on the Makefiles, but it isn't at all for Kconfig.
True, was blinded by similar comments in that Kconfig, such as
this:
# input before char - char/joystick depends on it. As does USB.
Anyway I took out the comment.
>> +menuconfig PINCTRL
>> + ? ? bool "PINCTRL Support"
>> + ? ? depends on SYSFS && EXPERIMENTAL
>
> Why depends on SYSFS? ?That shouldn't be a consideration at all.
Dropped last week. Leftover from when I registered this
bus...
>> +
>> +/**
>> + * get_pctldev_from_dev() - look up pin controller device
>
> Naming abbreviation is a little agressive. ?Please use pinctrl
> everywhere instead of a mix between pctl and pinctrl.
OK. Renamed a few functions accordingly.
Also added a fixup to the SirfPrimaII driver to use
the new names.
>> + ? ? if (found)
>> + ? ? ? ? ? ? return pctldev;
>> +
>> + ? ? return NULL;
>
> or simply:
>
> ? ? ? ?return found ? pctldev : NULL;
OK.
>> + ? ? /* Loop over the pin controllers */
>> + ? ? mutex_lock(&pinctrldev_list_mutex);
>> + ? ? list_for_each_entry(pctldev, &pinctrldev_list, node) {
>> + ? ? ? ? ? ? struct pinctrl_gpio_range *range;
>> +
>> + ? ? ? ? ? ? range = pinctrl_match_gpio_range(pctldev, gpio);
>> + ? ? ? ? ? ? if (range != NULL) {
>> + ? ? ? ? ? ? ? ? ? ? *outdev = pctldev;
>> + ? ? ? ? ? ? ? ? ? ? *outrange = range;
>> + ? ? ? ? ? ? ? ? ? ? return 0;
>
> Neglected to drop mutex
Yep noted by another reviewe also today...
>> +/**
>> + * pinctrl_register() - register a pin controller device
>> + * @pctldesc: descriptor for this pin controller
>> + * @dev: parent device for this pin controller
>> + * @driver_data: private pin controller data for this pin controller
>> + */
>> +struct pinctrl_dev *pinctrl_register(struct pinctrl_desc *pctldesc,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? struct device *dev, void *driver_data)
>> +{
>> + ? ? static atomic_t pinmux_no = ATOMIC_INIT(0);
>> + ? ? struct pinctrl_dev *pctldev;
>> + ? ? int ret;
>> +
>> + ? ? if (pctldesc == NULL)
>> + ? ? ? ? ? ? return ERR_PTR(-EINVAL);
>
> I urge you to consider carefully before relying on the ERR_PTR()
> pattern. ?It isn't easy to for a compiler or a human to check if
> ERR_PTR values are being handled properly, and is therefore a likely
> source of bugs. ?Unless it is *absolutely critical* to return an error
> code instead of NULL on error, I strongly recommend returning NULL
> from this function on failure.
>
> From what I see from this function, the error codes are less useful
> that using pr_err() calls.
Agreed, we return NULL on error instead.
Design pattern comes from clocks and regulators, it's not
that useful in this case, maybe not in the other cases either.
Fixed usage in U300 and Sirf controller as well.
> I think this is pretty close to ready. ?If you address the comments
> I've made above then you can add my Acked-by:
>
> Acked-by: Grant Likely <grant.likely@secretlab.ca>
Thanks a lot Grant!
Yours,
Linus Walleij
^ permalink raw reply
* [RFC 1/5] ARM: dev_archdata: add private iommu extension
From: Russell King - ARM Linux @ 2011-10-13 10:42 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAK=Wgbau+XZ-c21FCT8DT0NYfm2uD6WZv3=cU2R23pV7LUkxEA@mail.gmail.com>
On Thu, Oct 13, 2011 at 11:15:39AM +0200, Ohad Ben-Cohen wrote:
> On Wed, Oct 5, 2011 at 11:26 AM, Ohad Ben-Cohen <ohad@wizery.com> wrote:
> > Hi Russell,
> >
> > On Sun, Sep 25, 2011 at 1:58 PM, Ohad Ben-Cohen <ohad@wizery.com> wrote:
> >> Add a private iommu pointer to the ARM-specific arch data in the
> >> device struct, which will be used to attach iommu-specific data
> >> to devices which require iommu support.
> >
> > Can you please take a look and ack/nack ?
>
> gentle reminder
Ok, I'll take it if you put it in the patch system.
Thanks.
^ permalink raw reply
* [PATCH 5/7] ARM: EXYNOS4: Add support external GIC
From: Russell King - ARM Linux @ 2011-10-13 11:09 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <4E92ECD1.2010303@arm.com>
On Mon, Oct 10, 2011 at 02:02:09PM +0100, Marc Zyngier wrote:
> On 07/10/11 16:16, Will Deacon wrote:
> > On Fri, Oct 07, 2011 at 10:44:59AM +0100, Marc Zyngier wrote:
> >> +static void __cpuinit exynos4_secondary_init(unsigned int cpu)
> >> {
> >> void __iomem *dist_base = S5P_VA_GIC_DIST +
> >> - (gic_bank_offset * smp_processor_id());
> >> + (gic_bank_offset * cpu_logical_map(cpu));
> >
> > Again, I'm deeply suspicious of this code :) Is there not a common memory
> > alias for the distributor across all of the CPUs?
>
> Kukjin, could you please comment on the presence of a common memory
> region for the distributor? This seem quite odd...
It's not odd when you consider that there's per-CPU registers within the
GIC distributor as well (for the first 32 GIC IRQs) as the per-CPU GIC
CPU interfaces.
^ permalink raw reply
* [PATCH 4/9] ARM: SPMP8000: Add ADC driver
From: Linus Walleij @ 2011-10-13 11:09 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20111013094717.GL21648@n2100.arm.linux.org.uk>
On Thu, Oct 13, 2011 at 11:47 AM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Mon, Oct 10, 2011 at 12:44:08PM +0100, Mark Brown wrote:
>> On Mon, Oct 10, 2011 at 01:42:30PM +0200, Zoltan Devai wrote:
>>
>> > Where should the adc driver go in this case ?
>>
>> At the minute it seems to me that arch/arm is as good a place as any
>> really - currently this code is getting dumped wherever the main device
>> is.
>
> No it isn't - we want drivers out of arch/arm (it's already been a topic
> of flame for Linus, so it's something that we should try _really_ hard
> to avoid.)
Amen to that.
> As this is clearly a device driver (it has a device driver struct, it
> relies upon a struct device, etc) then it needs to live outside of
> arch/arm/ - and I think Arnd's suggestion of drivers/adc is probably
> the right place for it to move to (even though its probably the first
> to create the directory.) ?More stuff can come along in the future,
> and then its all together to start creating a common API in there.
That sort of suggest that the drivers/staging/iio/adc is in
trouble now. It will have an in-kernel competitor for similar
stuff.
Having drivers/adc and drivers/iio/adc compete about
taking drivers on preference as to whether they were intended
for userspace or kernelspace does not seem like a good
thing.
^ permalink raw reply
* [PATCH 4/9] ARM: SPMP8000: Add ADC driver
From: Mark Brown @ 2011-10-13 11:35 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20111013094717.GL21648@n2100.arm.linux.org.uk>
On Thu, Oct 13, 2011 at 10:47:17AM +0100, Russell King - ARM Linux wrote:
> On Mon, Oct 10, 2011 at 12:44:08PM +0100, Mark Brown wrote:
> > At the minute it seems to me that arch/arm is as good a place as any
> > really - currently this code is getting dumped wherever the main device
> > is.
> No it isn't - we want drivers out of arch/arm (it's already been a topic
> of flame for Linus, so it's something that we should try _really_ hard
> to avoid.)
I said it was as good a place as any, I didn't say it was a good place.
Currently we don't have a good place for this code but just dumping the
code somewhere else isn't really helping anything either unless there's
some meaningful subsytem (or at least effort towards a subsystem) to
look after it, it's bookkeeping.
> As this is clearly a device driver (it has a device driver struct, it
> relies upon a struct device, etc) then it needs to live outside of
> arch/arm/ - and I think Arnd's suggestion of drivers/adc is probably
> the right place for it to move to (even though its probably the first
> to create the directory.) More stuff can come along in the future,
> and then its all together to start creating a common API in there.
Creating drivers/adc just seems like a sucky idea. We've already got at
least one ongoing effort at creating a general purpose ADC/DAC interface
in the form of the IIO subsystem (which is currently mostly targeted at
higher volume devices and userspace only but the abstractions needed
aren't terribly different depending on the data volumes) and does have
the rather substantial advantage that there's people actually trying to
make a subsystem. On the other hand it's in staging for now so it'd
create pain trying to merge the in-kernel clients that go along with
these devices which doesn't seem constructive.
There's also the fact that ADCs and DACs are pretty much the same thing
to software with the data line driven from the opposite end of the link
sO if we've got a subsystem for ADCs only it seems clear we're doing
something wrong.
> What we *do* need to avoid is having the spmp8000 ADC callback function
> data structure, the foo ADC callback function data structure, the bar
> ADC callback function data structure and so forth. We need to think
> *NOW* about defining a common ADC callback structure so that we don't
> spawn a new problem which'll take effort to solve.
The cat's already well out of the bag on that one, we've got a bunch of
these things in tree already - the Cragganmore system I've got sitting
here on my desk has three auxiliary ADCs I'm aware of, all of which are
supported in mainline, and they're far from the only ones.
I think we should just carry on as we are while the IIO work proceeds,
either we'll decide that that's the way forward and we can then kick all
the ADCs into there or we'll get fed up, decide that's never going to
work then go and can do something else.
^ permalink raw reply
* [PATCH 4/9] ARM: SPMP8000: Add ADC driver
From: Jonathan Cameron @ 2011-10-13 11:35 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CACRpkdZMXjoHdEaPhTFS9Q7=5tWfFHDiwt-gdKd2CGiW+Evdmw@mail.gmail.com>
On 10/13/11 12:09, Linus Walleij wrote:
> On Thu, Oct 13, 2011 at 11:47 AM, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
>> On Mon, Oct 10, 2011 at 12:44:08PM +0100, Mark Brown wrote:
>>> On Mon, Oct 10, 2011 at 01:42:30PM +0200, Zoltan Devai wrote:
>>>
>>>> Where should the adc driver go in this case ?
>>>
>>> At the minute it seems to me that arch/arm is as good a place as any
>>> really - currently this code is getting dumped wherever the main device
>>> is.
>>
>> No it isn't - we want drivers out of arch/arm (it's already been a topic
>> of flame for Linus, so it's something that we should try _really_ hard
>> to avoid.)
>
> Amen to that.
>
>> As this is clearly a device driver (it has a device driver struct, it
>> relies upon a struct device, etc) then it needs to live outside of
>> arch/arm/ - and I think Arnd's suggestion of drivers/adc is probably
>> the right place for it to move to (even though its probably the first
>> to create the directory.) More stuff can come along in the future,
>> and then its all together to start creating a common API in there.
>
> That sort of suggest that the drivers/staging/iio/adc is in
> trouble now. It will have an in-kernel competitor for similar
> stuff.
>
> Having drivers/adc and drivers/iio/adc compete about
> taking drivers on preference as to whether they were intended
> for userspace or kernelspace does not seem like a good
> thing.
The problem with a straight adc directory is the fact that many of
these devices are combined input and output. The also tend to
have numerous things that to a casual don't look like ADC's
(light sensors etc). We only did the grouping in IIO for the
sake of not having a huge flat directory. Many of the drivers
overlap between the different categories.
>
>>From a quick review of the IIO ADC subsystem I cannot really
> find a big problem with it other than it brings the entire IIO
> subsystem along, including the userspace interfaces I guess.
>
> The circumstances seem to suggest that drivers/iio/adc should
> be moved down to drivers/adc without the rest of the iio
> stuff, with a in-kernel API only and then have iio use that
> (this is probably true for some more drivers in IIO like
> dac, accel, addac, gyro...).
>
> So the concept of IIO stuff would change from the
> userspace-first centric view to the kernelspace-first
> centric, with optional userspace interfaces in the form
> of IIO.
>
> I'm not sure about what Jonathan thinks about that
> though.
That wouldn't be too hard to do. It's just a case of breaking
iio_register_device function in two. Right now (with the in kernel
RFC) on top. The only thing needed for in kernel access is that the
configured struct iio_dev gets added to the list of available devices.
I think we need to maintain the form of struct iio_chan_spec
(or whatever does the same job) with the intent of it directly mapping
to userspace interfaces (keeps things clean and makes sure there is enough
information to actually use the channel for anything).
The key thing is the unified specification of channel capabilities
and the ability to read them. We have one way of doing that which
has developed against a 'lot' of drivers.
The nasty corner is optimized reading of channel scans. They tend to
be irrelevant for memory mapped device but are vital for many devices
on low bandwidth buses. Right now we leave that entirely to the
individual drivers (they push this data out rather than it being
pulled from the core like everything else). It may be possible to
flip this around with out too much pain, but I'm not sure.
The other nasty bits are:
* hardware buffering
* triggered capture control - we really care about this for IIO
and I don't think anyone else does. Could maybe get away with using
device tree or similar to specify defaults for this and not provide
any means of changing it for devices other than those with full IIO
userspace in place.
As you can imagine I'm a little wary of sending IIO through another
refactoring right now (proposal to move the core out of staging is
scheduled for my next free afternoon!). So in principal in favour
of this approach but in practice not sure I'll be in a position
to do it (need to eat and my funding source is about to run out -
awaiting decision on more).
So my personal preference would be to add the relevant access functions
etc to IIO as is. Two models:
1) Pull from drivers - (give me reading now).
Done - we need to nick the clkdev mapping approach to join everything
up and play a few tricks to get a unique consistent name.
2) Push from triggered capture - (here is a reading).
Easiest option from our point of view is to provide this as a 'buffer'
implementation. Normally we'd notify userspace of new data, here we
notify in kernel users (or just push it into their callbacks).
Post that we start to make IIO userspace interfaces optional.
It gives a progression towards what you are suggesting and doesn't
hold us up in staging for another long cycle.
There are exceptions. I'd argue any element that is really built for
say hwmon or input ought to register directly with those subsytems.
That includes the touchscreen element of the adc that started this
conversation. No point in adding overhead and generality to a non
general element.
Sorry for quick reply, mad day.
Jonathan
^ permalink raw reply
* [PATCH v3 2/4] dmaengine: at_hdmac: add device tree support
From: Nicolas Ferre @ 2011-10-13 11:54 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20111013003422.GE14042@ponder.secretlab.ca>
On 10/13/2011 02:34 AM, Grant Likely :
> On Wed, Oct 12, 2011 at 06:57:12PM +0200, Nicolas Ferre wrote:
>> Add device tree probe support for atmel at_hdmac DMA driver.
>> Bindings are added to specify DMA controller configuration.
>>
>> Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
>> ---
>> .../devicetree/bindings/dma/atmel-dma.txt | 14 +++++++++
>> drivers/dma/at_hdmac.c | 30 +++++++++++++++++++-
>> 2 files changed, 43 insertions(+), 1 deletions(-)
>> create mode 100644 Documentation/devicetree/bindings/dma/atmel-dma.txt
>>
>> diff --git a/Documentation/devicetree/bindings/dma/atmel-dma.txt b/Documentation/devicetree/bindings/dma/atmel-dma.txt
>> new file mode 100644
>> index 0000000..3c046ee
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/dma/atmel-dma.txt
>> @@ -0,0 +1,14 @@
>> +* Atmel Direct Memory Access Controller (DMA)
>> +
>> +Required properties:
>> +- compatible: Should be "atmel,<chip>-dma"
>> +- reg: Should contain DMA registers location and length
>> +- interrupts: Should contain DMA interrupt
>> +
>> +Examples:
>> +
>> +dma at ffffec00 {
>> + compatible = "atmel,at91sam9g45-dma";
>> + reg = <0xffffec00 0x200>;
>> + interrupts = <21>;
>> +};
>> diff --git a/drivers/dma/at_hdmac.c b/drivers/dma/at_hdmac.c
>> index d1869c5..42bd64c 100644
>> --- a/drivers/dma/at_hdmac.c
>> +++ b/drivers/dma/at_hdmac.c
>> @@ -23,6 +23,8 @@
>> #include <linux/module.h>
>> #include <linux/platform_device.h>
>> #include <linux/slab.h>
>> +#include <linux/of.h>
>> +#include <linux/of_device.h>
>>
>> #include "at_hdmac_regs.h"
>>
>> @@ -1175,6 +1177,20 @@ static void atc_free_chan_resources(struct dma_chan *chan)
>>
>> /*-- Module Management -----------------------------------------------*/
>>
>> +#if defined(CONFIG_OF)
>> +static const struct of_device_id atmel_dma_dt_ids[] = {
>> + {
>> + .compatible = "atmel,at91sam9rl-dma",
>> + .data = (void *)ATDMA_DEVTYPE_SAM9RL
>> + }, {
>> + .compatible = "atmel,at91sam9g45-dma",
>> + .data = (void *)ATDMA_DEVTYPE_SAM9G45
>> + }, { /* sentinel */ }
>> +};
>> +
>> +MODULE_DEVICE_TABLE(of, atmel_dma_dt_ids);
>> +#endif
>> +
>> static struct platform_device_id atdma_devtypes[] = {
>> {
>> .name = "at91sam9rl_dma",
>> @@ -1187,6 +1203,17 @@ static struct platform_device_id atdma_devtypes[] = {
>> }
>> };
>>
>> +static inline enum atdma_devtype __init at_dma_get_driver_data(
>> + struct platform_device *pdev)
>> +{
>> + if (pdev->dev.of_node) {
>> + const struct of_device_id *match;
>> + match = of_match_node(atmel_dma_dt_ids, pdev->dev.of_node);
>
> You'll need to check the return value. It is possible (though
> unlikely) to have a device with an of_node but still happens to get
> bound via the platform_driver id_table. In which case match would be
> NULL here.
Ok, I add:
if (match == NULL)
return ATDMA_DEVTYPE_UNDEFINED;
Which will take care about this case.
> Otherwise, you can add my a-b after fixing that bug.
>
> Acked-by: Grant Likely <grant.likely@secretlab.ca>
Thanks a lot for your reviews Grant,
Bye,
--
Nicolas Ferre
^ permalink raw reply
* [RFC 1/5] ARM: dev_archdata: add private iommu extension
From: Ohad Ben-Cohen @ 2011-10-13 11:55 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20111013104246.GA22076@n2100.arm.linux.org.uk>
On Thu, Oct 13, 2011 at 12:42 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> Ok, I'll take it if you put it in the patch system.
Done, thanks.
^ permalink raw reply
* [PATCH] atmel_lcdfb: support new-style palette format
From: Peter Korsgaard @ 2011-10-13 12:03 UTC (permalink / raw)
To: linux-arm-kernel
The newer Atmel SoCs use normal 16bit 565 BGR/RGB for the palette data,
rather than the special intensity + 555 format.
Fill out palette data correctly on these devices, and at the same time
respect the RGB/BGR wiring mode.
Signed-off-by: Peter Korsgaard <jacmet@sunsite.dk>
---
drivers/video/atmel_lcdfb.c | 32 ++++++++++++++++++++++++--------
1 files changed, 24 insertions(+), 8 deletions(-)
diff --git a/drivers/video/atmel_lcdfb.c b/drivers/video/atmel_lcdfb.c
index 63409c1..0e8c38e 100644
--- a/drivers/video/atmel_lcdfb.c
+++ b/drivers/video/atmel_lcdfb.c
@@ -682,14 +682,30 @@ static int atmel_lcdfb_setcolreg(unsigned int regno, unsigned int red,
case FB_VISUAL_PSEUDOCOLOR:
if (regno < 256) {
- val = ((red >> 11) & 0x001f);
- val |= ((green >> 6) & 0x03e0);
- val |= ((blue >> 1) & 0x7c00);
-
- /*
- * TODO: intensity bit. Maybe something like
- * ~(red[10] ^ green[10] ^ blue[10]) & 1
- */
+ if (cpu_is_at91sam9261() || cpu_is_at91sam9263()
+ || cpu_is_at91sam9rl()) {
+ /* old style I+BGR:555 */
+ val = ((red >> 11) & 0x001f);
+ val |= ((green >> 6) & 0x03e0);
+ val |= ((blue >> 1) & 0x7c00);
+
+ /*
+ * TODO: intensity bit. Maybe something like
+ * ~(red[10] ^ green[10] ^ blue[10]) & 1
+ */
+ } else {
+ /* new style BGR:565 / RGB:565 */
+ if (sinfo->lcd_wiring_mode ==
+ ATMEL_LCDC_WIRING_RGB) {
+ val = ((red >> 11) & 0x001f);
+ val |= ((blue >> 0) & 0xf800);
+ } else {
+ val = ((blue >> 11) & 0x001f);
+ val |= ((red >> 0) & 0xf800);
+ }
+
+ val |= ((green >> 5) & 0x07e0);
+ }
lcdc_writel(sinfo, ATMEL_LCDC_LUT(regno), val);
ret = 0;
--
1.7.6.3
^ permalink raw reply related
* [PATCH 4/9] ARM: SPMP8000: Add ADC driver
From: Russell King - ARM Linux @ 2011-10-13 12:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20111013113506.GJ1098@sirena.org.uk>
On Thu, Oct 13, 2011 at 12:35:06PM +0100, Mark Brown wrote:
> On Thu, Oct 13, 2011 at 10:47:17AM +0100, Russell King - ARM Linux wrote:
> > On Mon, Oct 10, 2011 at 12:44:08PM +0100, Mark Brown wrote:
>
> > > At the minute it seems to me that arch/arm is as good a place as any
> > > really - currently this code is getting dumped wherever the main device
> > > is.
>
> > No it isn't - we want drivers out of arch/arm (it's already been a topic
> > of flame for Linus, so it's something that we should try _really_ hard
> > to avoid.)
>
> I said it was as good a place as any, I didn't say it was a good place.
I'm saying it is a *BAD* place. I'm saying that we've been flamed over
this several times before. We need to change our behaviour RIGHT NOW,
not continue on ignoring the problem, and demonstrating to Linus that
we don't take his concerns seriously.
If we can't find a place for it that's outside of arch/arm, then it
doesn't go in.
> I think we should just carry on as we are while the IIO work proceeds,
> either we'll decide that that's the way forward and we can then kick all
> the ADCs into there or we'll get fed up, decide that's never going to
> work then go and can do something else.
You mean like other stuff which is already in the kernel gets fixed?
It doesn't get fixed. We persist with per-driver private data structures
which multiply all over the place.
Look at what happened with MTD and the flash_platform_data structure.
That got ignored and instead people just continued merrily creating their
own private crap time and time again.
We can not continue like this. We can not continue to be soo permissive.
I'm now saying a big NO to this - I don't care that the "cat is already
out of the bag" - that's a defeatest attitude. I'm saying no *now* so
that there _is_ some kind of movement towards fixing this problem. I
don't care whether that means it shares an existing ADC drivers callback
data structures or what - I just don't want to see yet another driver
private data structure being created for NO REASON what so ever.
Or maybe you'd like to be on the receiving end of another Linus flame?
Some people would like to avoid such things - maybe you feed off them,
that's your decision. I personally am on the side of the folk who'd
like to avoid being on the receiving end of the next Linus flame.
So. No new drivers in arch/arm. And I'm going to be saying no to any
new per-driver data structures in mach/*.h for stuff which should be
generic which haven't been justified for why they need to be different
from someone elses.
^ 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