Linux userland API discussions
 help / color / mirror / Atom feed
* Re: futex(2) man page update help request
From: Darren Hart @ 2015-01-17  0:46 UTC (permalink / raw)
  To: Michael Kerrisk (man-pages), Thomas Gleixner
  Cc: Carlos O'Donell, Ingo Molnar, Jakub Jelinek,
	linux-man-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, lkml,
	Davidlohr Bueso, Arnd Bergmann, Steven Rostedt, Peter Zijlstra,
	Linux API, Torvald Riegel, Roland McGrath, Darren Hart,
	Anton Blanchard, Petr Baudis, Eric Dumazet, bill o gallmeister,
	Jan Kiszka, Daniel Wagner, Rich Felker
In-Reply-To: <54B97A72.2050205-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>





On 1/16/15, 12:54 PM, "Michael Kerrisk (man-pages)"
<mtk.manpages-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:

>On 01/16/2015 04:20 PM, Thomas Gleixner wrote:
>> On Fri, 16 Jan 2015, Michael Kerrisk (man-pages) wrote:
>> 
>>> Hello Thomas,
>>>
>>> On 01/15/2015 11:23 PM, Thomas Gleixner wrote:
>>>> On Thu, 15 Jan 2015, Michael Kerrisk (man-pages) wrote:
>>>>>> [EINVAL] uaddr equal uaddr2. Requeue to same futex.
>>>>>
>>>>> ??? I added this, but does this error not occur only for PI requeues?
>>>>
>>>> It's equally wrong for normal futexes. And its actually the same code
>>>> checking for this for all variants.
>>>
>>> I don't understand "equally wrong" in your reply, I'm sorry. Do you
>>> mean:
>>>
>>> a) This error text should be there for both normal and PI requeues
>> 
>> It is there for both. The requeue code has that check independent of
>> the requeue type (normal/pi). It never makes sense to requeue
>> something to itself whether normal or pi futex. We added this for PI,
>> because there it is harmful, but we did not special case it. So normal
>> futexes get the same treatment.
>
>Hello Thomas, 
>
>Color me stupid, but I can't see this in futex_requeue(). Where is that
>check that is "independent of the requeue type (normal/pi)"?
>
>When I look through futex_requeue(), all the likely looking sources
>of EINVAL are governed by a check on the 'requeue_pi' argument.


Right, in the non-PI case, I believe there are valid use cases: move to
the back of the FIFO, for example (OK, maybe the only example?). Both
tests ensuring uaddr1 != uaddr2 are under the requeue_pi conditional
block. The second compares the keys in case they are not FUTEX_PRIVATE
(uaddrs would be different, but still the same backing store).

Thomas, am I missing a test for this someplace else?


-- 
Darren Hart
Intel Open Source Technology Center

^ permalink raw reply

* Re: futex(2) man page update help request
From: Davidlohr Bueso @ 2015-01-17  0:56 UTC (permalink / raw)
  To: Michael Kerrisk (man-pages)
  Cc: Thomas Gleixner, Carlos O'Donell, Darren Hart, Ingo Molnar,
	Jakub Jelinek, linux-man@vger.kernel.org, lkml, Arnd Bergmann,
	Steven Rostedt, Peter Zijlstra, Linux API, Torvald Riegel,
	Roland McGrath, Darren Hart, Anton Blanchard, Petr Baudis,
	Eric Dumazet, bill o gallmeister, Jan Kiszka, Daniel Wagner,
	Rich Felker
In-Reply-To: <54B97A72.2050205@gmail.com>

On Fri, 2015-01-16 at 21:54 +0100, Michael Kerrisk (man-pages) wrote:
> On 01/16/2015 04:20 PM, Thomas Gleixner wrote:
> > On Fri, 16 Jan 2015, Michael Kerrisk (man-pages) wrote:
> > 
> >> Hello Thomas,
> >>
> >> On 01/15/2015 11:23 PM, Thomas Gleixner wrote:
> >>> On Thu, 15 Jan 2015, Michael Kerrisk (man-pages) wrote:
> >>>>> [EINVAL] uaddr equal uaddr2. Requeue to same futex.
> >>>>
> >>>> ??? I added this, but does this error not occur only for PI requeues?
> >>>
> >>> It's equally wrong for normal futexes. And its actually the same code
> >>> checking for this for all variants.
> >>
> >> I don't understand "equally wrong" in your reply, I'm sorry. Do you
> >> mean:
> >>
> >> a) This error text should be there for both normal and PI requeues
> > 
> > It is there for both. The requeue code has that check independent of
> > the requeue type (normal/pi). It never makes sense to requeue
> > something to itself whether normal or pi futex. We added this for PI,
> > because there it is harmful, but we did not special case it. So normal
> > futexes get the same treatment.
> 
> Hello Thomas, 
> 
> Color me stupid, but I can't see this in futex_requeue(). Where is that
> check that is "independent of the requeue type (normal/pi)"?
> 
> When I look through futex_requeue(), all the likely looking sources
> of EINVAL are governed by a check on the 'requeue_pi' argument.

Yeah, its not very straightforward, I was also scratching my head. First
we do:

	if (requeue_pi) {
		/*
		 * Requeue PI only works on two distinct uaddrs. This
		 * check is only valid for private futexes. See below.
		 */
		if (uaddr1 == uaddr2)
			return -EINVAL;

Then:

	/*
	 * The check above which compares uaddrs is not sufficient for
	 * shared futexes. We need to compare the keys:
	 */
	if (requeue_pi && match_futex(&key1, &key2)) {
		ret = -EINVAL;
		goto out_put_keys;
	}

I wonder why we're checking for requeue_pi again, when, at least
according to the comments, it should be for shared. I guess it would
make sense depending on the mappings as the keys are the only true way
of determining if both futexes are the same, so perhaps:

	if ((requeue_pi || (flags & FLAGS_SHARED)) && match_futex())

That would also align with the retry labels.

Thanks,
Davidlohr

^ permalink raw reply

* Re: futex(2) man page update help request
From: Darren Hart @ 2015-01-17  1:11 UTC (permalink / raw)
  To: Davidlohr Bueso, Michael Kerrisk (man-pages)
  Cc: Thomas Gleixner, Carlos O'Donell, Ingo Molnar, Jakub Jelinek,
	linux-man-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, lkml,
	Arnd Bergmann, Steven Rostedt, Peter Zijlstra, Linux API,
	Torvald Riegel, Roland McGrath, Darren Hart, Anton Blanchard,
	Petr Baudis, Eric Dumazet, bill o gallmeister, Jan Kiszka,
	Daniel Wagner, Rich Felker
In-Reply-To: <1421456216.27134.2.camel-h16yJtLeMjHk1uMJSBkQmQ@public.gmane.org>

On 1/16/15, 4:56 PM, "Davidlohr Bueso" <dave-h16yJtLeMjHk1uMJSBkQmQ@public.gmane.org> wrote:


>On Fri, 2015-01-16 at 21:54 +0100, Michael Kerrisk (man-pages) wrote:
>> On 01/16/2015 04:20 PM, Thomas Gleixner wrote:
>> > On Fri, 16 Jan 2015, Michael Kerrisk (man-pages) wrote:
>> > 
>> >> Hello Thomas,
>> >>
>> >> On 01/15/2015 11:23 PM, Thomas Gleixner wrote:
>> >>> On Thu, 15 Jan 2015, Michael Kerrisk (man-pages) wrote:
>> >>>>> [EINVAL] uaddr equal uaddr2. Requeue to same futex.
>> >>>>
>> >>>> ??? I added this, but does this error not occur only for PI
>>requeues?
>> >>>
>> >>> It's equally wrong for normal futexes. And its actually the same
>>code
>> >>> checking for this for all variants.
>> >>
>> >> I don't understand "equally wrong" in your reply, I'm sorry. Do you
>> >> mean:
>> >>
>> >> a) This error text should be there for both normal and PI requeues
>> > 
>> > It is there for both. The requeue code has that check independent of
>> > the requeue type (normal/pi). It never makes sense to requeue
>> > something to itself whether normal or pi futex. We added this for PI,
>> > because there it is harmful, but we did not special case it. So normal
>> > futexes get the same treatment.
>> 
>> Hello Thomas, 
>> 
>> Color me stupid, but I can't see this in futex_requeue(). Where is that
>> check that is "independent of the requeue type (normal/pi)"?
>> 
>> When I look through futex_requeue(), all the likely looking sources
>> of EINVAL are governed by a check on the 'requeue_pi' argument.
>
>Yeah, its not very straightforward, I was also scratching my head. First
>we do:
>
>	if (requeue_pi) {
>		/*
>		 * Requeue PI only works on two distinct uaddrs. This
>		 * check is only valid for private futexes. See below.
>		 */
>		if (uaddr1 == uaddr2)
>			return -EINVAL;

We check here to abort as early as possible for the usual security reasons.

>
>Then:
>
>	/*
>	 * The check above which compares uaddrs is not sufficient for
>	 * shared futexes. We need to compare the keys:
>	 */
>	if (requeue_pi && match_futex(&key1, &key2)) {
>		ret = -EINVAL;
>		goto out_put_keys;
>	}
>
>I wonder why we're checking for requeue_pi again, when, at least
>according to the comments, it should be for shared. I guess it would
>make sense depending on the mappings as the keys are the only true way
>of determining if both futexes are the same, so perhaps:
>
>	if ((requeue_pi || (flags & FLAGS_SHARED)) && match_futex())

No, the rule only applies to requeue_pi. This check is the for-sure
version of the uaddr comparison above. We could add if flags &
FLAGS_SHARED, but I'm not sure it's worth it.

--
Darren Hart
Intel Open Source Technology Center



--
To unsubscribe from this list: send the line "unsubscribe linux-man" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: futex(2) man page update help request
From: Darren Hart @ 2015-01-17  1:33 UTC (permalink / raw)
  To: Michael Kerrisk (man-pages), Thomas Gleixner
  Cc: Carlos O'Donell, Ingo Molnar, Jakub Jelinek,
	linux-man-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, lkml,
	Arnd Bergmann, Steven Rostedt, Peter Zijlstra, Linux API,
	Davidlohr Bueso
In-Reply-To: <54B7D8D4.2070203-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

Corrected Davidlohr's email address.

On 1/15/15, 7:12 AM, "Michael Kerrisk (man-pages)"
<mtk.manpages-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:

>Hello Darren,
>
>I give you the same apology as to Thomas for the
>long-delayed response to your mail.
>
>And I repeat my note to Thomas:
>In the next day or two, I hope to send out the new version
>of the futex(2) page for review. The new draft is a bit
>bigger (okay -- 4 x bigger) than the current page. And there
>are a quite number of FIXMEs that I've placed in the page
>for various points--some minor, but a few major--that need
>to be checked or fixed. Would you have some time to review
>that page?

I'll make the time for that. I've wanted to see this for a while, so thank
you for working on it!

> 
>
>In the meantime, I have a couple of questions, which, if
>you could answer them, I would work some changes into the
>page before sending.
>
>1. In various places, distinction is made between non-PI
>   futexs and PI futexes. But what determines that distinction?
>   From the kernel's perspective, hat make a futex one type
>   or another? I presume it is to do with the types of blocking
>   waiters on the futex, but it would be good to have a formal
>   definition.

You're right in that a uaddr is a uaddr is a uaddr. Also "there is no such
thing as a futex", it doesn't exist as any kind of identifiable object, so
these discussions can get rather confusing :-)

