Thursday, November 7, 2013

linux.kernel - 26 new messages in 16 topics - digest

linux.kernel
http://groups.google.com/group/linux.kernel?hl=en

linux.kernel@googlegroups.com

Today's topics:

* LEDS: tca6507 - fix bugs in parsing of device-tree configuration. - 1
messages, 1 author
http://groups.google.com/group/linux.kernel/t/1eb3aff0df8d6fb5?hl=en
* irq_work: Provide a irq work that can be processed on any cpu - 8 messages,
3 authors
http://groups.google.com/group/linux.kernel/t/7bc00cc28be7d4b5?hl=en
* staging: zsmalloc: Ensure handle is never 0 on success - 1 messages, 1
author
http://groups.google.com/group/linux.kernel/t/56e7b28ac067dee2?hl=en
* kmod: Run usermodehelpers only on cpus allowed for kthreadd V2 - 1 messages,
1 author
http://groups.google.com/group/linux.kernel/t/a1700157e52e62ff?hl=en
* wire up CPU features to udev based module loading - 1 messages, 1 author
http://groups.google.com/group/linux.kernel/t/7bd137213e7534f5?hl=en
* Introducing Device Tree Overlays - 2 messages, 2 authors
http://groups.google.com/group/linux.kernel/t/7c26bee9afcc62d1?hl=en
* dmaengine: add msm bam dma driver - 1 messages, 1 author
http://groups.google.com/group/linux.kernel/t/eb7ef92f0916cf63?hl=en
* ext4: Fix reading of extended tv_sec (bug 23732) - 2 messages, 2 authors
http://groups.google.com/group/linux.kernel/t/1c061384ec63b78b?hl=en
* linux-next: reminder - 1 messages, 1 author
http://groups.google.com/group/linux.kernel/t/6baf83471f093f73?hl=en
* common location for devicetree files - 1 messages, 1 author
http://groups.google.com/group/linux.kernel/t/036feb035f29aa02?hl=en
* Embeddable Position Independent Executable - 1 messages, 1 author
http://groups.google.com/group/linux.kernel/t/5e545fba8eaa67b6?hl=en
* ARM: at91: move to common clk framework - 1 messages, 1 author
http://groups.google.com/group/linux.kernel/t/2487628f16b80970?hl=en
* LEDS: tca6502: add device-tree support for GPIO configuration. - 2 messages,
2 authors
http://groups.google.com/group/linux.kernel/t/ca1c50996760ad7e?hl=en
* linux-next: manual merge of the gpio tree with the arm-soc and cortex trees -
1 messages, 1 author
http://groups.google.com/group/linux.kernel/t/5f1d2168387c350b?hl=en
* cifs: Use data structures to compute NTLMv2 response offsets - 1 messages, 1
author
http://groups.google.com/group/linux.kernel/t/acd5fb058fd9bf01?hl=en
* ARM: OMAP2+: omap_device: maintain sane runtime pm status around suspend/
resume - 1 messages, 1 author
http://groups.google.com/group/linux.kernel/t/ae07929ea3c9826b?hl=en

==============================================================================
TOPIC: LEDS: tca6507 - fix bugs in parsing of device-tree configuration.
http://groups.google.com/group/linux.kernel/t/1eb3aff0df8d6fb5?hl=en
==============================================================================

== 1 of 1 ==
Date: Thurs, Nov 7 2013 3:00 pm
From: Bryan Wu


On Thu, Oct 31, 2013 at 7:33 PM, NeilBrown <neilb@suse.de> wrote:
>
>
> 1/ The led_info array must be allocated to allow the full number
> of LEDs even if not all are present. The array maybe be sparsely
> filled but it is indexed by device address so we must at least
> allocate as many slots as the highest address used. It is easiest
> just to allocate all 7.
>
> 2/ range check the 'reg' value properly.
>
> 3/ led.flags must be initialised to zero, else all leds could
> be treated as GPIOs (depending on what happens to be on the stack).
>
> Signed-off-by: NeilBrown <neilb@suse.de>
>

Thanks, queued into devel branch of my tree for 3.14 kernel.

-Bryan

> diff --git a/drivers/leds/leds-tca6507.c b/drivers/leds/leds-tca6507.c
> index 8cc304f36728..f5063f447463 100644
> --- a/drivers/leds/leds-tca6507.c
> +++ b/drivers/leds/leds-tca6507.c
> @@ -682,7 +682,7 @@ tca6507_led_dt_init(struct i2c_client *client)
> return ERR_PTR(-ENODEV);
>
> tca_leds = devm_kzalloc(&client->dev,
> - sizeof(struct led_info) * count, GFP_KERNEL);
> + sizeof(struct led_info) * NUM_LEDS, GFP_KERNEL);
> if (!tca_leds)
> return ERR_PTR(-ENOMEM);
>
> @@ -695,9 +695,9 @@ tca6507_led_dt_init(struct i2c_client *client)
> of_get_property(child, "label", NULL) ? : child->name;
> led.default_trigger =
> of_get_property(child, "linux,default-trigger", NULL);
> -
> + led.flags = 0;
> ret = of_property_read_u32(child, "reg", &reg);
> - if (ret != 0)
> + if (ret != 0 || reg < 0 || reg >= NUM_LEDS)
> continue;
>
> tca_leds[reg] = led;
> @@ -708,7 +708,7 @@ tca6507_led_dt_init(struct i2c_client *client)
> return ERR_PTR(-ENOMEM);
>
> pdata->leds.leds = tca_leds;
> - pdata->leds.num_leds = count;
> + pdata->leds.num_leds = NUM_LEDS;
>
> return pdata;
> }
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/





==============================================================================
TOPIC: irq_work: Provide a irq work that can be processed on any cpu
http://groups.google.com/group/linux.kernel/t/7bc00cc28be7d4b5?hl=en
==============================================================================

== 1 of 8 ==
Date: Thurs, Nov 7 2013 3:00 pm
From: Frederic Weisbecker


On Thu, Nov 07, 2013 at 11:50:34PM +0100, Jan Kara wrote:
> On Thu 07-11-13 23:23:14, Frederic Weisbecker wrote:
> > On Thu, Nov 07, 2013 at 11:19:04PM +0100, Jan Kara wrote:
> > > On Thu 07-11-13 23:13:39, Frederic Weisbecker wrote:
> > > > But then, who's going to process that work if every CPUs is idle?
> > > Have a look into irq_work_queue(). There is:
> > > /*
> > > * If the work is not "lazy" or the tick is stopped, raise the irq
> > > * work interrupt (if supported by the arch), otherwise, just wait
> > > * for the next tick. We do this even for unbound work to make sure
> > > * *some* CPU will be doing the work.
> > > */
> > > if (!(work->flags & IRQ_WORK_LAZY) || tick_nohz_tick_stopped()) {
> > > if (!this_cpu_cmpxchg(irq_work_raised, 0, 1))
> > > arch_irq_work_raise();
> > > }
> > >
> > > So we raise an interrupt if there would be no timer ticking (which is
> > > what I suppose you mean by "CPU is idle"). That is nothing changed by my
> > > patches...
> >
> > Ok but we raise that interrupt locally, not to the other CPUs.
> True, but that doesn't really matter in this case. Any CPU (including the
> local one) can handle the unbound work. So from the definition of the
> unbound work things are OK.

I don't see how that can be ok. You want to offline a work because the local CPU
can't handle it, right? If the local CPU can handle it you can just use local
irq works.

>
> Regarding my use for printk - if all (other) CPUs are idle then we can
> easily afford making the current cpu busy printing, that's not a problem.
> There's nothing else to do than to print what's remaining in the printk
> buffer...

So if the current CPU can handle it, what is the problem?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/




== 2 of 8 ==
Date: Thurs, Nov 7 2013 3:00 pm
From: Jan Kara


On Thu 07-11-13 23:23:14, Frederic Weisbecker wrote:
> On Thu, Nov 07, 2013 at 11:19:04PM +0100, Jan Kara wrote:
> > On Thu 07-11-13 23:13:39, Frederic Weisbecker wrote:
> > > But then, who's going to process that work if every CPUs is idle?
> > Have a look into irq_work_queue(). There is:
> > /*
> > * If the work is not "lazy" or the tick is stopped, raise the irq
> > * work interrupt (if supported by the arch), otherwise, just wait
> > * for the next tick. We do this even for unbound work to make sure
> > * *some* CPU will be doing the work.
> > */
> > if (!(work->flags & IRQ_WORK_LAZY) || tick_nohz_tick_stopped()) {
> > if (!this_cpu_cmpxchg(irq_work_raised, 0, 1))
> > arch_irq_work_raise();
> > }
> >
> > So we raise an interrupt if there would be no timer ticking (which is
> > what I suppose you mean by "CPU is idle"). That is nothing changed by my
> > patches...
>
> Ok but we raise that interrupt locally, not to the other CPUs.
True, but that doesn't really matter in this case. Any CPU (including the
local one) can handle the unbound work. So from the definition of the
unbound work things are OK.

Regarding my use for printk - if all (other) CPUs are idle then we can
easily afford making the current cpu busy printing, that's not a problem.
There's nothing else to do than to print what's remaining in the printk
buffer...

