* [PATCH] power: introduce library for device-specific OPPs
From: Menon, Nishanth @ 2010-10-07 22:03 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <201010072354.57011.rjw@sisk.pl>
> -----Original Message-----
> From: Rafael J. Wysocki [mailto:rjw at sisk.pl]
> Sent: Thursday, October 07, 2010 4:55 PM
>
> Hi,
>
> On Wednesday, October 06, 2010, Nishanth Menon wrote:
> > SoCs have a standard set of tuples consisting of frequency and
> > voltage pairs that the device will support per voltage domain. These
> > are called Operating Performance Points or OPPs. The actual
> > definitions of OPP varies over silicon versions. For a specific domain,
> > we can have a set of {frequency, voltage} pairs. As the kernel boots
> > and more information is available, a default set of these are activated
> > based on the precise nature of device. Further on operation, based on
> > conditions prevailing in the system (such as temperature), some OPP
> > availability may be temporarily controlled by the SoC frameworks.
> >
> > To implement an OPP, some sort of power management support is necessary
> > hence this library depends on CONFIG_PM.
>
> The patch generally looks good to me, I only have a couple of cosmetic
Thanks for the great reviews.. It did bump up the resultant patch.
> remarks
> (below).
>
> ...
> > +static int opp_set_availability(struct device *dev, unsigned long freq,
> > + bool availability_req)
> > +{
> > + struct device_opp *tmp_dev_opp, *dev_opp = NULL;
> > + struct opp *new_opp, *tmp_opp, *opp = ERR_PTR(-ENODEV);
> > + int r = 0;
> > +
> > + /* keep the node allocated */
> > + new_opp = kmalloc(sizeof(struct opp), GFP_KERNEL);
> > + if (!new_opp) {
> > + pr_warning("Unable to allocate opp\n");
>
> Please add an identification string to the messages, something like
> "OPP: Unable to allocat object\n" (and similarly in the other messages).
> That would help to find the source of a message in case there's any
> problem.
pr_fmt has been reformatted for this. The actual message which will appear
is as follows:
opp_set_availability: Unable to allocate opp
is'nt that good enough considering that all functions are opp_ prefixed?
I can modify pr_fmt to add "OPP:" but I kinda think it is redundant. But I
have no strong opinions on that and look forward to your recommendations.
>
>
> > + return -ENOMEM;
> > + }
> > +
> > + mutex_lock(&dev_opp_list_lock);
> > +
> > + /* Find the device_opp */
> > + list_for_each_entry(tmp_dev_opp, &dev_opp_list, node) {
> > + if (dev == tmp_dev_opp->dev) {
> > + dev_opp = tmp_dev_opp;
> > + break;
> > + }
> > + }
> > + if (IS_ERR(dev_opp)) {
> > + r = PTR_ERR(dev_opp);
> > + pr_warning("Unable to find device\n");
> > + goto out1;
>
> I'd prefer this lable to be called "unlock". It will be a bit more
> informative.
Ack. Fixing in v6 if you are ok with the above.
Regards,
Nishanth Menon
^ permalink raw reply
* Re: [PATCH] power: introduce library for device-specific OPPs
From: Menon, Nishanth @ 2010-10-07 22:03 UTC (permalink / raw)
To: Rafael J. Wysocki; +Cc: Paul, linux-pm, l-o, lkml, l-a
In-Reply-To: <201010072354.57011.rjw@sisk.pl>
> -----Original Message-----
> From: Rafael J. Wysocki [mailto:rjw@sisk.pl]
> Sent: Thursday, October 07, 2010 4:55 PM
>
> Hi,
>
> On Wednesday, October 06, 2010, Nishanth Menon wrote:
> > SoCs have a standard set of tuples consisting of frequency and
> > voltage pairs that the device will support per voltage domain. These
> > are called Operating Performance Points or OPPs. The actual
> > definitions of OPP varies over silicon versions. For a specific domain,
> > we can have a set of {frequency, voltage} pairs. As the kernel boots
> > and more information is available, a default set of these are activated
> > based on the precise nature of device. Further on operation, based on
> > conditions prevailing in the system (such as temperature), some OPP
> > availability may be temporarily controlled by the SoC frameworks.
> >
> > To implement an OPP, some sort of power management support is necessary
> > hence this library depends on CONFIG_PM.
>
> The patch generally looks good to me, I only have a couple of cosmetic
Thanks for the great reviews.. It did bump up the resultant patch.
> remarks
> (below).
>
> ...
> > +static int opp_set_availability(struct device *dev, unsigned long freq,
> > + bool availability_req)
> > +{
> > + struct device_opp *tmp_dev_opp, *dev_opp = NULL;
> > + struct opp *new_opp, *tmp_opp, *opp = ERR_PTR(-ENODEV);
> > + int r = 0;
> > +
> > + /* keep the node allocated */
> > + new_opp = kmalloc(sizeof(struct opp), GFP_KERNEL);
> > + if (!new_opp) {
> > + pr_warning("Unable to allocate opp\n");
>
> Please add an identification string to the messages, something like
> "OPP: Unable to allocat object\n" (and similarly in the other messages).
> That would help to find the source of a message in case there's any
> problem.
pr_fmt has been reformatted for this. The actual message which will appear
is as follows:
opp_set_availability: Unable to allocate opp
is'nt that good enough considering that all functions are opp_ prefixed?
I can modify pr_fmt to add "OPP:" but I kinda think it is redundant. But I
have no strong opinions on that and look forward to your recommendations.
>
>
> > + return -ENOMEM;
> > + }
> > +
> > + mutex_lock(&dev_opp_list_lock);
> > +
> > + /* Find the device_opp */
> > + list_for_each_entry(tmp_dev_opp, &dev_opp_list, node) {
> > + if (dev == tmp_dev_opp->dev) {
> > + dev_opp = tmp_dev_opp;
> > + break;
> > + }
> > + }
> > + if (IS_ERR(dev_opp)) {
> > + r = PTR_ERR(dev_opp);
> > + pr_warning("Unable to find device\n");
> > + goto out1;
>
> I'd prefer this lable to be called "unlock". It will be a bit more
> informative.
Ack. Fixing in v6 if you are ok with the above.
Regards,
Nishanth Menon
^ permalink raw reply
* RE: [PATCH] power: introduce library for device-specific OPPs
From: Menon, Nishanth @ 2010-10-07 22:03 UTC (permalink / raw)
To: Rafael J. Wysocki; +Cc: Paul, linux-pm, l-o, lkml, l-a
In-Reply-To: <201010072354.57011.rjw@sisk.pl>
> -----Original Message-----
> From: Rafael J. Wysocki [mailto:rjw@sisk.pl]
> Sent: Thursday, October 07, 2010 4:55 PM
>
> Hi,
>
> On Wednesday, October 06, 2010, Nishanth Menon wrote:
> > SoCs have a standard set of tuples consisting of frequency and
> > voltage pairs that the device will support per voltage domain. These
> > are called Operating Performance Points or OPPs. The actual
> > definitions of OPP varies over silicon versions. For a specific domain,
> > we can have a set of {frequency, voltage} pairs. As the kernel boots
> > and more information is available, a default set of these are activated
> > based on the precise nature of device. Further on operation, based on
> > conditions prevailing in the system (such as temperature), some OPP
> > availability may be temporarily controlled by the SoC frameworks.
> >
> > To implement an OPP, some sort of power management support is necessary
> > hence this library depends on CONFIG_PM.
>
> The patch generally looks good to me, I only have a couple of cosmetic
Thanks for the great reviews.. It did bump up the resultant patch.
> remarks
> (below).
>
> ...
> > +static int opp_set_availability(struct device *dev, unsigned long freq,
> > + bool availability_req)
> > +{
> > + struct device_opp *tmp_dev_opp, *dev_opp = NULL;
> > + struct opp *new_opp, *tmp_opp, *opp = ERR_PTR(-ENODEV);
> > + int r = 0;
> > +
> > + /* keep the node allocated */
> > + new_opp = kmalloc(sizeof(struct opp), GFP_KERNEL);
> > + if (!new_opp) {
> > + pr_warning("Unable to allocate opp\n");
>
> Please add an identification string to the messages, something like
> "OPP: Unable to allocat object\n" (and similarly in the other messages).
> That would help to find the source of a message in case there's any
> problem.
pr_fmt has been reformatted for this. The actual message which will appear
is as follows:
opp_set_availability: Unable to allocate opp
is'nt that good enough considering that all functions are opp_ prefixed?
I can modify pr_fmt to add "OPP:" but I kinda think it is redundant. But I
have no strong opinions on that and look forward to your recommendations.
>
>
> > + return -ENOMEM;
> > + }
> > +
> > + mutex_lock(&dev_opp_list_lock);
> > +
> > + /* Find the device_opp */
> > + list_for_each_entry(tmp_dev_opp, &dev_opp_list, node) {
> > + if (dev == tmp_dev_opp->dev) {
> > + dev_opp = tmp_dev_opp;
> > + break;
> > + }
> > + }
> > + if (IS_ERR(dev_opp)) {
> > + r = PTR_ERR(dev_opp);
> > + pr_warning("Unable to find device\n");
> > + goto out1;
>
> I'd prefer this lable to be called "unlock". It will be a bit more
> informative.
Ack. Fixing in v6 if you are ok with the above.
Regards,
Nishanth Menon
^ permalink raw reply
* Re: [PATCH] serial: DCC(JTAG) serial and console emulation support
From: Greg KH @ 2010-10-07 21:52 UTC (permalink / raw)
To: Daniel Walker
Cc: Alan Cox, linux-kernel, Hyok S. Choi, Tony Lindgren,
Jeff Ohlstein, Ben Dooks, Alan Cox, Kukjin Kim, Mike Frysinger,
Feng Tang, Tobias Klauser, Jason Wessel, Philippe Langlais
In-Reply-To: <1286488038.23836.101.camel@c-dwalke-linux.qualcomm.com>
On Thu, Oct 07, 2010 at 02:47:18PM -0700, Daniel Walker wrote:
> On Thu, 2010-10-07 at 14:15 -0700, Greg KH wrote:
> > On Thu, Oct 07, 2010 at 01:51:18PM -0700, Daniel Walker wrote:
> > > Your making too many assumptions .. You might be able to modify the
> > > kernel, and not the userspace. So you couldn't tweak the device
> > > creation .. It's much easier in the server world ..
> >
> > You're saying it's easier to replace an embedded kernel than a userspace
> > file on an embedded system? Heh, that's funny.
>
> I'm saying it _can_ happen that it's easier to replace the kernel.. I'm
> not saying it's always the case. We're talking about debugging
> situations in an embedded environment which don't always follow a normal
> path.
Nothing is "normal", so stop trying to say it is easier in the "server"
world please.
> > > > We've said no over a period of about ten years to a lot of attempts to
> > > > just borrow the ttyS0 range. If we'd said yes it would have been a
> > > > complete mess by now.
> > > >
> > > > So the answer is no.
> > >
> > > Nothing can be unilateral, there's always room for exceptions. You
> > > should say something more like "it's possible, but unlikely".
> >
> > Hm, how about this, as the TTY and serial driver[1] maintainer, I will
> > not accept this kind of patch at all.
> >
> > Is that final enough for you?
>
> So you don't like it, that's fair enough .. <thinks>I wonder what other
> maintainers I can send this too</thinks> ;)
>
> Can you be more specific about your objections
See everything that Mike and Alan wrote, do I need to repeat it?
greg k-h
^ permalink raw reply
* Re: memory clobber in rx path, maybe related to ath9k.
From: Luis R. Rodriguez @ 2010-10-07 21:59 UTC (permalink / raw)
To: Johannes Berg; +Cc: Ben Greear, linux-wireless@vger.kernel.org
In-Reply-To: <AANLkTi=t-P_5c14brJohM8UgxeOknv+3syO=7W=ix6qf@mail.gmail.com>
On Thu, Oct 7, 2010 at 2:36 PM, Luis R. Rodriguez <mcgrof@gmail.com> wrote:
> On Thu, Oct 7, 2010 at 2:31 PM, Luis R. Rodriguez <mcgrof@gmail.com> wrote:
>> On Thu, Oct 7, 2010 at 12:27 PM, Johannes Berg
>> <johannes@sipsolutions.net> wrote:
>>> On Thu, 2010-10-07 at 12:22 -0700, Ben Greear wrote:
>>>
>>>> After reboot, and re-run of the script,
>>>> I saw this in the logs, and shortly after,
>>>> the SLUB poison warning dumped to screen.
>>>>
>>>> Maybe those DMA errors are serious?
>>>
>>>> ath: Failed to stop TX DMA in 100 msec after killing last frame
>>>> ath: Failed to stop TX DMA. Resetting hardware!
>>>
>>> That's TX DMA, it can hardly result in invalid memory writes like the
>>> ones you've been seeing.
>>>
>>> I'm still convinced something is wrong with ath9k RX DMA, as you've seen
>>> the contents of frames written to already freed memory regions. Since I
>>> don't know anything about ath9k, you should probably not rely on me
>>> though :-)
>>
>> I'm on this now. Lets play.
>>
>> I had to remove /lib/udev/rules.d/75-persistent-net-generator.rules
>> to avoid Ubuntu trying to remember the device names and it creating
>> stax_rename names.
>> I just ran your script with some modifications. You can find it here:
>>
>> http://www.kernel.org/pub/linux/kernel/people/mcgrof/scripts/poo.pl
>>
>> I then ran:
>>
>> for i in $(seq 0 31) ; do sudo dhclient seq$i; done
>>
>> It took about 10 minutes to get IP addresses for all interfaces but it
>> got there eventually. Odd enough I was unable to ping the AP from any
>> interface though. Not sure what that was about. But I got no oops, no
>> slub dump. All I got was some more delba warnings which seems to
>> indicate we haven't caught all the cases for its use:
>>
>> [ 3622.660344] addBA response timer expired on tid 0
>> [ 3622.660373] Tx BA session stop requested for 68:7f:74:3b:b1:0f tid 0
>> [ 3622.680133] addBA response timer expired on tid 0
>> [ 3622.687196] Tx BA session stop requested for 68:7f:74:3b:b1:0f tid 0
>> [ 3623.110077] addBA response timer expired on tid 0
>> [ 3623.110123] Tx BA session stop requested for 68:7f:74:3b:b1:0f tid 0
>> [ 3628.935895] sta10: authenticate with 68:7f:74:3b:b1:10 (try 1)
>> [ 3628.937194] switched off addBA timer for tid 0
>> [ 3628.937196] Aggregation is on for tid 0
>> [ 3628.937239] Stopping Tx BA session for 68:7f:74:3b:b1:0f tid 0
>> [ 3628.937243] ------------[ cut here ]------------
>> [ 3628.937263] WARNING: at include/net/mac80211.h:2694
>> rate_control_send_low+0xd3/0x140 [mac80211]()
>> [ 3628.937265] Hardware name: 6460DWU
>> [ 3628.937266] Modules linked in: binfmt_misc ppdev
>> snd_hda_codec_analog rfcomm sco bridge joydev stp bnep l2cap nouveau
>> ath9k snd_hda_intel mac80211 snd_hda_codec snd_hwdep snd_pcm ttm btusb
>> ath9k_common thinkpad_acpi ath9k_hw bluetooth drm_kms_helper
>> snd_seq_midi snd_rawmidi pcmcia snd_seq_midi_event drm snd_seq ath
>> snd_timer snd_seq_device tpm_tis i2c_algo_bit cfg80211 snd nvram tpm
>> tpm_bios yenta_socket pcmcia_rsrc video psmouse output pcmcia_core
>> serio_raw soundcore snd_page_alloc intel_agp lp parport ohci1394
>> e1000e ieee1394 ahci libahci
>> [ 3628.937307] Pid: 49, comm: kworker/u:3 Tainted: G W
>> 2.6.36-rc6-wl+ #263
>> [ 3628.937310] Call Trace:
>> [ 3628.937317] [<ffffffff8105ffcf>] warn_slowpath_common+0x7f/0xc0
>> [ 3628.937320] [<ffffffff8106002a>] warn_slowpath_null+0x1a/0x20
>> [ 3628.937329] [<ffffffffa032f603>] rate_control_send_low+0xd3/0x140 [mac80211]
>> [ 3628.937336] [<ffffffffa038bfd8>] ath_get_rate+0x48/0x570 [ath9k]
>> [ 3628.937340] [<ffffffff812b9f39>] ? put_dec+0x59/0x60
>> [ 3628.937349] [<ffffffffa032f42e>] rate_control_get_rate+0x8e/0x190 [mac80211]
>> [ 3628.937360] [<ffffffffa0339928>]
>> ieee80211_tx_h_rate_ctrl+0x1a8/0x4e0 [mac80211]
>> [ 3628.937370] [<ffffffffa033a000>] invoke_tx_handlers+0x100/0x140 [mac80211]
>> [ 3628.937379] [<ffffffffa033a0c5>] ieee80211_tx+0x85/0x240 [mac80211]
>> [ 3628.937384] [<ffffffff8147b890>] ? skb_release_data+0xd0/0xe0
>> [ 3628.937386] [<ffffffff8147d72f>] ? pskb_expand_head+0x10f/0x1a0
>> [ 3628.937397] [<ffffffffa033a336>] ieee80211_xmit+0xb6/0x1d0 [mac80211]
>> [ 3628.937399] [<ffffffff8147b9d3>] ? __alloc_skb+0x83/0x170
>> [ 3628.937409] [<ffffffffa033a4a4>] ieee80211_tx_skb+0x54/0x70 [mac80211]
>> [ 3628.937418] [<ffffffffa03230ad>] ieee80211_send_delba+0x11d/0x190 [mac80211]
>> [ 3628.937427] [<ffffffffa0323a18>]
>> ieee80211_stop_tx_ba_cb+0x1b8/0x240 [mac80211]
>> [ 3628.937431] [<ffffffff81036c89>] ? default_spin_lock_flags+0x9/0x10
>> [ 3628.937440] [<ffffffffa032e031>] ieee80211_iface_work+0x271/0x340 [mac80211]
>> [ 3628.937450] [<ffffffffa032ddc0>] ? ieee80211_iface_work+0x0/0x340 [mac80211]
>> [ 3628.937453] [<ffffffff8107a203>] process_one_work+0x123/0x440
>> [ 3628.937457] [<ffffffff8107c750>] worker_thread+0x170/0x400
>> [ 3628.937460] [<ffffffff8107c5e0>] ? worker_thread+0x0/0x400
>> [ 3628.937463] [<ffffffff81080b76>] kthread+0x96/0xa0
>> [ 3628.937466] [<ffffffff8100bea4>] kernel_thread_helper+0x4/0x10
>> [ 3628.937469] [<ffffffff81080ae0>] ? kthread+0x0/0xa0
>> [ 3628.937472] [<ffffffff8100bea0>] ? kernel_thread_helper+0x0/0x10
>> [ 3628.937474] ---[ end trace 9dd0d025ccb9b75c ]---
>> [ 3628.937980] switched off addBA timer for tid 0
>> [ 3628.937982] Aggregation is on for tid 0
>>
>> But other than this I got nothing. I left the box sit there for about
>> 1 hour and came back and it was still going with no issues. Mind you,
>> I can't ping but that seems like another issue.
>>
>> You can find my logs here:
>>
>> http://www.kernel.org/pub/linux/kernel/people/mcgrof/logs/2010/10-07-stress-sta-01/
>
> Doh, I did not have CONFIG_SLUB_DEBUG_ON=y so building now with that.
Yay I can reproduce now. I'll be back, going to dig now.
Luis
^ permalink raw reply
* + percpu_counter-use-this_cpu_ptr-instead-of-per_cpu_ptr.patch added to -mm tree
From: akpm @ 2010-10-07 21:59 UTC (permalink / raw)
To: mm-commits; +Cc: cl, eric.dumazet, tj
The patch titled
percpu_counter: use this_cpu_ptr() instead of per_cpu_ptr()
has been added to the -mm tree. Its filename is
percpu_counter-use-this_cpu_ptr-instead-of-per_cpu_ptr.patch
Before you just go and hit "reply", please:
a) Consider who else should be cc'ed
b) Prefer to cc a suitable mailing list as well
c) Ideally: find the original patch on the mailing list and do a
reply-to-all to that, adding suitable additional cc's
*** Remember to use Documentation/SubmitChecklist when testing your code ***
See http://userweb.kernel.org/~akpm/stuff/added-to-mm.txt to find
out what to do about this
The current -mm tree may be found at http://userweb.kernel.org/~akpm/mmotm/
------------------------------------------------------
Subject: percpu_counter: use this_cpu_ptr() instead of per_cpu_ptr()
From: Christoph Lameter <cl@linux.com>
this_cpu_ptr() avoids an array lookup and can use the percpu offset of the
local cpu directly.
Signed-off-by: Christoph Lameter <cl@linux.com>
Cc: Eric Dumazet <eric.dumazet@gmail.com>
Cc: Tejun Heo <tj@kernel.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---
lib/percpu_counter.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff -puN lib/percpu_counter.c~percpu_counter-use-this_cpu_ptr-instead-of-per_cpu_ptr lib/percpu_counter.c
--- a/lib/percpu_counter.c~percpu_counter-use-this_cpu_ptr-instead-of-per_cpu_ptr
+++ a/lib/percpu_counter.c
@@ -73,9 +73,9 @@ void __percpu_counter_add(struct percpu_
{
s64 count;
s32 *pcount;
- int cpu = get_cpu();
- pcount = per_cpu_ptr(fbc->counters, cpu);
+ preempt_disable();
+ pcount = this_cpu_ptr(fbc->counters);
count = *pcount + amount;
if (count >= batch || count <= -batch) {
spin_lock(&fbc->lock);
@@ -85,7 +85,7 @@ void __percpu_counter_add(struct percpu_
} else {
*pcount = count;
}
- put_cpu();
+ preempt_enable();
}
EXPORT_SYMBOL(__percpu_counter_add);
_
Patches currently in -mm which might be from cl@linux.com are
linux-next.patch
mm-compaction-fix-compactpagefailed-counting.patch
mm-mempolicy-check-return-code-of-check_range.patch
percpu_counter-use-this_cpu_ptr-instead-of-per_cpu_ptr.patch
^ permalink raw reply
* Re: 2.6.36-rc7 warning in __cfg80211_auth_remove
From: Johannes Stezenbach @ 2010-10-07 21:59 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless
In-Reply-To: <1286482766.20974.39.camel@jlt3.sipsolutions.net>
On Thu, Oct 07, 2010 at 10:19:26PM +0200, Johannes Berg wrote:
> On Thu, 2010-10-07 at 17:18 +0200, Johannes Stezenbach wrote:
> >
> > just got the following while testing with rt73usb on
> > kernel 2.6.36-rc7 with wpa_supplicant v0.6.10:
> >
> > wlan0: authentication with 00:22:b0:xx:xx:xx timed out
> > ------------[ cut here ]------------
> > WARNING: at net/wireless/mlme.c:284 __cfg80211_auth_remove+0xc3/0xd0 [cfg80211]()
>
> > [<ffffffffa002d980>] __cfg80211_auth_remove+0xc3/0xd0 [cfg80211]
> > [<ffffffffa002e27c>] cfg80211_send_auth_timeout+0x93/0xaa [cfg80211]
> > [<ffffffffa004c3eb>] ieee80211_probe_auth_done+0x2a/0x97 [mac80211]
> > [<ffffffffa004e551>] ieee80211_work_work+0xecd/0xf3e [mac80211]
>
> Oh *crap*. I thought we'd fixed these :-(
>
> But then I really don't see how it might have happened. Was there
> anything in the log before this?
Nothing interesting. I started and ^C'ed wpa_supplicant
a few times in quick succession for testing because of
http://rt2x00.serialmonkey.com/pipermail/users_rt2x00.serialmonkey.com/2010-October/002152.html
Johannes
^ permalink raw reply
* [PATCH 1/2] ASoC: Add CS4271 codec support
From: Ryan Mallon @ 2010-10-07 21:59 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <4CAE3FAC.6010508@freescale.com>
On 10/08/2010 10:46 AM, Timur Tabi wrote:
> Ryan Mallon wrote:
>> + * For setups with variable MCLKs, pass 0 as 'freq' argument. This will cause
>> + * theoretically possible sample rates to be enabled. Call it again with a
>> + * proper value set one the external clock is set (most probably you would do
>> + * that from a machine's driver 'hw_param' hook.
>
> If you're going to copy/paste parts of my driver verbatim into yours, you should
> put something like this in the comments:
>
> Based on the CS4270 driver by Timur Tabi <timur@freescale.com>
>
Thanks. CC'ed Alexander, the driver author, who I stupidly left of the
reply list.
~Ryan
--
Bluewater Systems Ltd - ARM Technology Solution Centre
Ryan Mallon 5 Amuri Park, 404 Barbadoes St
ryan at bluewatersys.com PO Box 13 889, Christchurch 8013
http://www.bluewatersys.com New Zealand
Phone: +64 3 3779127 Freecall: Australia 1800 148 751
Fax: +64 3 3779135 USA 1800 261 2934
^ permalink raw reply
* Re: [PATCH] dell-laptop: Add hwswitch_only module parameter
From: Dmitry Torokhov @ 2010-10-07 21:58 UTC (permalink / raw)
To: Matthew Garrett
Cc: Mario Limonciello, Keng-Yu Lin, len.brown, alan-jenkins,
platform-driver-x86, linux-kernel
In-Reply-To: <20101007215028.GB2292@srcf.ucam.org>
On Thu, Oct 07, 2010 at 10:50:28PM +0100, Matthew Garrett wrote:
> On Thu, Oct 07, 2010 at 02:46:28PM -0700, Dmitry Torokhov wrote:
> > On Thu, Oct 07, 2010 at 10:37:54PM +0100, Matthew Garrett wrote:
> > > Dell laptop driver is reasonable. All we need is a list of hardware, and
> > > I'd really hope that Dell know which BIOS versions contain this code!
> > >
> >
> > Unfortunately this kind of crap tends to flow from one BIOS version to
> > another.. How many DMI entries do we know about so far? Did Dell issue
> > firmware updates correcting the issue so new boxes will not exhibit the
> > problem?
>
> Pushing this out to userspace doesn't help. If it's sufficiently
> prevelant that it's impossible to have a comprehensive list, then a
> module option makes it too easy for people to just post the fix on a
> forum somewhere and never actually get it upstream.
>
Forcing users patching the kernel reduces chances that it will be fixed
at all. Also there is load of patches that never make into distributions
and even less in mainline. Or if they make it it takes looong time...
so I do not think it is a valid argument here.
--
Dmitry
^ permalink raw reply
* Re: [PATCH 1/2] ASoC: Add CS4271 codec support
From: Ryan Mallon @ 2010-10-07 21:59 UTC (permalink / raw)
To: Timur Tabi
Cc: alsa-devel@alsa-project.org, Mark Brown, Alexander,
linux-arm-kernel, Liam Girdwood
In-Reply-To: <4CAE3FAC.6010508@freescale.com>
On 10/08/2010 10:46 AM, Timur Tabi wrote:
> Ryan Mallon wrote:
>> + * For setups with variable MCLKs, pass 0 as 'freq' argument. This will cause
>> + * theoretically possible sample rates to be enabled. Call it again with a
>> + * proper value set one the external clock is set (most probably you would do
>> + * that from a machine's driver 'hw_param' hook.
>
> If you're going to copy/paste parts of my driver verbatim into yours, you should
> put something like this in the comments:
>
> Based on the CS4270 driver by Timur Tabi <timur@freescale.com>
>
Thanks. CC'ed Alexander, the driver author, who I stupidly left of the
reply list.
~Ryan
--
Bluewater Systems Ltd - ARM Technology Solution Centre
Ryan Mallon 5 Amuri Park, 404 Barbadoes St
ryan@bluewatersys.com PO Box 13 889, Christchurch 8013
http://www.bluewatersys.com New Zealand
Phone: +64 3 3779127 Freecall: Australia 1800 148 751
Fax: +64 3 3779135 USA 1800 261 2934
^ permalink raw reply
* Re: [PATCH] serial: DCC(JTAG) serial and console emulation support
From: Daniel Walker @ 2010-10-07 21:58 UTC (permalink / raw)
To: Tony Lindgren
Cc: linux-arm-kernel, linux-arm-msm, Hyok S. Choi, Jeff Ohlstein
In-Reply-To: <20101007212707.GI7558@atomide.com>
On Thu, 2010-10-07 at 14:27 -0700, Tony Lindgren wrote:
> Can you please pass the read and write functions to the driver
> in platform_data? We are already booting kernels with both
> ARMv6 and 7 compiled in.
What kind of situation did you want to use it in ? I was thinking about
how arm could have these functions in a header file, that way we
wouldn't duplicate in multiple places.
> Also, as it's a driver, other architectures may want to use it too.
I'm not sure if this model fits other architectures tho. They may not
have the same read/write/status sorts of stuff. Atleast I don't know
enough about other architectures jtags .
Daniel
--
Sent by a consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
^ permalink raw reply
* [PATCH] serial: DCC(JTAG) serial and console emulation support
From: Daniel Walker @ 2010-10-07 21:58 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20101007212707.GI7558@atomide.com>
On Thu, 2010-10-07 at 14:27 -0700, Tony Lindgren wrote:
> Can you please pass the read and write functions to the driver
> in platform_data? We are already booting kernels with both
> ARMv6 and 7 compiled in.
What kind of situation did you want to use it in ? I was thinking about
how arm could have these functions in a header file, that way we
wouldn't duplicate in multiple places.
> Also, as it's a driver, other architectures may want to use it too.
I'm not sure if this model fits other architectures tho. They may not
have the same read/write/status sorts of stuff. Atleast I don't know
enough about other architectures jtags .
Daniel
--
Sent by a consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
^ permalink raw reply
* Re: [PATCH] dell-laptop: Add hwswitch_only module parameter
From: Mario Limonciello @ 2010-10-07 21:57 UTC (permalink / raw)
To: Matthew Garrett
Cc: Dmitry Torokhov, Keng-Yu Lin, len.brown, alan-jenkins,
platform-driver-x86, linux-kernel
In-Reply-To: <20101007215028.GB2292@srcf.ucam.org>
On Thu, Oct 7, 2010 at 16:50, Matthew Garrett <mjg59@srcf.ucam.org> wrote:
> On Thu, Oct 07, 2010 at 02:46:28PM -0700, Dmitry Torokhov wrote:
>> On Thu, Oct 07, 2010 at 10:37:54PM +0100, Matthew Garrett wrote:
>> > Dell laptop driver is reasonable. All we need is a list of hardware, and
>> > I'd really hope that Dell know which BIOS versions contain this code!
>> >
>>
>> Unfortunately this kind of crap tends to flow from one BIOS version to
>> another.. How many DMI entries do we know about so far? Did Dell issue
>> firmware updates correcting the issue so new boxes will not exhibit the
>> problem?
>
> Pushing this out to userspace doesn't help. If it's sufficiently
> prevelant that it's impossible to have a comprehensive list, then a
> module option makes it too easy for people to just post the fix on a
> forum somewhere and never actually get it upstream.
>
> --
> Matthew Garrett | mjg59@srcf.ucam.org
>
Making it more difficult to enable the alternative functionality
wouldn't necessarily make the list any more comprehensive, it just
raises the bar for users who want to try to help. If users report the
bugs to their distros, distros will help to forward them upstream.
You can't expect to fix a social problem here just by making it more
difficult to fix the original problem at hand for individuals.
--
Mario Limonciello
superm1@gmail.com
^ permalink raw reply
* Re: ext4 support on pvgrub
From: Samuel Thibault @ 2010-10-07 21:56 UTC (permalink / raw)
To: M A Young; +Cc: xen-devel
In-Reply-To: <alpine.LFD.2.00.1010072238110.13579@vega1.dur.ac.uk>
M A Young, le Thu 07 Oct 2010 22:48:04 +0100, a écrit :
> I have this working on Fedora. It turned out to be very easy to add this
> as I simply took the grub-ext4-support.patch from the Fedora grub package
> and put it in xen-4.0.1/stubdom/grub.patches/ . Using pvgrub (without a
> vif on the virtual machine as I was having problems if I did give it a
> network interface) I could read and boot from ext4 partition with this
> patch but not without it.
>
> Is this a good way to add this feature to xen more generally?
Yes, it's definitely the way to go. I haven't reviewed the patch at
all, but since that's just what Fedora uses it's probably simply ok.
Samuel
^ permalink raw reply
* Re: [Qemu-devel] [PATCH] ceph/rbd block driver for qemu-kvm (v4)
From: Anthony Liguori @ 2010-10-07 21:55 UTC (permalink / raw)
To: Yehuda Sadeh Weinraub
Cc: Kevin Wolf, ceph-devel, qemu-devel, kvm, Christian Brunner
In-Reply-To: <AANLkTinzdiJZFxacD1BDCPWt44ChokNaNVU6_E=GtcTD@mail.gmail.com>
On 10/07/2010 04:49 PM, Yehuda Sadeh Weinraub wrote:
> On Thu, Oct 7, 2010 at 2:04 PM, Anthony Liguori<anthony@codemonkey.ws> wrote:
>
>> On 10/07/2010 03:47 PM, Yehuda Sadeh Weinraub wrote:
>>
>>>> How is that possible? Are the callbacks delivered in the context of a
>>>> different thread? If so, don't you need locking?
>>>>
>>>>
>>> Not sure I'm completely following you. The callbacks are delivered in
>>> the context of a different thread, but won't run concurrently.
>>>
>> Concurrently to what? How do you prevent them from running concurrently
>> with qemu?
>>
> There are two types of callbacks. The first is for rados aio
> completions, and the second one is the one added later for the fd glue
> layer.
>
This is a bad architecture for something like qemu. You could create a
pipe and use the pipe to signal to qemu. Same principle as eventfd.
Ideally, you would do this in the library itself.
Regards,
Anthony Liguori
> The first callback, called by librados whenever aio completes, runs in
> the context of a single librados thread:
>
> +static void rbd_finish_aiocb(rados_completion_t c, RADOSCB *rcb)
> +{
> + RBDAIOCB *acb = rcb->acb;
> rcb is per a single aio. Was created before and will be destroyed
> here, whereas acb is shared between a few aios, however, it was
> generated before the first aio was created.
>
> + int64_t r;
> + uint64_t buf = 1;
> + int i;
> +
> + acb->aiocnt--;
>
> acb->aiocnt has been set before initiating all the aios, so it's ok to
> touch it now. Same goes to all acb fields.
>
> + r = rados_aio_get_return_value(c);
> + rados_aio_release(c);
> + if (acb->write) {
> + if (r< 0) {
> + acb->ret = r;
> + acb->error = 1;
> + } else if (!acb->error) {
> + acb->ret += rcb->segsize;
> + }
> + } else {
> + if (r == -ENOENT) {
> + memset(rcb->buf, 0, rcb->segsize);
> + if (!acb->error) {
> + acb->ret += rcb->segsize;
> + }
> + } else if (r< 0) {
> + acb->ret = r;
> + acb->error = 1;
> + } else if (r< rcb->segsize) {
> + memset(rcb->buf + r, 0, rcb->segsize - r);
> + if (!acb->error) {
> + acb->ret += rcb->segsize;
> + }
> + } else if (!acb->error) {
> + acb->ret += r;
> + }
> + }
> + if (write(acb->s->efd,&buf, sizeof(buf))< 0)
> This will wake up the io_read()
>
> + error_report("failed writing to acb->s->efd\n");
> + qemu_free(rcb);
> + i = 0;
> + if (!acb->aiocnt&& acb->bh) {
> + qemu_bh_schedule(acb->bh);
> This is the only qemu related call in here, seems safe to call it.
>
> + }
> +}
>
> The scheduled bh function will be called only after all aios that
> relate to this specific aio set are done, so the following seems ok,
> as there's no more acb references.
> +static void rbd_aio_bh_cb(void *opaque)
> +{
> + RBDAIOCB *acb = opaque;
> + uint64_t buf = 1;
> +
> + if (!acb->write) {
> + qemu_iovec_from_buffer(acb->qiov, acb->bounce, acb->qiov->size);
> + }
> + qemu_vfree(acb->bounce);
> + acb->common.cb(acb->common.opaque, (acb->ret> 0 ? 0 : acb->ret));
> + qemu_bh_delete(acb->bh);
> + acb->bh = NULL;
> +
> + if (write(acb->s->efd,&buf, sizeof(buf))< 0)
> + error_report("failed writing to acb->s->efd\n");
> + qemu_aio_release(acb);
> +}
>
> Now, the second ones are the io_read(), in which we have our glue fd.
> We send uint64 per each completed io
>
> +static void rbd_aio_completion_cb(void *opaque)
> +{
> + BDRVRBDState *s = opaque;
> +
> + uint64_t val;
> + ssize_t ret;
> +
> + do {
> + if ((ret = read(s->efd,&val, sizeof(val)))> 0) {
> + s->qemu_aio_count -= val;
> There is an issue here with s->qemu_aio_count which needs to be
> protected by a mutex. Other than that, it just reads from s->efd.
>
> + }
> + } while (ret< 0&& errno == EINTR);
> +
> + return;
> +}
> +
> +static int rbd_aio_flush_cb(void *opaque)
> +{
> + BDRVRBDState *s = opaque;
> +
> + return (s->qemu_aio_count> 0);
> Same here as with the previous one, needs a mutex around s->qemu_aio_count.
>
> +}
>
>
>> If you saw lock ups, I bet that's what it was from.
>>
>>
> As I explained before, before introducing the fd glue layer, the lack
> of fd associated with our block device caused that there was no way
> for qemu to check whether all aios were flushed or not, which didn't
> work well when doing migration/savevm.
>
> Thanks,
> Yehuda
>
^ permalink raw reply
* Re: [Qemu-devel] [PATCH] ceph/rbd block driver for qemu-kvm (v4)
From: Anthony Liguori @ 2010-10-07 21:55 UTC (permalink / raw)
To: Yehuda Sadeh Weinraub
Cc: Kevin Wolf, kvm, qemu-devel, ceph-devel, Christian Brunner
In-Reply-To: <AANLkTinzdiJZFxacD1BDCPWt44ChokNaNVU6_E=GtcTD@mail.gmail.com>
On 10/07/2010 04:49 PM, Yehuda Sadeh Weinraub wrote:
> On Thu, Oct 7, 2010 at 2:04 PM, Anthony Liguori<anthony@codemonkey.ws> wrote:
>
>> On 10/07/2010 03:47 PM, Yehuda Sadeh Weinraub wrote:
>>
>>>> How is that possible? Are the callbacks delivered in the context of a
>>>> different thread? If so, don't you need locking?
>>>>
>>>>
>>> Not sure I'm completely following you. The callbacks are delivered in
>>> the context of a different thread, but won't run concurrently.
>>>
>> Concurrently to what? How do you prevent them from running concurrently
>> with qemu?
>>
> There are two types of callbacks. The first is for rados aio
> completions, and the second one is the one added later for the fd glue
> layer.
>
This is a bad architecture for something like qemu. You could create a
pipe and use the pipe to signal to qemu. Same principle as eventfd.
Ideally, you would do this in the library itself.
Regards,
Anthony Liguori
> The first callback, called by librados whenever aio completes, runs in
> the context of a single librados thread:
>
> +static void rbd_finish_aiocb(rados_completion_t c, RADOSCB *rcb)
> +{
> + RBDAIOCB *acb = rcb->acb;
> rcb is per a single aio. Was created before and will be destroyed
> here, whereas acb is shared between a few aios, however, it was
> generated before the first aio was created.
>
> + int64_t r;
> + uint64_t buf = 1;
> + int i;
> +
> + acb->aiocnt--;
>
> acb->aiocnt has been set before initiating all the aios, so it's ok to
> touch it now. Same goes to all acb fields.
>
> + r = rados_aio_get_return_value(c);
> + rados_aio_release(c);
> + if (acb->write) {
> + if (r< 0) {
> + acb->ret = r;
> + acb->error = 1;
> + } else if (!acb->error) {
> + acb->ret += rcb->segsize;
> + }
> + } else {
> + if (r == -ENOENT) {
> + memset(rcb->buf, 0, rcb->segsize);
> + if (!acb->error) {
> + acb->ret += rcb->segsize;
> + }
> + } else if (r< 0) {
> + acb->ret = r;
> + acb->error = 1;
> + } else if (r< rcb->segsize) {
> + memset(rcb->buf + r, 0, rcb->segsize - r);
> + if (!acb->error) {
> + acb->ret += rcb->segsize;
> + }
> + } else if (!acb->error) {
> + acb->ret += r;
> + }
> + }
> + if (write(acb->s->efd,&buf, sizeof(buf))< 0)
> This will wake up the io_read()
>
> + error_report("failed writing to acb->s->efd\n");
> + qemu_free(rcb);
> + i = 0;
> + if (!acb->aiocnt&& acb->bh) {
> + qemu_bh_schedule(acb->bh);
> This is the only qemu related call in here, seems safe to call it.
>
> + }
> +}
>
> The scheduled bh function will be called only after all aios that
> relate to this specific aio set are done, so the following seems ok,
> as there's no more acb references.
> +static void rbd_aio_bh_cb(void *opaque)
> +{
> + RBDAIOCB *acb = opaque;
> + uint64_t buf = 1;
> +
> + if (!acb->write) {
> + qemu_iovec_from_buffer(acb->qiov, acb->bounce, acb->qiov->size);
> + }
> + qemu_vfree(acb->bounce);
> + acb->common.cb(acb->common.opaque, (acb->ret> 0 ? 0 : acb->ret));
> + qemu_bh_delete(acb->bh);
> + acb->bh = NULL;
> +
> + if (write(acb->s->efd,&buf, sizeof(buf))< 0)
> + error_report("failed writing to acb->s->efd\n");
> + qemu_aio_release(acb);
> +}
>
> Now, the second ones are the io_read(), in which we have our glue fd.
> We send uint64 per each completed io
>
> +static void rbd_aio_completion_cb(void *opaque)
> +{
> + BDRVRBDState *s = opaque;
> +
> + uint64_t val;
> + ssize_t ret;
> +
> + do {
> + if ((ret = read(s->efd,&val, sizeof(val)))> 0) {
> + s->qemu_aio_count -= val;
> There is an issue here with s->qemu_aio_count which needs to be
> protected by a mutex. Other than that, it just reads from s->efd.
>
> + }
> + } while (ret< 0&& errno == EINTR);
> +
> + return;
> +}
> +
> +static int rbd_aio_flush_cb(void *opaque)
> +{
> + BDRVRBDState *s = opaque;
> +
> + return (s->qemu_aio_count> 0);
> Same here as with the previous one, needs a mutex around s->qemu_aio_count.
>
> +}
>
>
>> If you saw lock ups, I bet that's what it was from.
>>
>>
> As I explained before, before introducing the fd glue layer, the lack
> of fd associated with our block device caused that there was no way
> for qemu to check whether all aios were flushed or not, which didn't
> work well when doing migration/savevm.
>
> Thanks,
> Yehuda
>
^ permalink raw reply
* [PATCH] power: introduce library for device-specific OPPs
From: Rafael J. Wysocki @ 2010-10-07 21:54 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1286357180-21565-1-git-send-email-nm@ti.com>
Hi,
On Wednesday, October 06, 2010, Nishanth Menon wrote:
> SoCs have a standard set of tuples consisting of frequency and
> voltage pairs that the device will support per voltage domain. These
> are called Operating Performance Points or OPPs. The actual
> definitions of OPP varies over silicon versions. For a specific domain,
> we can have a set of {frequency, voltage} pairs. As the kernel boots
> and more information is available, a default set of these are activated
> based on the precise nature of device. Further on operation, based on
> conditions prevailing in the system (such as temperature), some OPP
> availability may be temporarily controlled by the SoC frameworks.
>
> To implement an OPP, some sort of power management support is necessary
> hence this library depends on CONFIG_PM.
The patch generally looks good to me, I only have a couple of cosmetic remarks
(below).
...
> +static int opp_set_availability(struct device *dev, unsigned long freq,
> + bool availability_req)
> +{
> + struct device_opp *tmp_dev_opp, *dev_opp = NULL;
> + struct opp *new_opp, *tmp_opp, *opp = ERR_PTR(-ENODEV);
> + int r = 0;
> +
> + /* keep the node allocated */
> + new_opp = kmalloc(sizeof(struct opp), GFP_KERNEL);
> + if (!new_opp) {
> + pr_warning("Unable to allocate opp\n");
Please add an identification string to the messages, something like
"OPP: Unable to allocat object\n" (and similarly in the other messages).
That would help to find the source of a message in case there's any problem.
> + return -ENOMEM;
> + }
> +
> + mutex_lock(&dev_opp_list_lock);
> +
> + /* Find the device_opp */
> + list_for_each_entry(tmp_dev_opp, &dev_opp_list, node) {
> + if (dev == tmp_dev_opp->dev) {
> + dev_opp = tmp_dev_opp;
> + break;
> + }
> + }
> + if (IS_ERR(dev_opp)) {
> + r = PTR_ERR(dev_opp);
> + pr_warning("Unable to find device\n");
> + goto out1;
I'd prefer this lable to be called "unlock". It will be a bit more informative.
> + }
> +
> + /* Do we have the frequency? */
> + list_for_each_entry(tmp_opp, &dev_opp->opp_list, node) {
> + if (tmp_opp->rate == freq) {
> + opp = tmp_opp;
> + break;
> + }
> + }
> + if (IS_ERR(opp)) {
> + r = PTR_ERR(opp);
> + goto out1;
> + }
> +
> + /* Is update really needed? */
> + if (opp->available == availability_req)
> + goto out1;
> + /* copy the old data over */
> + *new_opp = *opp;
> +
> + /* plug in new node */
> + new_opp->available = availability_req;
> +
> + list_replace_rcu(&opp->node, &new_opp->node);
> + mutex_unlock(&dev_opp_list_lock);
> + synchronize_rcu();
> +
> + /* clean up old opp */
> + new_opp = opp;
> + goto out;
> +
> +out1:
+unlock:
> + mutex_unlock(&dev_opp_list_lock);
> +out:
> + kfree(new_opp);
> + return r;
> +}
Thanks,
Rafael
^ permalink raw reply
* Re: [PATCH] power: introduce library for device-specific OPPs
From: Rafael J. Wysocki @ 2010-10-07 21:54 UTC (permalink / raw)
To: Nishanth Menon; +Cc: Paul, linux-pm, l-o, lkml, l-a
In-Reply-To: <1286357180-21565-1-git-send-email-nm@ti.com>
Hi,
On Wednesday, October 06, 2010, Nishanth Menon wrote:
> SoCs have a standard set of tuples consisting of frequency and
> voltage pairs that the device will support per voltage domain. These
> are called Operating Performance Points or OPPs. The actual
> definitions of OPP varies over silicon versions. For a specific domain,
> we can have a set of {frequency, voltage} pairs. As the kernel boots
> and more information is available, a default set of these are activated
> based on the precise nature of device. Further on operation, based on
> conditions prevailing in the system (such as temperature), some OPP
> availability may be temporarily controlled by the SoC frameworks.
>
> To implement an OPP, some sort of power management support is necessary
> hence this library depends on CONFIG_PM.
The patch generally looks good to me, I only have a couple of cosmetic remarks
(below).
...
> +static int opp_set_availability(struct device *dev, unsigned long freq,
> + bool availability_req)
> +{
> + struct device_opp *tmp_dev_opp, *dev_opp = NULL;
> + struct opp *new_opp, *tmp_opp, *opp = ERR_PTR(-ENODEV);
> + int r = 0;
> +
> + /* keep the node allocated */
> + new_opp = kmalloc(sizeof(struct opp), GFP_KERNEL);
> + if (!new_opp) {
> + pr_warning("Unable to allocate opp\n");
Please add an identification string to the messages, something like
"OPP: Unable to allocat object\n" (and similarly in the other messages).
That would help to find the source of a message in case there's any problem.
> + return -ENOMEM;
> + }
> +
> + mutex_lock(&dev_opp_list_lock);
> +
> + /* Find the device_opp */
> + list_for_each_entry(tmp_dev_opp, &dev_opp_list, node) {
> + if (dev == tmp_dev_opp->dev) {
> + dev_opp = tmp_dev_opp;
> + break;
> + }
> + }
> + if (IS_ERR(dev_opp)) {
> + r = PTR_ERR(dev_opp);
> + pr_warning("Unable to find device\n");
> + goto out1;
I'd prefer this lable to be called "unlock". It will be a bit more informative.
> + }
> +
> + /* Do we have the frequency? */
> + list_for_each_entry(tmp_opp, &dev_opp->opp_list, node) {
> + if (tmp_opp->rate == freq) {
> + opp = tmp_opp;
> + break;
> + }
> + }
> + if (IS_ERR(opp)) {
> + r = PTR_ERR(opp);
> + goto out1;
> + }
> +
> + /* Is update really needed? */
> + if (opp->available == availability_req)
> + goto out1;
> + /* copy the old data over */
> + *new_opp = *opp;
> +
> + /* plug in new node */
> + new_opp->available = availability_req;
> +
> + list_replace_rcu(&opp->node, &new_opp->node);
> + mutex_unlock(&dev_opp_list_lock);
> + synchronize_rcu();
> +
> + /* clean up old opp */
> + new_opp = opp;
> + goto out;
> +
> +out1:
+unlock:
> + mutex_unlock(&dev_opp_list_lock);
> +out:
> + kfree(new_opp);
> + return r;
> +}
Thanks,
Rafael
^ permalink raw reply
* Re: [PATCH] power: introduce library for device-specific OPPs
From: Rafael J. Wysocki @ 2010-10-07 21:54 UTC (permalink / raw)
To: Nishanth Menon; +Cc: linux-pm, lkml, l-o, l-a, Paul
In-Reply-To: <1286357180-21565-1-git-send-email-nm@ti.com>
Hi,
On Wednesday, October 06, 2010, Nishanth Menon wrote:
> SoCs have a standard set of tuples consisting of frequency and
> voltage pairs that the device will support per voltage domain. These
> are called Operating Performance Points or OPPs. The actual
> definitions of OPP varies over silicon versions. For a specific domain,
> we can have a set of {frequency, voltage} pairs. As the kernel boots
> and more information is available, a default set of these are activated
> based on the precise nature of device. Further on operation, based on
> conditions prevailing in the system (such as temperature), some OPP
> availability may be temporarily controlled by the SoC frameworks.
>
> To implement an OPP, some sort of power management support is necessary
> hence this library depends on CONFIG_PM.
The patch generally looks good to me, I only have a couple of cosmetic remarks
(below).
...
> +static int opp_set_availability(struct device *dev, unsigned long freq,
> + bool availability_req)
> +{
> + struct device_opp *tmp_dev_opp, *dev_opp = NULL;
> + struct opp *new_opp, *tmp_opp, *opp = ERR_PTR(-ENODEV);
> + int r = 0;
> +
> + /* keep the node allocated */
> + new_opp = kmalloc(sizeof(struct opp), GFP_KERNEL);
> + if (!new_opp) {
> + pr_warning("Unable to allocate opp\n");
Please add an identification string to the messages, something like
"OPP: Unable to allocat object\n" (and similarly in the other messages).
That would help to find the source of a message in case there's any problem.
> + return -ENOMEM;
> + }
> +
> + mutex_lock(&dev_opp_list_lock);
> +
> + /* Find the device_opp */
> + list_for_each_entry(tmp_dev_opp, &dev_opp_list, node) {
> + if (dev == tmp_dev_opp->dev) {
> + dev_opp = tmp_dev_opp;
> + break;
> + }
> + }
> + if (IS_ERR(dev_opp)) {
> + r = PTR_ERR(dev_opp);
> + pr_warning("Unable to find device\n");
> + goto out1;
I'd prefer this lable to be called "unlock". It will be a bit more informative.
> + }
> +
> + /* Do we have the frequency? */
> + list_for_each_entry(tmp_opp, &dev_opp->opp_list, node) {
> + if (tmp_opp->rate == freq) {
> + opp = tmp_opp;
> + break;
> + }
> + }
> + if (IS_ERR(opp)) {
> + r = PTR_ERR(opp);
> + goto out1;
> + }
> +
> + /* Is update really needed? */
> + if (opp->available == availability_req)
> + goto out1;
> + /* copy the old data over */
> + *new_opp = *opp;
> +
> + /* plug in new node */
> + new_opp->available = availability_req;
> +
> + list_replace_rcu(&opp->node, &new_opp->node);
> + mutex_unlock(&dev_opp_list_lock);
> + synchronize_rcu();
> +
> + /* clean up old opp */
> + new_opp = opp;
> + goto out;
> +
> +out1:
+unlock:
> + mutex_unlock(&dev_opp_list_lock);
> +out:
> + kfree(new_opp);
> + return r;
> +}
Thanks,
Rafael
^ permalink raw reply
* RE: [PATCH 1/2] drivers:bluetooth: TI_ST bluetooth driver
From: Savoy, Pavan @ 2010-10-07 21:53 UTC (permalink / raw)
To: Gustavo F. Padovan
Cc: linux-bluetooth@vger.kernel.org, marcel@holtmann.org,
greg@kroah.com, linux-kernel@vger.kernel.org
In-Reply-To: <20101007184534.GB13602@vigoh>
Gustavo / Marcel,
> -----Original Message-----
> From: Gustavo F. Padovan [mailto:pao@profusion.mobi] On Behalf Of Gustavo F.
> Padovan
> Sent: Thursday, October 07, 2010 1:46 PM
> To: Savoy, Pavan
> Cc: linux-bluetooth@vger.kernel.org; marcel@holtmann.org; greg@kroah.com;
> linux-kernel@vger.kernel.org
> Subject: Re: [PATCH 1/2] drivers:bluetooth: TI_ST bluetooth driver
>
> Hi Pavan,
>
> Change the commit subject to "Bluetooth: TI_ST bluetooth driver"
>
> * pavan_savoy@ti.com <pavan_savoy@ti.com> [2010-10-07 14:47:16 -0400]:
>
> > From: Pavan Savoy <pavan_savoy@ti.com>
> >
> > This is the bluetooth protocol driver for the TI WiLink7 chipsets.
> > Texas Instrument's WiLink chipsets combine wireless technologies
> > like BT, FM, GPS and WLAN onto a single chip.
> >
> > This Bluetooth driver works on top of the TI_ST shared transport
> > line discipline driver which also allows other drivers like
> > FM V4L2 and GPS character driver to make use of the same UART interface.
> >
> > Signed-off-by: Pavan Savoy <pavan_savoy@ti.com>
> > ---
> > drivers/bluetooth/bt_ti.c | 489
> +++++++++++++++++++++++++++++++++++++++++++++
> > 1 files changed, 489 insertions(+), 0 deletions(-)
> > create mode 100644 drivers/bluetooth/bt_ti.c
>
> We don't have filename with bt_.. in drivers/bluetooth/. Maybe ti_st.c
> should be a better name, or something like that.
>
> >
> > diff --git a/drivers/bluetooth/bt_ti.c b/drivers/bluetooth/bt_ti.c
> > new file mode 100644
> > index 0000000..dffbb56
> > --- /dev/null
> > +++ b/drivers/bluetooth/bt_ti.c
> > @@ -0,0 +1,489 @@
> > +/*
> > + * Texas Instrument's Bluetooth Driver For Shared Transport.
> > + *
> > + * Bluetooth Driver acts as interface between HCI CORE and
> > + * TI Shared Transport Layer.
> > + *
> > + * Copyright (C) 2009-2010 Texas Instruments
> > + * Author: Raja Mani <raja_mani@ti.com>
> > + * Pavan Savoy <pavan_savoy@ti.com>
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2 as
> > + * published by the Free Software Foundation.
> > + *
> > + * 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.
> > + *
> > + * You should have received a copy of the GNU General Public License
> > + * along with this program; if not, write to the Free Software
> > + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307
> USA
> > + *
> > + */
> > +
> > +#include <linux/platform_device.h>
> > +#include <net/bluetooth/bluetooth.h>
> > +#include <net/bluetooth/hci_core.h>
> > +
> > +#include <linux/ti_wilink_st.h>
> > +
> > +/* Bluetooth Driver Version */
> > +#define VERSION "1.0"
> > +
> > +/* Defines number of seconds to wait for reg completion
> > + * callback getting called from ST (in case,registration
> > + * with ST returns PENDING status)
> > + */
> > +#define BT_REGISTER_TIMEOUT 6000 /* 6 sec */
> > +
> > +/* BT driver's local status */
> > +#define BT_DRV_RUNNING 0
> > +#define BT_ST_REGISTERED 1
> > +
> > +/**
> > + * struct hci_st - BT driver operation structure
> > + * @hdev: hci device pointer which binds to bt driver
> > + * @flags: used locally,to maintain various BT driver status
> > + * @streg_cbdata: to hold ST registration callback status
> > + * @st_write: write function pointer of ST driver
> > + * @wait_for_btdrv_reg_completion - completion sync between hci_st_open
> > + * and hci_st_registration_completion_cb.
> > + */
> > +struct hci_st {
> > + struct hci_dev *hdev;
> > + unsigned long flags;
> > + char streg_cbdata;
> > + long (*st_write) (struct sk_buff *);
> > + struct completion wait_for_btdrv_reg_completion;
> > +};
> > +
> > +static int reset;
> > +
> > +/* Increments HCI counters based on pocket ID (cmd,acl,sco) */
> > +static inline void hci_st_tx_complete(struct hci_st *hst, int pkt_type)
> > +{
> > + struct hci_dev *hdev;
> > + hdev = hst->hdev;
> > +
> > + /* Update HCI stat counters */
> > + switch (pkt_type) {
> > + case HCI_COMMAND_PKT:
> > + hdev->stat.cmd_tx++;
> > + break;
> > +
> > + case HCI_ACLDATA_PKT:
> > + hdev->stat.acl_tx++;
> > + break;
> > +
> > + case HCI_SCODATA_PKT:
> > + hdev->stat.cmd_tx++;
>
> it should be sco_tx here.
>
> > + break;
> > + }
> > +}
> > +
> > +/* ------- Interfaces to Shared Transport ------ */
> > +
> > +/* Called by ST layer to indicate protocol registration completion
> > + * status.hci_st_open() function will wait for signal from this
> > + * API when st_register() function returns ST_PENDING.
> > + */
> > +static void hci_st_registration_completion_cb(void *priv_data, char data)
>
> That is not the hci layer, so rename this function (and the others) to
> something that reflect where they are really doing.
>
> > +{
> > + struct hci_st *lhst = (struct hci_st *)priv_data;
> > + /* hci_st_open() function needs value of 'data' to know
> > + * the registration status(success/fail),So have a back
> > + * up of it.
> > + */
> > + lhst->streg_cbdata = data;
> > +
> > + /* Got a feedback from ST for BT driver registration
> > + * request.Wackup hci_st_open() function to continue
> > + * it's open operation.
> > + */
> > + complete(&lhst->wait_for_btdrv_reg_completion);
> > +}
> > +
> > +/* Called by Shared Transport layer when receive data is
> > + * available */
> > +static long hci_st_receive(void *priv_data, struct sk_buff *skb)
> > +{
> > + int err;
> > + int len;
>
> you can put err and len in the same line.
>
> > + struct hci_st *lhst = (struct hci_st *)priv_data;
> > +
> > + err = 0;
> > + len = 0;
>
> and no need to set them to 0 here.
>
> > +
> > + if (skb == NULL) {
> > + BT_ERR("Invalid SKB received from ST");
> > + return -EFAULT;
> > + }
>
> We need a empty line here.
>
> > + if (!lhst) {
> > + kfree_skb(skb);
> > + BT_ERR("Invalid hci_st memory,freeing SKB");
> > + return -EFAULT;
> > + }
>
> And also here. Check the rest of the code for similar issues.
>
> > + if (!test_bit(BT_DRV_RUNNING, &lhst->flags)) {
> > + kfree_skb(skb);
> > + BT_ERR("Device is not running,freeing SKB");
> > + return -EINVAL;
> > + }
>
> If you are here, your device is running, right? Or am I missing
> something?
>
> > +
> > + len = skb->len;
> > + skb->dev = (struct net_device *)lhst->hdev;
> > +
> > + /* Forward skb to HCI CORE layer */
> > + err = hci_recv_frame(skb);
> > + if (err) {
> > + kfree_skb(skb);
> > + BT_ERR("Unable to push skb to HCI CORE(%d),freeing SKB",
> > + err);
> > + return err;
> > + }
> > + lhst->hdev->stat.byte_rx += len;
>
> actually you even don't need len, just use skb->len
>
> > +
> > + return 0;
> > +}
> > +
> > +/* ------- Interfaces to HCI layer ------ */
> > +
> > +/* Called from HCI core to initialize the device */
> > +static int hci_st_open(struct hci_dev *hdev)
> > +{
> > + static struct st_proto_s hci_st_proto;
> > + unsigned long timeleft;
> > + struct hci_st *hst;
> > + int err;
> > + err = 0;
> > +
> > + BT_DBG("%s %p", hdev->name, hdev);
> > + hst = hdev->driver_data;
> > +
> > + /* Populate BT driver info required by ST */
> > + memset(&hci_st_proto, 0, sizeof(hci_st_proto));
> > +
> > + /* BT driver ID */
> > + hci_st_proto.type = ST_BT;
> > +
> > + /* Receive function which called from ST */
> > + hci_st_proto.recv = hci_st_receive;
> > +
> > + /* Packet match function may used in future */
> > + hci_st_proto.match_packet = NULL;
>
> It is already NULL, you dua a memset.
>
> > +
> > + /* Callback to be called when registration is pending */
> > + hci_st_proto.reg_complete_cb = hci_st_registration_completion_cb;
> > +
> > + /* This is write function pointer of ST. BT driver will make use of
> this
> > + * for sending any packets to chip. ST will assign and give to us, so
> > + * make it as NULL */
> > + hci_st_proto.write = NULL;
>
> Same here.
>
> > +
> > + /* send in the hst to be received at registration complete callback
> > + * and during st's receive
> > + */
> > + hci_st_proto.priv_data = hst;
> > +
> > + /* Register with ST layer */
> > + err = st_register(&hci_st_proto);
> > + if (err == -EINPROGRESS) {
> > + /* Prepare wait-for-completion handler data structures.
> > + * Needed to syncronize this and st_registration_completion_cb()
> > + * functions.
> > + */
> > + init_completion(&hst->wait_for_btdrv_reg_completion);
>
> I'm not liking that, but I'll leave for Marcel and others comment.
Thanks for the comments, I will take care of them all, but I want to
tell why I do this here.
"st_register" function takes time. It takes about 2 seconds for us
to download the firmware. (or longer based on UART baud-rate).
This is intended to happen in the process context. Say hciconfig hci0 up
takes a while longer than usual or if someone doing an ioctl(HCIDEVUP)
and this is also the reason I have a callback, as and when firmware download
completes, I call the Bluetooth driver's callback.
and I await marcel's comment on this.
> > +
> > + /* Reset ST registration callback status flag , this value
> > + * will be updated in hci_st_registration_completion_cb()
> > + * function whenever it called from ST driver.
> > + */
> > + hst->streg_cbdata = -EINPROGRESS;
> > +
> > + /* ST is busy with other protocol registration(may be busy with
> > + * firmware download).So,Wait till the registration callback
> > + * (passed as a argument to st_register() function) getting
> > + * called from ST.
> > + */
> > + BT_DBG(" %s waiting for reg completion signal from ST",
> > + __func__);
> > +
> > + timeleft =
> > + wait_for_completion_timeout
> > + (&hst->wait_for_btdrv_reg_completion,
> > + msecs_to_jiffies(BT_REGISTER_TIMEOUT));
> > + if (!timeleft) {
> > + BT_ERR("Timeout(%d sec),didn't get reg"
> > + "completion signal from ST",
> > + BT_REGISTER_TIMEOUT / 1000);
> > + return -ETIMEDOUT;
> > + }
> > +
> > + /* Is ST registration callback called with ERROR value? */
> > + if (hst->streg_cbdata != 0) {
> > + BT_ERR("ST reg completion CB called with invalid"
> > + "status %d", hst->streg_cbdata);
> > + return -EAGAIN;
> > + }
> > + err = 0;
> > + } else if (err == -1) {
>
> Use the proper error macro instead "-1"
>
> > + BT_ERR("st_register failed %d", err);
> > + return -EAGAIN;
> > + }
> > +
> > + /* Do we have proper ST write function? */
> > + if (hci_st_proto.write != NULL) {
> > + /* We need this pointer for sending any Bluetooth pkts */
> > + hst->st_write = hci_st_proto.write;
> > + } else {
> > + BT_ERR("failed to get ST write func pointer");
> > +
> > + /* Undo registration with ST */
> > + err = st_unregister(ST_BT);
> > + if (err < 0)
> > + BT_ERR("st_unregister failed %d", err);
> > +
> > + hst->st_write = NULL;
> > + return -EAGAIN;
> > + }
> > +
> > + /* Registration with ST layer is completed successfully,
> > + * now chip is ready to accept commands from HCI CORE.
> > + * Mark HCI Device flag as RUNNING
> > + */
> > + set_bit(HCI_RUNNING, &hdev->flags);
> > +
> > + /* Registration with ST successful */
> > + set_bit(BT_ST_REGISTERED, &hst->flags);
> > +
> > + return err;
> > +}
> > +
> > +/* Close device */
> > +static int hci_st_close(struct hci_dev *hdev)
> > +{
> > + int err;
> > + struct hci_st *hst;
>
> Skip a line after declarations.
>
> > + err = 0;
>
> you can set err to 0 in the declaration if you really need that.
>
> > +
> > + hst = hdev->driver_data;
> > + /* Unregister from ST layer */
> > + if (test_and_clear_bit(BT_ST_REGISTERED, &hst->flags)) {
> > + err = st_unregister(ST_BT);
> > + if (err != 0) {
> > + BT_ERR("st_unregister failed %d", err);
> > + return -EBUSY;
> > + }
> > + }
> > +
> > + hst->st_write = NULL;
> > +
> > + /* ST layer would have moved chip to inactive state.
> > + * So,clear HCI device RUNNING flag.
> > + */
> > + if (!test_and_clear_bit(HCI_RUNNING, &hdev->flags))
> > + return 0;
>
> Looks you are screwing up the flags here, if it fails on st_unregister()
> and returns HCI_RUNNING should keep set?
>
> > +
> > + return err;
>
> Rethink how you are doing error handling here, it should no be
> complicated like that.
>
> > +}
> > +
> > +/* Called from HCI CORE , Sends frames to Shared Transport */
> > +static int hci_st_send_frame(struct sk_buff *skb)
> > +{
> > + struct hci_dev *hdev;
> > + struct hci_st *hst;
> > + long len;
> > +
> > + if (skb == NULL) {
> > + BT_ERR("Invalid skb received from HCI CORE");
> > + return -ENOMEM;
> > + }
> > + hdev = (struct hci_dev *)skb->dev;
> > + if (!hdev) {
> > + BT_ERR("SKB received for invalid HCI Device (hdev=NULL)");
> > + return -ENODEV;
> > + }
> > + if (!test_bit(HCI_RUNNING, &hdev->flags)) {
> > + BT_ERR("Device is not running");
> > + return -EBUSY;
> > + }
> > +
> > + hst = (struct hci_st *)hdev->driver_data;
> > +
> > + /* Prepend skb with frame type */
> > + memcpy(skb_push(skb, 1), &bt_cb(skb)->pkt_type, 1);
> > +
> > + BT_DBG(" %s: type %d len %d", hdev->name, bt_cb(skb)->pkt_type,
> > + skb->len);
> > +
> > + /* Insert skb to shared transport layer's transmit queue.
> > + * Freeing skb memory is taken care in shared transport layer,
> > + * so don't free skb memory here.
> > + */
> > + if (!hst->st_write) {
> > + kfree_skb(skb);
> > + BT_ERR(" Can't write to ST, st_write null?");
> > + return -EAGAIN;
> > + }
> > + len = hst->st_write(skb);
> > + if (len < 0) {
> > + /* Something went wrong in st write , free skb memory */
>
> IMHO we don't need comments like that, clearly we now that something
> went wrong.
>
> > + kfree_skb(skb);
> > + BT_ERR(" ST write failed (%ld)", len);
> > + return -EAGAIN;
> > + }
> > +
> > + /* ST accepted our skb. So, Go ahead and do rest */
> > + hdev->stat.byte_tx += len;
> > + hci_st_tx_complete(hst, bt_cb(skb)->pkt_type);
> > +
> > + return 0;
>
> goto might be better to handle error here.
>
>
> --
> Gustavo F. Padovan
> ProFUSION embedded systems - http://profusion.mobi
^ permalink raw reply
* RE: [PATCH 1/2] drivers:bluetooth: TI_ST bluetooth driver
From: Savoy, Pavan @ 2010-10-07 21:53 UTC (permalink / raw)
To: Gustavo F. Padovan
Cc: linux-bluetooth@vger.kernel.org, marcel@holtmann.org,
greg@kroah.com, linux-kernel@vger.kernel.org
In-Reply-To: <20101007184534.GB13602@vigoh>
Gustavo / Marcel,
=20
> -----Original Message-----
> From: Gustavo F. Padovan [mailto:pao@profusion.mobi] On Behalf Of Gustavo=
F.
> Padovan
> Sent: Thursday, October 07, 2010 1:46 PM
> To: Savoy, Pavan
> Cc: linux-bluetooth@vger.kernel.org; marcel@holtmann.org; greg@kroah.com;
> linux-kernel@vger.kernel.org
> Subject: Re: [PATCH 1/2] drivers:bluetooth: TI_ST bluetooth driver
>=20
> Hi Pavan,
>=20
> Change the commit subject to "Bluetooth: TI_ST bluetooth driver"
>=20
> * pavan_savoy@ti.com <pavan_savoy@ti.com> [2010-10-07 14:47:16 -0400]:
>=20
> > From: Pavan Savoy <pavan_savoy@ti.com>
> >
> > This is the bluetooth protocol driver for the TI WiLink7 chipsets.
> > Texas Instrument's WiLink chipsets combine wireless technologies
> > like BT, FM, GPS and WLAN onto a single chip.
> >
> > This Bluetooth driver works on top of the TI_ST shared transport
> > line discipline driver which also allows other drivers like
> > FM V4L2 and GPS character driver to make use of the same UART interface=
.
> >
> > Signed-off-by: Pavan Savoy <pavan_savoy@ti.com>
> > ---
> > drivers/bluetooth/bt_ti.c | 489
> +++++++++++++++++++++++++++++++++++++++++++++
> > 1 files changed, 489 insertions(+), 0 deletions(-)
> > create mode 100644 drivers/bluetooth/bt_ti.c
>=20
> We don't have filename with bt_.. in drivers/bluetooth/. Maybe ti_st.c
> should be a better name, or something like that.
>=20
> >
> > diff --git a/drivers/bluetooth/bt_ti.c b/drivers/bluetooth/bt_ti.c
> > new file mode 100644
> > index 0000000..dffbb56
> > --- /dev/null
> > +++ b/drivers/bluetooth/bt_ti.c
> > @@ -0,0 +1,489 @@
> > +/*
> > + * Texas Instrument's Bluetooth Driver For Shared Transport.
> > + *
> > + * Bluetooth Driver acts as interface between HCI CORE and
> > + * TI Shared Transport Layer.
> > + *
> > + * Copyright (C) 2009-2010 Texas Instruments
> > + * Author: Raja Mani <raja_mani@ti.com>
> > + * Pavan Savoy <pavan_savoy@ti.com>
> > + *
> > + * This program is free software; you can redistribute it and/or modi=
fy
> > + * it under the terms of the GNU General Public License version 2 as
> > + * published by the Free Software Foundation.
> > + *
> > + * 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.
> > + *
> > + * You should have received a copy of the GNU General Public License
> > + * along with this program; if not, write to the Free Software
> > + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-13=
07
> USA
> > + *
> > + */
> > +
> > +#include <linux/platform_device.h>
> > +#include <net/bluetooth/bluetooth.h>
> > +#include <net/bluetooth/hci_core.h>
> > +
> > +#include <linux/ti_wilink_st.h>
> > +
> > +/* Bluetooth Driver Version */
> > +#define VERSION "1.0"
> > +
> > +/* Defines number of seconds to wait for reg completion
> > + * callback getting called from ST (in case,registration
> > + * with ST returns PENDING status)
> > + */
> > +#define BT_REGISTER_TIMEOUT 6000 /* 6 sec */
> > +
> > +/* BT driver's local status */
> > +#define BT_DRV_RUNNING 0
> > +#define BT_ST_REGISTERED 1
> > +
> > +/**
> > + * struct hci_st - BT driver operation structure
> > + * @hdev: hci device pointer which binds to bt driver
> > + * @flags: used locally,to maintain various BT driver status
> > + * @streg_cbdata: to hold ST registration callback status
> > + * @st_write: write function pointer of ST driver
> > + * @wait_for_btdrv_reg_completion - completion sync between hci_st_ope=
n
> > + * and hci_st_registration_completion_cb.
> > + */
> > +struct hci_st {
> > + struct hci_dev *hdev;
> > + unsigned long flags;
> > + char streg_cbdata;
> > + long (*st_write) (struct sk_buff *);
> > + struct completion wait_for_btdrv_reg_completion;
> > +};
> > +
> > +static int reset;
> > +
> > +/* Increments HCI counters based on pocket ID (cmd,acl,sco) */
> > +static inline void hci_st_tx_complete(struct hci_st *hst, int pkt_type=
)
> > +{
> > + struct hci_dev *hdev;
> > + hdev =3D hst->hdev;
> > +
> > + /* Update HCI stat counters */
> > + switch (pkt_type) {
> > + case HCI_COMMAND_PKT:
> > + hdev->stat.cmd_tx++;
> > + break;
> > +
> > + case HCI_ACLDATA_PKT:
> > + hdev->stat.acl_tx++;
> > + break;
> > +
> > + case HCI_SCODATA_PKT:
> > + hdev->stat.cmd_tx++;
>=20
> it should be sco_tx here.
>=20
> > + break;
> > + }
> > +}
> > +
> > +/* ------- Interfaces to Shared Transport ------ */
> > +
> > +/* Called by ST layer to indicate protocol registration completion
> > + * status.hci_st_open() function will wait for signal from this
> > + * API when st_register() function returns ST_PENDING.
> > + */
> > +static void hci_st_registration_completion_cb(void *priv_data, char da=
ta)
>=20
> That is not the hci layer, so rename this function (and the others) to
> something that reflect where they are really doing.
>=20
> > +{
> > + struct hci_st *lhst =3D (struct hci_st *)priv_data;
> > + /* hci_st_open() function needs value of 'data' to know
> > + * the registration status(success/fail),So have a back
> > + * up of it.
> > + */
> > + lhst->streg_cbdata =3D data;
> > +
> > + /* Got a feedback from ST for BT driver registration
> > + * request.Wackup hci_st_open() function to continue
> > + * it's open operation.
> > + */
> > + complete(&lhst->wait_for_btdrv_reg_completion);
> > +}
> > +
> > +/* Called by Shared Transport layer when receive data is
> > + * available */
> > +static long hci_st_receive(void *priv_data, struct sk_buff *skb)
> > +{
> > + int err;
> > + int len;
>=20
> you can put err and len in the same line.
>=20
> > + struct hci_st *lhst =3D (struct hci_st *)priv_data;
> > +
> > + err =3D 0;
> > + len =3D 0;
>=20
> and no need to set them to 0 here.
>=20
> > +
> > + if (skb =3D=3D NULL) {
> > + BT_ERR("Invalid SKB received from ST");
> > + return -EFAULT;
> > + }
>=20
> We need a empty line here.
>=20
> > + if (!lhst) {
> > + kfree_skb(skb);
> > + BT_ERR("Invalid hci_st memory,freeing SKB");
> > + return -EFAULT;
> > + }
>=20
> And also here. Check the rest of the code for similar issues.
>=20
> > + if (!test_bit(BT_DRV_RUNNING, &lhst->flags)) {
> > + kfree_skb(skb);
> > + BT_ERR("Device is not running,freeing SKB");
> > + return -EINVAL;
> > + }
>=20
> If you are here, your device is running, right? Or am I missing
> something?
>=20
> > +
> > + len =3D skb->len;
> > + skb->dev =3D (struct net_device *)lhst->hdev;
> > +
> > + /* Forward skb to HCI CORE layer */
> > + err =3D hci_recv_frame(skb);
> > + if (err) {
> > + kfree_skb(skb);
> > + BT_ERR("Unable to push skb to HCI CORE(%d),freeing SKB",
> > + err);
> > + return err;
> > + }
> > + lhst->hdev->stat.byte_rx +=3D len;
>=20
> actually you even don't need len, just use skb->len
>=20
> > +
> > + return 0;
> > +}
> > +
> > +/* ------- Interfaces to HCI layer ------ */
> > +
> > +/* Called from HCI core to initialize the device */
> > +static int hci_st_open(struct hci_dev *hdev)
> > +{
> > + static struct st_proto_s hci_st_proto;
> > + unsigned long timeleft;
> > + struct hci_st *hst;
> > + int err;
> > + err =3D 0;
> > +
> > + BT_DBG("%s %p", hdev->name, hdev);
> > + hst =3D hdev->driver_data;
> > +
> > + /* Populate BT driver info required by ST */
> > + memset(&hci_st_proto, 0, sizeof(hci_st_proto));
> > +
> > + /* BT driver ID */
> > + hci_st_proto.type =3D ST_BT;
> > +
> > + /* Receive function which called from ST */
> > + hci_st_proto.recv =3D hci_st_receive;
> > +
> > + /* Packet match function may used in future */
> > + hci_st_proto.match_packet =3D NULL;
>=20
> It is already NULL, you dua a memset.
>=20
> > +
> > + /* Callback to be called when registration is pending */
> > + hci_st_proto.reg_complete_cb =3D hci_st_registration_completion_cb;
> > +
> > + /* This is write function pointer of ST. BT driver will make use of
> this
> > + * for sending any packets to chip. ST will assign and give to us, so
> > + * make it as NULL */
> > + hci_st_proto.write =3D NULL;
>=20
> Same here.
>=20
> > +
> > + /* send in the hst to be received at registration complete callback
> > + * and during st's receive
> > + */
> > + hci_st_proto.priv_data =3D hst;
> > +
> > + /* Register with ST layer */
> > + err =3D st_register(&hci_st_proto);
> > + if (err =3D=3D -EINPROGRESS) {
> > + /* Prepare wait-for-completion handler data structures.
> > + * Needed to syncronize this and st_registration_completion_cb()
> > + * functions.
> > + */
> > + init_completion(&hst->wait_for_btdrv_reg_completion);
>=20
> I'm not liking that, but I'll leave for Marcel and others comment.
Thanks for the comments, I will take care of them all, but I want to
tell why I do this here.
"st_register" function takes time. It takes about 2 seconds for us
to download the firmware. (or longer based on UART baud-rate).
This is intended to happen in the process context. Say hciconfig hci0 up
takes a while longer than usual or if someone doing an ioctl(HCIDEVUP)
and this is also the reason I have a callback, as and when firmware downloa=
d
completes, I call the Bluetooth driver's callback.
and I await marcel's comment on this.
> > +
> > + /* Reset ST registration callback status flag , this value
> > + * will be updated in hci_st_registration_completion_cb()
> > + * function whenever it called from ST driver.
> > + */
> > + hst->streg_cbdata =3D -EINPROGRESS;
> > +
> > + /* ST is busy with other protocol registration(may be busy with
> > + * firmware download).So,Wait till the registration callback
> > + * (passed as a argument to st_register() function) getting
> > + * called from ST.
> > + */
> > + BT_DBG(" %s waiting for reg completion signal from ST",
> > + __func__);
> > +
> > + timeleft =3D
> > + wait_for_completion_timeout
> > + (&hst->wait_for_btdrv_reg_completion,
> > + msecs_to_jiffies(BT_REGISTER_TIMEOUT));
> > + if (!timeleft) {
> > + BT_ERR("Timeout(%d sec),didn't get reg"
> > + "completion signal from ST",
> > + BT_REGISTER_TIMEOUT / 1000);
> > + return -ETIMEDOUT;
> > + }
> > +
> > + /* Is ST registration callback called with ERROR value? */
> > + if (hst->streg_cbdata !=3D 0) {
> > + BT_ERR("ST reg completion CB called with invalid"
> > + "status %d", hst->streg_cbdata);
> > + return -EAGAIN;
> > + }
> > + err =3D 0;
> > + } else if (err =3D=3D -1) {
>=20
> Use the proper error macro instead "-1"
>=20
> > + BT_ERR("st_register failed %d", err);
> > + return -EAGAIN;
> > + }
> > +
> > + /* Do we have proper ST write function? */
> > + if (hci_st_proto.write !=3D NULL) {
> > + /* We need this pointer for sending any Bluetooth pkts */
> > + hst->st_write =3D hci_st_proto.write;
> > + } else {
> > + BT_ERR("failed to get ST write func pointer");
> > +
> > + /* Undo registration with ST */
> > + err =3D st_unregister(ST_BT);
> > + if (err < 0)
> > + BT_ERR("st_unregister failed %d", err);
> > +
> > + hst->st_write =3D NULL;
> > + return -EAGAIN;
> > + }
> > +
> > + /* Registration with ST layer is completed successfully,
> > + * now chip is ready to accept commands from HCI CORE.
> > + * Mark HCI Device flag as RUNNING
> > + */
> > + set_bit(HCI_RUNNING, &hdev->flags);
> > +
> > + /* Registration with ST successful */
> > + set_bit(BT_ST_REGISTERED, &hst->flags);
> > +
> > + return err;
> > +}
> > +
> > +/* Close device */
> > +static int hci_st_close(struct hci_dev *hdev)
> > +{
> > + int err;
> > + struct hci_st *hst;
>=20
> Skip a line after declarations.
>=20
> > + err =3D 0;
>=20
> you can set err to 0 in the declaration if you really need that.
>=20
> > +
> > + hst =3D hdev->driver_data;
> > + /* Unregister from ST layer */
> > + if (test_and_clear_bit(BT_ST_REGISTERED, &hst->flags)) {
> > + err =3D st_unregister(ST_BT);
> > + if (err !=3D 0) {
> > + BT_ERR("st_unregister failed %d", err);
> > + return -EBUSY;
> > + }
> > + }
> > +
> > + hst->st_write =3D NULL;
> > +
> > + /* ST layer would have moved chip to inactive state.
> > + * So,clear HCI device RUNNING flag.
> > + */
> > + if (!test_and_clear_bit(HCI_RUNNING, &hdev->flags))
> > + return 0;
>=20
> Looks you are screwing up the flags here, if it fails on st_unregister()
> and returns HCI_RUNNING should keep set?
>=20
> > +
> > + return err;
>=20
> Rethink how you are doing error handling here, it should no be
> complicated like that.
>=20
> > +}
> > +
> > +/* Called from HCI CORE , Sends frames to Shared Transport */
> > +static int hci_st_send_frame(struct sk_buff *skb)
> > +{
> > + struct hci_dev *hdev;
> > + struct hci_st *hst;
> > + long len;
> > +
> > + if (skb =3D=3D NULL) {
> > + BT_ERR("Invalid skb received from HCI CORE");
> > + return -ENOMEM;
> > + }
> > + hdev =3D (struct hci_dev *)skb->dev;
> > + if (!hdev) {
> > + BT_ERR("SKB received for invalid HCI Device (hdev=3DNULL)");
> > + return -ENODEV;
> > + }
> > + if (!test_bit(HCI_RUNNING, &hdev->flags)) {
> > + BT_ERR("Device is not running");
> > + return -EBUSY;
> > + }
> > +
> > + hst =3D (struct hci_st *)hdev->driver_data;
> > +
> > + /* Prepend skb with frame type */
> > + memcpy(skb_push(skb, 1), &bt_cb(skb)->pkt_type, 1);
> > +
> > + BT_DBG(" %s: type %d len %d", hdev->name, bt_cb(skb)->pkt_type,
> > + skb->len);
> > +
> > + /* Insert skb to shared transport layer's transmit queue.
> > + * Freeing skb memory is taken care in shared transport layer,
> > + * so don't free skb memory here.
> > + */
> > + if (!hst->st_write) {
> > + kfree_skb(skb);
> > + BT_ERR(" Can't write to ST, st_write null?");
> > + return -EAGAIN;
> > + }
> > + len =3D hst->st_write(skb);
> > + if (len < 0) {
> > + /* Something went wrong in st write , free skb memory */
>=20
> IMHO we don't need comments like that, clearly we now that something
> went wrong.
>=20
> > + kfree_skb(skb);
> > + BT_ERR(" ST write failed (%ld)", len);
> > + return -EAGAIN;
> > + }
> > +
> > + /* ST accepted our skb. So, Go ahead and do rest */
> > + hdev->stat.byte_tx +=3D len;
> > + hci_st_tx_complete(hst, bt_cb(skb)->pkt_type);
> > +
> > + return 0;
>=20
> goto might be better to handle error here.
>=20
>=20
> --
> Gustavo F. Padovan
> ProFUSION embedded systems - http://profusion.mobi
^ permalink raw reply
* Re: XFS causes Kernel trace with Jeremy's pv_ops kernel
From: Jeremy Fitzhardinge @ 2010-10-07 21:53 UTC (permalink / raw)
To: Ulf Kreutzberg; +Cc: xen-devel
In-Reply-To: <4CAD9D97.5070800@hosteurope.de>
On 10/07/2010 03:14 AM, Ulf Kreutzberg wrote:
> Hi Jeremy,
> hi all,
>
>
> I have checked out Jeremy's last git built of dom0 pv_ops kernel 2.6.32.23:
>
> commit 41a85de5caef68bbd58e11ed7b781b7867d96eaa
> Merge: 62bf426... 0ee0f94...
> Author: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
> Date: Tue Sep 28 11:20:40 2010 -0700
>
> I have set up on a Dell PE1950 2xDualXeon 5130 an LVM2 partition
> containing XFS. I have built Jeremy's kernel successfully, so the
> running dom0 Kernel is the one mentioned above.
> System is Debian-Lenny, Xen and Xen-tools are self-built.
>
> If I make some I/O, eg:
> cd linux-2.6-xen-git (which is then on the lv with the XFS filesystem)
> make -j 7 ; make install; make -j 7 modules; make modules_install
>
> in dmesg appears a kernel trace after a while. But compiling continues
> and all keeps up and running. This only happens if the kernel is running
> under xen 4.0.2-rc1-pre hypervisor. Nothing strange happens if I run it
> on bare-metal (same machine) without hypervisor.
> I have not tested bonnie or iozone yet.
>
> I have attached booting output of xen 4.0.2-rc1-pre
> as well as the trace (file console-kerneltrace.txt).
> Kernel config is also attached (config-2.6.32.23.txt)
This doesn't look obviously Xen specific. At least at one point XFS was
prone to spurious lockdep warnings that were hard to eliminate. I
wonder if its possible you're seeing this under Xen because of some kind
of timing issue?
Of course it could be Xen related too, but as I said, nothing stands out.
J
> Perhaps you can help. Please let me know if I have forgotten any
> important information.
>
> Many thanks and best regards,
> Ulf Kreutzberg
>
>
> Viele Grüße,
> Ulf
>
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
^ permalink raw reply
* Re: [PATCH] dell-laptop: Add hwswitch_only module parameter
From: Mario Limonciello @ 2010-10-07 21:52 UTC (permalink / raw)
To: Matthew Garrett
Cc: Dmitry Torokhov, Keng-Yu Lin, len.brown, alan-jenkins,
platform-driver-x86, linux-kernel
In-Reply-To: <20101007214927.GA2292@srcf.ucam.org>
On Thu, Oct 7, 2010 at 16:49, Matthew Garrett <mjg59@srcf.ucam.org> wrote:
> On Thu, Oct 07, 2010 at 04:45:22PM -0500, Mario Limonciello wrote:
>> Matthew:
>>
>> On Thu, Oct 7, 2010 at 16:42, Matthew Garrett <mjg59@srcf.ucam.org> wrote:
>> > I don't understand this. You know which code is broken - surely you're
>> > able to determine which products shipped with BIOSes derived from the
>> > broken code?
>>
>> Not all product's BIOS are developed in-house. The ones that are, yes
>> it quite possible to identify. Matter of fact, this issue tends to
>> not exist on the in-house developed BIOS codebase.
>
> Ok, so let's approach this differently. What is it that results in this
> issue not affecting Windows?
>
> --
> Matthew Garrett | mjg59@srcf.ucam.org
>
Depending on the third-party vendor that worked on the BIOS, they may
have an additional method for modifying the wireless and another tool
in addition to the standard one used on a majority of the machines.
That method is not necessarily documented.
--
Mario Limonciello
superm1@gmail.com
^ permalink raw reply
* Re: memory clobber in rx path, maybe related to ath9k.
From: Ben Greear @ 2010-10-07 21:52 UTC (permalink / raw)
To: Luis R. Rodriguez; +Cc: Johannes Berg, linux-wireless@vger.kernel.org
In-Reply-To: <AANLkTimxp703Gy_Atg8=wpFszRDR=OJCXwOUKRh-a-d7@mail.gmail.com>
On 10/07/2010 02:31 PM, Luis R. Rodriguez wrote:
> On Thu, Oct 7, 2010 at 12:27 PM, Johannes Berg
> <johannes@sipsolutions.net> wrote:
>> On Thu, 2010-10-07 at 12:22 -0700, Ben Greear wrote:
>>
>>> After reboot, and re-run of the script,
>>> I saw this in the logs, and shortly after,
>>> the SLUB poison warning dumped to screen.
>>>
>>> Maybe those DMA errors are serious?
>>
>>> ath: Failed to stop TX DMA in 100 msec after killing last frame
>>> ath: Failed to stop TX DMA. Resetting hardware!
>>
>> That's TX DMA, it can hardly result in invalid memory writes like the
>> ones you've been seeing.
>>
>> I'm still convinced something is wrong with ath9k RX DMA, as you've seen
>> the contents of frames written to already freed memory regions. Since I
>> don't know anything about ath9k, you should probably not rely on me
>> though :-)
>
> I'm on this now. Lets play.
>
> I had to remove /lib/udev/rules.d/75-persistent-net-generator.rules
> to avoid Ubuntu trying to remember the device names and it creating
> stax_rename names.
Right, we disable udev for 'sta*' devices.
> I just ran your script with some modifications. You can find it here:
>
> http://www.kernel.org/pub/linux/kernel/people/mcgrof/scripts/poo.pl
Can you post your kernel .config somewhere, and confirm which
kernel you are using?
Also, what ath9k NIC, platform, etc?
We see the problem on two different systems (haven't tried more). I can
figure out the brands of the NICs if that helps, and have included
lspci information below.
I've uploaded my kernel config to here:
http://www.candelatech.com/~greearb/ctwl_kernel.cfg
* Dual core Intel Pentium-D 32-bit
2GB RAM
Fedora 13 (but with custom compiled top-of-tree iw, hostap, libnl
Atheros NIC: from lspci -vv:
08:01.0 Network controller: Atheros Communications Inc. AR922X Wireless Network Adapter (rev 01)
Subsystem: Device 0777:4002
Control: I/O- Mem+ BusMaster- SpecCycle- MemWINV+ VGASnoop- ParErr- Stepping- SERR+ FastB2B+ DisINTx-
Status: Cap+ 66MHz+ UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
Interrupt: pin A routed to IRQ 17
Region 0: Memory at d0300000 (32-bit, non-prefetchable) [size=64K]
Capabilities: [44] Power Management version 2
Flags: PMEClk- DSI- D1- D2- AuxCurrent=100mA PME(D0+,D1-,D2-,D3hot+,D3cold-)
Status: D0 NoSoftRst- PME-Enable- DSel=0 DScale=0 PME-
Kernel modules: ath9k
* Dual-core Intel Atom, 32-bit
1GB RAM
Fedora 13 (custom compiled top-of-tree iw, hostap, libnl)
Atheros mini-pcie NIC, from lspci -vv:
03:00.0 Network controller: Atheros Communications Inc. AR928X Wireless Network Adapter (PCI-Express) (rev 01)
Subsystem: Device 0777:4e05
Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx-
Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort+ >SERR- <PERR- INTx-
Latency: 0, Cache Line Size: 64 bytes
Interrupt: pin A routed to IRQ 19
Region 0: Memory at febf0000 (64-bit, non-prefetchable) [size=64K]
Capabilities: [40] Power Management version 2
Flags: PMEClk- DSI- D1+ D2- AuxCurrent=375mA PME(D0+,D1+,D2-,D3hot+,D3cold-)
Status: D0 NoSoftRst- PME-Enable- DSel=0 DScale=0 PME-
Capabilities: [50] MSI: Enable- Count=1/1 Maskable- 64bit-
Address: 00000000 Data: 0000
Capabilities: [60] Express (v1) Legacy Endpoint, MSI 00
DevCap: MaxPayload 128 bytes, PhantFunc 0, Latency L0s <512ns, L1 <64us
ExtTag- AttnBtn- AttnInd- PwrInd- RBE- FLReset-
DevCtl: Report errors: Correctable- Non-Fatal- Fatal- Unsupported-
RlxdOrd+ ExtTag- PhantFunc- AuxPwr- NoSnoop-
MaxPayload 128 bytes, MaxReadReq 512 bytes
DevSta: CorrErr- UncorrErr+ FatalErr- UnsuppReq+ AuxPwr- TransPend-
LnkCap: Port #0, Speed 2.5GT/s, Width x1, ASPM unknown, Latency L0 <512ns, L1 <64us
ClockPM- Surprise- LLActRep- BwNot-
LnkCtl: ASPM Disabled; RCB 128 bytes Disabled- Retrain- CommClk+
ExtSynch- ClockPM- AutWidDis- BWInt- AutBWInt-
LnkSta: Speed 2.5GT/s, Width x1, TrErr- Train- SlotClk+ DLActive- BWMgmt- ABWMgmt-
Capabilities: [90] MSI-X: Enable- Count=1 Masked-
Vector table: BAR=0 offset=00000000
PBA: BAR=0 offset=00000000
Capabilities: [100 v1] Advanced Error Reporting
UESta: DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt- UnxCmplt- RxOF- MalfTLP- ECRC- UnsupReq+ ACSViol-
UEMsk: DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt- UnxCmplt- RxOF- MalfTLP- ECRC- UnsupReq- ACSViol-
UESvrt: DLP+ SDES- TLP- FCP+ CmpltTO- CmpltAbrt- UnxCmplt- RxOF+ MalfTLP+ ECRC- UnsupReq- ACSViol-
CESta: RxErr- BadTLP- BadDLLP- Rollover- Timeout- NonFatalErr-
CEMsk: RxErr- BadTLP- BadDLLP- Rollover- Timeout- NonFatalErr-
AERCap: First Error Pointer: 14, GenCap+ CGenEn- ChkCap+ ChkEn-
Capabilities: [140 v1] Virtual Channel
Caps: LPEVC=0 RefClk=100ns PATEntrySize=0
Arb: Fixed- WRR32- WRR64- WRR128- 100ns- onfig- - - TableOffset=0
Ctrl: ArbSelect=Fixed
Status: InProgress-
VC0: Caps: PATOffset=00 MaxTimeSlots=1 RejSnoopTrans-
Arb: Fixed- WRR32- WRR64- WRR128- TWRR128- WRR256- Fixed- RR32-
Ctrl: Enable+ ID=0 ArbSelect=Fixed TC/VC=01
Status: NegoPending- InProgress-
Capabilities: [160 v1] Device Serial Number 00-00-00-00-00-00-00-00
Kernel driver in use: ath9k
Kernel modules: ath9k
>
> I then ran:
>
> for i in $(seq 0 31) ; do sudo dhclient seq$i; done
>
> It took about 10 minutes to get IP addresses for all interfaces but it
> got there eventually. Odd enough I was unable to ping the AP from any
> interface though. Not sure what that was about. But I got no oops, no
> slub dump. All I got was some more delba warnings which seems to
> indicate we haven't caught all the cases for its use:
If you just create one or two interfaces, can you ping as expected?
Thanks,
Ben
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply
* Re: [PATCH 3/3] compat-wireless: backport kfifo for rt2x00
From: Luis R. Rodriguez @ 2010-10-07 21:51 UTC (permalink / raw)
To: Hauke Mehrtens; +Cc: linux-wireless, mcgrof
In-Reply-To: <1286486469-18904-3-git-send-email-hauke@hauke-m.de>
On Thu, Oct 7, 2010 at 2:21 PM, Hauke Mehrtens <hauke@hauke-m.de> wrote:
> Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
Applied all 3 thanks!!!
Luis
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.