A "futex" becomes a PI futex when it is "created" via a PI futex op code.
At that point, the syscall will ensure a pi_state is populated for the
futex_q entry. See futex_lock_pi() for example. Before the locks are
taken, there is a call to refill_pi_state_cache() which preps a pi_state
for assignment later in futex_lock_pi_atomic(). This pi_state provides the
necessary linkage to perform the priority boosting in the event of a
priority inversion. This is handled externally from the futexes via the
rt_mutex construct.

Clear as mud?


>
>2. Can you say something about the pairing requirements of
>   FUTEX_WAIT_REQUEUE_PI and FUTEX_CMP_REQUEUE_PI.
>   What is the requirement and why do we need it?

Briefly, these op codes exist to support a fairly specific use case:
support for PI aware pthread condvars (glibc patch acceptance STILL
PENDING FOR LOVE OF EVERYTHING HOLY WHY?!?!?! But is shipped with various
PREEMPT_RT enabled Linux systems. Because these calls are paired, and more
of the logic can happen on the kernel side (to preserve ownership of an
rt_mutex with waiters), so in order to ensure userspace and kernelspace
remain in sync, we pre-specify the target of the requeue in
futex_wait_requeue_pi. This also limits the attack surface by only
supporting exactly what it was meant to do. The corner cases get insane
otherwise.

We could walk through the various ways in which it would break if these
pairing restrictions were not in place, but I'll have to take some serious
time to page all those into working memory. Let me know if we need more
detail here and I will.

>
>Most of the rest of this mail is just a checklist noting
>what I did with your comments. No response is needed
>in most cases, but there is one that I have marked with
>"???". If you could reply to that. I'd be grateful.

...

>> For all the PI opcodes, we should probably mention something about the
>> futex value scheme (TID), whereas the other opcodes do not require any
>> specific value scheme.
>> 
>> No Owner:	0
>> Owner:		TID
>> Waiters:	TID | FUTEX_WAITERS
>> 
>> This is the relevant section from the referenced paper:
>> 				
>> The PI futex operations diverge from the oth-
>> ers in that they impose a policy describing how
>> the futex value is to be used. If the lock is un-
>> owned, the futex value shall be 0. If owned, it
>> shall be the thread id (tid) of the owning thread.
>> If there are threads contending for the lock, then
>> the FUTEX_WAITERS flag is set. With this policy in
>> place, userspace can atomically acquire an unowned
>> lock or release an uncontended lock using an atomic
>> instruction and their own tid. A non-zero futex
>> value will force waiters into the kernel to lock. The
>> FUTEX_WAITERS flag forces the owner into the kernel
>> to unlock. If the callers are forced into the kernel,
>> they then deal directly with an underlying rt_mutex
>> which implements the priority inheritance semantics.
>> After the rt_mutex is acquired, the futex value is up-
>> dated accordingly, before the calling thread returns
>> to userspace.
>>
>> It is important to note that the kernel will update the futex value
>>prior
>> to returning to userspace. Unlike other futex op codes,
>> FUTEX_CMP_REUQUE_PI (and FUTEX_WAIT_REQUEUE_PI, FUTEX_LOCK_PI are
>>designed
>> for the implementation of very specific IPC mechanisms).
>
>??? Great text. May I presume that I can take this text
>and freely adapt it for the man page? (Actually, this is a
>request for forgiveness, rather than permission :-).)

Thanks, and no objection from me.

--
Darren Hart
Intel Open Source Technology Center

^ permalink raw reply

* Re: [PATCH v5 2/5] Documentation: DT: Add bindings for Spreadtrum SoC Platform
From: Orson Zhai @ 2015-01-17  8:10 UTC (permalink / raw)
  To: Mark Rutland
  Cc: Lyra Zhang, Chunyan Zhang,
	gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org,
	arnd-r2nGTMty4D4@public.gmane.org,
	gnomes-qBU/x9rampVanCEyBjwyrvXRex20P6io@public.gmane.org,
	broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org, Pawel Moll,
	ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org,
	galak-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org, Will Deacon,
	Catalin Marinas, jslaby-AlSwsSmVLrQ@public.gmane.org,
	jason-NLaQJdtUoK4Be96aLqz0jA@public.gmane.org,
	heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org,
	florian.vaussard-p8DiymsW2f8@public.gmane.org,
	andrew-g2DYL2Zd6BY@public.gmane.org,
	rrichter-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org,
	hytszk-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
	grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org
In-Reply-To: <20150116141109.GC22569@leverpostej>

On Fri, Jan 16, 2015 at 10:11 PM, Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org> wrote:
> On Fri, Jan 16, 2015 at 12:53:16PM +0000, Lyra Zhang wrote:
>> Hi, Mark
>>
>> >> +
>> >> +Required properties:
>> >> +- compatible: must be "sprd,sc9836-uart"
>> >> +- reg: offset and length of the register set for the device
>> >> +- interrupts: exactly one interrupt specifier
>> >> +- clocks: phandles to input clocks.
>> >
>> > The order and relevance of each should be specified. If you have
>> > multiple clocks I would strongly recommend you use clock-names to
>> > distinguish them.
>> >
>>
>> Thank you for the recommendation.
>> but, since we haven't made the clock driver ready, for this initial
>> commit, we just let 4 UARTs share a single fixed 26 MHz clock source.
>> we'll do like you've recommended when we will submit the clock driver
>> in the future.
>
> I'm on about the clock input lines on the UART instance, not the
> providers they come from.
>
> Is there only a single clock input line on each UART? Perhaps multiple
> input lines which are currently fed by the same clock?

________
| 26MHz |-------------------------------------------------
-------------        |                  |
                     |                  |
                _______       ________
               | UART1 |      | UART2 |    .........
               --------------      -------------

the hardware is something like the diagram.

4 Uart modules are all connected to a fixed 26Mhz crystal by power-on default.

There should be a clock-mux between uart and 26Mhz which
could select other clock source such as some pll output.

But as initial commit , we are not ready to describe other inputs by
these muxes.
So  we treat the UART as a simple model with only one fixed-clock input.
And we plan to add the other inputs path back in a not very far future.
Is it appropriate  to do like this?

  Orson


>
> Thanks,
> Mark.

^ permalink raw reply

* Re: futex(2) man page update help request
From: Michael Kerrisk (man-pages) @ 2015-01-17  9:16 UTC (permalink / raw)
  To: Darren Hart, Thomas Gleixner
  Cc: mtk.manpages, Carlos O'Donell, Ingo Molnar, Jakub Jelinek,
	linux-man@vger.kernel.org, lkml, Arnd Bergmann, Steven Rostedt,
	Peter Zijlstra, Linux API, Davidlohr Bueso, Jan Kiszka
In-Reply-To: <D0DEF1AE.B7EDE%dvhart@linux.intel.com>

Hello Darren,

On 01/17/2015 02:33 AM, Darren Hart wrote:
> Corrected Davidlohr's email address.

Thanks!

> On 1/15/15, 7:12 AM, "Michael Kerrisk (man-pages)"
> <mtk.manpages@gmail.com> wrote:
> 
>> Hello Darren,
>>
>> I give you the same apology as to Thomas for the
>> long-delayed response to your mail.
>>
>> And I repeat my note to Thomas:
>> In the next day or two, I hope to send out the new version
>> of the futex(2) page for review. The new draft is a bit
>> bigger (okay -- 4 x bigger) than the current page. And there
>> are a quite number of FIXMEs that I've placed in the page
>> for various points--some minor, but a few major--that need
>> to be checked or fixed. Would you have some time to review
>> that page?
> 
> I'll make the time for that. I've wanted to see this for a while, so thank
> you for working on it!

Great!

>> In the meantime, I have a couple of questions, which, if
>> you could answer them, I would work some changes into the
>> page before sending.
>>
>> 1. In various places, distinction is made between non-PI
>>   futexs and PI futexes. But what determines that distinction?
>>   From the kernel's perspective, hat make a futex one type
>>   or another? I presume it is to do with the types of blocking
>>   waiters on the futex, but it would be good to have a formal
>>   definition.
> 
> You're right in that a uaddr is a uaddr is a uaddr. Also "there is no such
> thing as a futex", it doesn't exist as any kind of identifiable object, so
> these discussions can get rather confusing :-)

So, I want to make sure that I am clear on what you mean you say this.
You say "there is no such thing as a futex" because from the kernel's
perspective there is no visible entity in the uncontended case
(where everything can be dealt with in user space). And from user-space,
in the uncontended case all we're doing is memory operations. Right?

On the other hand, from a kernel perspective, we could say that a 
futex "exists" in the contended phases, since the kernel has allocated
state associated with the uaddr. Right?

> A "futex" becomes a PI futex when it is "created" via a PI futex op code.

Precisely which PI op codes? Is it: FUTEX_LOCK_PI, FUTEX_TRYLOCK_PI, and
FUTEX_CMP_REQUEUE_PI, and not FUTEX_WAIT_REQUEUE_PI or FUTEX_UNLOCK_PI?

> At that point, the syscall will ensure a pi_state is populated for the
> futex_q entry. See futex_lock_pi() for example. Before the locks are
> taken, there is a call to refill_pi_state_cache() which preps a pi_state
> for assignment later in futex_lock_pi_atomic(). This pi_state provides the
> necessary linkage to perform the priority boosting in the event of a
> priority inversion. This is handled externally from the futexes via the
> rt_mutex construct.
> 
> Clear as mud?

Not quite that bad, but... The thing is, still, the man page has text
such as the following (based on your wording):

       FUTEX_CMP_REQUEUE_PI (since Linux 2.6.31)
              This operation is a PI-aware variant of FUTEX_CMP_REQUEUE.
              It    requeues    waiters    that    are    blocked    via
              FUTEX_WAIT_REQUEUE_PI  on uaddr from a non-PI source futex
              (uaddr) to a PI target futex (uaddr2).

And elsewhere you said

    EINVAL is returned if the non-pi to pi or 
    op pairing semantics are violated.

When someone in user-land (e.g., me) reads pieces like that, they then 
want to find somewhere in the man page a description of what makes a 
futex a *PI futex* and probably some statements of the distinction 
between PI and non-PI futexes. And those statements should be from a 
perspective that is somewhat comprehensible to user-space. I'm not
yet confident that I can do that. Do you care to take a shot at it?