Honza
--
Jan Kara <jack@suse.cz>
SUSE Labs, CR
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/




== 3 of 8 ==
Date: Thurs, Nov 7 2013 3:10 pm
From: Jan Kara


On Thu 07-11-13 23:54:10, Frederic Weisbecker wrote:
> On Thu, Nov 07, 2013 at 11:50:34PM +0100, Jan Kara wrote:
> > On Thu 07-11-13 23:23:14, Frederic Weisbecker wrote:
> > > On Thu, Nov 07, 2013 at 11:19:04PM +0100, Jan Kara wrote:
> > > > On Thu 07-11-13 23:13:39, Frederic Weisbecker wrote:
> > > > > But then, who's going to process that work if every CPUs is idle?
> > > > Have a look into irq_work_queue(). There is:
> > > > /*
> > > > * If the work is not "lazy" or the tick is stopped, raise the irq
> > > > * work interrupt (if supported by the arch), otherwise, just wait
> > > > * for the next tick. We do this even for unbound work to make sure
> > > > * *some* CPU will be doing the work.
> > > > */
> > > > if (!(work->flags & IRQ_WORK_LAZY) || tick_nohz_tick_stopped()) {
> > > > if (!this_cpu_cmpxchg(irq_work_raised, 0, 1))
> > > > arch_irq_work_raise();
> > > > }
> > > >
> > > > So we raise an interrupt if there would be no timer ticking (which is
> > > > what I suppose you mean by "CPU is idle"). That is nothing changed by my
> > > > patches...
> > >
> > > Ok but we raise that interrupt locally, not to the other CPUs.
> > True, but that doesn't really matter in this case. Any CPU (including the
> > local one) can handle the unbound work. So from the definition of the
> > unbound work things are OK.
>
> I don't see how that can be ok. You want to offline a work because the
> local CPU can't handle it, right? If the local CPU can handle it you can
> just use local irq works.
>
> > Regarding my use for printk - if all (other) CPUs are idle then we can
> > easily afford making the current cpu busy printing, that's not a problem.
> > There's nothing else to do than to print what's remaining in the printk
> > buffer...
>
> So if the current CPU can handle it, what is the problem?
I hope this gets cleared out in my other email. But to make sure: If
other CPUs are idle (i.e. not appending to the printk buffer), we can well
handle the printing on the current CPU (with some breaks to allow
interrupts to be served etc.). If other CPUs are also appending to printk
buffer, that's where we really want to push the work to other CPUs.

Honza
--
Jan Kara <jack@suse.cz>
SUSE Labs, CR
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/




== 4 of 8 ==
Date: Thurs, Nov 7 2013 3:30 pm
From: Frederic Weisbecker


On Thu, Nov 07, 2013 at 11:57:33PM +0100, Jan Kara wrote:
> On Thu 07-11-13 23:43:52, Frederic Weisbecker wrote:
> > 2013/11/7 Jan Kara <jack@suse.cz>:
> > > A CPU can be caught in console_unlock() for a long time (tens of seconds
> > > are reported by our customers) when other CPUs are using printk heavily
> > > and serial console makes printing slow. Despite serial console drivers
> > > are calling touch_nmi_watchdog() this triggers softlockup warnings
> > > because interrupts are disabled for the whole time console_unlock() runs
> > > (e.g. vprintk() calls console_unlock() with interrupts disabled). Thus
> > > IPIs cannot be processed and other CPUs get stuck spinning in calls like
> > > smp_call_function_many(). Also RCU eventually starts reporting lockups.
> > >
> > > In my artifical testing I can also easily trigger a situation when disk
> > > disappears from the system apparently because interrupt from it wasn't
> > > served for too long. This is why just silencing watchdogs isn't a
> > > reliable solution to the problem and we simply have to avoid spending
> > > too long in console_unlock() with interrupts disabled.
> > >
> > > The solution this patch works toward is to postpone printing to a later
> > > moment / different CPU when we already printed over X characters in
> > > current console_unlock() invocation. This is a crude heuristic but
> > > measuring time we spent printing doesn't seem to be really viable - we
> > > cannot rely on high resolution time being available and with interrupts
> > > disabled jiffies are not updated. User can tune the value X via
> > > printk.offload_chars kernel parameter.
> > >
> > > Reviewed-by: Steven Rostedt <rostedt@goodmis.org>
> > > Signed-off-by: Jan Kara <jack@suse.cz>
> >
> > When a message takes tens of seconds to be printed, it usually means
> > we are in trouble somehow :)
> > I wonder what printk source can trigger such a high volume.
> Machines with tens of processors and thousands of scsi devices. When
> device discovery happens on boot, all processors are busily reporting new
> scsi devices and one poor looser is bound to do the printing for ever and
> ever until the machine dies...
>
> Or try running sysrq-t on a large machine with serial console connected. The
> machine will die because of lockups (although in this case I agree it is more
> of a problem of sysrq-t doing lots of printing in interrupt-disabled
> context).
>
> > May be cutting some huge message into smaller chunks could help? That
> > would re enable interrupts between each call.
> >
> > It's hard to tell without the context, but using other CPUs for
> > rescuing doesn't look like a good solution. What if the issue happens
> > in UP to begin with?
> The real trouble in practice is that while one cpu is doing printing,
> other cpus are appending to the printk buffer. So the cpu can be printing
> for a *long* time. So offloading the work to other cpus which are also
> appending messages seems as a fair thing to do.

Ok I see now.

But then this irq_work based solution won't work if, say, you run in full dynticks
mode. Also the hook on the timer interrupt is something that I wish we get rid
of on archs that can trigger self-IPIs.

Notwithstanding it's going to have scalibility issues as irq work then converges
to a single list for unbound works.

Offloading to a workqueue would be perhaps better, and writing to the serial
console could then be done with interrupts enabled, preemptible context, etc...

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/




== 5 of 8 ==
Date: Thurs, Nov 7 2013 3:40 pm
From: Steven Rostedt


On Fri, 8 Nov 2013 00:21:51 +0100
Frederic Weisbecker <fweisbec@gmail.com> wrote:

> Ok I see now.
>
> But then this irq_work based solution won't work if, say, you run in full dynticks
> mode. Also the hook on the timer interrupt is something that I wish we get rid
> of on archs that can trigger self-IPIs.

Do we really want that? What about users that use LAZY? That is, the
work isn't that important to trigger right now (added interrupt
expense).

>
> Notwithstanding it's going to have scalibility issues as irq work then converges
> to a single list for unbound works.

Well, it doesn't seem to be something that would be called often. All
CPUs checking a variable that is seldom changed should not have any
scalability issues. Unless of course it happens to share a cache line
with a variable that does change often.

>
> Offloading to a workqueue would be perhaps better, and writing to the serial
> console could then be done with interrupts enabled, preemptible context, etc...

Oh God no ;-) Adding workqueue logic into printk just spells a
nightmare of much more complexity for a critical kernel infrastructure.

-- Steve
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/




== 6 of 8 ==
Date: Thurs, Nov 7 2013 3:40 pm
From: Steven Rostedt


On Fri, 8 Nov 2013 00:01:11 +0100
Jan Kara <jack@suse.cz> wrote:

> On Thu 07-11-13 23:54:10, Frederic Weisbecker wrote:

> > So if the current CPU can handle it, what is the problem?
> I hope this gets cleared out in my other email. But to make sure: If
> other CPUs are idle (i.e. not appending to the printk buffer), we can well
> handle the printing on the current CPU (with some breaks to allow
> interrupts to be served etc.). If other CPUs are also appending to printk
> buffer, that's where we really want to push the work to other CPUs.

I guess the question is, how does it migrate? I guess that's not so
clear. Or do you just hope that the timer tick on another CPU will come
in first and finish the job for you? As the list is global and all CPUs
get to see it.

-- Steve
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/




== 7 of 8 ==
Date: Thurs, Nov 7 2013 3:50 pm
From: Frederic Weisbecker


On Thu, Nov 07, 2013 at 06:37:17PM -0500, Steven Rostedt wrote:
> On Fri, 8 Nov 2013 00:21:51 +0100
> Frederic Weisbecker <fweisbec@gmail.com> wrote:
> >
> > Offloading to a workqueue would be perhaps better, and writing to the serial
> > console could then be done with interrupts enabled, preemptible context, etc...
>
> Oh God no ;-) Adding workqueue logic into printk just spells a
> nightmare of much more complexity for a critical kernel infrastructure.

But yeah that's scary, that means workqueues itself can't printk that safely.
So, you're right after all.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/




== 8 of 8 ==
Date: Thurs, Nov 7 2013 3:50 pm
From: Frederic Weisbecker


On Thu, Nov 07, 2013 at 06:37:17PM -0500, Steven Rostedt wrote:
> On Fri, 8 Nov 2013 00:21:51 +0100
> Frederic Weisbecker <fweisbec@gmail.com> wrote:
>
> > Ok I see now.
> >
> > But then this irq_work based solution won't work if, say, you run in full dynticks
> > mode. Also the hook on the timer interrupt is something that I wish we get rid
> > of on archs that can trigger self-IPIs.
>
> Do we really want that? What about users that use LAZY? That is, the
> work isn't that important to trigger right now (added interrupt
> expense).