>> 2. Can you say something about the pairing requirements of
>>   FUTEX_WAIT_REQUEUE_PI and FUTEX_CMP_REQUEUE_PI.
>>   What is the requirement and why do we need it?
> 
> Briefly, these op codes exist to support a fairly specific use case:
> support for PI aware pthread condvars (glibc patch acceptance STILL
> PENDING FOR LOVE OF EVERYTHING HOLY WHY?!?!?! 

Yes, Jan Kiszka recently alerted me to the existence of 
https://sourceware.org/bugzilla/show_bug.cgi?id=11588
and I still have some text that you proposed (mail titled
("Pthread Condition Variables and Priority Inversion")
quite a long time ago for the pthread_cond_timedwait() page.
One day, when that page exists, I'll try to remember to add it.

> But is shipped with various
> PREEMPT_RT enabled Linux systems. Because these calls are paired, and more
> of the logic can happen on the kernel side (to preserve ownership of an
> rt_mutex with waiters), so in order to ensure userspace and kernelspace
> remain in sync, we pre-specify the target of the requeue in
> futex_wait_requeue_pi. This also limits the attack surface by only
> supporting exactly what it was meant to do. The corner cases get insane
> otherwise.

Thanks. I've added some text on pairing, based on your text above.

> We could walk through the various ways in which it would break if these
> pairing restrictions were not in place, but I'll have to take some serious
> time to page all those into working memory. Let me know if we need more
> detail here and I will.

I don't think we need that much level of detail.

>> Most of the rest of this mail is just a checklist noting
>> what I did with your comments. No response is needed
>> in most cases, but there is one that I have marked with
>> "???". If you could reply to that. I'd be grateful.
> 
> ...
> 
>>> For all the PI opcodes, we should probably mention something about the
>>> futex value scheme (TID), whereas the other opcodes do not require any
>>> specific value scheme.
>>>
>>> No Owner:	0
>>> Owner:		TID
>>> Waiters:	TID | FUTEX_WAITERS
>>>
>>> This is the relevant section from the referenced paper:
>>> 				
>>> The PI futex operations diverge from the oth-
>>> ers in that they impose a policy describing how
>>> the futex value is to be used. If the lock is un-
>>> owned, the futex value shall be 0. If owned, it
>>> shall be the thread id (tid) of the owning thread.
>>> If there are threads contending for the lock, then
>>> the FUTEX_WAITERS flag is set. With this policy in
>>> place, userspace can atomically acquire an unowned
>>> lock or release an uncontended lock using an atomic
>>> instruction and their own tid. A non-zero futex
>>> value will force waiters into the kernel to lock. The
>>> FUTEX_WAITERS flag forces the owner into the kernel
>>> to unlock. If the callers are forced into the kernel,
>>> they then deal directly with an underlying rt_mutex
>>> which implements the priority inheritance semantics.
>>> After the rt_mutex is acquired, the futex value is up-
>>> dated accordingly, before the calling thread returns
>>> to userspace.
>>>
>>> It is important to note that the kernel will update the futex value
>>> prior
>>> to returning to userspace. Unlike other futex op codes,
>>> FUTEX_CMP_REUQUE_PI (and FUTEX_WAIT_REQUEUE_PI, FUTEX_LOCK_PI are
>>> designed
>>> for the implementation of very specific IPC mechanisms).
>>
>> ??? Great text. May I presume that I can take this text
>> and freely adapt it for the man page? (Actually, this is a
>> request for forgiveness, rather than permission :-).)
> 
> Thanks, and no objection from me.

Thanks.

Cheers,

Michael


-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

^ permalink raw reply

* Re: futex(2) man page update help request
From: Darren Hart @ 2015-01-17 19:26 UTC (permalink / raw)
  To: Michael Kerrisk (man-pages), Thomas Gleixner
  Cc: Carlos O'Donell, Ingo Molnar, Jakub Jelinek,
	linux-man-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, lkml,
	Arnd Bergmann, Steven Rostedt, Peter Zijlstra, Linux API,
	Davidlohr Bueso, Jan Kiszka
In-Reply-To: <54BA2872.5040003-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>


On 1/17/15, 1:16 AM, "Michael Kerrisk (man-pages)"
<mtk.manpages-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:

>Hello Darren,
>
>On 01/17/2015 02:33 AM, Darren Hart wrote:
>> Corrected Davidlohr's email address.
>
>Thanks!
>
>> On 1/15/15, 7:12 AM, "Michael Kerrisk (man-pages)"
>> <mtk.manpages-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> 
>>> Hello Darren,
>>>
>>> I give you the same apology as to Thomas for the
>>> long-delayed response to your mail.
>>>
>>> And I repeat my note to Thomas:
>>> In the next day or two, I hope to send out the new version
>>> of the futex(2) page for review. The new draft is a bit
>>> bigger (okay -- 4 x bigger) than the current page. And there
>>> are a quite number of FIXMEs that I've placed in the page
>>> for various points--some minor, but a few major--that need
>>> to be checked or fixed. Would you have some time to review
>>> that page?
>> 
>> I'll make the time for that. I've wanted to see this for a while, so
>>thank
>> you for working on it!
>
>Great!
>
>>> In the meantime, I have a couple of questions, which, if
>>> you could answer them, I would work some changes into the
>>> page before sending.
>>>
>>> 1. In various places, distinction is made between non-PI
>>>   futexs and PI futexes. But what determines that distinction?
>>>   From the kernel's perspective, hat make a futex one type
>>>   or another? I presume it is to do with the types of blocking
>>>   waiters on the futex, but it would be good to have a formal
>>>   definition.
>> 
>> You're right in that a uaddr is a uaddr is a uaddr. Also "there is no
>>such
>> thing as a futex", it doesn't exist as any kind of identifiable object,
>>so
>> these discussions can get rather confusing :-)
>
>So, I want to make sure that I am clear on what you mean you say this.
>You say "there is no such thing as a futex" because from the kernel's
>perspective there is no visible entity in the uncontended case
>(where everything can be dealt with in user space). And from user-space,
>in the uncontended case all we're doing is memory operations. Right?
>
>On the other hand, from a kernel perspective, we could say that a
>futex "exists" in the contended phases, since the kernel has allocated
>state associated with the uaddr. Right?


Sorry, this was more anecdotal, and probably more of a distraction than
constructive. I just meant that unlike other things which you can point to
a specific struct for (task, rt_mutex, etc.), a "futex" has it's state
distributed across the backing store (uaddr), the queue (futex_q), the
pi_state, the rt_mutex, etc, and these span kernel space and userspace.
Your description above is correct.

>
>> A "futex" becomes a PI futex when it is "created" via a PI futex op
>>code.
>
>Precisely which PI op codes? Is it: FUTEX_LOCK_PI, FUTEX_TRYLOCK_PI, and
>FUTEX_CMP_REQUEUE_PI, and not FUTEX_WAIT_REQUEUE_PI or FUTEX_UNLOCK_PI?

Based on your wording below about taking a user POV on this, I'm going to
say "yes" here. These opcodes paired with the PI futex value policy
(described below) defines a "futex" as PI aware. These were created very
specifically in support of PI pthread_mutexes, so it makes a lot more
sense to talk about a PI aware pthread_mutex, than a PI aware futex, since
there is a lot of policy and scaffolding that has to be built up around it
to use it properly (this is what a PI pthread_mutex is).

>> At that point, the syscall will ensure a pi_state is populated for the
>> futex_q entry. See futex_lock_pi() for example. Before the locks are
>> taken, there is a call to refill_pi_state_cache() which preps a pi_state
>> for assignment later in futex_lock_pi_atomic(). This pi_state provides
>>the
>> necessary linkage to perform the priority boosting in the event of a
>> priority inversion. This is handled externally from the futexes via the
>> rt_mutex construct.
>> 
>> Clear as mud?
>
>Not quite that bad, but... The thing is, still, the man page has text
>such as the following (based on your wording):
>
>       FUTEX_CMP_REQUEUE_PI (since Linux 2.6.31)
>              This operation is a PI-aware variant of FUTEX_CMP_REQUEUE.
>              It    requeues    waiters    that    are    blocked    via
>              FUTEX_WAIT_REQUEUE_PI  on uaddr from a non-PI source futex
>              (uaddr) to a PI target futex (uaddr2).
>
>And elsewhere you said
>
>    EINVAL is returned if the non-pi to pi or
>    op pairing semantics are violated.
>
>When someone in user-land (e.g., me) reads pieces like that, they then
>want to find somewhere in the man page a description of what makes a
>futex a *PI futex* and probably some statements of the distinction
>between PI and non-PI futexes. And those statements should be from a
>perspective that is somewhat comprehensible to user-space. I'm not
>yet confident that I can do that. Do you care to take a shot at it?

Hrm, tricky indeed. From userspace, what makes a "futex" PI is the policy
agreement between kernel and userspace (which is the value of the futex:
0, TID, TID|WAITERS, and never just WAITERS, and the use of PI aware futex
op codes when making the futex syscalls.

For a longer discussion of this policy, see Documentation/pi-futex.txt.
Also note that this policy can be combined with that for robust futexes,
adding the OWNERDIED component.

--
Darren Hart
Intel Open Source Technology Center






--
To unsubscribe from this list: send the line "unsubscribe linux-man" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: futex(2) man page update help request
From: Michael Kerrisk (man-pages) @ 2015-01-18 10:18 UTC (permalink / raw)
  To: Darren Hart, Thomas Gleixner
  Cc: mtk.manpages-Re5JQEeQqe8AvxtiuMwx3w, Carlos O'Donell,
	Ingo Molnar, Jakub Jelinek,
	linux-man-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, lkml,
	Arnd Bergmann, Steven Rostedt, Peter Zijlstra, Linux API,
	Davidlohr Bueso, Jan Kiszka
In-Reply-To: <D0DFF430.B7F94%dvhart-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>

Hello Darren,

On 01/17/2015 08:26 PM, Darren Hart wrote:
> 
> On 1/17/15, 1:16 AM, "Michael Kerrisk (man-pages)"
> <mtk.manpages-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:

[...]

>>>> In the meantime, I have a couple of questions, which, if
>>>> you could answer them, I would work some changes into the
>>>> page before sending.
>>>>
>>>> 1. In various places, distinction is made between non-PI
>>>>   futexs and PI futexes. But what determines that distinction?
>>>>   From the kernel's perspective, hat make a futex one type
>>>>   or another? I presume it is to do with the types of blocking
>>>>   waiters on the futex, but it would be good to have a formal
>>>>   definition.
>>>
>>> You're right in that a uaddr is a uaddr is a uaddr. Also "there is no
>>> such
>>> thing as a futex", it doesn't exist as any kind of identifiable object,
>>> so
>>> these discussions can get rather confusing :-)
>>
>> So, I want to make sure that I am clear on what you mean you say this.
>> You say "there is no such thing as a futex" because from the kernel's
>> perspective there is no visible entity in the uncontended case
>> (where everything can be dealt with in user space). And from user-space,
>> in the uncontended case all we're doing is memory operations. Right?
>>
>> On the other hand, from a kernel perspective, we could say that a
>> futex "exists" in the contended phases, since the kernel has allocated
>> state associated with the uaddr. Right?
> 
> 
> Sorry, this was more anecdotal, and probably more of a distraction than
> constructive. I just meant that unlike other things which you can point to
> a specific struct for (task, rt_mutex, etc.), a "futex" has it's state
> distributed across the backing store (uaddr), the queue (futex_q), the
> pi_state, the rt_mutex, etc, and these span kernel space and userspace.
> Your description above is correct.

Okay. Thanks. I've added a few more words to the page noting that
the kernel maintains no state for a futex in the uncontended state.

>>> A "futex" becomes a PI futex when it is "created" via a PI futex op
>>> code.
>>
>> Precisely which PI op codes? Is it: FUTEX_LOCK_PI, FUTEX_TRYLOCK_PI, and
>> FUTEX_CMP_REQUEUE_PI, and not FUTEX_WAIT_REQUEUE_PI or FUTEX_UNLOCK_PI?
> 
> Based on your wording below about taking a user POV on this, I'm going to
> say "yes" here. These opcodes paired with the PI futex value policy
> (described below) defines a "futex" as PI aware. These were created very
> specifically in support of PI pthread_mutexes, so it makes a lot more
> sense to talk about a PI aware pthread_mutex, than a PI aware futex, since
> there is a lot of policy and scaffolding that has to be built up around it
> to use it properly (this is what a PI pthread_mutex is).

See below.

>>> At that point, the syscall will ensure a pi_state is populated for the
>>> futex_q entry. See futex_lock_pi() for example. Before the locks are
>>> taken, there is a call to refill_pi_state_cache() which preps a pi_state
>>> for assignment later in futex_lock_pi_atomic(). This pi_state provides
>>> the
>>> necessary linkage to perform the priority boosting in the event of a
>>> priority inversion. This is handled externally from the futexes via the
>>> rt_mutex construct.
>>>
>>> Clear as mud?
>>
>> Not quite that bad, but... The thing is, still, the man page has text
>> such as the following (based on your wording):
>>
>>       FUTEX_CMP_REQUEUE_PI (since Linux 2.6.31)
>>              This operation is a PI-aware variant of FUTEX_CMP_REQUEUE.
>>              It    requeues    waiters    that    are    blocked    via
>>              FUTEX_WAIT_REQUEUE_PI  on uaddr from a non-PI source futex
>>              (uaddr) to a PI target futex (uaddr2).
>>
>> And elsewhere you said
>>
>>    EINVAL is returned if the non-pi to pi or
>>    op pairing semantics are violated.
>>
>> When someone in user-land (e.g., me) reads pieces like that, they then
>> want to find somewhere in the man page a description of what makes a
>> futex a *PI futex* and probably some statements of the distinction
>> between PI and non-PI futexes. And those statements should be from a
>> perspective that is somewhat comprehensible to user-space. I'm not
>> yet confident that I can do that. Do you care to take a shot at it?
> 
> Hrm, tricky indeed. From userspace, what makes a "futex" PI is the policy
> agreement between kernel and userspace (which is the value of the futex:
> 0, TID, TID|WAITERS, and never just WAITERS, and the use of PI aware futex
> op codes when making the futex syscalls.

Okay -- I've attempted to capture this in some text that I added to the 
page.

> For a longer discussion of this policy, see Documentation/pi-futex.txt.

Sad to say, that document doesn't supply that much more detail, in
my reading of it, at least.

> Also note that this policy can be combined with that for robust futexes,
> adding the OWNERDIED component.

Now there's two other stories that have yet to be dealt with ;-). 

I have a FIXME already in the page regarding OWNERDIED, and
get_robust_list(2) is another page that seems like it could do with 
a fair bit of work, but that's a story for another day.

Cheers,

Michael


-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

^ permalink raw reply

* Re: [PATCH v4 0/4] power: Add max77693 charger driver
From: Lee Jones @ 2015-01-18 18:00 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Sebastian Reichel, Dmitry Eremin-Solenikov, David Woodhouse,
	Samuel Ortiz, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-pm-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1420453019-28614-1-git-send-email-k.kozlowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>

Tell us what you need?

> Changes since v3
> ================
> 1. Patch 3/4: Don't create platform data structure for charger settings
>    because driver won't be used on non-DT SoCs (its for Exynos based
>    Trats2).
> 2. Patch 3/4: Drop Sebastian's ack [1] because of change above.
> 
> Changes since v2
> ================
> 1. Add ack from Sebastian Reichel (charger driver).
> 2. Drop patch "mfd: max77693: Map charger device to its own of_node"
>    (applied by Lee Jones).
> 
> 
> Changes since v1
> ================
> 1. Add patch 2/5: mfd: max77693: Map charger device to its own of_node
>    (forgot to send it in v1)
> 2. Remove patches for DTS and configs. I'll send them later.
> 3. Add ack from Lee Jones (patch 3/5).
> 
> 
> Description
> ===========
> The patchset adds max77693 charger driver present on Trats2 board
> (and Galaxy S III). The driver configures battery charger and exposes
> power supply interface.
> 
> Driver is necessary to provide full charging stack on Trats2 device
> (extcon, charger-manager etc.).
> 
> [1] https://lkml.org/lkml/2014/10/27/774
> 
> 
> Best regards,
> Krzysztof
> 
> 
> 
> Krzysztof Kozlowski (4):
>   devicetree: power/mfd: max77693: Document new bindings for charger
>   mfd: max77693: Add defines for MAX77693 charger driver
>   power: max77693: Add charger driver for Maxim 77693
>   Documentation: power: max77693-charger: Document exported sysfs entry
> 
>  Documentation/ABI/testing/sysfs-class-power        |  42 ++
>  Documentation/devicetree/bindings/mfd/max77693.txt |  45 ++
>  drivers/power/Kconfig                              |   6 +
>  drivers/power/Makefile                             |   1 +
>  drivers/power/max77693_charger.c                   | 758 +++++++++++++++++++++
>  include/linux/mfd/max77693-private.h               | 108 +++
>  6 files changed, 960 insertions(+)
>  create mode 100644 drivers/power/max77693_charger.c
> 

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH] MIPS,prctl: add PR_[GS]ET_FP_MODE prctl options for MIPS
From: Paul Burton @ 2015-01-18 21:47 UTC (permalink / raw)
  To: Markos Chandras
  Cc: linux-mips-6z/3iImG2C8G8FEW9MqTrA, Matthew Fortune, LKML,
	linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <54B519B6.5040604-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>

On Tue, Jan 13, 2015 at 01:12:22PM +0000, Markos Chandras wrote:
> Hi,
> 
> I think the "MIPS,prctl" in the title should be "MIPS: prctl"

I used the comma to denote a list - that is, this change affects both
MIPS & the generic prctl code. To me your "MIPS: prctl" suggestion would
indicate that the changes are confined to arch/mips.

> I have also CC'd the LKML and the linux-api mailing lists since this
> touches the kernel ABI with the new PR_[GS]ET_FP_MODE definitions.

Thanks,
    Paul

^ permalink raw reply

* Re: [PATCH 1/6] selftests: Introduce minimal shared logic for running tests
From: Michael Ellerman @ 2015-01-19  0:35 UTC (permalink / raw)
  To: Shuah Khan
  Cc: linux-kernel, mmarek, gregkh, akpm, rostedt, mingo, davem,
	keescook, tranmanphong, cov, dh.herrmann, hughd, bobby.prani,
	serge.hallyn, ebiederm, tim.bird, josh, koct9i, linux-kbuild,
	linux-api, netdev
In-Reply-To: <54B95006.3080502@osg.samsung.com>

On Fri, 2015-01-16 at 10:53 -0700, Shuah Khan wrote:
> On 01/09/2015 02:06 AM, Michael Ellerman wrote:
> > This adds a Make include file which most selftests can then include to
> > get the run_tests logic.
> > 
> > On its own this has the advantage of some reduction in repetition, and
> > also means the pass/fail message is defined in fewer places.
> > 
> > However the key advantage is it will allow us to implement install very
> > simply in a subsequent patch.
> > 
> > The default implementation just executes each program in $(TEST_PROGS).
> > 
> > We use a variable to hold the default implementation of $(RUN_TESTS)
> > because that gives us a clean way to override it if necessary, ie. using
> > override. The mount, memory-hotplug and mqueue tests use that to provide
> > a different implementation.
> > 
> > Tests are not run via /bin/bash, so if they are scripts they must be
> > executable, we add u+x to several.
> > 
> > Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
> 
> I like the shared logic approach in general provided it leaves the
> flexibility to not use the shared logic if a test have the need to
> do so.

Yes of course it does, it's entirely optional to include lib.mk.

> This series requires some patch planning. shared logic patch
> followed by individual test patches as opposed a single patch.

It could be a single patch too, but there's no reason to do it that way. The
series works fine as I sent it.

> I would like to see the shared logic work done on top of my patch v4
> series.

That's a waste of time. This series replaces your v4. Doing this "on top" of
your v4 would just mean reverting your v4 series and then applying this.

cheers

^ permalink raw reply

* Re: [PATCH 4/6] kbuild: add a new kselftest_install make target to install selftests
From: Michael Ellerman @ 2015-01-19  0:35 UTC (permalink / raw)
  To: Shuah Khan
  Cc: linux-kernel, mmarek, gregkh, akpm, rostedt, mingo, davem,
	keescook, tranmanphong, cov, dh.herrmann, hughd, bobby.prani,
	serge.hallyn, ebiederm, tim.bird, josh, koct9i, linux-kbuild,
	linux-api, netdev
In-Reply-To: <54B93DA1.2010601@osg.samsung.com>

On Fri, 2015-01-16 at 09:34 -0700, Shuah Khan wrote:
> On 01/09/2015 02:06 AM, Michael Ellerman wrote:
> > Add a new make target to install kernel selftests. This new target will
> > build and install selftests.
> > 
> > The default is just $(objtree)/selftests. This is preferable to
> > something based on $(INSTALL_MOD_PATH) (which defaults to /), as it
> > allows a normal user to install the tests. This is similar to the
> > default behaviour of make headers_install.
> 
> A normal user can install tests at any location they choose by
> overriding the default path. For example:
> 
> INSTALL_MOD_PATH=/tmp make kselftest_install
> 
> will install under tmp.

Why default to a directory that most users can't write to? That's not helpful.

Users who are root can override the path, for example:

INSTALL_MOD_PATH=/ make kselftest_install

> The approach I used also ties test installs to kernel release.
> This addresses an important use-case for kernel developers
> that want to compare results from release to release.

Sure, I'm happy to add the kernel release, so the default would be
$(objtree)/selftests/$(kernel-release)/.

> The use-case for any user to be able to install tests at
> any location is addressed by the above example.

The default should work for most users most of the time, / does not achieve
that.

> I would like these two above use-cases continued to be supported,
> especially the one that tries the test installs to kernel release.

That's fine, I'm happy to update this to use kernel release. But defaulting to
/ doesn't make sense.

> Another goal is to keep changes to the main Makefile minimal and
> the rest of the install support belongs under selftests/Makefile
> and any other include file (like the one you proposed).

Yes, this patch does just that.

cheers

^ permalink raw reply

* Re: [PATCH 2/6] selftests: Add install target
From: Michael Ellerman @ 2015-01-19  0:35 UTC (permalink / raw)
  To: Shuah Khan
  Cc: linux-kernel, mmarek, gregkh, akpm, rostedt, mingo, davem,
	keescook, tranmanphong, cov, dh.herrmann, hughd, bobby.prani,
	serge.hallyn, ebiederm, tim.bird, josh, koct9i, linux-kbuild,
	linux-api, netdev
In-Reply-To: <54B94E75.8030504@osg.samsung.com>

On Fri, 2015-01-16 at 10:46 -0700, Shuah Khan wrote:
> On 01/09/2015 02:06 AM, Michael Ellerman wrote:
> > This adds make install support to selftests. The basic usage is:
> > 
> > $ cd tools/testing/selftests
> > $ make install
> > 
> > That installs into tools/testing/selftests/install, which can then be
> > copied where ever necessary.
> > 
> > The install destination is also configurable using eg:
> > 
> > $ INSTALL_PATH=/mnt/selftests make install
> 
> Please see my response to [PATCH 4/6] kbuild: add a new
> kselftest_install make target to install selftests
> 
> These are addressed by the current approach to use existing
> INSTALL_MOD_PATH.

No that's a separate issue.

This patch adds install support for tools/testing/selftests, *completely
separate* from the kbuild infrastructure. 

That is an existing use case, ie. using selftests on its own, which this patch
easily continues to support.

cheers

^ permalink raw reply

* Re: [PATCH v4] gpio: lib-sysfs: Add 'wakeup' attribute
From: Alexandre Courbot @ 2015-01-19  4:20 UTC (permalink / raw)
  To: Sören Brinkmann
  Cc: Johan Hovold, Linus Walleij, linux-api-u79uwXL29TY76Z2rM5mHXA,
	Linux Kernel Mailing List,
	linux-gpio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-doc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <a4610e13d0094d40b7e0574992509ec6-reflc3kr++NteXefQoUsnuhlVc3/7hDbVaz/vdPVXQ4@public.gmane.org>