Ah right, I forgot that :)

>
> >
> > Notwithstanding it's going to have scalibility issues as irq work then converges
> > to a single list for unbound works.
>
> Well, it doesn't seem to be something that would be called often. All
> CPUs checking a variable that is seldom changed should not have any
> scalability issues. Unless of course it happens to share a cache line
> with a variable that does change often.

Right, if it was printk specific code I wouldn't mind much in fact. But having that in
such a globally available API like irq work make me feel uncomfortable.

>
> >
> > Offloading to a workqueue would be perhaps better, and writing to the serial
> > console could then be done with interrupts enabled, preemptible context, etc...
>
> Oh God no ;-) Adding workqueue logic into printk just spells a
> nightmare of much more complexity for a critical kernel infrastructure.

True, in fact I was thinking about raising an irq work that enqueues a workqueue, circumvoluted right? ;)

But it would make it safe, as irq work can be enqueued everywhere, and as it seems that
we can have high bandwidth of stuffs to print to the serial device, a process context looks much saner to me.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/





==============================================================================
TOPIC: staging: zsmalloc: Ensure handle is never 0 on success
http://groups.google.com/group/linux.kernel/t/56e7b28ac067dee2?hl=en
==============================================================================

== 1 of 1 ==
Date: Thurs, Nov 7 2013 3:00 pm
From: Olav Haugan


On 11/6/2013 7:06 PM, Greg KH wrote:
> On Wed, Nov 06, 2013 at 04:00:02PM -0800, Olav Haugan wrote:
>> On 11/5/2013 5:56 PM, Greg KH wrote:
>>> On Tue, Nov 05, 2013 at 04:54:12PM -0800, Olav Haugan wrote:
>>>> zsmalloc encodes a handle using the page pfn and an object
>>>> index. On some hardware platforms the pfn could be 0 and this
>>>> causes the encoded handle to be 0 which is interpreted as an
>>>> allocation failure.
>>>
>>> What platforms specifically have this issue?
>>
>> Currently some of Qualcomm SoC's have physical memory that starts at
>> address 0x0 which causes this problem.
>
> Then say this, and list the exact SoC's that can have this problem so
> people know how to evaluate the bugfix and see if it is relevant for
> their systems.
>
>> I believe this could be a problem
>> on any platforms if memory is configured to be starting at physical
>> address 0x0 for these platforms.
>
> Have you seen this be a problem? So it's just a theoretical issue at
> this point in time?

Yes, I can consistently reproduce it. It is not just theoretical.

Thanks,

Olav Haugan

--
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/





==============================================================================
TOPIC: kmod: Run usermodehelpers only on cpus allowed for kthreadd V2
http://groups.google.com/group/linux.kernel/t/a1700157e52e62ff?hl=en
==============================================================================

== 1 of 1 ==
Date: Thurs, Nov 7 2013 3:00 pm
From: Frederic Weisbecker


On Thu, Nov 07, 2013 at 04:43:11PM +0000, Christoph Lameter wrote:
> usermodehelper() threads can currently run on all processors.
> This is an issue for low latency cores. Spawnig a new thread causes
> cpu holdoffs in the range of hundreds of microseconds to a few
> milliseconds. Not good for cores on which processes run that need
> to react as fast as possible.
>
> kthreadd threads can be restricted using taskset to a limited set
> of processors. Then the kernel thread pool will not fork processes
> on those anymore thereby protecting those processors from additional
> latencies.
>
> Make usermodehelper() threads obey the limitations that kthreadd is
> restricted to. Kthreadd is not the parent of usermodehelper threads
> so we need to explicitly get the allowed processors for kthreadd.
>
> Before this patch there is no way to limit the cpus that usermodehelper
> can run on since the affinity is set when the thread is spawned to
> all processors.
>
> Signed-off-by: Christoph Lameter <cl@linux.com>
>
> Index: linux/include/linux/kthread.h
> ===================================================================
> --- linux.orig/include/linux/kthread.h 2013-11-07 10:31:46.667807582 -0600
> +++ linux/include/linux/kthread.h 2013-11-07 10:31:46.663807693 -0600
> @@ -51,6 +51,7 @@ void kthread_parkme(void);
> int kthreadd(void *unused);
> extern struct task_struct *kthreadd_task;
> extern int tsk_fork_get_node(struct task_struct *tsk);
> +void set_kthreadd_affinity(void);
>
> /*
> * Simple work processor based on kthread.
> Index: linux/kernel/kmod.c
> ===================================================================
> --- linux.orig/kernel/kmod.c 2013-11-07 10:31:46.667807582 -0600
> +++ linux/kernel/kmod.c 2013-11-07 10:35:28.825645008 -0600
> @@ -40,6 +40,7 @@
> #include <linux/ptrace.h>
> #include <linux/async.h>
> #include <asm/uaccess.h>
> +#include <linux/kthread.h>
>
> #include <trace/events/module.h>
>
> @@ -209,8 +210,13 @@ static int ____call_usermodehelper(void
> flush_signal_handlers(current, 1);
> spin_unlock_irq(&current->sighand->siglock);
>
> - /* We can run anywhere, unlike our parent keventd(). */
> - set_cpus_allowed_ptr(current, cpu_all_mask);
> + /*
> + * Kthreadd can be restricted to a set of processors if the user wants to
> + * protect other processors from OS latencies. If that has happened then
> + * we do not want to disturb the other processors here either so we start
> + * the usermode helper threads only on the processors allowed for kthreadd.
> + */
> + set_kthreadd_affinity();

I'm sorry to burden again on this but this looks too odd.

usermodehelper works are created via workqueues, right? And workqueues are an issue as
well for those who want CPU isolation.

So this looks like a more general problem than just call_usermodehelper.

Last time I checked, it seemed to me that this is an unbound workqueue? If so can't we tune
the affinity of this workqueue? If not perhaps that's something we want to solve and which
could be useful for all the users who don't want their CPU to be disturbed.

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/





==============================================================================
TOPIC: wire up CPU features to udev based module loading
http://groups.google.com/group/linux.kernel/t/7bd137213e7534f5?hl=en
==============================================================================

== 1 of 1 ==
Date: Thurs, Nov 7 2013 3:00 pm
From: "H. Peter Anvin"


On 11/07/2013 02:15 PM, Ard Biesheuvel wrote:
>
> That would involve repurposing/generalizing a bit more of the existing
> x86-only code than I did the first time around, but if you (as x86
> maintainers) are happy with that, I'm all for it.
>
> I do have a couple of questions then
> - the module aliases host tool has no arch specific dependencies at
> all except having x86cpu as one of the entries: would you mind
> dropping the x86 prefix there? Or rather add dependencies on $ARCH?
> (If we drop it there, we basically end up with 'cpu:' everywhere)

I think it makes sense to indicate what kind of CPU the string refers
to, as the top-level indicator of what is going on. This might be
possible to macroize the generation of this prefix, though.

> - in the vendor/family/model case, it may be preferable to drop these
> fields entirely from certain modules' aliases if they match on 'any'
> (provided that the module tools permit this) rather than add
> architecture, variant, revision, etc fields for all architectures if
> they can only ever match on one

I think that can be CPU dependent.

> - some of the X86_ macros would probable be redefined in terms of the
> generic macros rather than the other way around, which would result in
> some changes under arch/x86 as well, is that acceptable for you?

If you are talking about X86_FEATURE_* then almost certainly no,
although I'm willing to listen to what you have in mind.

-hpa


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/





==============================================================================
TOPIC: Introducing Device Tree Overlays
http://groups.google.com/group/linux.kernel/t/7c26bee9afcc62d1?hl=en
==============================================================================

== 1 of 2 ==
Date: Thurs, Nov 7 2013 3:10 pm
From: Guenter Roeck


On Thu, Nov 07, 2013 at 09:46:26PM +0100, Sebastian Andrzej Siewior wrote:
> On 07.11.13, Pantelis Antoniou wrote:
> > Hi Sebastian,
> Hi Pantelis,
>
> > FWIW DT has been ported to x86. And is present on arm/powerpc/mips/arc and possibly
> > others.
>
> Yes, I know. I am the one that did the work for CE4100, the first one
> that boots with DT on x86.
>
> > So what are we talking about again? If you care about the non-DT case, why
> > don't you make a patch about how you could support Guenter's use case on
> > the x86.
>
> I am only saying that this "hot-plug a device at a non hot-plugagle bus at
> runtime" is not limited to DT but this solution is. X86 + ACPI is not
> the only limitation. ARM is (forced) going to ACPI as well as far I
> know. And this solution is limited to DT. This is what I am pointing
> out.
>
I can't tell about ARM, but I am not entirely sure how ACPI support on ARM
is going to help us on powerpc.

> > His use case is not uncommon, believe it or not, and x86 would benefit from
> > something this flexible.
>
> I *think* a more flexible solution would be something like bus_type which is
> exposed via configfs. It would be attached behind a certain device/bus where
> the "physical" hotplug interface is. The user would then be able to read the
> configuration based on whatever information he has and could then create
> devices he likes at runtime. This wouldn't depend much on the firmware that is
> used but would require a little more work I think.
>
Quite frankly, I am interested at a solution that works and solves our problems.
I am not looking for something that is 100% perfect and may never be delivered.

Fortunately, the Linux kernel was willing to adopt multiple different file
systems, and still accepts new ones on a regular basis. If a new file system
is better, it will start getting used, and old file systems are being phased out
as fewer people use them. I would hope the same should be possible with DT
overlays and possible other future solutions for the same problem, and that
we won't have to wait for the perfect solution from day 1.

Guenter
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/




== 2 of 2 ==
Date: Thurs, Nov 7 2013 3:40 pm
From: delicious quinoa


On Tue, Nov 5, 2013 at 12:41 PM, Pantelis Antoniou
<panto@antoniou-consulting.com> wrote:
> +
> + pr_info("%s: Applied #%d overlay segments @%d\n", __func__,
> + od->ovinfo_cnt, od->id);
> +

This could be pr_debug so that we get normally get silence unless
something fails, like insmod.

Also please take out the '#'.

We see it working with our fpga ip driver and it looks really great.

Alan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/





==============================================================================
TOPIC: dmaengine: add msm bam dma driver
http://groups.google.com/group/linux.kernel/t/eb7ef92f0916cf63?hl=en
==============================================================================

== 1 of 1 ==
Date: Thurs, Nov 7 2013 3:10 pm
From: Andy Gross


On Thu, Oct 31, 2013 at 04:46:21PM -0500, Andy Gross wrote:
> On Thu, Oct 31, 2013 at 10:29:48PM +0530, Vinod Koul wrote:
> > On Fri, Oct 25, 2013 at 03:24:02PM -0500, Andy Gross wrote:
> >
> > This should be sent to dmaengine@vger.kernel.org
>
> I'll add the list when I send the second iteration or should I send it over mid
> stream?
>
> > > Add the DMA engine driver for the MSM Bus Access Manager (BAM) DMA controller
> > > found in the MSM 8x74 platforms.
> > >
> > > Each BAM DMA device is associated with a specific on-chip peripheral. Each
> > > channel provides a uni-directional data transfer engine that is capable of
> > > transferring data between the peripheral and system memory (System mode) or
> > > between two peripherals (BAM2BAM).
> > >
> > > The initial release of this driver only supports slave transfers between
> > > peripherals and system memory.
> > >
> > > Signed-off-by: Andy Gross <agross@codeaurora.org>
> > > +/*
> > > + * bam_alloc_chan - Allocate channel resources for DMA channel.
> > > + * @chan: specified channel
> > > + *
> > > + * This function allocates the FIFO descriptor memory and resets the channel
> > > + */
> > > +static int bam_alloc_chan(struct dma_chan *chan)
> > > +{
> > > + struct bam_chan *bchan = to_bam_chan(chan);
> > > + struct bam_device *bdev = bchan->device;
> > > + u32 val;
> > > + union bam_pipe_ctrl pctrl;
> > > +
> > > + /* check for channel activity */
> > > + pctrl.value = ioread32(bdev->regs + BAM_P_CTRL(bchan->id));
> > > + if (pctrl.bits.p_en) {
> > > + dev_err(bdev->dev, "channel already active\n");
> > > + return -EINVAL;
> > > + }
> > > +
> > > + /* allocate FIFO descriptor space */
> > > + bchan->fifo_virt = (struct bam_desc_hw *)dma_alloc_coherent(bdev->dev,
> > > + BAM_DESC_FIFO_SIZE, &bchan->fifo_phys,
> > > + GFP_KERNEL);
> > > +
> > > + if (!bchan->fifo_virt) {
> > > + dev_err(bdev->dev, "Failed to allocate descriptor fifo\n");
> > > + return -ENOMEM;
> > > + }
> > > +
> > > + /* reset channel */
> > > + iowrite32(1, bdev->regs + BAM_P_RST(bchan->id));
> > > + iowrite32(0, bdev->regs + BAM_P_RST(bchan->id));
> > > +
> > > + /* configure fifo address/size in bam channel registers */
> > > + iowrite32(bchan->fifo_phys, bdev->regs +
> > > + BAM_P_DESC_FIFO_ADDR(bchan->id));
> > > + iowrite32(BAM_DESC_FIFO_SIZE, bdev->regs +
> > > + BAM_P_FIFO_SIZES(bchan->id));
> > > +
> > > + /* unmask and enable interrupts for defined EE, bam and error irqs */
> > > + iowrite32(BAM_IRQ_MSK, bdev->regs + BAM_IRQ_SRCS_EE(bchan->ee));
> > > +
> > > + /* enable the per pipe interrupts, enable EOT and INT irqs */
> > > + iowrite32(P_DEFAULT_IRQS_EN, bdev->regs + BAM_P_IRQ_EN(bchan->id));
> > > +
> > > + /* unmask the specific pipe and EE combo */
> > > + val = ioread32(bdev->regs + BAM_IRQ_SRCS_MSK_EE(bchan->ee));
> > > + val |= 1 << bchan->id;
> > > + iowrite32(val, bdev->regs + BAM_IRQ_SRCS_MSK_EE(bchan->ee));
> > > +
> > > + /* set fixed direction and mode, then enable channel */
> > I was going to question why you are doing hw specfic stuff in alloc channel but
> > now why do you enable seems to be a bigger question in mind?
>
> The fifo_virt is used to store the hardware descriptors that are used directly
> by the dma controller. I have to at least fill in the descriptor FIFO address
> and size and reset the channel to clear the fifo offset inside the hardware.
> After reset the internal fifo offset is 0. And every subsequent transaction
> increments this. That is how it knows which descriptors to work on inside the
> descriptor fifo memory.
>
> I can definitely defer the rest of hte h/w interactions until the point that I
> need to actually kick off the dma controller.
>
>
> > > + pctrl.value = 0;
> > > + pctrl.bits.p_direction =
> > > + (bchan->bam_slave.slave.direction == DMA_DEV_TO_MEM) ?
> > > + BAM_PIPE_PRODUCER : BAM_PIPE_CONSUMER;
> > > + pctrl.bits.p_sys_mode = BAM_PIPE_MODE_SYSTEM;
> > > + pctrl.bits.p_en = 1;
> > > +
> > > + iowrite32(pctrl.value, bdev->regs + BAM_P_CTRL(bchan->id));
> > > +
> > > + /* set desc threshold */
> > > + /* do bookkeeping for tracking used EEs, used during IRQ handling */
> > > + set_bit(bchan->ee, &bdev->enabled_ees);
> > > +
> > > + bchan->head = 0;
> > > + bchan->tail = 0;
> > > +
> > > + return 0;
> > You said you are going to allocate descriptors so right thing would be return
> > number of allocated desc here!
>
> OK, I missed that.
>
> > > +}
> > > +
> > > +/*
> > > + * bam_free_chan - Frees dma resources associated with specific channel
> > > + * @chan: specified channel
> > > + *
> > > + * Free the allocated fifo descriptor memory and channel resources
> > > + *
> > > + */
> > > +static void bam_free_chan(struct dma_chan *chan)
> > > +{
> > > + struct bam_chan *bchan = to_bam_chan(chan);
> > > + struct bam_device *bdev = bchan->device;
> > > + u32 val;
> > > +
> > Shouldn't you check if channel is busy?
> >
>
> Yes, I'll add that in. With no return code, how useful is this to the caller?
>
>
> > > + /* reset channel */
> > > + iowrite32(1, bdev->regs + BAM_P_RST(bchan->id));
> > > + iowrite32(0, bdev->regs + BAM_P_RST(bchan->id));
> > > +
> > > + dma_free_coherent(bdev->dev, BAM_DESC_FIFO_SIZE, bchan->fifo_virt,
> > > + bchan->fifo_phys);
> > > +
> > > + /* mask irq for pipe/channel */
> > > + val = ioread32(bdev->regs + BAM_IRQ_SRCS_MSK_EE(bchan->ee));
> > > + val &= ~(1 << bchan->id);
> > > + iowrite32(val, bdev->regs + BAM_IRQ_SRCS_MSK_EE(bchan->ee));
> > > +
> > > + /* disable irq */
> > > + iowrite32(0, bdev->regs + BAM_P_IRQ_EN(bchan->id));
> > > +
> > > + clear_bit(bchan->ee, &bdev->enabled_ees);
> > > +}
> > > +
> > > +/*
> > > + * bam_slave_config - set slave configuration for channel
> > > + * @chan: dma channel
> > > + * @cfg: slave configuration
> > > + *
> > > + * Sets slave configuration for channel
> > > + * Only allow setting direction once. BAM channels are unidirectional
> > > + * and the direction is set in hardware.
> > > + *
> > > + */
> > > +static void bam_slave_config(struct bam_chan *bchan,
> > > + struct bam_dma_slave_config *bcfg)
> >
> > > +{
> > > + struct bam_device *bdev = bchan->device;
> > > +
> > > + bchan->bam_slave.desc_threshold = bcfg->desc_threshold;
> > what does the desc_threshold mean?
>
> The desc threshhold determines the minimum number of bytes of descriptor that
> causes a write event to be communicated to the peripheral. I default to 4 bytes
> (1 descriptor), but this is configurable through the DMA_SLAVE_CONFIG interface
> using the extended slave_config structure.
>
> > > +
> > > + /* set desc threshold */
> > > + iowrite32(bcfg->desc_threshold, bdev->regs + BAM_DESC_CNT_TRSHLD);
> > > +}
> > > +
> > > +/*
> > > + * bam_start_dma - loads up descriptors and starts dma
> > > + * @chan: dma channel
> > > + *
> > > + * Loads descriptors into descriptor fifo and starts dma controller
> > > + *
> > > + * NOTE: Must hold channel lock
> > > +*/
> > > +static void bam_start_dma(struct bam_chan *bchan)
> > > +{
> > > + struct bam_device *bdev = bchan->device;
> > > + struct bam_async_desc *async_desc, *_adesc;
> > > + u32 curr_len, val;
> > > + u32 num_processed = 0;
> > > +
> > > + if (list_empty(&bchan->pending))
> > > + return;
> > > +
> > > + curr_len = (bchan->head <= bchan->tail) ?
> > > + bchan->tail - bchan->head :
> > > + MAX_DESCRIPTORS - bchan->head + bchan->tail;
> > > +
> > > + list_for_each_entry_safe(async_desc, _adesc, &bchan->pending, node) {
> > > +
> > > + /* bust out if we are out of room */
> > > + if (async_desc->num_desc + curr_len > MAX_DESCRIPTORS)
> > > + break;
> > > +
> > > + /* copy descriptors into fifo */
> > > + if (bchan->tail + async_desc->num_desc > MAX_DESCRIPTORS) {
> > > + u32 partial = MAX_DESCRIPTORS - bchan->tail;
> > > +
> > > + memcpy(&bchan->fifo_virt[bchan->tail], async_desc->desc,
> > > + partial * sizeof(struct bam_desc_hw));
> > > + memcpy(bchan->fifo_virt, &async_desc->desc[partial],
> > > + (async_desc->num_desc - partial) *
> > > + sizeof(struct bam_desc_hw));
> > memcpy for device memory copy?
>
> My initial thought was that I needed to wait until now to fill in the
> descriptors on the fifo that was allocated. The fifo memory is accessed from
> the dma controller. The controller just manages an internal offset that rolls
> over based on the size of the configured fifo memory. I was keeping the
> descriptors on hand until I needed to program them into the fifo memory, hence
> the copy.
>
> > > + } else {
> > > + memcpy(&bchan->fifo_virt[bchan->tail], async_desc->desc,
> > > + async_desc->num_desc *
> > > + sizeof(struct bam_desc_hw));
> > > + }
> > > +
> > > + num_processed++;
> > > + bchan->tail += async_desc->num_desc;
> > > + bchan->tail %= MAX_DESCRIPTORS;
> > > + curr_len += async_desc->num_desc;
> > > +
> > > + list_move_tail(&async_desc->node, &bchan->active);
> > > + }
> > > +
> > > + /* bail if we didn't queue anything to the active queue */
> > > + if (!num_processed)
> > > + return;
> > > +
> > > + async_desc = list_first_entry(&bchan->active, struct bam_async_desc,
> > > + node);
> > > +
> > > + val = ioread32(bdev->regs + BAM_P_SW_OFSTS(bchan->id));
> > > + val &= P_SW_OFSTS_MASK;
> > > +
> > > + /* kick off dma by forcing a write event to the pipe */
> > > + iowrite32((bchan->tail * sizeof(struct bam_desc_hw)),
> > > + bdev->regs + BAM_P_EVNT_REG(bchan->id));
> > > +}
> > > +
> > > +/*
> > > + * bam_tx_submit - Adds transaction to channel pending queue
> > > + * @tx: transaction to queue
> > > + *
> > > + * Adds dma transaction to pending queue for channel
> > > + *
> > > +*/
> > > +static dma_cookie_t bam_tx_submit(struct dma_async_tx_descriptor *tx)
> > > +{
> > > + struct bam_chan *bchan = to_bam_chan(tx->chan);
> > > + struct bam_async_desc *desc = to_bam_async_desc(tx);
> > > + dma_cookie_t cookie;
> > > +
> > > + spin_lock_bh(&bchan->lock);
> > > +
> > > + cookie = dma_cookie_assign(tx);
> > > + list_add_tail(&desc->node, &bchan->pending);
> > > +
> > > + spin_unlock_bh(&bchan->lock);
> > > +
> > > + return cookie;
> > > +}
> > Okay you are *NOT* using virt-dma layer, all this can be removed if you do that,
> > this is similar to what vchan_tx_submit() does!
> >
>
> I'll look into reworking and utilizing the virt-dma layer.
>

Is it acceptable to pick and choose the pieces of the virt-dma layer that
benefit me? Or do I have to absorb all of it. The smaller helper structs and
functions are fine, but in some cases they force a certain implementation.

The bam_tx_submit and perhaps the cleanup functions are about the only pieces
I'd be able to leverage from the virt-dma, aside from the structures.

The main issue with the rest of the code is that it doesn't fit the behavior of
my dma controller. Because i have a fixed sized circular buffer for my
descriptor FIFO, I have to copy in the new descriptors before I start up the
dma channel.

Using the vchan_cookie_complete() forces me to start the next transaction within
the interrupt, or schedule another tasklet to do that work for me. The overhead
for copying what could be a large number of descriptors into the FIFO might
introduce too much latency inside the IRQ handler (especially since this is done
to non-cached memory). Using another tasklet for just restarting the dma
controller seems klunky to me.

I would rather start the next transaction within the tasklet queued from the IRQ
(vchan_cookie_complete), but because it uses it's own tasklet, I wouldn't be
able to leverage that.

The vchan_cookie_complete() usage within the IRQ handler also implys that only 1
dma transaction is completed per IRQ. That might not be the case for me,
because I can advance the DMA FIFO offset to tell the controller to keep going
to the next transaction. By the time I get to servicing the IRQ, I might have
completed more than 1 transaction. I suppose you could call
vchan_cookie_complete() multiple times, but that seems wrong to me due to the
multiple calls to tasklet_schedule.


> > > +
> > > +/*
> > > + * bam_prep_slave_sg - Prep slave sg transaction
> > > + *
> > > + * @chan: dma channel
> > > + * @sgl: scatter gather list
> > > + * @sg_len: length of sg
> > > + * @direction: DMA transfer direction
> > > + * @flags: DMA flags
> > > + * @context: transfer context (unused)
> > > + */
> > > +static struct dma_async_tx_descriptor *bam_prep_slave_sg(struct dma_chan *chan,
> > > + struct scatterlist *sgl, unsigned int sg_len,
> > > + enum dma_transfer_direction direction, unsigned long flags,
> > > + void *context)
> > > +{
> > > + struct bam_chan *bchan = to_bam_chan(chan);
> > > + struct bam_device *bdev = bchan->device;
> > > + struct bam_async_desc *async_desc = NULL;
> > > + struct scatterlist *sg;
> > > + u32 i;
> > > + struct bam_desc_hw *desc;
> > > +
> > > +
> > > + if (!is_slave_direction(direction)) {
> > > + dev_err(bdev->dev, "invalid dma direction\n");
> > > + goto err_out;
> > > + }
> > > +
> > > + /* direction has to match pipe configuration from the slave config */
> > > + if (direction != bchan->bam_slave.slave.direction) {
> > > + dev_err(bdev->dev,
> > > + "trans does not match channel configuration\n");
> > > + goto err_out;
> > > + }
> > > +
> > > + /* make sure number of descriptors will fit within the fifo */
> > > + if (sg_len > MAX_DESCRIPTORS) {
> > > + dev_err(bdev->dev, "not enough fifo descriptor space\n");
> > > + goto err_out;
> > > + }
> > what prevents you from assigning more virtual descriptors to this case and then
> > submit those after HW descriptors are done.
>
> I hadn't considered the virtual descriptors. I'll see what I can do to utilize
> that and rework this. I can see the virtual descriptors simplifying this a good
> bit.
>
> > > +
> > > + /* allocate enough room to accomodate the number of entries */
> > > + async_desc = kzalloc(sizeof(*async_desc) +
> > > + (sg_len * sizeof(struct bam_desc_hw)), GFP_KERNEL);
> > this cna be called from non sleepy context and the recommedation is to use
> > GFP_NOWAIT for memory allocation
> >
>
> I missed this. I'll change it.
>
> > > +static int bam_control(struct dma_chan *chan, enum dma_ctrl_cmd cmd,
> > > + unsigned long arg)
> > > +{
> > > + struct bam_chan *bchan = to_bam_chan(chan);
> > > + struct bam_device *bdev = bchan->device;
> > > + struct bam_dma_slave_config *bconfig;
> > > + int ret = 0;
> > > +
> > > + switch (cmd) {
> > > + case DMA_PAUSE:
> > > + spin_lock_bh(&bchan->lock);
> > > + iowrite32(1, bdev->regs + BAM_P_HALT(bchan->id));
> > > + spin_unlock_bh(&bchan->lock);
> > > + break;
> > > + case DMA_RESUME:
> > > + spin_lock_bh(&bchan->lock);
> > > + iowrite32(0, bdev->regs + BAM_P_HALT(bchan->id));
> > > + spin_unlock_bh(&bchan->lock);
> > > + break;
> > > + case DMA_TERMINATE_ALL:
> > > + bam_dma_terminate_all(chan);
> > > + break;
> > > + case DMA_SLAVE_CONFIG:
> > > + bconfig = (struct bam_dma_slave_config *)arg;
> > > + bam_slave_config(bchan, bconfig);
> > DMA_SLAVE_CONFIG expects arg as struct dma_slave_config, not this. Pls dont
> > voilate APIs
>
> So for extended slave_config structures, the caller needs to send in a ptr to
> the dma_slave_config and then I can determine the bam_dma_slave_config. Will
> fix.
>
> > > + break;
> > > + default:
> > > + ret = -EIO;
> > I would expect -ENXIO here!
> >
>
> OK.
>
> > > + break;
> > > + }
> > > +
> > > + return ret;
> > > +}
> > > +
> > > +/*
> > > + * bam_tx_status - returns status of transaction
> > > + * @chan: dma channel
> > > + * @cookie: transaction cookie
> > > + * @txstate: DMA transaction state
> > > + *
> > > + * Return status of dma transaction
> > > + */
> > > +static enum dma_status bam_tx_status(struct dma_chan *chan, dma_cookie_t cookie,
> > > + struct dma_tx_state *txstate)
> > > +{
> > > + return dma_cookie_status(chan, cookie, txstate);
> > hmmm, this wont work in many cases for slave. For example if you try to get this
> > working with audio you need to provide the residue values.
> > The results you are providing here will not be useful, so I would recommedn
> > fixing it
> >
>
> Ok so in this case I'd need to read where I am in the descriptor chain and
> calculate the residual. That shouldn't be a problem.
>
> > > +
> > > +static int bam_dma_probe(struct platform_device *pdev)
> > > +{
> > > + struct bam_device *bdev;
> > > + int err, i;
> > > +
> > > + bdev = kzalloc(sizeof(*bdev), GFP_KERNEL);
> > devm_ pls
> >
>
> will fix.
>
> > > + if (!bdev) {
> > > + dev_err(&pdev->dev, "insufficient memory for private data\n");
> > > + err = -ENOMEM;
> > > + goto err_no_bdev;
> > > + }
> > > +
> > > + bdev->dev = &pdev->dev;
> > > + dev_set_drvdata(bdev->dev, bdev);
> > > +
> > > + bdev->regs = of_iomap(pdev->dev.of_node, 0);
> > > + if (!bdev->regs) {
> > > + dev_err(bdev->dev, "unable to ioremap base\n");
> > > + err = -ENOMEM;
> > > + goto err_free_bamdev;
> > > + }
> > > +
> > > + bdev->irq = irq_of_parse_and_map(pdev->dev.of_node, 0);
> > > + if (bdev->irq == NO_IRQ) {
> > > + dev_err(bdev->dev, "unable to map irq\n");
> > > + err = -EINVAL;
> > > + goto err_unmap_mem;
> > > + }
> > > +
> > > + bdev->bamclk = devm_clk_get(bdev->dev, "bam_clk");
> > > + if (IS_ERR(bdev->bamclk)) {
> > > + err = PTR_ERR(bdev->bamclk);
> > > + goto err_free_irq;
> > > + }
> > > +
> > > + err = clk_prepare_enable(bdev->bamclk);
> > > + if (err) {
> > > + dev_err(bdev->dev, "failed to prepare/enable clock");
> > > + goto err_free_irq;
> > > + }
> > > +
> > > + err = request_irq(bdev->irq, &bam_dma_irq, IRQF_TRIGGER_HIGH, "bam_dma",
> > > + bdev);
> > > + if (err) {
> > > + dev_err(bdev->dev, "error requesting irq\n");
> > > + err = -EINVAL;
> > > + goto err_disable_clk;
> > > + }
> > > +
> > > + if (bam_init(bdev)) {
> > > + dev_err(bdev->dev, "cannot initialize bam device\n");
> > > + err = -EINVAL;
> > > + goto err_disable_clk;
> > > + }
> > > +
> > > + bdev->channels = kzalloc(sizeof(*bdev->channels) * bdev->num_channels,
> > > + GFP_KERNEL);
> > > +
> > > + if (!bdev->channels) {
> > > + dev_err(bdev->dev, "unable to allocate channels\n");
> > > + err = -ENOMEM;
> > > + goto err_disable_clk;
> > > + }
> > > +
> > > + /* allocate and initialize channels */
> > > + INIT_LIST_HEAD(&bdev->common.channels);
> > > +
> > > + for (i = 0; i < bdev->num_channels; i++)
> > > + bam_channel_init(bdev, &bdev->channels[i], i);
> > > +
> > > + /* set max dma segment size */
> > > + bdev->common.dev = bdev->dev;
> > > + bdev->common.dev->dma_parms = &bdev->dma_parms;
> > > + if (dma_set_max_seg_size(bdev->common.dev, BAM_MAX_DATA_SIZE)) {
> > > + dev_err(bdev->dev, "cannot set maximum segment size\n");
> > > + goto err_disable_clk;
> > > + }
> > > +
> > > + /* set capabilities */
> > > + dma_cap_zero(bdev->common.cap_mask);
> > > + dma_cap_set(DMA_SLAVE, bdev->common.cap_mask);
> > > +
> > > + /* initialize dmaengine apis */
> > > + bdev->common.device_alloc_chan_resources = bam_alloc_chan;
> > > + bdev->common.device_free_chan_resources = bam_free_chan;
> > > + bdev->common.device_prep_slave_sg = bam_prep_slave_sg;
> > > + bdev->common.device_control = bam_control;
> > > + bdev->common.device_issue_pending = bam_issue_pending;
> > > + bdev->common.device_tx_status = bam_tx_status;
> > > + bdev->common.dev = bdev->dev;
> > > +
> > > + dma_async_device_register(&bdev->common);
> > > +
> > > + if (pdev->dev.of_node) {
> > > + err = of_dma_controller_register(pdev->dev.of_node,
> > > + bam_dma_xlate, &bdev->common);
> > > +
> > > + if (err) {
> > > + dev_err(bdev->dev, "failed to register of_dma\n");
> > > + goto err_unregister_dma;
> > > + }
> > > + }
> > > +
> > > + return 0;
> > > +
> > > +err_unregister_dma:
> > > + dma_async_device_unregister(&bdev->common);
> > > +err_free_irq:
> > > + free_irq(bdev->irq, bdev);
> > > +err_disable_clk:
> > > + clk_disable_unprepare(bdev->bamclk);
> > > +err_unmap_mem:
> > > + iounmap(bdev->regs);
> > > +err_free_bamdev:
> > > + if (bdev)
> > > + kfree(bdev->channels);
> > > + kfree(bdev);
> > > +err_no_bdev:
> > you need to get yourslef introduced with devm_ friends to ease this part!
> >
> > Overall I think driver needs to bit more plumbing and also needs to use the
> > virt-dma so that bunch of work already done is not redefined here.
>
> I'll rework for comments and see how I can incorporate the virt-dma.
>
> --
> sent by an employee of the Qualcomm Innovation Center, Inc.
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> hosted by The Linux Foundation
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

--
sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/





==============================================================================
TOPIC: ext4: Fix reading of extended tv_sec (bug 23732)
http://groups.google.com/group/linux.kernel/t/1c061384ec63b78b?hl=en
==============================================================================

== 1 of 2 ==
Date: Thurs, Nov 7 2013 3:20 pm
From: Jan Kara


On Thu 07-11-13 17:54:24, David Turner wrote:
> On Thu, 2013-11-07 at 17:03 +0100, Jan Kara wrote:
> > So I'm somewhat wondering: Previously we decoded tv_nsec regardless of
> > tv_sec size. After your patch we do it only if sizeof(time->tv_sec) > 4. Is
> > this an intended change? Why is it OK?
>
> This is an error. Here is a corrected version of the patch.
>
>
> --
>
> In ext4, the bottom two bits of {a,c,m}time_extra are used to extend
> the {a,c,m}time fields, deferring the year 2038 problem to the year
> 2446. The representation (which this patch does not alter) is a bit
> hackish, in that the most-significant bit is no longer (alone)
> sufficient to indicate the sign. That's because we're representing an
> asymmetric range, with seven times as many positive values as
> negative.
>
> When decoding these extended fields, for times whose bottom 32 bits
> would represent a negative number, sign extension causes the 64-bit
> extended timestamp to be negative as well, which is not what's
> intended. This patch corrects that issue, so that the only negative
> {a,c,m}times are those between 1901 and 1970 (as per 32-bit signed
> timestamps).
>
> Signed-off-by: David Turner <novalis@novalis.org>
> Reported-by: Mark Harris <mh8928@yahoo.com>
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=23732
> ---
> fs/ext4/ext4.h | 13 +++++++++----
> 1 file changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index af815ea..3c2d0b3 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -722,10 +722,15 @@ static inline __le32 ext4_encode_extra_time(struct timespec *time)
>
> static inline void ext4_decode_extra_time(struct timespec *time, __le32 extra)
> {
> - if (sizeof(time->tv_sec) > 4)
> - time->tv_sec |= (__u64)(le32_to_cpu(extra) & EXT4_EPOCH_MASK)
> - << 32;
> - time->tv_nsec = (le32_to_cpu(extra) & EXT4_NSEC_MASK) >> EXT4_EPOCH_BITS;
> + if (sizeof(time->tv_sec) > 4) {
> + u64 extra_bits = (__u64)(le32_to_cpu(extra) & EXT4_EPOCH_MASK);
^^^^
Still unnecessary type cast here (but that's a cosmetic issue).

Otherwise the patch looks good. You can add:
Reviewed-by: Jan Kara <jack@suse.cz>

Honza

> + if (time->tv_sec > 0 || extra_bits != EXT4_EPOCH_MASK) {
> + time->tv_sec &= 0xFFFFFFFF;
> + time->tv_sec |= extra_bits << 32;
> + }
> + }
> + time->tv_nsec = (le32_to_cpu(extra) & EXT4_NSEC_MASK) >>
> + EXT4_EPOCH_BITS;
> }
>
> #define EXT4_INODE_SET_XTIME(xtime, inode, raw_inode) \
> --
> 1.8.1.2
>
>
>
--
Jan Kara <jack@suse.cz>
SUSE Labs, CR
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/




== 2 of 2 ==
Date: Thurs, Nov 7 2013 3:30 pm
From: David Turner


On Fri, 2013-11-08 at 00:14 +0100, Jan Kara wrote:
> Still unnecessary type cast here (but that's a cosmetic issue).
...
> Otherwise the patch looks good. You can add:
> Reviewed-by: Jan Kara <jack@suse.cz>

Thanks. A version with this correction and the reviewed-by follows.

--
In ext4, the bottom two bits of {a,c,m}time_extra are used to extend
the {a,c,m}time fields, deferring the year 2038 problem to the year
2446. The representation (which this patch does not alter) is a bit
hackish, in that the most-significant bit is no longer (alone)
sufficient to indicate the sign. That's because we're representing an
asymmetric range, with seven times as many positive values as
negative.

When decoding these extended fields, for times whose bottom 32 bits
would represent a negative number, sign extension causes the 64-bit
extended timestamp to be negative as well, which is not what's
intended. This patch corrects that issue, so that the only negative
{a,c,m}times are those between 1901 and 1970 (as per 32-bit signed
timestamps).

Signed-off-by: David Turner <novalis@novalis.org>
Reported-by: Mark Harris <mh8928@yahoo.com>
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=23732
Reviewed-by
: Jan Kara <jack@suse.cz>
---
fs/ext4/ext4.h | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index af815ea..3c2d0b3 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -722,10 +722,15 @@ static inline __le32 ext4_encode_extra_time(struct timespec *time)

static inline void ext4_decode_extra_time(struct timespec *time, __le32 extra)
{
- if (sizeof(time->tv_sec) > 4)
- time->tv_sec |= (__u64)(le32_to_cpu(extra) & EXT4_EPOCH_MASK)
- << 32;
- time->tv_nsec = (le32_to_cpu(extra) & EXT4_NSEC_MASK) >> EXT4_EPOCH_BITS;
+ if (sizeof(time->tv_sec) > 4) {
+ u64 extra_bits = le32_to_cpu(extra) & EXT4_EPOCH_MASK;
+ if (time->tv_sec > 0 || extra_bits != EXT4_EPOCH_MASK) {
+ time->tv_sec &= 0xFFFFFFFF;
+ time->tv_sec |= extra_bits << 32;
+ }
+ }
+ time->tv_nsec = (le32_to_cpu(extra) & EXT4_NSEC_MASK) >>
+ EXT4_EPOCH_BITS;
}

#define EXT4_INODE_SET_XTIME(xtime, inode, raw_inode) \
--
1.8.1.2



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/





==============================================================================
TOPIC: linux-next: reminder
http://groups.google.com/group/linux.kernel/t/6baf83471f093f73?hl=en
==============================================================================

== 1 of 1 ==
Date: Thurs, Nov 7 2013 3:30 pm
From: Stephen Rothwell


Hi all,

Just a reminder to *not* add any v3.14 material to linux-next until after
v3.13-rc1 has been released.

Also, please resist the temptation to rebase your trees before sending
them upstream.

--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au





==============================================================================
TOPIC: common location for devicetree files
http://groups.google.com/group/linux.kernel/t/036feb035f29aa02?hl=en
==============================================================================

== 1 of 1 ==
Date: Thurs, Nov 7 2013 3:30 pm
From: Kumar Gala


As we start having more sharing of device trees between architectures (arm & arm64, arm & powerpc, guessing maybe mips & arm) we need dts to live in location that

I was wondering what people felt about doing:

arch/dts/<VENDOR>/

as a common location that could be shared. I'm up for other suggestions.

- k

--
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/





==============================================================================
TOPIC: Embeddable Position Independent Executable
http://groups.google.com/group/linux.kernel/t/5e545fba8eaa67b6?hl=en
==============================================================================

== 1 of 1 ==
Date: Thurs, Nov 7 2013 3:30 pm
From: Kevin Hilman


On Tue, Sep 17, 2013 at 5:43 AM, Russ Dill <Russ.Dill@ti.com> wrote:
> This patch adds support for and demonstrates the usage of an embedded
> position independent executable (PIE). The goal is to allow the use of C
> code in situations where carefully written position independent assembly
> was previously required.

Hmm, I thought I had already replied to this but it appears that I haven't.

I really like this approach and think it's a major improvement on what
we're already doing on OMAP and a few other ARM platforms. This
approach is done in a way that's significantly easier to maintain and
extend IMO.

As a next step, I think you should drop the RFC and split it up into a
few series that can go via the right trees:

- asm-generic and lib/devres (patches 1, 2, 4)
- drivers/sram (patch 3)
- generic PIE: (patch 5)
- core PIE: (patch 5-8)
- AM33xx support (patch 9-11)

Kevin
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/





==============================================================================
TOPIC: ARM: at91: move to common clk framework
http://groups.google.com/group/linux.kernel/t/2487628f16b80970?hl=en
==============================================================================

== 1 of 1 ==
Date: Thurs, Nov 7 2013 3:30 pm
From: Mike Turquette


Quoting Nicolas Ferre (2013-10-18 01:18:04)
> On 11/10/2013 09:37, Boris BREZILLON :
> > Hello,
> >
> > This patch series is the 4th version of the new at91 clock implementation
> > (using common clk framework).
>
> Mike, DT maintainers,
>
> Can you have a look at this series converting the AT91 family to common
> clock framework and associated Device Tree description?
>
> Here is the thread on gmane:
> http://thread.gmane.org/gmane.linux.ports.arm.kernel/272550
>
> Mike, I think that you already reviewed a previous version of this
> series and I guess that we have finalized it.
>
> So gentlemen, It can be good for us if you can give us your "review" or
> "acknowledgement" tags...

I guess you want to take this through your tree? If so please add:

Acked-by: Mike Turquette <mturquette@linaro.org>

Regards,
Mike

>
> Thanks, best regards,
>
>
> > Most of the clock provided by the PMC (Power Management Controller) are
> > implemented :
> > - main clock (main oscillator)
> > - pll clocks
> > - master clock
> > - programmable clocks
> > - utmi clock
> > - peripheral clocks
> > - system clocks
> >
> > This implementation is only compatible with device tree definition.
> > The goal is to define the whole clock tree in the device tree.
> >
> > Could a dt maintainer take a look at these dt bindinds.
> >
> > This patch series is based on linus tree (but should apply correctly on
> > linux-next) and has been tested on sama5d31ek.
> >
> > Best Regards,
> > Boris
> >
> > Changes since v3:
> > - simplify master clk implementation (drop set_rate/parent support)
> > - fix bug in set_rate function of pll driver
> > - fix coding style issues
> > - define macros and constants where needed
> > - remove peripheral id macro references
> > - remove sam9g35 specific handling (sam9g35 = sam9x5)
> > - rework main clk prepare function to handle automatic rate calculation
> >
> > Changes since v2:
> > - fix several bugs in clk implementations
> > - drop non-dt boards support
> > - split the series to ease review and tests:
> > * 1 patch series for new clk implementations (this series)
> > * 1 patch series to move each at91 SoC to common clk framework (coming soon)
> > - modify dt-bindings (add atmel,clk- prefix to atmel specific properties)
> > - add clk macros for dt-bindings
> > - add pmc framework (helper function to access pmc registers)
> > - add interrupt support to enable passive wait in clk_prepare functions
> >
> > Changes since v1:
> > - fix bugs in pll, programmable and system clock implementations
> > (wrong bit position).
> > - add usb clock configuration support (ohci and udc drivers +
> > clk_lookup for non dt boards)
> > - rework of the system clock interfaces (no need to define a parent clock,
> > system clock is a gate with no rate info)
> > - change system, peripheral and programmable clk dt bindings (1 master node
> > and multiple child nodes each defining a system/peripheral or prog clock)
> > - fix bugs in sama5 dt definition
> >
> > Boris BREZILLON (17):
> > ARM: at91: move at91_pmc.h to include/linux/clk/at91_pmc.h
> > ARM: at91: add Kconfig options for common clk support
> > clk: at91: add PMC base support
> > clk: at91: add PMC macro file for dt definitions
> > clk: at91: add PMC main clock
> > clk: at91: add PMC pll clocks
> > clk: at91: add PMC master clock
> > clk: at91: add PMC system clocks
> > clk: at91: add PMC peripheral clocks
> > clk: at91: add peripheral clk macros for peripheral clk dt bindings
> > clk: at91: add PMC programmable clocks
> > clk: at91: add PMC utmi clock
> > clk: at91: add PMC usb clock
> > clk: at91: add PMC smd clock
> > clk: at91: add PMC clk device tree binding doc.
> > ARM: at91: move pit timer to common clk framework
> > ARM: at91: add new compatible strings for pmc driver
> >
> > .../devicetree/bindings/clock/at91-clock.txt | 328 ++++++++++++
> > arch/arm/mach-at91/Kconfig | 44 ++
> > arch/arm/mach-at91/Kconfig.non_dt | 6 +
> > arch/arm/mach-at91/Makefile | 2 +-
> > arch/arm/mach-at91/at91rm9200.c | 2 +-
> > arch/arm/mach-at91/at91sam9260.c | 2 +-
> > arch/arm/mach-at91/at91sam9261.c | 2 +-
> > arch/arm/mach-at91/at91sam9263.c | 2 +-
> > arch/arm/mach-at91/at91sam926x_time.c | 14 +-
> > arch/arm/mach-at91/at91sam9g45.c | 2 +-
> > arch/arm/mach-at91/at91sam9n12.c | 2 +-
> > arch/arm/mach-at91/at91sam9rl.c | 2 +-
> > arch/arm/mach-at91/at91sam9x5.c | 2 +-
> > arch/arm/mach-at91/clock.c | 7 +-
> > arch/arm/mach-at91/generic.h | 3 +-
> > arch/arm/mach-at91/pm.c | 2 +-
> > arch/arm/mach-at91/pm_slowclock.S | 2 +-
> > arch/arm/mach-at91/sama5d3.c | 2 +-
> > arch/arm/mach-at91/setup.c | 8 +-
> > drivers/clk/Makefile | 1 +
> > drivers/clk/at91/Makefile | 12 +
> > drivers/clk/at91/clk-main.c | 189 +++++++
> > drivers/clk/at91/clk-master.c | 278 ++++++++++
> > drivers/clk/at91/clk-peripheral.c | 407 +++++++++++++++
> > drivers/clk/at91/clk-pll.c | 539 ++++++++++++++++++++
> > drivers/clk/at91/clk-plldiv.c | 137 +++++
> > drivers/clk/at91/clk-programmable.c | 423 +++++++++++++++
> > drivers/clk/at91/clk-smd.c | 173 +++++++
> > drivers/clk/at91/clk-system.c | 193 +++++++
> > drivers/clk/at91/clk-usb.c | 400 +++++++++++++++
> > drivers/clk/at91/clk-utmi.c | 162 ++++++
> > drivers/clk/at91/pmc.c | 372 ++++++++++++++
> > drivers/clk/at91/pmc.h | 113 ++++
> > drivers/usb/gadget/atmel_usba_udc.c | 2 +-
> > include/dt-bindings/clk/at91.h | 28 +
> > .../include/mach => include/linux/clk}/at91_pmc.h | 4 +-
> > 36 files changed, 3847 insertions(+), 20 deletions(-)
> > create mode 100644 Documentation/devicetree/bindings/clock/at91-clock.txt
> > create mode 100644 drivers/clk/at91/Makefile
> > create mode 100644 drivers/clk/at91/clk-main.c
> > create mode 100644 drivers/clk/at91/clk-master.c
> > create mode 100644 drivers/clk/at91/clk-peripheral.c
> > create mode 100644 drivers/clk/at91/clk-pll.c
> > create mode 100644 drivers/clk/at91/clk-plldiv.c
> > create mode 100644 drivers/clk/at91/clk-programmable.c
> > create mode 100644 drivers/clk/at91/clk-smd.c
> > create mode 100644 drivers/clk/at91/clk-system.c
> > create mode 100644 drivers/clk/at91/clk-usb.c
> > create mode 100644 drivers/clk/at91/clk-utmi.c
> > create mode 100644 drivers/clk/at91/pmc.c
> > create mode 100644 drivers/clk/at91/pmc.h
> > create mode 100644 include/dt-bindings/clk/at91.h
> > rename {arch/arm/mach-at91/include/mach => include/linux/clk}/at91_pmc.h (98%)
> >
>
>
> --
> Nicolas Ferre
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/





==============================================================================
TOPIC: LEDS: tca6502: add device-tree support for GPIO configuration.
http://groups.google.com/group/linux.kernel/t/ca1c50996760ad7e?hl=en
==============================================================================

== 1 of 2 ==
Date: Thurs, Nov 7 2013 3:40 pm
From: Bryan Wu


On Thu, Oct 31, 2013 at 7:41 PM, NeilBrown <neilb@suse.de> wrote:
>

[PATCH 2/2] LEDS: tca6502: add device-tree support for GPIO configuration.

I think it should be tca6507, right? Typo?

For other parts of this patch, I'm OK for them.

And just a quick scan of the leds-tca6507, I found bunch of typos in
the comments:

* An led-tca6507 device must be provided with platform data. This data
* lists for each output: the name, default trigger, and whether the signal
* is being used as a GPiO rather than an led. 'struct led_plaform_data'

Can we unify to use GPIO and gpio?

* is used for this. If 'name' is NULL, the output isn't used. If 'flags'
* is TCA6507_MAKE_CPIO, the output is a GPO.

Here should be TCA6507_MAKE_GPIO and GPIO instead of GPO

* The "struct led_platform_data" can be embedded in a
* "struct tca6507_platform_data" which adds a 'gpio_base' for the GPiOs,
* and a 'setup' callback which is called once the GPiOs are available.

Don't use GPiO, please.

Could you please review the code again and submit a cleanup patch to
fix those typos?

Thanks,
-Bryan


> The 7 lines driven by the TCA6507 can either drive LEDs or act as output-only
> GPIOs.
>
> To make this distinction in devicetree we use the "compatible" property.
>
> If the device attached to a line is "compatible" with "gpio", we treat it
> like a GPIO. If it is "compatible" with "led" (or if no "compatible" value
> is set) we treat it like an LED.
>
> Signed-off-by: NeilBrown <neilb@suse.de>
>
> diff --git a/Documentation/devicetree/bindings/leds/tca6507.txt b/Documentation/devicetree/bindings/leds/tca6507.txt
> index 80ff3dfb1f32..d7221b84987c 100644
> --- a/Documentation/devicetree/bindings/leds/tca6507.txt
> +++ b/Documentation/devicetree/bindings/leds/tca6507.txt
> @@ -2,6 +2,13 @@ LEDs connected to tca6507
>
> Required properties:
> - compatible : should be : "ti,tca6507".
> +- #address-cells: must be 1
> +- #size-cells: must be 0
> +- reg: typically 0x45.
> +
> +Optional properties:
> +- gpio-controller: allows lines to be used as output-only GPIOs.
> +- #gpio-cells: if present, must be 0.
>
> Each led is represented as a sub-node of the ti,tca6507 device.
>
> @@ -10,6 +17,7 @@ LED sub-node properties:
> - reg : number of LED line (could be from 0 to 6)
> - linux,default-trigger : (optional)
> see Documentation/devicetree/bindings/leds/common.txt
> +- compatible: either "led" (the default) or "gpio".
>
> Examples:
>
> @@ -19,6 +27,9 @@ tca6507@45 {
> #size-cells = <0>;
> reg = <0x45>;
>
> + gpio-controller;
> + #gpio-cells = <2>;
> +
> led0: red-aux@0 {
> label = "red:aux";
> reg = <0x0>;
> @@ -29,5 +40,10 @@ tca6507@45 {
> reg = <0x5>;
> linux,default-trigger = "default-on";
> };
> +
> + wifi-reset@6 {
> + reg = <0x6>;
> + compatible = "gpio";
> + };
> };
>
> diff --git a/drivers/leds/leds-tca6507.c b/drivers/leds/leds-tca6507.c
> index f5063f447463..93a2b1759054 100644
> --- a/drivers/leds/leds-tca6507.c
> +++ b/drivers/leds/leds-tca6507.c
> @@ -638,6 +638,9 @@ static int tca6507_probe_gpios(struct i2c_client *client,
> tca->gpio.direction_output = tca6507_gpio_direction_output;
> tca->gpio.set = tca6507_gpio_set_value;
> tca->gpio.dev = &client->dev;
> +#ifdef CONFIG_OF_GPIO
> + tca->gpio.of_node = of_node_get(client->dev.of_node);
> +

0 Comments:

Post a Comment

Subscribe to Post Comments [Atom]

<< Home


Real Estate