On Sat, Jan 17, 2015 at 1:49 AM, Sören Brinkmann
<soren.brinkmann-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org> wrote:
> On Fri, 2015-01-16 at 12:11PM +0100, Johan Hovold wrote:
>> On Thu, Jan 15, 2015 at 11:49:49AM -0800, Soren Brinkmann wrote:
>> > Add an attribute 'wakeup' to the GPIO sysfs interface which allows
>> > marking/unmarking a GPIO as wake IRQ.
>> > The file 'wakeup' is created in each exported GPIOs directory, if an IRQ
>> > is associated with that GPIO and the irqchip implements set_wake().
>> > Writing 'enabled' to that file will enable wake for that GPIO, while
>> > writing 'disabled' will disable wake.
>> > Reading that file will return either 'disabled' or 'enabled' depening on
>> > the currently set flag for the GPIO's IRQ.
>> >
>> > Signed-off-by: Soren Brinkmann <soren.brinkmann-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>
>> > Reviewed-by: Alexandre Courbot <acourbot-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>> > ---
>> > Hi Linus, Johan,
>> >
>> > I rebased my patch. And things look good.
>>
>> I took at closer look at this patch now and I really don't think it
>> should be merged at all.
>>
>> We have a mechanism for handling wake-up sources (documented in
>> Documentation/power/devices.txt) as well as an ABI to enable/disable
>> them using the power/wakeup device attribute from userspace.
>
> Doesn't work for GPIOs AFAIK.
>
>>
>> Implementing proper wakeup support for unclaimed GPIOs would take some
>> work (if at all desired), but that is not a reason to be adding custom
>> implementations that violates the kernel's power policies and new ABIs
>> that would need to be maintained forever.
>
> These are claimed, by the sysfs interface.
>
>>
>> [ And we really shouldn't be adding anything to the broken gpio sysfs
>> interface until it's been redesigned. ]
>>
>> Meanwhile you can (should) use gpio-keys if you need to wake your system
>> on gpio events.
>
> We had that discussion and I don't think GPIO keys is the right solution
> for every use-case.

Sorry, it has been a while - can you remind us of why?

>>
>> > But the 'is_visible' things does not behave the way I expected it to.
>> > It seems to be only triggered on an export but not when attributes
>> > change. Hence, in my case, everything was visiible since the inital
>> > state matches that, but even when changing the direction or things
>> > like that, attributes don't disappear. Is that something still worked
>> > on? Expected
>>
>> That's expected. We generally don't want attributes to appear or
>> disappear after the device has been registered (although there is a
>> mechanism for cases were it makes sense). This is no different from
>> how your v3 patch worked either.
>
> Sure, but the is_visible thing is effectively broken for GPIO. I think a
> GPIO is in a defined state when exported and the checks all work on that
> state during export. But then this state can be changed through the
> sysfs interface. So, if the initial state hides something it becomes
> unavailable for all times and, vice versa, if the initial state makes
> something visible, it will stay even when it is no longer a valid
> property to change.

Is there anything that prevents you from patching other GPIO sysfs
hooks to remove/reenable the wakeup property as needed?

^ permalink raw reply

* Re: [PATCH v4 0/4] power: Add max77693 charger driver
From: Krzysztof Kozlowski @ 2015-01-19  7:40 UTC (permalink / raw)
  To: Lee Jones
  Cc: Sebastian Reichel, Dmitry Eremin-Solenikov, David Woodhouse,
	Samuel Ortiz, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-pm-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20150118180002.GJ3574@x1>

On nie, 2015-01-18 at 18:00 +0000, Lee Jones wrote:
> Tell us what you need?

Hi Lee,


All I need is... acks or applying of patchset by Sebastian. Sebastian
acked main patch (3/4) but then I changed it [1].

You already acked the MFD change (patch 2/4: mfd: max77693: Add defines
for MAX77693 charger driver).

The patch with bindings (1/4: devicetree: power/mfd: max77693: Document
new bindings for charger) actually adds bindings for charger so rather
Sebastian's opinion is needed.

Other patches are from power supply subsystem.
Thanks for asking!


Dear Sebastian,

You already looked at the patchset [1], so maybe you could pick it up?

[1] https://lkml.org/lkml/2014/10/27/774

Best regards,
Krzysztof

> 
> > Changes since v3
> > ================
> > 1. Patch 3/4: Don't create platform data structure for charger settings
> >    because driver won't be used on non-DT SoCs (its for Exynos based
> >    Trats2).
> > 2. Patch 3/4: Drop Sebastian's ack [1] because of change above.
> > 
> > Changes since v2
> > ================
> > 1. Add ack from Sebastian Reichel (charger driver).
> > 2. Drop patch "mfd: max77693: Map charger device to its own of_node"
> >    (applied by Lee Jones).
> > 
> > 
> > Changes since v1
> > ================
> > 1. Add patch 2/5: mfd: max77693: Map charger device to its own of_node
> >    (forgot to send it in v1)
> > 2. Remove patches for DTS and configs. I'll send them later.
> > 3. Add ack from Lee Jones (patch 3/5).
> > 
> > 
> > Description
> > ===========
> > The patchset adds max77693 charger driver present on Trats2 board
> > (and Galaxy S III). The driver configures battery charger and exposes
> > power supply interface.
> > 
> > Driver is necessary to provide full charging stack on Trats2 device
> > (extcon, charger-manager etc.).
> > 
> > [1] https://lkml.org/lkml/2014/10/27/774
> > 
> > 
> > Best regards,
> > Krzysztof
> > 
> > 
> > 
> > Krzysztof Kozlowski (4):
> >   devicetree: power/mfd: max77693: Document new bindings for charger
> >   mfd: max77693: Add defines for MAX77693 charger driver
> >   power: max77693: Add charger driver for Maxim 77693
> >   Documentation: power: max77693-charger: Document exported sysfs entry
> > 
> >  Documentation/ABI/testing/sysfs-class-power        |  42 ++
> >  Documentation/devicetree/bindings/mfd/max77693.txt |  45 ++
> >  drivers/power/Kconfig                              |   6 +
> >  drivers/power/Makefile                             |   1 +
> >  drivers/power/max77693_charger.c                   | 758 +++++++++++++++++++++
> >  include/linux/mfd/max77693-private.h               | 108 +++
> >  6 files changed, 960 insertions(+)
> >  create mode 100644 drivers/power/max77693_charger.c
> > 
> 

^ permalink raw reply

* [PATCH] tpm, tpm_crb: fix build error
From: Jarkko Sakkinen @ 2015-01-19  8:43 UTC (permalink / raw)
  To: Peter Huewe, Ashley Lai, Marcel Selhorst
  Cc: tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, josh-iaAMLnmF4UmaiuxdJuQwMA,
	christophe.ricard-Re5JQEeQqe8AvxtiuMwx3w,
	jason.gunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/,
	stefanb-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8,
	linux-api-u79uwXL29TY76Z2rM5mHXA,
	trousers-tech-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, Jarkko Sakkinen

SIMPLE_DEV_PM_OPS() was inside #ifdef CONFIG_PM_SLEEP.

Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
---
 drivers/char/tpm/tpm_crb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
index c248a35..3dd23cf 100644
--- a/drivers/char/tpm/tpm_crb.c
+++ b/drivers/char/tpm/tpm_crb.c
@@ -107,9 +107,9 @@ static int crb_resume(struct device *dev)
 
 	return rc;
 }
+#endif
 
 static SIMPLE_DEV_PM_OPS(crb_pm, tpm_pm_suspend, crb_resume);
-#endif
 
 static u8 crb_status(struct tpm_chip *chip)
 {
-- 
2.1.0

^ permalink raw reply related

* Re: [PATCH v4] gpio: lib-sysfs: Add 'wakeup' attribute
From: Linus Walleij @ 2015-01-19  8:54 UTC (permalink / raw)
  To: Alexandre Courbot
  Cc: Sören Brinkmann, Johan Hovold, linux-api,
	Linux Kernel Mailing List, linux-gpio@vger.kernel.org,
	linux-doc@vger.kernel.org
In-Reply-To: <CAAVeFu+K0W96mi0=QR8JGSe9BpH2nC5WV_9qbSvm6LJGA8gc7Q@mail.gmail.com>

On Mon, Jan 19, 2015 at 5:20 AM, Alexandre Courbot <gnurou@gmail.com> wrote:
> On Sat, Jan 17, 2015 at 1:49 AM, Sören Brinkmann <soren.brinkmann@xilinx.com> wrote:
>> On Fri, 2015-01-16 at 12:11PM +0100, Johan Hovold wrote:

>>> Implementing proper wakeup support for unclaimed GPIOs would take some
>>> work (if at all desired), but that is not a reason to be adding custom
>>> implementations that violates the kernel's power policies and new ABIs
>>> that would need to be maintained forever.
(...)
>>> Meanwhile you can (should) use gpio-keys if you need to wake your system
>>> on gpio events.
>>
>> We had that discussion and I don't think GPIO keys is the right solution
>> for every use-case.
>
> Sorry, it has been a while - can you remind us of why?

There are such cases. Of course keys should be handled by GPIO-keys
and these will trigger the right wakeup events in such cases.

This is for more esoteric cases: we cannot have a kernel module for
everything people want to do with GPIOs, and the use case I accept
is GPIOs used in automatic control etc, think factory lines or doors.
We can't have a "door" driver or "punch arm" or "fire alarm" driver
in the kernel. Those are userspace things.

Still such embedded systems need to be able to go to idle and
sleep to conerve power, and then they need to put wakeups on
these GPIOs.

So it is a feature userspace needs, though as with much of the
sysfs ABI it is very often abused for things like keys and LEDs which
is an abomination but we can't do much about it :(

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH tip 0/9] tracing: attach eBPF programs to tracepoints/syscalls/kprobe
From: Masami Hiramatsu @ 2015-01-19  9:52 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Ingo Molnar, Steven Rostedt, Namhyung Kim,
	Arnaldo Carvalho de Melo, Jiri Olsa, David S. Miller,
	Daniel Borkmann, Hannes Frederic Sowa, Brendan Gregg,
	linux-api-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1421381770-4866-1-git-send-email-ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org>

(2015/01/16 13:16), Alexei Starovoitov wrote:
> Hi Ingo, Steven,
> 
> This patch set is based on tip/master.
> It adds ability to attach eBPF programs to tracepoints, syscalls and kprobes.
> 
> Mechanism of attaching:
> - load program via bpf() syscall and receive program_fd
> - event_fd = open("/sys/kernel/debug/tracing/events/.../filter")
> - write 'bpf-123' to event_fd where 123 is program_fd
> - program will be attached to particular event and event automatically enabled
> - close(event_fd) will detach bpf program from event and event disabled
> 
> Program attach point and input arguments:
> - programs attached to kprobes receive 'struct pt_regs *' as an input.
>   See tracex4_kern.c that demonstrates how users can write a C program like:
>   SEC("events/kprobes/sys_write")
>   int bpf_prog4(struct pt_regs *regs)
>   {
>      long write_size = regs->dx; 
>      // here user need to know the proto of sys_write() from kernel
>      // sources and x64 calling convention to know that register $rdx
>      // contains 3rd argument to sys_write() which is 'size_t count'
> 
>   it's obviously architecture dependent, but allows building sophisticated
>   user tools on top, that can see from debug info of vmlinux which variables
>   are in which registers or stack locations and fetch it from there.
>   'perf probe' can potentialy use this hook to generate programs in user space
>   and insert them instead of letting kernel parse string during kprobe creation.

Actually, this program just shows raw pt_regs for handlers, but I guess it is also
possible to pass event arguments from perf probe which given by user and perf-probe.
If we can write the script as

int bpf_prog4(s64 write_size)
{
   ...
}

This will be much easier to play with.

> - programs attached to tracepoints and syscalls receive 'struct bpf_context *':
>   u64 arg1, arg2, ..., arg6;
>   for syscalls they match syscall arguments.
>   for tracepoints these args match arguments passed to tracepoint.
>   For example:
>   trace_sched_migrate_task(p, new_cpu); from sched/core.c
>   arg1 <- p        which is 'struct task_struct *'
>   arg2 <- new_cpu  which is 'unsigned int'
>   arg3..arg6 = 0
>   the program can use bpf_fetch_u8/16/32/64/ptr() helpers to walk 'task_struct'
>   or any other kernel data structures.
>   These helpers are using probe_kernel_read() similar to 'perf probe' which is
>   not 100% safe in both cases, but good enough.
>   To access task_struct's pid inside 'sched_migrate_task' tracepoint
>   the program can do:
>   struct task_struct *task = (struct task_struct *)ctx->arg1;
>   u32 pid = bpf_fetch_u32(&task->pid);
>   Since struct layout is kernel configuration specific such programs are not
>   portable and require access to kernel headers to be compiled,
>   but in this case we don't need debug info.
>   llvm with bpf backend will statically compute task->pid offset as a constant
>   based on kernel headers only.
>   The example of this arbitrary pointer walking is tracex1_kern.c
>   which does skb->dev->name == "lo" filtering.

At least I would like to see this way on kprobes event too, since it should be
treated as a traceevent.

> In all cases the programs are called before trace buffer is allocated to
> minimize the overhead, since we want to filter huge number of events, but
> buffer alloc/free and argument copy for every event is too costly.
> Theoretically we can invoke programs after buffer is allocated, but it
> doesn't seem needed, since above approach is faster and achieves the same.
> 
> Note, tracepoint/syscall and kprobe programs are two different types:
> BPF_PROG_TYPE_TRACING_FILTER and BPF_PROG_TYPE_KPROBE_FILTER,
> since they expect different input.
> Both use the same set of helper functions:
> - map access (lookup/update/delete)
> - fetch (probe_kernel_read wrappers)
> - memcmp (probe_kernel_read + memcmp)
> - dump_stack
> - trace_printk
> The last two are mainly to debug the programs and to print data for user
> space consumptions.
> 
> Portability:
> - kprobe programs are architecture dependent and need user scripting
>   language like ktap/stap/dtrace/perf that will dynamically generate
>   them based on debug info in vmlinux

If we can use kprobe event as a normal traceevent, user scripting can be
architecture independent too. Only perf-probe fills the gap. All other
userspace tools can collaborate with perf-probe to setup the events.
If so, we can avoid redundant works on debuginfo. That is my point.

Thank you,

> - tracepoint programs are architecture independent, but if arbitrary pointer
>   walking (with fetch() helpers) is used, they need data struct layout to match.
>   Debug info is not necessary
> - for networking use case we need to access 'struct sk_buff' fields in portable
>   way (user space needs to fetch packet length without knowing skb->len offset),
>   so for some frequently used data structures we will add helper functions
>   or pseudo instructions to access them. I've hacked few ways specifically
>   for skb, but abandoned them in favor of more generic type/field infra.
>   That work is still wip. Not part of this set.
>   Once it's ready tracepoint programs that access common data structs
>   will be kernel independent.
> 
> Program return value:
> - programs return 0 to discard an event
> - and return non-zero to proceed with event (allocate trace buffer, copy
>   arguments there and print it eventually in trace_pipe in traditional way)
> 
> Examples:
> - dropmon.c - simple kfree_skb() accounting in eBPF assembler, similar
>   to dropmon tool
> - tracex1_kern.c - does net/netif_receive_skb event filtering
>   for dev->skb->name == "lo" condition
> - tracex2_kern.c - same kfree_skb() accounting like dropmon, but now in C
>   plus computes histogram of all write sizes from sys_write syscall
>   and prints the histogram in userspace
> - tracex3_kern.c - most sophisticated example that computes IO latency
>   between block/block_rq_issue and block/block_rq_complete events
>   and prints 'heatmap' using gray shades of text terminal.
>   Useful to analyze disk performance.
> - tracex4_kern.c - computes histogram of write sizes from sys_write syscall
>   using kprobe mechanism instead of syscall. Since kprobe is optimized into
>   ftrace the overhead of instrumentation is smaller than in example 2.
> 
> The user space tools like ktap/dtrace/systemptap/perf that has access
> to debug info would probably want to use kprobe attachment point, since kprobe
> can be inserted anywhere and all registers are avaiable in the program.
> tracepoint attachments are useful without debug info, so standalone tools
> like iosnoop will use them.
> 
> The main difference vs existing perf_probe/ftrace infra is in kernel aggregation
> and conditional walking of arbitrary data structures.
> 
> Thanks!
> 
> Alexei Starovoitov (9):
>   tracing: attach eBPF programs to tracepoints and syscalls
>   tracing: allow eBPF programs to call bpf_printk()
>   tracing: allow eBPF programs to call ktime_get_ns()
>   samples: bpf: simple tracing example in eBPF assembler
>   samples: bpf: simple tracing example in C
>   samples: bpf: counting example for kfree_skb tracepoint and write
>     syscall
>   samples: bpf: IO latency analysis (iosnoop/heatmap)
>   tracing: attach eBPF programs to kprobe/kretprobe
>   samples: bpf: simple kprobe example
> 
>  include/linux/ftrace_event.h       |    6 +
>  include/trace/bpf_trace.h          |   25 ++++
>  include/trace/ftrace.h             |   30 +++++
>  include/uapi/linux/bpf.h           |   11 ++
>  kernel/trace/Kconfig               |    1 +
>  kernel/trace/Makefile              |    1 +
>  kernel/trace/bpf_trace.c           |  250 ++++++++++++++++++++++++++++++++++++
>  kernel/trace/trace.h               |    3 +
>  kernel/trace/trace_events.c        |   41 +++++-
>  kernel/trace/trace_events_filter.c |   80 +++++++++++-
>  kernel/trace/trace_kprobe.c        |   11 +-
>  kernel/trace/trace_syscalls.c      |   31 +++++
>  samples/bpf/Makefile               |   18 +++
>  samples/bpf/bpf_helpers.h          |   18 +++
>  samples/bpf/bpf_load.c             |   62 ++++++++-
>  samples/bpf/bpf_load.h             |    3 +
>  samples/bpf/dropmon.c              |  129 +++++++++++++++++++
>  samples/bpf/tracex1_kern.c         |   28 ++++
>  samples/bpf/tracex1_user.c         |   24 ++++
>  samples/bpf/tracex2_kern.c         |   71 ++++++++++
>  samples/bpf/tracex2_user.c         |   95 ++++++++++++++
>  samples/bpf/tracex3_kern.c         |   96 ++++++++++++++
>  samples/bpf/tracex3_user.c         |  146 +++++++++++++++++++++
>  samples/bpf/tracex4_kern.c         |   36 ++++++
>  samples/bpf/tracex4_user.c         |   83 ++++++++++++
>  25 files changed, 1290 insertions(+), 9 deletions(-)
>  create mode 100644 include/trace/bpf_trace.h
>  create mode 100644 kernel/trace/bpf_trace.c
>  create mode 100644 samples/bpf/dropmon.c
>  create mode 100644 samples/bpf/tracex1_kern.c
>  create mode 100644 samples/bpf/tracex1_user.c
>  create mode 100644 samples/bpf/tracex2_kern.c
>  create mode 100644 samples/bpf/tracex2_user.c
>  create mode 100644 samples/bpf/tracex3_kern.c
>  create mode 100644 samples/bpf/tracex3_user.c
>  create mode 100644 samples/bpf/tracex4_kern.c
>  create mode 100644 samples/bpf/tracex4_user.c
> 


-- 
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt-FCd8Q96Dh0JBDgjK7y7TUQ@public.gmane.org

^ permalink raw reply

* Re: [PATCH v5 5/5] tty/serial: Add Spreadtrum sc9836-uart driver support
From: Lyra Zhang @ 2015-01-19  9:55 UTC (permalink / raw)
  To: Rob Herring
  Cc: Chunyan Zhang, Greg Kroah-Hartman, Mark Rutland, Arnd Bergmann,
	One Thousand Gnomes, Mark Brown, Rob Herring, Pawel Moll,
	Ian Campbell, Kumar Gala, Will Deacon, Catalin Marinas,
	Jiri Slaby, Jason Cooper, Heiko Stübner, Florian Vaussard,
	Andrew Lunn, Robert Richter, Hayato Suzuki, Grant Likely,
	Antony Pavlov, Joel Schopp
In-Reply-To: <CAL_JsqJg=BtanmR_rhYthCUhKbErs8viZ7MMMXr-PmPjUV4BRQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On Sat, Jan 17, 2015 at 12:41 AM, Rob Herring <robherring2-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On Fri, Jan 16, 2015 at 4:00 AM, Chunyan Zhang
> <chunyan.zhang-lxIno14LUO0EEoCn2XhGlw@public.gmane.org> wrote:
>> Add a full sc9836-uart driver for SC9836 SoC which is based on the
>> spreadtrum sharkl64 platform.
>> This driver also support earlycon.
>> This patch also replaced the spaces between the macros and their
>> values with the tabs in serial_core.h
>>
>> Signed-off-by: Chunyan Zhang <chunyan.zhang-lxIno14LUO0EEoCn2XhGlw@public.gmane.org>
>> Signed-off-by: Orson Zhai <orson.zhai-lxIno14LUO0EEoCn2XhGlw@public.gmane.org>
>> Originally-by: Lanqing Liu <lanqing.liu-lxIno14LUO0EEoCn2XhGlw@public.gmane.org>
>> ---
>>  drivers/tty/serial/Kconfig       |   18 +
>>  drivers/tty/serial/Makefile      |    1 +
>>  drivers/tty/serial/sprd_serial.c |  772 ++++++++++++++++++++++++++++++++++++++
>>  include/uapi/linux/serial_core.h |    3 +
>>  4 files changed, 794 insertions(+)
>>  create mode 100644 drivers/tty/serial/sprd_serial.c
>>
>> diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
>> index c79b43c..969d3cd 100644
>> --- a/drivers/tty/serial/Kconfig
>> +++ b/drivers/tty/serial/Kconfig
>> @@ -1577,6 +1577,24 @@ config SERIAL_MEN_Z135
>>           This driver can also be build as a module. If so, the module will be called
>>           men_z135_uart.ko
>>
>> +config SERIAL_SPRD
>> +       tristate "Support for SPRD serial"
>
> Can the menu text spell out Spreadtrum. What SPRD means is not obvious.
>

Ok, I'll address it.

>> +       depends on ARCH_SPRD
>> +       select SERIAL_CORE
>> +       help
>> +         This enables the driver for the Spreadtrum's serial.
>> +
>> +config SERIAL_SPRD_CONSOLE
>> +       bool "SPRD UART console support"
>
> Same here.
>
>> +       depends on SERIAL_SPRD=y
>> +       select SERIAL_CORE_CONSOLE
>> +       select SERIAL_EARLYCON
>> +       help
>> +         Support for early debug console using Spreadtrum's serial. This enables
>> +         the console before standard serial driver is probed. This is enabled
>> +         with "earlycon" on the kernel command line. The console is
>> +         enabled when early_param is processed.
>> +
>>  endmenu
>>
>>  config SERIAL_MCTRL_GPIO
>> diff --git a/drivers/tty/serial/Makefile b/drivers/tty/serial/Makefile
>> index 9a548ac..4801aca 100644
>> --- a/drivers/tty/serial/Makefile
>> +++ b/drivers/tty/serial/Makefile
>> @@ -93,6 +93,7 @@ obj-$(CONFIG_SERIAL_ARC)      += arc_uart.o
>>  obj-$(CONFIG_SERIAL_RP2)       += rp2.o
>>  obj-$(CONFIG_SERIAL_FSL_LPUART)        += fsl_lpuart.o
>>  obj-$(CONFIG_SERIAL_MEN_Z135)  += men_z135_uart.o
>> +obj-$(CONFIG_SERIAL_SPRD) += sprd_serial.o
>>
>>  # GPIOLIB helpers for modem control lines
>>  obj-$(CONFIG_SERIAL_MCTRL_GPIO)        += serial_mctrl_gpio.o
>> diff --git a/drivers/tty/serial/sprd_serial.c b/drivers/tty/serial/sprd_serial.c
>> new file mode 100644
>> index 0000000..81839e4
>> --- /dev/null
>> +++ b/drivers/tty/serial/sprd_serial.c
>> @@ -0,0 +1,772 @@
>> +/*
>> + * Copyright (C) 2012 Spreadtrum Communications Inc.
>
> This is unchanged in 3 years?
>

ok, I'll change it to 2012-2015.

>> + *
>> + * This software is licensed under the terms of the GNU General Public
>> + * License version 2, as published by the Free Software Foundation, and
>> + * may be copied, distributed, and modified under those terms.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + * GNU General Public License for more details.
>> + */
>> +
>> +#include <linux/clk.h>
>> +#include <linux/console.h>
>> +#include <linux/delay.h>
>> +#include <linux/io.h>
>> +#include <linux/ioport.h>
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/serial_core.h>
>> +#include <linux/serial.h>
>> +#include <linux/slab.h>
>> +#include <linux/tty.h>
>> +#include <linux/tty_flip.h>
>> +
>> +/* device name */
>> +#define UART_NR_MAX            8
>> +#define SPRD_TTY_NAME          "ttySPX"
>
> We really want to get away from per SOC serial names and use ttyS for
> serial port /dev names. There's issues switching existing drivers
> because this creates an ABI, so we can only have new drivers follow
> this.
>

ok. i see. I'll convert it to ttyS in the next version.

> [...]
>
>> +static struct platform_driver sprd_platform_driver = {
>> +       .probe          = sprd_probe,
>> +       .remove         = sprd_remove,
>> +       .driver         = {
>> +               .name   = "sprd_serial",
>> +               .of_match_table = of_match_ptr(serial_ids),
>> +               .pm     = &sprd_pm_ops,
>> +       },
>> +};
>> +
>> +static int __init sprd_serial_init(void)
>> +{
>> +       int ret = 0;
>> +
>> +       ret = uart_register_driver(&sprd_uart_driver);
>
> This can be done in probe now. Then you can use module_platform_driver().
>

Question:
1. there are 4 uart ports configured in dt for sprd_serial, so probe
will be called 4 times, but uart_register_driver only needs to be
called one time, so can we use uart_driver.state to check if
uart_register_driver has already been called ?

2. if I use module_platform_driver() instead of
module_init(sprd_serial_init)  and  module_exit(sprd_serial_exit) , I
must move uart_unregister_driver() which is now processed in
sprd_serial_exit() to sprd_remove(), there is a similar problem with
probe(), sprd_remove() will also be called 4 times, and actually it
should be called only one time. How can we deal with this case?

3. for the second question, we can check the platform_device->id, if
it is equal to the index of last port (e.g. 4 for this case), then
uart_unregister_driver() can be called. Does it work correctly? since
for this case, we must keep the order of releasing ports.


Thanks,
Chunyan


> Rob

^ permalink raw reply

* Re: [PATCH v4] gpio: lib-sysfs: Add 'wakeup' attribute
From: Johan Hovold @ 2015-01-19 10:10 UTC (permalink / raw)
  To: Sören Brinkmann
  Cc: Johan Hovold, Linus Walleij, Alexandre Courbot, linux-api,
	linux-kernel, linux-gpio, linux-doc
In-Reply-To: <a4610e13d0094d40b7e0574992509ec6@BN1AFFO11FD044.protection.gbl>

On Fri, Jan 16, 2015 at 08:49:17AM -0800, Sören Brinkmann wrote:
> On Fri, 2015-01-16 at 12:11PM +0100, Johan Hovold wrote:
> > On Thu, Jan 15, 2015 at 11:49:49AM -0800, Soren Brinkmann wrote:
> > > Add an attribute 'wakeup' to the GPIO sysfs interface which allows
> > > marking/unmarking a GPIO as wake IRQ.
> > > The file 'wakeup' is created in each exported GPIOs directory, if an IRQ
> > > is associated with that GPIO and the irqchip implements set_wake().
> > > Writing 'enabled' to that file will enable wake for that GPIO, while
> > > writing 'disabled' will disable wake.
> > > Reading that file will return either 'disabled' or 'enabled' depening on
> > > the currently set flag for the GPIO's IRQ.
> > > 
> > > Signed-off-by: Soren Brinkmann <soren.brinkmann@xilinx.com>
> > > Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>
> > > ---
> > > Hi Linus, Johan,
> > > 
> > > I rebased my patch. And things look good.
> > 
> > I took at closer look at this patch now and I really don't think it
> > should be merged at all.
> > 
> > We have a mechanism for handling wake-up sources (documented in
> > Documentation/power/devices.txt) as well as an ABI to enable/disable
> > them using the power/wakeup device attribute from userspace.
> 
> Doesn't work for GPIOs AFAIK.

Not today no, that's why I said it would take some work.

> > Implementing proper wakeup support for unclaimed GPIOs would take some
> > work (if at all desired), but that is not a reason to be adding custom
> > implementations that violates the kernel's power policies and new ABIs
> > that would need to be maintained forever.
> 
> These are claimed, by the sysfs interface.

Unclaimed by a proper device and driver in the driver model.

> > [ And we really shouldn't be adding anything to the broken gpio sysfs
> > interface until it's been redesigned. ]
> > 
> > Meanwhile you can (should) use gpio-keys if you need to wake your system
> > on gpio events.
> 
> We had that discussion and I don't think GPIO keys is the right solution
> for every use-case.

I can see that, but this still needs to be implemented properly and not
just as a quick hack on top of the already fragile gpio sysfs-interface.

Since pretty much everyone agrees that the current interface needs to be
replaced, we really shouldn't be adding more stuff to the broken
interface before that happens.

> > > But the 'is_visible' things does not behave the way I expected it to.
> > > It seems to be only triggered on an export but not when attributes
> > > change. Hence, in my case, everything was visiible since the inital
> > > state matches that, but even when changing the direction or things
> > > like that, attributes don't disappear. Is that something still worked
> > > on? Expected
> > 
> > That's expected. We generally don't want attributes to appear or
> > disappear after the device has been registered (although there is a
> > mechanism for cases were it makes sense). This is no different from
> > how your v3 patch worked either.
> 
> Sure, but the is_visible thing is effectively broken for GPIO. I think a
> GPIO is in a defined state when exported and the checks all work on that
> state during export. But then this state can be changed through the
> sysfs interface. So, if the initial state hides something it becomes
> unavailable for all times and, vice versa, if the initial state makes
> something visible, it will stay even when it is no longer a valid
> property to change.

Again, this is exactly how the interface has always worked, and that's
exactly how your v3, which added the attributes manually, also worked.

The group-visibility mechanism is not broken. What's broken is interface
designs based on attributes magically disappearing and reappearing after
the device has been created.

Johan

^ permalink raw reply

* Re: futex(2) man page update help request
From: Thomas Gleixner @ 2015-01-19 10:45 UTC (permalink / raw)
  To: Darren Hart
  Cc: Michael Kerrisk (man-pages), Carlos O'Donell, Ingo Molnar,
	Jakub Jelinek, linux-man-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	lkml, Davidlohr Bueso, Arnd Bergmann, Steven Rostedt,
	Peter Zijlstra, Linux API, Torvald Riegel, Roland McGrath,
	Darren Hart, Anton Blanchard, Petr Baudis, Eric Dumazet,
	bill o gallmeister, Jan Kiszka, Daniel Wagner, Rich Felker
In-Reply-To: <D0DEECA2.B7EAD%dvhart-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>

On Fri, 16 Jan 2015, Darren Hart wrote:
> On 1/16/15, 12:54 PM, "Michael Kerrisk (man-pages)"
> <mtk.manpages-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> 
> >On 01/16/2015 04:20 PM, Thomas Gleixner wrote:
> >> On Fri, 16 Jan 2015, Michael Kerrisk (man-pages) wrote:
> >> 
> >>> Hello Thomas,
> >>>
> >>> On 01/15/2015 11:23 PM, Thomas Gleixner wrote:
> >>>> On Thu, 15 Jan 2015, Michael Kerrisk (man-pages) wrote:
> >>>>>> [EINVAL] uaddr equal uaddr2. Requeue to same futex.
> >>>>>
> >>>>> ??? I added this, but does this error not occur only for PI requeues?
> >>>>
> >>>> It's equally wrong for normal futexes. And its actually the same code
> >>>> checking for this for all variants.
> >>>
> >>> I don't understand "equally wrong" in your reply, I'm sorry. Do you
> >>> mean:
> >>>
> >>> a) This error text should be there for both normal and PI requeues
> >> 
> >> It is there for both. The requeue code has that check independent of
> >> the requeue type (normal/pi). It never makes sense to requeue
> >> something to itself whether normal or pi futex. We added this for PI,
> >> because there it is harmful, but we did not special case it. So normal
> >> futexes get the same treatment.
> >
> >Hello Thomas, 
> >
> >Color me stupid, but I can't see this in futex_requeue(). Where is that
> >check that is "independent of the requeue type (normal/pi)"?
> >
> >When I look through futex_requeue(), all the likely looking sources
> >of EINVAL are governed by a check on the 'requeue_pi' argument.
> 
> 
> Right, in the non-PI case, I believe there are valid use cases: move to
> the back of the FIFO, for example (OK, maybe the only example?). Both
> tests ensuring uaddr1 != uaddr2 are under the requeue_pi conditional
> block. The second compares the keys in case they are not FUTEX_PRIVATE
> (uaddrs would be different, but still the same backing store).
> 
> Thomas, am I missing a test for this someplace else?

No, I had a short look at the code misread it. So, yes, it's a valid
operation for the non PI case. Sorry for the confusion.

Thanks,

	tglx

^ permalink raw reply

* Re: futex(2) man page update help request
From: Michael Kerrisk (man-pages) @ 2015-01-19 14:07 UTC (permalink / raw)
  To: Thomas Gleixner, Darren Hart
  Cc: mtk.manpages, Carlos O'Donell, Ingo Molnar, Jakub Jelinek,
	linux-man@vger.kernel.org, lkml, Davidlohr Bueso, Arnd Bergmann,
	Steven Rostedt, Peter Zijlstra, Linux API, Torvald Riegel,
	Roland McGrath, Darren Hart, Anton Blanchard, Petr Baudis,
	Eric Dumazet, bill o gallmeister, Jan Kiszka, Daniel Wagner,
	Rich Felker
In-Reply-To: <alpine.DEB.2.11.1501191144320.5526@nanos>

On 01/19/2015 11:45 AM, Thomas Gleixner wrote:
> On Fri, 16 Jan 2015, Darren Hart wrote:
>> On 1/16/15, 12:54 PM, "Michael Kerrisk (man-pages)"
>> <mtk.manpages@gmail.com> wrote:
>>
>>> On 01/16/2015 04:20 PM, Thomas Gleixner wrote:
>>>> On Fri, 16 Jan 2015, Michael Kerrisk (man-pages) wrote:
>>>>
>>>>> Hello Thomas,
>>>>>
>>>>> On 01/15/2015 11:23 PM, Thomas Gleixner wrote:
>>>>>> On Thu, 15 Jan 2015, Michael Kerrisk (man-pages) wrote:
>>>>>>>> [EINVAL] uaddr equal uaddr2. Requeue to same futex.
>>>>>>>
>>>>>>> ??? I added this, but does this error not occur only for PI requeues?
>>>>>>
>>>>>> It's equally wrong for normal futexes. And its actually the same code
>>>>>> checking for this for all variants.
>>>>>
>>>>> I don't understand "equally wrong" in your reply, I'm sorry. Do you
>>>>> mean:
>>>>>
>>>>> a) This error text should be there for both normal and PI requeues
>>>>
>>>> It is there for both. The requeue code has that check independent of
>>>> the requeue type (normal/pi). It never makes sense to requeue
>>>> something to itself whether normal or pi futex. We added this for PI,
>>>> because there it is harmful, but we did not special case it. So normal
>>>> futexes get the same treatment.
>>>
>>> Hello Thomas, 
>>>
>>> Color me stupid, but I can't see this in futex_requeue(). Where is that
>>> check that is "independent of the requeue type (normal/pi)"?
>>>
>>> When I look through futex_requeue(), all the likely looking sources
>>> of EINVAL are governed by a check on the 'requeue_pi' argument.
>>
>>
>> Right, in the non-PI case, I believe there are valid use cases: move to
>> the back of the FIFO, for example (OK, maybe the only example?). Both
>> tests ensuring uaddr1 != uaddr2 are under the requeue_pi conditional
>> block. The second compares the keys in case they are not FUTEX_PRIVATE
>> (uaddrs would be different, but still the same backing store).
>>
>> Thomas, am I missing a test for this someplace else?
> 
> No, I had a short look at the code misread it. So, yes, it's a valid
> operation for the non PI case. Sorry for the confusion.

Thanks for the confirmation, Thomas. Page updated to remove 
FUTEX_CMP_REQUEUE from that error case.

Cheers,

Michael


-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

^ permalink raw reply

* Re: [PATCH v5 5/5] tty/serial: Add Spreadtrum sc9836-uart driver support
From: Rob Herring @ 2015-01-19 14:11 UTC (permalink / raw)
  To: Lyra Zhang
  Cc: Chunyan Zhang, Greg Kroah-Hartman, Mark Rutland, Arnd Bergmann,
	One Thousand Gnomes, Mark Brown, Rob Herring, Pawel Moll,
	Ian Campbell, Kumar Gala, Will Deacon, Catalin Marinas,
	Jiri Slaby, Jason Cooper, Heiko Stübner, Florian Vaussard,
	Andrew Lunn, Robert Richter, Hayato Suzuki, Grant Likely,
	Antony Pavlov, Joel Schopp
In-Reply-To: <CAAfSe-vqqpvbYe0bEeD-3_QsLYz4r7pxMuA1-L4cYRw9TY=NgA@mail.gmail.com>

On Mon, Jan 19, 2015 at 3:55 AM, Lyra Zhang <zhang.lyra@gmail.com> wrote:
> On Sat, Jan 17, 2015 at 12:41 AM, Rob Herring <robherring2@gmail.com> wrote:
>> On Fri, Jan 16, 2015 at 4:00 AM, Chunyan Zhang
>> <chunyan.zhang@spreadtrum.com> wrote:
>>> Add a full sc9836-uart driver for SC9836 SoC which is based on the
>>> spreadtrum sharkl64 platform.
>>> This driver also support earlycon.
>>> This patch also replaced the spaces between the macros and their
>>> values with the tabs in serial_core.h

[...]

>>> +static int __init sprd_serial_init(void)
>>> +{
>>> +       int ret = 0;
>>> +
>>> +       ret = uart_register_driver(&sprd_uart_driver);
>>
>> This can be done in probe now. Then you can use module_platform_driver().
>>
>
> Question:
> 1. there are 4 uart ports configured in dt for sprd_serial, so probe
> will be called 4 times, but uart_register_driver only needs to be
> called one time, so can we use uart_driver.state to check if
> uart_register_driver has already been called ?

Yes.

> 2. if I use module_platform_driver() instead of
> module_init(sprd_serial_init)  and  module_exit(sprd_serial_exit) , I
> must move uart_unregister_driver() which is now processed in
> sprd_serial_exit() to sprd_remove(), there is a similar problem with
> probe(), sprd_remove() will also be called 4 times, and actually it
> should be called only one time. How can we deal with this case?

Look at pl01x or Samsung UART drivers which have done this conversion.

> 3. for the second question, we can check the platform_device->id, if
> it is equal to the index of last port (e.g. 4 for this case), then
> uart_unregister_driver() can be called. Does it work correctly? since
> for this case, we must keep the order of releasing ports.

The id will not be the line index in the DT case. I don't think you
can guarantee the order either.

It would be better to make uart_{un}register_driver deal with being
called multiple times so drivers don't have to deal with getting this
correct. I'm not sure if that is feasible though.

Rob

^ permalink raw reply

* Re: [PATCH 4/6] kbuild: add a new kselftest_install make target to install selftests
From: Shuah Khan @ 2015-01-19 16:34 UTC (permalink / raw)
  To: Michael Ellerman
  Cc: linux-kernel, mmarek, gregkh, akpm, rostedt, mingo, davem,
	keescook, tranmanphong, cov, dh.herrmann, hughd, bobby.prani,
	serge.hallyn, ebiederm, tim.bird, josh, koct9i, linux-kbuild,
	linux-api, netdev
In-Reply-To: <1421627747.3787.7.camel@ellerman.id.au>

On 01/18/2015 05:35 PM, Michael Ellerman wrote:
> On Fri, 2015-01-16 at 09:34 -0700, Shuah Khan wrote:
>> On 01/09/2015 02:06 AM, Michael Ellerman wrote:
>>> Add a new make target to install kernel selftests. This new target will
>>> build and install selftests.
>>>
>>> The default is just $(objtree)/selftests. This is preferable to
>>> something based on $(INSTALL_MOD_PATH) (which defaults to /), as it
>>> allows a normal user to install the tests. This is similar to the
>>> default behaviour of make headers_install.
>>
>> A normal user can install tests at any location they choose by
>> overriding the default path. For example:
>>
>> INSTALL_MOD_PATH=/tmp make kselftest_install
>>
>> will install under tmp.
> 
> Why default to a directory that most users can't write to? That's not helpful.
> 
> Users who are root can override the path, for example:
> 
> INSTALL_MOD_PATH=/ make kselftest_install
> 
>> The approach I used also ties test installs to kernel release.
>> This addresses an important use-case for kernel developers
>> that want to compare results from release to release.
> 
> Sure, I'm happy to add the kernel release, so the default would be
> $(objtree)/selftests/$(kernel-release)/.
> 
>> The use-case for any user to be able to install tests at
>> any location is addressed by the above example.
> 
> The default should work for most users most of the time, / does not achieve
> that.
> 
>> I would like these two above use-cases continued to be supported,
>> especially the one that tries the test installs to kernel release.
> 
> That's fine, I'm happy to update this to use kernel release. But defaulting to
> / doesn't make sense.
> 

I want to keep the kselftest installs to default to the location
other kernel installs such as firmware and modules default to.
This keep the use-case the same as other kernel installs. As user
can override the location, I don't any problems with this.

For most kernel developers, INSTALL_MOD_PATH is the familiar place
to cleanup kernels and I don't see any reason to change that.

You are welcome to send patches to simply the install process for
the individual tests for the next release. I want individual patches
for each test and a separate patch for the shared logic patch.

thanks,
-- Shuah


-- 
Shuah Khan
Sr. Linux Kernel Developer
Open Source Innovation Group
Samsung Research America (Silicon Valley)
shuahkh@osg.samsung.com | (970) 217-8978

^ permalink raw reply

* Re: [PATCH 2/6] selftests: Add install target
From: Shuah Khan @ 2015-01-19 16:35 UTC (permalink / raw)
  To: Michael Ellerman
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA, mmarek-AlSwsSmVLrQ,
	gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
	akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
	rostedt-nx8X9YLhiw1AfugRpC6u6w, mingo-H+wXaHxf7aLQT0dZR+AlfA,
	davem-fT/PcQaiUtIeIZ0/mPfg9Q, keescook-F7+t8E8rja9g9hUCZPvPmw,
	tranmanphong-Re5JQEeQqe8AvxtiuMwx3w, cov-sgV2jX0FEOL9JmXXK+q4OQ,
	dh.herrmann-Re5JQEeQqe8AvxtiuMwx3w, hughd-hpIqsD4AKlfQT0dZR+AlfA,
	bobby.prani-Re5JQEeQqe8AvxtiuMwx3w,
	serge.hallyn-GeWIH/nMZzLQT0dZR+AlfA,
	ebiederm-aS9lmoZGLiVWk0Htik3J/w, tim.bird-/MT0OVThwyLZJqsBc5GL+g,
	josh-iaAMLnmF4UmaiuxdJuQwMA, koct9i-Re5JQEeQqe8AvxtiuMwx3w,
	linux-kbuild-u79uwXL29TY76Z2rM5mHXA,
	linux-api-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1421627752.3787.8.camel-Gsx/Oe8HsFggBc27wqDAHg@public.gmane.org>

On 01/18/2015 05:35 PM, Michael Ellerman wrote:
> On Fri, 2015-01-16 at 10:46 -0700, Shuah Khan wrote:
>> On 01/09/2015 02:06 AM, Michael Ellerman wrote:
>>> This adds make install support to selftests. The basic usage is:
>>>
>>> $ cd tools/testing/selftests
>>> $ make install
>>>
>>> That installs into tools/testing/selftests/install, which can then be
>>> copied where ever necessary.
>>>
>>> The install destination is also configurable using eg:
>>>
>>> $ INSTALL_PATH=/mnt/selftests make install
>>
>> Please see my response to [PATCH 4/6] kbuild: add a new
>> kselftest_install make target to install selftests
>>
>> These are addressed by the current approach to use existing
>> INSTALL_MOD_PATH.
> 
> No that's a separate issue.
> 
> This patch adds install support for tools/testing/selftests, *completely
> separate* from the kbuild infrastructure. 
> 

What's the use-case for this feature? I don't see why we need multiple
ways to do the install?

thanks,
-- Shuah


-- 
Shuah Khan
Sr. Linux Kernel Developer
Open Source Innovation Group
Samsung Research America (Silicon Valley)
shuahkh-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org | (970) 217-8978

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox