2.6.32-rc5-git3: Reported regressions from 2.6.31

2.6.32-rc5-git3: Reported regressions from 2.6.31

[Rafael J. Wysocki]

This message contains a list of some regressions from 2.6.31, for which there
are no fixes in the mainline I know of.  If any of them have been fixed already,
please let me know.

If you know of any other unresolved regressions from 2.6.31, please let me know
either and I'll add them to the list.  Also, please let me know if any of the
entries below are invalid.

Each entry from the list will be sent additionally in an automatic reply to
this message with CCs to the people involved in reporting and handling the
issue.


Listed regressions statistics:

  Date          Total  Pending  Unresolved
  ----------------------------------------
  2009-10-26       66       42          37
  2009-10-12       48       31          27
  2009-10-02       22       15           9


Unresolved regressions
----------------------

Bug-Entry	: http://bugzilla.kernel.org/show_bug.cgi?id=14485
Subject		: System lockup running "cat /sys/kernel/debug/dri/0/i915_regs"
Submitter	: Miles Lane <miles.lane-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Date		: 2009-10-26 4:00 (1 days old)
References	: http://marc.info/?l=linux-kernel&m=125652968117713&w=4


Bug-Entry	: http://bugzilla.kernel.org/show_bug.cgi?id=14484
Subject		: no video output after suspend
Submitter	: Riccardo Magliocchetti <riccardo.magliocchetti-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Date		: 2009-10-25 20:57 (2 days old)
References	: http://marc.info/?l=linux-kernel&m=125650430123713&w=4
Handled-By	: Jesse Barnes <jbarnes-Y1mF5jBUw70BENJcbMCuUQ@public.gmane.org>


Bug-Entry	: http://bugzilla.kernel.org/show_bug.cgi?id=14483
Subject		: WARNING: at drivers/base/sys.c:353 __sysdev_resume+0x54/0xca()
Submitter	: Justin Mattock <justinmattock-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Date		: 2009-10-25 19:58 (2 days old)
References	: http://marc.info/?l=linux-kernel&m=125650070420168&w=4


Bug-Entry	: http://bugzilla.kernel.org/show_bug.cgi?id=14482
Subject		: kernel BUG at fs/dcache.c:670 +lvm +md +ext3
Submitter	: Alexander Clouter <alex-L4GPcECwBoDe9xe1eoZjHA@public.gmane.org>
Date		: 2009-10-23 10:30 (4 days old)
References	: http://lkml.org/lkml/2009/10/23/50


Bug-Entry	: http://bugzilla.kernel.org/show_bug.cgi?id=14481
Subject		: umount blocked for more than 120 seconds after USB drive removal
Submitter	: Robert Hancock <hancockrwd-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Date		: 2009-10-21 5:26 (6 days old)
References	: http://marc.info/?l=linux-kernel&m=125610280532245&w=4


Bug-Entry	: http://bugzilla.kernel.org/show_bug.cgi?id=14479
Subject		: nfs oops
Submitter	: Egon Alter <egon.alter-hi6Y0CQ0nG0@public.gmane.org>
Date		: 2009-10-19 16:03 (8 days old)
References	: http://marc.info/?l=linux-kernel&m=125596822630410&w=4


Bug-Entry	: http://bugzilla.kernel.org/show_bug.cgi?id=14477
Subject		: possible circular locking dependency in ISDN PPP
Submitter	: Tilman Schmidt <tilman-ZTO5kqT2PaM@public.gmane.org>
Date		: 2009-10-18 22:16 (9 days old)
References	: http://marc.info/?l=linux-kernel&m=125590423416087&w=4


Bug-Entry	: http://bugzilla.kernel.org/show_bug.cgi?id=14473
Subject		: ATA related kernel warning after resume
Submitter	: Tino Keitel <tino.keitel-rAwCM5oiXHA@public.gmane.org>
Date		: 2009-10-14 6:55 (13 days old)
References	: http://marc.info/?l=linux-kernel&m=125550466624678&w=4


Bug-Entry	: http://bugzilla.kernel.org/show_bug.cgi?id=14472
Subject		: EXT4 corruption
Submitter	: Shawn Starr <shawn.starr-bJEeYj9oJeDQT0dZR+AlfA@public.gmane.org>
Date		: 2009-10-13 2:07 (14 days old)
References	: http://marc.info/?l=linux-kernel&m=125539997508256&w=4
Handled-By	: Theodore Tso <tytso-3s7WtUTddSA@public.gmane.org>


Bug-Entry	: http://bugzilla.kernel.org/show_bug.cgi?id=14467
Subject		: Linker errors on ia64 with NR_CPUS=4096
Submitter	: Jeff Mahoney <jeffm-IBi9RG/b67k@public.gmane.org>
Date		: 2009-10-18 22:28 (9 days old)
First-Bad-Commit: http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=34d76c41554a05425613d16efebb3069c4c545f0
References	: http://marc.info/?l=linux-kernel&m=125590493116720&w=4


Bug-Entry	: http://bugzilla.kernel.org/show_bug.cgi?id=14466
Subject		: EFI boot on x86 fails in .32
Submitter	: Matthew Garrett <mjg59-1xO5oi07KQx4cg9Nei1l7Q@public.gmane.org>
Date		: 2009-10-20 0:34 (7 days old)
First-Bad-Commit: http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=7bd867dfb4e0357e06a3211ab2bd0e714110def3
References	: http://marc.info/?l=linux-kernel&m=125599887314290&w=4
Handled-By	: Feng Tang <feng.tang-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>


Bug-Entry	: http://bugzilla.kernel.org/show_bug.cgi?id=14442
Subject		: resume after hibernate: /dev/sdb drops and returns as /dev/sde
Submitter	: Duncan <1i5t5.duncan-j9pdmedNgrk@public.gmane.org>
Date		: 2009-10-20 01:52 (7 days old)


Bug-Entry	: http://bugzilla.kernel.org/show_bug.cgi?id=14430
Subject		: sync() hangs in bdi_sched_wait
Submitter	: Petr Vandrovec <petr-vPk2MGR0e28uaRcfnNAh7A@public.gmane.org>
Date		: 2009-10-17 19:14 (10 days old)


Bug-Entry	: http://bugzilla.kernel.org/show_bug.cgi?id=14415
Subject		: Reboot on kernel load
Submitter	: Brian Beardall <brian-sVkzCUl/XCrR7s880joybQ@public.gmane.org>
Date		: 2009-10-15 23:57 (12 days old)


Bug-Entry	: http://bugzilla.kernel.org/show_bug.cgi?id=14408
Subject		: sysctl check failed
Submitter	: Peter Teoh <htmldeveloper-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Date		: 2009-10-14 22:59 (13 days old)


Bug-Entry	: http://bugzilla.kernel.org/show_bug.cgi?id=14406
Subject		: uvcvideo stopped work on Toshiba
Submitter	: okias <d.okias-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Date		: 2009-10-14 19:08 (13 days old)


Bug-Entry	: http://bugzilla.kernel.org/show_bug.cgi?id=14390
Subject		: "bind" a device to a driver doesn't not work anymore
Submitter	: Éric Piel <Eric.Piel-VkQ1JFuSMpfAbQlEx87xDw@public.gmane.org>
Date		: 2009-10-11 0:04 (16 days old)
References	: http://marc.info/?l=linux-kernel&m=125521979921241&w=4


Bug-Entry	: http://bugzilla.kernel.org/show_bug.cgi?id=14389
Subject		: Build system issue
Submitter	: Peter Zijlstra <peterz-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
Date		: 2009-10-09 8:58 (18 days old)
First-Bad-Commit: http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=575543347b5baed0ca927cb90ba8807396fe9cc9
References	: http://marc.info/?l=linux-kernel&m=125507914909152&w=4


Bug-Entry	: http://bugzilla.kernel.org/show_bug.cgi?id=14387
Subject		: deadlock with fallocate
Submitter	: Thomas Neumann <tneumann-Rn4VEauK+AKRv+LV9MX5uipxlwaOVQ5f@public.gmane.org>
Date		: 2009-10-07 3:00 (20 days old)
References	: http://marc.info/?l=linux-kernel&m=125488495526471&w=4
Handled-By	: Christoph Hellwig <hch-jcswGhMUV9g@public.gmane.org>


Bug-Entry	: http://bugzilla.kernel.org/show_bug.cgi?id=14384
Subject		: tbench regression with 2.6.32-rc1
Submitter	: Zhang, Yanmin <yanmin_zhang-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Date		: 2009-10-09 9:51 (18 days old)
First-Bad-Commit: http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=59abf02644c45f1591e1374ee7bb45dc757fcb88
References	: http://marc.info/?l=linux-kernel&m=125508216713138&w=4
Handled-By	: Peter Zijlstra <a.p.zijlstra-/NLkJaSkS4VmR6Xm/wNWPw@public.gmane.org>


Bug-Entry	: http://bugzilla.kernel.org/show_bug.cgi?id=14383
Subject		: hackbench regression with kernel 2.6.32-rc1
Submitter	: Zhang, Yanmin <yanmin_zhang-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Date		: 2009-10-09 9:19 (18 days old)
First-Bad-Commit: http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=29cd8bae396583a2ee9a3340db8c5102acf9f6fd
References	: http://marc.info/?l=linux-kernel&m=125508007510274&w=4
Handled-By	: Peter Zijlstra <a.p.zijlstra-/NLkJaSkS4VmR6Xm/wNWPw@public.gmane.org>


Bug-Entry	: http://bugzilla.kernel.org/show_bug.cgi?id=14381
Subject		: iwlagn lost connection after s2ram (with warnings)
Submitter	: Carlos R. Mafra <crmafra2-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Date		: 2009-10-07 14:20 (20 days old)
References	: http://marc.info/?l=linux-kernel&m=125492569119947&w=4


Bug-Entry	: http://bugzilla.kernel.org/show_bug.cgi?id=14378
Subject		: Problems with net/core/skbuff.c
Submitter	: Massimo Cetra <mcetra-BBpJ+9iBSNKonA0d6jMUrA@public.gmane.org>
Date		: 2009-10-08 14:51 (19 days old)
References	: http://marc.info/?l=linux-kernel&m=125501488220358&w=4


Bug-Entry	: http://bugzilla.kernel.org/show_bug.cgi?id=14376
Subject		: Kernel NULL pointer dereference/ kvm subsystem
Submitter	: Don Dupuis <dondster-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Date		: 2009-10-06 14:38 (21 days old)
References	: http://marc.info/?l=linux-kernel&m=125484025021737&w=4


Bug-Entry	: http://bugzilla.kernel.org/show_bug.cgi?id=14373
Subject		: Task blocked for more than 120 seconds
Submitter	: Zeno Davatz <zdavatz-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Date		: 2009-10-02 10:16 (25 days old)
References	: http://marc.info/?l=linux-kernel&m=125447858618412&w=4


Bug-Entry	: http://bugzilla.kernel.org/show_bug.cgi?id=14372
Subject		: ath5k wireless not working after suspend-resume - eeepc
Submitter	: Fabio Comolli <fabio.comolli-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Date		: 2009-10-03 15:36 (24 days old)
References	: http://lkml.org/lkml/2009/10/3/91


Bug-Entry	: http://bugzilla.kernel.org/show_bug.cgi?id=14355
Subject		: USB serial regression after 2.6.31.1 with Huawei E169 GSM modem
Submitter	: Benjamin Herrenschmidt <benh-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r@public.gmane.org>
Date		: 2009-10-10 03:07 (17 days old)
References	: http://marc.info/?l=linux-kernel&m=125513456327542&w=4


Bug-Entry	: http://bugzilla.kernel.org/show_bug.cgi?id=14354
Subject		: Bad corruption with 2.6.32-rc1 and upwards
Submitter	: Holger Freyther <zecke-MQnelBtSfJRAfugRpC6u6w@public.gmane.org>
Date		: 2009-10-09 15:42 (18 days old)


Bug-Entry	: http://bugzilla.kernel.org/show_bug.cgi?id=14353
Subject		: BUG: sleeping function called from invalid context at kernel/mutex.c:280
Submitter	: Miles Lane <miles.lane-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Date		: 2009-10-05 3:39 (22 days old)
References	: http://marc.info/?l=linux-kernel&m=125471432208671&w=4


Bug-Entry	: http://bugzilla.kernel.org/show_bug.cgi?id=14352
Subject		: WARNING: at net/mac80211/scan.c:267
Submitter	: Maciej Rutecki <maciej.rutecki-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Date		: 2009-10-08 00:30 (19 days old)
References	: http://bugzilla.intellinuxwireless.org/show_bug.cgi?id=2089#c7


Bug-Entry	: http://bugzilla.kernel.org/show_bug.cgi?id=14334
Subject		: pcmcia suspend regression from 2.6.31.1 to 2.6.31.2 - Dell Inspiron 600m
Submitter	: Jose Marino <braket-PkbjNfxxIARBDgjK7y7TUQ@public.gmane.org>
Date		: 2009-10-06 15:44 (21 days old)


Bug-Entry	: http://bugzilla.kernel.org/show_bug.cgi?id=14331
Subject		: Radeon XPRESS 200M: System hang with radeon DRI and Fedora 10 userspace unless DRI=off
Submitter	: Alex Villacis Lasso <avillaci-x0m+Mc+nT7uljOmnV8AmnkElSqmLX1BE@public.gmane.org>
Date		: 2009-10-06 00:29 (21 days old)


Bug-Entry	: http://bugzilla.kernel.org/show_bug.cgi?id=14299
Subject		: oops in wireless, iwl3945 related?
Submitter	: Pavel Machek <pavel-+ZI9xUNit7I@public.gmane.org>
Date		: 2009-09-29 17:12 (28 days old)
References	: http://marc.info/?l=linux-kernel&m=125424439725743&w=4


Bug-Entry	: http://bugzilla.kernel.org/show_bug.cgi?id=14298
Subject		: warning at manage.c:361 (set_irq_wake), matrix-keypad related?
Submitter	: Pavel Machek <pavel-+ZI9xUNit7I@public.gmane.org>
Date		: 2009-09-30 20:07 (27 days old)
References	: http://marc.info/?l=linux-kernel&m=125434130703538&w=4


Bug-Entry	: http://bugzilla.kernel.org/show_bug.cgi?id=14297
Subject		: console resume broken since ba15ab0e8d
Submitter	: Sascha Hauer <s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
Date		: 2009-09-30 15:11 (27 days old)
References	: http://marc.info/?l=linux-kernel&m=125432349404060&w=4


Bug-Entry	: http://bugzilla.kernel.org/show_bug.cgi?id=14296
Subject		: spitz boots but suspend/resume is broken
Submitter	: Pavel Machek <pavel-+ZI9xUNit7I@public.gmane.org>
Date		: 2009-09-30 12:06 (27 days old)
References	: http://marc.info/?l=linux-kernel&m=125431244516449&w=4


Bug-Entry	: http://bugzilla.kernel.org/show_bug.cgi?id=14277
Subject		: Caught 8-bit read from freed memory in b43 driver at association
Submitter	: Christian Casteyde <casteyde.christian-GANU6spQydw@public.gmane.org>
Date		: 2009-09-30 18:06 (27 days old)


Regressions with patches
------------------------

Bug-Entry	: http://bugzilla.kernel.org/show_bug.cgi?id=14480
Subject		: 2 locks held by cat -- running "find /sys | head -c 4" --> system hang
Submitter	: Miles Lane <miles.lane-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Date		: 2009-10-20 16:11 (7 days old)
References	: http://marc.info/?l=linux-kernel&m=125605511728088&w=4
Handled-By	: Chris Wilson <chris-Y6uKTt2uX1cEflXRtASbqLVCufUGDwFn@public.gmane.org>
Patch		: http://patchwork.kernel.org/patch/54974/


Bug-Entry	: http://bugzilla.kernel.org/show_bug.cgi?id=14380
Subject		: Video tearing/glitching with T400 laptops
Submitter	: Theodore Ts'o <tytso-3s7WtUTddSA@public.gmane.org>
Date		: 2009-10-02 22:40 (25 days old)
References	: http://marc.info/?l=linux-kernel&m=125452324520623&w=4
Handled-By	: Jesse Barnes <jbarnes-Y1mF5jBUw70BENJcbMCuUQ@public.gmane.org>
Patch		: http://marc.info/?l=linux-kernel&m=125591495325000&w=4


Bug-Entry	: http://bugzilla.kernel.org/show_bug.cgi?id=14379
Subject		: ACPI Warning for _SB_.BAT0._BIF: Converted Buffer to expected String
Submitter	: Justin Mattock <justinmattock-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Date		: 2009-10-08 21:46 (19 days old)
First-Bad-Commit: http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=d9adc2e031bd22d5d9607a53a8d3b30e0b675f39
References	: http://marc.info/?l=linux-kernel&m=125504031328941&w=4
Handled-By	: Alexey Starikovskiy <astarikovskiy-l3A5Bk7waGM@public.gmane.org>
Patch		: http://bugzilla.kernel.org/attachment.cgi?id=23347


Bug-Entry	: http://bugzilla.kernel.org/show_bug.cgi?id=14375
Subject		: Intel(R) I/OAT DMA Engine init failed
Submitter	: Alexander Beregalov <a.beregalov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Date		: 2009-10-02 9:46 (25 days old)
References	: http://marc.info/?l=linux-kernel&m=125447680016160&w=4
Handled-By	: Dan Williams <dan.j.williams-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Patch		: http://patchwork.kernel.org/patch/51808/


Bug-Entry	: http://bugzilla.kernel.org/show_bug.cgi?id=14302
Subject		: Kernel panic on i386 machine when booting with profile=2
Submitter	: Shi, Alex <alex.shi-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Date		: 2009-10-01 3:23 (26 days old)
References	: http://marc.info/?l=linux-kernel&m=125436749607199&w=4
Handled-By	: Alex Shi <alex.shi-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Patch		: http://patchwork.kernel.org/patch/50813/


For details, please visit the bug entries and follow the links given in
references.

As you can see, there is a Bugzilla entry for each of the listed regressions.
There also is a Bugzilla entry used for tracking the regressions from 2.6.31,
unresolved as well as resolved, at:

http://bugzilla.kernel.org/show_bug.cgi?id=14230

Please let me know if there are any Bugzilla entries that should be added to
the list in there.

Thanks,
Rafael

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

Re: [Bug #14379] ACPI Warning for _SB_.BAT0._BIF: Converted Buffer to expected String

[Rafael J. Wysocki]

On Monday 26 October 2009, Justin P. Mattock wrote:
> Rafael J. Wysocki wrote:
> > This message has been generated automatically as a part of a report
> > of recent regressions.
> >
> > The following bug entry is on the current list of known regressions
> > from 2.6.31.  Please verify if it still should be listed and let me know
> > (either way).
> >
> >
> > Bug-Entry	: http://bugzilla.kernel.org/show_bug.cgi?id=14379
> > Subject		: ACPI Warning for _SB_.BAT0._BIF: Converted Buffer to expected String
> > Submitter	: Justin Mattock<justinmattock@gmail.com>
> > Date		: 2009-10-08 21:46 (19 days old)
> > First-Bad-Commit: http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=d9adc2e031bd22d5d9607a53a8d3b30e0b675f39
> > References	: http://marc.info/?l=linux-kernel&m=125504031328941&w=4
> > Handled-By	: Alexey Starikovskiy<astarikovskiy@suse.de>
> > Patch		: http://bugzilla.kernel.org/attachment.cgi?id=23347
> >
> >
> >
> >    
> The patch submitted gets rid of the warning
> for people crying wolf!! but probably would not close
> the bugreport until the patch makes it into the main
> kernel.

Yup.

Thanks,
Rafael

Re: [Bug #14483] WARNING: at drivers/base/sys.c:353 __sysdev_resume+0x54/0xca()

[Rafael J. Wysocki]

On Monday 26 October 2009, Justin P. Mattock wrote:
> Rafael J. Wysocki wrote:
> > This message has been generated automatically as a part of a report
> > of recent regressions.
> >
> > The following bug entry is on the current list of known regressions
> > from 2.6.31.  Please verify if it still should be listed and let me know
> > (either way).
> >
> >
> > Bug-Entry	: http://bugzilla.kernel.org/show_bug.cgi?id=14483
> > Subject		: WARNING: at drivers/base/sys.c:353 __sysdev_resume+0x54/0xca()
> > Submitter	: Justin Mattock<justinmattock-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > Date		: 2009-10-25 19:58 (2 days old)
> > References	: http://marc.info/?l=linux-kernel&m=125650070420168&w=4
> >
> >
> >
> >    
> yeah this happens on the first go of
> echo mem > /sys/power/state
> for both of my imac's I have here.
> (on the second  try, this message does not appear)

Thanks for the update.

Rafael

Help needed, Re: [Bug #14334] pcmcia suspend regression from 2.6.31.1 to 2.6.31.2 - Dell Inspiron 600m

[Rafael J. Wysocki]

Hi,

On Monday 26 October 2009, Rafael J. Wysocki wrote:
> This message has been generated automatically as a part of a report
> of recent regressions.
> 
> The following bug entry is on the current list of known regressions
> from 2.6.31.  Please verify if it still should be listed and let me know
> (either way).

Here's a puzzle for whoever likes hardware detective work.

Suspend to RAM worked just fine on the Jose's box before commit
53024df259e37ad49ee3d1f3721d4cecdd7bc357 (PM / yenta: Fix cardbus
suspend/resume regression, mainline commit
0c570cdeb8fdfcb354a3e9cd81bfc6a09c19de0c) that made the resume of CardBus
devices actually work.  After this commit, the box hangs during resume 100% of
the time.

We've done quite some debugging and here's the summary of findings:

1) Resume works if pcmcia_socket_dev_resume(dev) is moved to the "regular"
   resume phase, after resume_device_irqs().

2) Resume works if yenta_set_power() is not called during resume
   (from yenta_set_socket()).

3) It doesn't help to resume all ACPI devices before yenta.

4) It doesn't help to disable yenta events during resume (unconditionally).

5) There are two CardBus bridges in the box and according to the PM_TRACE
   information the _second_ one is the last device we attempt to wake up
   during a failing resume.

Please look into http://bugzilla.kernel.org/show_bug.cgi?id=14334 for details.

OK, any ideas anyone?

Rafael

Re: Help needed, Re: [Bug #14334] pcmcia suspend regression from 2.6.31.1 to 2.6.31.2 - Dell Inspiron 600m

[Linus Torvalds]

On Fri, 30 Oct 2009, Rafael J. Wysocki wrote:
> 
>
> 1) Resume works if pcmcia_socket_dev_resume(dev) is moved to the "regular"
>    resume phase, after resume_device_irqs().

Hmm. We really probably shouldn't call pcmcia_socket_dev_resume() in 
early_resume. It takes mutexes etc, and it calls "socket_resume()", which 
sleeps etc. That per se should be ok these days (since we don't actualyl 
disable CPU irq's, just device irqs), but it also does that whole card 
insertion events etc. And _that_ code I wouldn't trust at all.

The PCMCIA code is better than it used to be a long time ago, but some of 
it is still pretty crazy. 

I get the feeling that we should just revert that commit 0c570cdeb, and 
instead always do PCMCIA suspend as a "eject" event. That way we have no 
driver behind it to resume at resume time - and we'll see any plugged-in 
device as just a new insertion. 

		Linus

Re: Help needed, Re: [Bug #14334] pcmcia suspend regression from 2.6.31.1 to 2.6.31.2 - Dell Inspiron 600m

[Rafael J. Wysocki]

On Friday 30 October 2009, Linus Torvalds wrote:
> 
> On Fri, 30 Oct 2009, Rafael J. Wysocki wrote:
> > 
> >
> > 1) Resume works if pcmcia_socket_dev_resume(dev) is moved to the "regular"
> >    resume phase, after resume_device_irqs().
> 
> Hmm. We really probably shouldn't call pcmcia_socket_dev_resume() in 
> early_resume. It takes mutexes etc, and it calls "socket_resume()", which 
> sleeps etc. That per se should be ok these days (since we don't actualyl 
> disable CPU irq's, just device irqs), but it also does that whole card 
> insertion events etc. And _that_ code I wouldn't trust at all.

I thought so when I worked on commit 0c570cdeb, but then it turned out to
work just fine with a number of boxes.

> The PCMCIA code is better than it used to be a long time ago, but some of 
> it is still pretty crazy. 
> 
> I get the feeling that we should just revert that commit 0c570cdeb,

Well, there's nothing wrong with doing the PCI stuff and restoring the state at
the _noirq stage IMO, so instead of reverting it altogether, I'd add
yenta_dev_suspend|resume() that would just call
pcmcia_socket_dev_suspend|resume() during "regular" suspend|resume.

> and instead always do PCMCIA suspend as a "eject" event. That way we have no 
> driver behind it to resume at resume time - and we'll see any plugged-in 
> device as just a new insertion. 

In fact I thought about that.

It looks like I need to find a CardBus adapter somewhere and clean that thing up.

That said, I'd really like to know what's going on in there. :-)

Thanks,
Rafael

Re: Help needed, Re: [Bug #14334] pcmcia suspend regression from 2.6.31.1 to 2.6.31.2 - Dell Inspiron 600m

[Linus Torvalds]

On Fri, 30 Oct 2009, Rafael J. Wysocki wrote:

> On Friday 30 October 2009, Linus Torvalds wrote:

> > The PCMCIA code is better than it used to be a long time ago, but some of 
> > it is still pretty crazy. 
> > 
> > I get the feeling that we should just revert that commit 0c570cdeb,
> 
> Well, there's nothing wrong with doing the PCI stuff and restoring the state at
> the _noirq stage IMO, so instead of reverting it altogether, I'd add
> yenta_dev_suspend|resume() that would just call
> pcmcia_socket_dev_suspend|resume() during "regular" suspend|resume.

Oh, that sounds fine too. I didn't mean that we had to do an outright 
revert, since we'd still need to do something else to fix the original 
issue.

> > and instead always do PCMCIA suspend as a "eject" event. That way we have no 
> > driver behind it to resume at resume time - and we'll see any plugged-in 
> > device as just a new insertion. 
> 
> In fact I thought about that.
> 
> It looks like I need to find a CardBus adapter somewhere and clean that thing up.
> 
> That said, I'd really like to know what's going on in there. :-)

I have to admit that my last two laptops haven't even _had_ a PCMCIA slot, 
so I'm unlikely to be able to help much. But I could dig out an old one, 
and try to find the one wireless card I know I have in some corner.

And partly exactly _because_ even Cardbus is starting to be "legacy", I'd 
personally prefer to try to simplify the model to the point where we don't 
have to think about all the subtle interactions. Just making suspend act 
as an eject would mean that we'd never have to worry about how the CardBus 
bridge interacts with the PCI layer at suspend/resume time.

		Linus

Re: Help needed, Re: [Bug #14334] pcmcia suspend regression from 2.6.31.1 to 2.6.31.2 - Dell Inspiron 600m

[Linus Torvalds]

On Fri, 30 Oct 2009, Linus Torvalds wrote:
> 
> And partly exactly _because_ even Cardbus is starting to be "legacy", I'd 
> personally prefer to try to simplify the model to the point where we don't 
> have to think about all the subtle interactions. Just making suspend act 
> as an eject would mean that we'd never have to worry about how the CardBus 
> bridge interacts with the PCI layer at suspend/resume time.

Put another way: five years ago I would have felt that it could be 
important that people can suspend and resume while they have a CD-ROM 
mounted through a PCMCIA IDE card. Or something like that where you want 
to keep session information.

These days, that scenario is less interesting to begin with, and we're 
generally better at some of the hotplug issues anyway. Example: one of the 
reasons I used to like not causing an unplug event was because I had 
network cards, and hated setting up the connection again. These days, all 
distros come with networkmanager or similar, and hotplug networking just 
works (even if the "CD-ROM mounted" case probably still would cause 
problems).

So I think we used to have good reasons to try to maintain state over a 
suspend event, but many of those reasons have become weaker, while at the 
same time USB has meant that PCMCIA itself has become more of a 
"maintenance burden" rather than a "primary subsystem".

			Linus

Re: Help needed, Re: [Bug #14334] pcmcia suspend regression from 2.6.31.1 to 2.6.31.2 - Dell Inspiron 600m

[Rafael J. Wysocki]

On Friday 30 October 2009, Linus Torvalds wrote:
> 
> On Fri, 30 Oct 2009, Linus Torvalds wrote:
> > 
> > And partly exactly _because_ even Cardbus is starting to be "legacy", I'd 
> > personally prefer to try to simplify the model to the point where we don't 
> > have to think about all the subtle interactions. Just making suspend act 
> > as an eject would mean that we'd never have to worry about how the CardBus 
> > bridge interacts with the PCI layer at suspend/resume time.
> 
> Put another way: five years ago I would have felt that it could be 
> important that people can suspend and resume while they have a CD-ROM 
> mounted through a PCMCIA IDE card. Or something like that where you want 
> to keep session information.
> 
> These days, that scenario is less interesting to begin with, and we're 
> generally better at some of the hotplug issues anyway. Example: one of the 
> reasons I used to like not causing an unplug event was because I had 
> network cards, and hated setting up the connection again. These days, all 
> distros come with networkmanager or similar, and hotplug networking just 
> works (even if the "CD-ROM mounted" case probably still would cause 
> problems).
> 
> So I think we used to have good reasons to try to maintain state over a 
> suspend event, but many of those reasons have become weaker, while at the 
> same time USB has meant that PCMCIA itself has become more of a 
> "maintenance burden" rather than a "primary subsystem".

I agree.

Rafael

Re: Help needed, Re: [Bug #14334] pcmcia suspend regression from 2.6.31.1 to 2.6.31.2 - Dell Inspiron 600m

[Benjamin Herrenschmidt]

> Put another way: five years ago I would have felt that it could be 
> important that people can suspend and resume while they have a CD-ROM 
> mounted through a PCMCIA IDE card. Or something like that where you want 
> to keep session information.

I think you are a bit too much laptop centric :-) It's sadly still
common in the embedded space to use PCMCIA or Cardbus for ... root
filesystems.

Cheers,
Ben.

Re: Help needed, Re: [Bug #14334] pcmcia suspend regression from 2.6.31.1 to 2.6.31.2 - Dell Inspiron 600m

[Benjamin Herrenschmidt]

On Fri, 2009-10-30 at 12:47 -0700, Linus Torvalds wrote:
> 
> On Fri, 30 Oct 2009, Rafael J. Wysocki wrote:
> > 
> >
> > 1) Resume works if pcmcia_socket_dev_resume(dev) is moved to the "regular"
> >    resume phase, after resume_device_irqs().
> 
> Hmm. We really probably shouldn't call pcmcia_socket_dev_resume() in 
> early_resume. It takes mutexes etc, and it calls "socket_resume()", which 
> sleeps etc. That per se should be ok these days (since we don't actualyl 
> disable CPU irq's, just device irqs), but it also does that whole card 
> insertion events etc. And _that_ code I wouldn't trust at all.
> 
> The PCMCIA code is better than it used to be a long time ago, but some of 
> it is still pretty crazy. 
> 
> I get the feeling that we should just revert that commit 0c570cdeb, and 
> instead always do PCMCIA suspend as a "eject" event. That way we have no 
> driver behind it to resume at resume time - and we'll see any plugged-in 
> device as just a new insertion. 

To me the proper approach would be to split it so that

 - early_resume() restores power & config space etc... so that existing
devices can move on (might check for removal). There's no other hotplug
activity

 - normal resume() restarts handling of events such as insertions

Now, while I do have some cardbus & pcmcia stuff somewhere, I also don't
have much time to hack on this right now...

Cheers,
Ben.

Re: Help needed, Re: [Bug #14334] pcmcia suspend regression from 2.6.31.1 to 2.6.31.2 - Dell Inspiron 600m

[Rafael J. Wysocki]

On Saturday 31 October 2009, Benjamin Herrenschmidt wrote:
> On Fri, 2009-10-30 at 12:47 -0700, Linus Torvalds wrote:
> > 
> > On Fri, 30 Oct 2009, Rafael J. Wysocki wrote:
> > > 
> > >
> > > 1) Resume works if pcmcia_socket_dev_resume(dev) is moved to the "regular"
> > >    resume phase, after resume_device_irqs().
> > 
> > Hmm. We really probably shouldn't call pcmcia_socket_dev_resume() in 
> > early_resume. It takes mutexes etc, and it calls "socket_resume()", which 
> > sleeps etc. That per se should be ok these days (since we don't actualyl 
> > disable CPU irq's, just device irqs), but it also does that whole card 
> > insertion events etc. And _that_ code I wouldn't trust at all.
> > 
> > The PCMCIA code is better than it used to be a long time ago, but some of 
> > it is still pretty crazy. 
> > 
> > I get the feeling that we should just revert that commit 0c570cdeb, and 
> > instead always do PCMCIA suspend as a "eject" event. That way we have no 
> > driver behind it to resume at resume time - and we'll see any plugged-in 
> > device as just a new insertion. 
> 
> To me the proper approach would be to split it so that

Well, agreed, but ...

>  - early_resume() restores power & config space etc... so that existing
> devices can move on (might check for removal). There's no other hotplug
> activity

... that's exactly what doesn't work at the moment.

Thanks,
Rafael

Re: Help needed, Re: [Bug #14334] pcmcia suspend regression from 2.6.31.1 to 2.6.31.2 - Dell Inspiron 600m

[Benjamin Herrenschmidt]

On Sat, 2009-10-31 at 10:31 +0100, Rafael J. Wysocki wrote:

> > To me the proper approach would be to split it so that
> 
> Well, agreed, but ...
> 
> >  - early_resume() restores power & config space etc... so that existing
> > devices can move on (might check for removal). There's no other hotplug
> > activity
> 
> ... that's exactly what doesn't work at the moment.

BTW. This is a PCMCIA problem, a Cardbus or both ? I'll see if I can dig
something on monday ?

>From the little email history I caught, it smells like pcmcia old style.

I don't see a problem per-se with the mutex usage with the new interrupt
masking style as Linus says. socket_resume() also looks reasonably sane,
it's the whole handling of removal that should be deferred. Maybe
instead of doing socket_remove_drivers()...send_event() etc.. in there,
we could simply just shut the socket down (PCMCIA drivers should cope
with sockets returning ffff's for a short amount of time), flag it dead
in skt->state and have the "late" resume  actually fire off the driver
removal and sending of the event.

BTW. Have we ever documented whether it's kosher to ->remove() a driver
before ->resume()'ing it ? (In which case obviously we wouldn't resume
it).

Cheers,  
Ben.

Re: Help needed, Re: [Bug #14334] pcmcia suspend regression from 2.6.31.1 to 2.6.31.2 - Dell Inspiron 600m

[Rafael J. Wysocki]

On Saturday 31 October 2009, Benjamin Herrenschmidt wrote:
> On Sat, 2009-10-31 at 10:31 +0100, Rafael J. Wysocki wrote:
> 
> > > To me the proper approach would be to split it so that
> > 
> > Well, agreed, but ...
> > 
> > >  - early_resume() restores power & config space etc... so that existing
> > > devices can move on (might check for removal). There's no other hotplug
> > > activity
> > 
> > ... that's exactly what doesn't work at the moment.
> 
> BTW. This is a PCMCIA problem, a Cardbus or both ?

I _think_ it's a CardBus problem.  Evidently, it only happens if the second
CardBus bridge is resumed right after the first one (which resumes entirely
correctly) and only if that happens in the early resume phase (ie. before
resume_device_irqs()).

> I'll see if I can dig something on monday ?

In the meantime I invented a patch that works, ie. apparently fixes the problem
and if there was a card in the socket during the suspend, it's standard config
space is restored correctly.  I tested it on one of my boxes with two different
CardBus adapters and Jose says it fixes the problem for him.

The patch is appended, please have a look.

> >From the little email history I caught, it smells like pcmcia old style.
> 
> I don't see a problem per-se with the mutex usage with the new interrupt
> masking style as Linus says. socket_resume() also looks reasonably sane,
> it's the whole handling of removal that should be deferred.

In the failing case we don't even get there.  There are two CardBus bridges in
the system, but both sockets are empty, so we take the socket_insert() code
path.

> Maybe instead of doing socket_remove_drivers()...send_event() etc.. in there,
> we could simply just shut the socket down (PCMCIA drivers should cope
> with sockets returning ffff's for a short amount of time), flag it dead
> in skt->state and have the "late" resume  actually fire off the driver
> removal and sending of the event.
> 
> BTW. Have we ever documented whether it's kosher to ->remove() a driver
> before ->resume()'ing it ? (In which case obviously we wouldn't resume
> it).

The PM core doesn't have a problem with that.  Whether or not it's sane from
the driver design point of view is a separate question, though.

Thanks,
Rafael

---
From: Rafael J. Wysocki <rjw@sisk.pl>
Subject: PM / yenta: Split resume into early and late parts

Commit 0c570cdeb8fdfcb354a3e9cd81bfc6a09c19de0c
(PM / yenta: Fix cardbus suspend/resume regression) caused resume to
fail on systems with two CardBus bridges.  While the exact nature
of the failure is not known at the moment, it can be worked around by
splitting the yenta resume into an early part, executed during the
early phase of resume, that will only resume the socket and power it
up if there was a card in it during suspend, and a late part,
executed during "regular" resume, that will carry out all of the
remaining yenta resume operations.

Fixes http://bugzilla.kernel.org/show_bug.cgi?id=14334, which is a
listed regression from 2.6.31.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 drivers/pcmcia/cs.c           |   79 +++++++++++++++++++++++++++++-------------
 drivers/pcmcia/yenta_socket.c |   12 +++++-
 include/pcmcia/ss.h           |    2 +
 3 files changed, 68 insertions(+), 25 deletions(-)

Index: linux-2.6/drivers/pcmcia/yenta_socket.c
===================================================================
--- linux-2.6.orig/drivers/pcmcia/yenta_socket.c
+++ linux-2.6/drivers/pcmcia/yenta_socket.c
@@ -1275,16 +1275,26 @@ static int yenta_dev_resume_noirq(struct
 	if (socket->type && socket->type->restore_state)
 		socket->type->restore_state(socket);
 
-	return pcmcia_socket_dev_resume(dev);
+	pcmcia_socket_dev_early_resume(dev);
+	return 0;
+}
+
+static int yenta_dev_resume(struct device *dev)
+{
+	pcmcia_socket_dev_late_resume(dev);
+	return 0;
 }
 
 static struct dev_pm_ops yenta_pm_ops = {
 	.suspend_noirq = yenta_dev_suspend_noirq,
 	.resume_noirq = yenta_dev_resume_noirq,
+	.resume = yenta_dev_resume,
 	.freeze_noirq = yenta_dev_suspend_noirq,
 	.thaw_noirq = yenta_dev_resume_noirq,
+	.thaw = yenta_dev_resume,
 	.poweroff_noirq = yenta_dev_suspend_noirq,
 	.restore_noirq = yenta_dev_resume_noirq,
+	.restore = yenta_dev_resume,
 };
 
 #define YENTA_PM_OPS	(&yenta_pm_ops)
Index: linux-2.6/drivers/pcmcia/cs.c
===================================================================
--- linux-2.6.orig/drivers/pcmcia/cs.c
+++ linux-2.6/drivers/pcmcia/cs.c
@@ -98,10 +98,13 @@ EXPORT_SYMBOL(pcmcia_socket_list_rwsem);
  * These functions check for the appropriate struct pcmcia_soket arrays,
  * and pass them to the low-level functions pcmcia_{suspend,resume}_socket
  */
+static int socket_early_resume(struct pcmcia_socket *skt);
+static int socket_late_resume(struct pcmcia_socket *skt);
 static int socket_resume(struct pcmcia_socket *skt);
 static int socket_suspend(struct pcmcia_socket *skt);
 
-int pcmcia_socket_dev_suspend(struct device *dev)
+static void pcmcia_socket_dev_run(struct device *dev,
+				  int (*cb)(struct pcmcia_socket *))
 {
 	struct pcmcia_socket *socket;
 
@@ -110,29 +113,34 @@ int pcmcia_socket_dev_suspend(struct dev
 		if (socket->dev.parent != dev)
 			continue;
 		mutex_lock(&socket->skt_mutex);
-		socket_suspend(socket);
+		cb(socket);
 		mutex_unlock(&socket->skt_mutex);
 	}
 	up_read(&pcmcia_socket_list_rwsem);
+}
 
+int pcmcia_socket_dev_suspend(struct device *dev)
+{
+	pcmcia_socket_dev_run(dev, socket_suspend);
 	return 0;
 }
 EXPORT_SYMBOL(pcmcia_socket_dev_suspend);
 
-int pcmcia_socket_dev_resume(struct device *dev)
+void pcmcia_socket_dev_early_resume(struct device *dev)
 {
-	struct pcmcia_socket *socket;
+	pcmcia_socket_dev_run(dev, socket_early_resume);
+}
+EXPORT_SYMBOL(pcmcia_socket_dev_early_resume);
 
-	down_read(&pcmcia_socket_list_rwsem);
-	list_for_each_entry(socket, &pcmcia_socket_list, socket_list) {
-		if (socket->dev.parent != dev)
-			continue;
-		mutex_lock(&socket->skt_mutex);
-		socket_resume(socket);
-		mutex_unlock(&socket->skt_mutex);
-	}
-	up_read(&pcmcia_socket_list_rwsem);
+void pcmcia_socket_dev_late_resume(struct device *dev)
+{
+	pcmcia_socket_dev_run(dev, socket_late_resume);
+}
+EXPORT_SYMBOL(pcmcia_socket_dev_late_resume);
 
+int pcmcia_socket_dev_resume(struct device *dev)
+{
+	pcmcia_socket_dev_run(dev, socket_resume);
 	return 0;
 }
 EXPORT_SYMBOL(pcmcia_socket_dev_resume);
@@ -546,23 +554,30 @@ static int socket_suspend(struct pcmcia_
 	return 0;
 }
 
-/*
- * Resume a socket.  If a card is present, verify its CIS against
- * our cached copy.  If they are different, the card has been
- * replaced, and we need to tell the drivers.
- */
-static int socket_resume(struct pcmcia_socket *skt)
+static void socket_start_resume(struct pcmcia_socket *skt)
 {
-	int ret;
-
-	if (!(skt->state & SOCKET_SUSPEND))
-		return -EBUSY;
-
 	skt->socket = dead_socket;
 	skt->ops->init(skt);
 	skt->ops->set_socket(skt, &skt->socket);
+}
+
+static int socket_early_resume(struct pcmcia_socket *skt)
+{
+	if ((skt->state & SOCKET_SUSPEND) && (skt->state & SOCKET_PRESENT))
+		socket_start_resume(skt);
+
+	return 0;
+}
+
+static int socket_late_resume(struct pcmcia_socket *skt)
+{
+	int ret;
+
+	if (!(skt->state & SOCKET_SUSPEND))
+		return 0;
 
 	if (!(skt->state & SOCKET_PRESENT)) {
+		socket_start_resume(skt);
 		skt->state &= ~SOCKET_SUSPEND;
 		return socket_insert(skt);
 	}
@@ -596,6 +611,22 @@ static int socket_resume(struct pcmcia_s
 	return 0;
 }
 
+/*
+ * Resume a socket.  If a card is present, verify its CIS against
+ * our cached copy.  If they are different, the card has been
+ * replaced, and we need to tell the drivers.
+ */
+static int socket_resume(struct pcmcia_socket *skt)
+{
+	if (!(skt->state & SOCKET_SUSPEND))
+		return -EBUSY;
+
+	if (skt->state & SOCKET_PRESENT)
+		socket_start_resume(skt);
+
+	return socket_late_resume(skt);
+}
+
 static void socket_remove(struct pcmcia_socket *skt)
 {
 	dev_printk(KERN_NOTICE, &skt->dev,
Index: linux-2.6/include/pcmcia/ss.h
===================================================================
--- linux-2.6.orig/include/pcmcia/ss.h
+++ linux-2.6/include/pcmcia/ss.h
@@ -280,6 +280,8 @@ extern struct pccard_resource_ops pccard
 
 /* socket drivers are expected to use these callbacks in their .drv struct */
 extern int pcmcia_socket_dev_suspend(struct device *dev);
+extern void pcmcia_socket_dev_early_resume(struct device *dev);
+extern void pcmcia_socket_dev_late_resume(struct device *dev);
 extern int pcmcia_socket_dev_resume(struct device *dev);
 
 /* socket drivers use this callback in their IRQ handler */

Re: Help needed, Re: [Bug #14334] pcmcia suspend regression from 2.6.31.1 to 2.6.31.2 - Dell Inspiron 600m

[Linus Torvalds]

On Sat, 31 Oct 2009, Rafael J. Wysocki wrote:
> 
> The patch is appended, please have a look.

Looks sane to me. It does the actual real socket ops early, and does the 
crazy pcmcia resume late. 

And I like how you abstracted out that dev->socket thing in 
pcmcia_socket_dev_run().

The only thing that looks odd is how you do "socket_start_resume()" in the 
"late_resume" path too - that has already been done by the early_resume, 
and as far as I can see you're now initializing the socket twice.

Is there a reason for that? Or am I misreading the patch (I didn't 
actually apply it, I just read the patch itself).

		Linus

Re: Help needed, Re: [Bug #14334] pcmcia suspend regression from 2.6.31.1 to 2.6.31.2 - Dell Inspiron 600m

[Rafael J. Wysocki]

On Saturday 31 October 2009, Linus Torvalds wrote:
> 
> On Sat, 31 Oct 2009, Rafael J. Wysocki wrote:
> > 
> > The patch is appended, please have a look.
> 
> Looks sane to me. It does the actual real socket ops early, and does the 
> crazy pcmcia resume late. 
> 
> And I like how you abstracted out that dev->socket thing in 
> pcmcia_socket_dev_run().
> 
> The only thing that looks odd is how you do "socket_start_resume()" in the 
> "late_resume" path too - that has already been done by the early_resume, 
> and as far as I can see you're now initializing the socket twice.
> 
> Is there a reason for that? Or am I misreading the patch (I didn't 
> actually apply it, I just read the patch itself).

Yes, there is, because socket_early_resume() only does it in
the (skt->state & SOCKET_PRESENT) case.  If that bit is not set, the
initialization is entirely postponed.

Rafael

Re: Help needed, Re: [Bug #14334] pcmcia suspend regression from 2.6.31.1 to 2.6.31.2 - Dell Inspiron 600m

[Linus Torvalds]

On Sat, 31 Oct 2009, Rafael J. Wysocki wrote:
> 
> Yes, there is, because socket_early_resume() only does it in
> the (skt->state & SOCKET_PRESENT) case.  If that bit is not set, the
> initialization is entirely postponed.

Ahh, ok. And what's the reason for that? It seems like the 

	skt->socket = dead_socket;
	skt->ops->init(skt);
	skt->ops->set_socket(skt, &skt->socket);

thing should always be safe, whether there is something present or not.. ?

		Linus

Re: Help needed, Re: [Bug #14334] pcmcia suspend regression from 2.6.31.1 to 2.6.31.2 - Dell Inspiron 600m

[Rafael J. Wysocki]

On Saturday 31 October 2009, Linus Torvalds wrote:
> 
> On Sat, 31 Oct 2009, Rafael J. Wysocki wrote:
> > 
> > Yes, there is, because socket_early_resume() only does it in
> > the (skt->state & SOCKET_PRESENT) case.  If that bit is not set, the
> > initialization is entirely postponed.
> 
> Ahh, ok. And what's the reason for that? It seems like the 
> 
> 	skt->socket = dead_socket;
> 	skt->ops->init(skt);
> 	skt->ops->set_socket(skt, &skt->socket);
> 
> thing should always be safe, whether there is something present or not.. ?

It should, but I'm not sure given the reported behavior so far.

I guess I'll prepare another patch that does this unconditionally in the
early phase and let's see how that works.

Rafael

Re: Help needed, Re: [Bug #14334] pcmcia suspend regression from 2.6.31.1 to 2.6.31.2 - Dell Inspiron 600m

[Benjamin Herrenschmidt]

On Sat, 2009-10-31 at 22:27 +0100, Rafael J. Wysocki wrote:

> In the meantime I invented a patch that works, ie. apparently fixes the problem
> and if there was a card in the socket during the suspend, it's standard config
> space is restored correctly.  I tested it on one of my boxes with two different
> CardBus adapters and Jose says it fixes the problem for him.
> 
> The patch is appended, please have a look.

Base idea of the patch sounds good, quick browse through looks good too,

I'll try to band on it with various PCMCIA & CB gear tomorrow.

Cheers,
Ben.

Re: Help needed, Re: [Bug #14334] pcmcia suspend regression from 2.6.31.1 to 2.6.31.2 - Dell Inspiron 600m

[Rafael J. Wysocki]

On Saturday 31 October 2009, Benjamin Herrenschmidt wrote:
> On Sat, 2009-10-31 at 22:27 +0100, Rafael J. Wysocki wrote:
> 
> > In the meantime I invented a patch that works, ie. apparently fixes the problem
> > and if there was a card in the socket during the suspend, it's standard config
> > space is restored correctly.  I tested it on one of my boxes with two different
> > CardBus adapters and Jose says it fixes the problem for him.
> > 
> > The patch is appended, please have a look.
> 
> Base idea of the patch sounds good, quick browse through looks good too,
> 
> I'll try to band on it with various PCMCIA & CB gear tomorrow.

Wait, I made a mistake when testing it, didn't notice that my distro ejected
PCMCIA cards automatically before suspend (sigh).

Working on a better patch right now, will hopefully post it in a while.

Thanks,
Rafael

Re: Help needed, Re: [Bug #14334] pcmcia suspend regression from 2.6.31.1 to 2.6.31.2 - Dell Inspiron 600m

[Rafael J. Wysocki]

On Sunday 01 November 2009, Rafael J. Wysocki wrote:
> On Saturday 31 October 2009, Benjamin Herrenschmidt wrote:
> > On Sat, 2009-10-31 at 22:27 +0100, Rafael J. Wysocki wrote:
> > 
> > > In the meantime I invented a patch that works, ie. apparently fixes the problem
> > > and if there was a card in the socket during the suspend, it's standard config
> > > space is restored correctly.  I tested it on one of my boxes with two different
> > > CardBus adapters and Jose says it fixes the problem for him.
> > > 
> > > The patch is appended, please have a look.
> > 
> > Base idea of the patch sounds good, quick browse through looks good too,
> > 
> > I'll try to band on it with various PCMCIA & CB gear tomorrow.
> 
> Wait, I made a mistake when testing it, didn't notice that my distro ejected
> PCMCIA cards automatically before suspend (sigh).
> 
> Working on a better patch right now, will hopefully post it in a while.

Here you go.

Waiting for feedback from the reporters of BKO #14334, so there's no sign-off
for now.

---
 drivers/pcmcia/cs.c           |   79 ++++++++++++++++++++++++++++--------------
 drivers/pcmcia/yenta_socket.c |   12 +++++-
 include/pcmcia/ss.h           |    4 ++
 3 files changed, 68 insertions(+), 27 deletions(-)

Index: linux-2.6/drivers/pcmcia/yenta_socket.c
===================================================================
--- linux-2.6.orig/drivers/pcmcia/yenta_socket.c
+++ linux-2.6/drivers/pcmcia/yenta_socket.c
@@ -1275,16 +1275,26 @@ static int yenta_dev_resume_noirq(struct
 	if (socket->type && socket->type->restore_state)
 		socket->type->restore_state(socket);
 
-	return pcmcia_socket_dev_resume(dev);
+	pcmcia_socket_dev_early_resume(dev);
+	return 0;
+}
+
+static int yenta_dev_resume(struct device *dev)
+{
+	pcmcia_socket_dev_late_resume(dev);
+	return 0;
 }
 
 static struct dev_pm_ops yenta_pm_ops = {
 	.suspend_noirq = yenta_dev_suspend_noirq,
 	.resume_noirq = yenta_dev_resume_noirq,
+	.resume = yenta_dev_resume,
 	.freeze_noirq = yenta_dev_suspend_noirq,
 	.thaw_noirq = yenta_dev_resume_noirq,
+	.thaw = yenta_dev_resume,
 	.poweroff_noirq = yenta_dev_suspend_noirq,
 	.restore_noirq = yenta_dev_resume_noirq,
+	.restore = yenta_dev_resume,
 };
 
 #define YENTA_PM_OPS	(&yenta_pm_ops)
Index: linux-2.6/drivers/pcmcia/cs.c
===================================================================
--- linux-2.6.orig/drivers/pcmcia/cs.c
+++ linux-2.6/drivers/pcmcia/cs.c
@@ -98,10 +98,13 @@ EXPORT_SYMBOL(pcmcia_socket_list_rwsem);
  * These functions check for the appropriate struct pcmcia_soket arrays,
  * and pass them to the low-level functions pcmcia_{suspend,resume}_socket
  */
+static int socket_early_resume(struct pcmcia_socket *skt);
+static int socket_late_resume(struct pcmcia_socket *skt);
 static int socket_resume(struct pcmcia_socket *skt);
 static int socket_suspend(struct pcmcia_socket *skt);
 
-int pcmcia_socket_dev_suspend(struct device *dev)
+static void pcmcia_socket_dev_run(struct device *dev,
+				  int (*cb)(struct pcmcia_socket *))
 {
 	struct pcmcia_socket *socket;
 
@@ -110,29 +113,34 @@ int pcmcia_socket_dev_suspend(struct dev
 		if (socket->dev.parent != dev)
 			continue;
 		mutex_lock(&socket->skt_mutex);
-		socket_suspend(socket);
+		cb(socket);
 		mutex_unlock(&socket->skt_mutex);
 	}
 	up_read(&pcmcia_socket_list_rwsem);
+}
 
+int pcmcia_socket_dev_suspend(struct device *dev)
+{
+	pcmcia_socket_dev_run(dev, socket_suspend);
 	return 0;
 }
 EXPORT_SYMBOL(pcmcia_socket_dev_suspend);
 
-int pcmcia_socket_dev_resume(struct device *dev)
+void pcmcia_socket_dev_early_resume(struct device *dev)
 {
-	struct pcmcia_socket *socket;
+	pcmcia_socket_dev_run(dev, socket_early_resume);
+}
+EXPORT_SYMBOL(pcmcia_socket_dev_early_resume);
 
-	down_read(&pcmcia_socket_list_rwsem);
-	list_for_each_entry(socket, &pcmcia_socket_list, socket_list) {
-		if (socket->dev.parent != dev)
-			continue;
-		mutex_lock(&socket->skt_mutex);
-		socket_resume(socket);
-		mutex_unlock(&socket->skt_mutex);
-	}
-	up_read(&pcmcia_socket_list_rwsem);
+void pcmcia_socket_dev_late_resume(struct device *dev)
+{
+	pcmcia_socket_dev_run(dev, socket_late_resume);
+}
+EXPORT_SYMBOL(pcmcia_socket_dev_late_resume);
 
+int pcmcia_socket_dev_resume(struct device *dev)
+{
+	pcmcia_socket_dev_run(dev, socket_resume);
 	return 0;
 }
 EXPORT_SYMBOL(pcmcia_socket_dev_resume);
@@ -546,29 +554,34 @@ static int socket_suspend(struct pcmcia_
 	return 0;
 }
 
-/*
- * Resume a socket.  If a card is present, verify its CIS against
- * our cached copy.  If they are different, the card has been
- * replaced, and we need to tell the drivers.
- */
-static int socket_resume(struct pcmcia_socket *skt)
+static void socket_start_resume(struct pcmcia_socket *skt)
 {
-	int ret;
-
-	if (!(skt->state & SOCKET_SUSPEND))
-		return -EBUSY;
-
 	skt->socket = dead_socket;
 	skt->ops->init(skt);
 	skt->ops->set_socket(skt, &skt->socket);
+	if (skt->state & SOCKET_PRESENT)
+		skt->resume_status = socket_setup(skt, resume_delay);
+}
+
+static int socket_early_resume(struct pcmcia_socket *skt)
+{
+	if (skt->state & SOCKET_SUSPEND)
+		socket_start_resume(skt);
+
+	return 0;
+}
+
+static int socket_late_resume(struct pcmcia_socket *skt)
+{
+	if (!(skt->state & SOCKET_SUSPEND))
+		return 0;
 
 	if (!(skt->state & SOCKET_PRESENT)) {
 		skt->state &= ~SOCKET_SUSPEND;
 		return socket_insert(skt);
 	}
 
-	ret = socket_setup(skt, resume_delay);
-	if (ret == 0) {
+	if (skt->resume_status == 0) {
 		/*
 		 * FIXME: need a better check here for cardbus cards.
 		 */
@@ -596,6 +609,20 @@ static int socket_resume(struct pcmcia_s
 	return 0;
 }
 
+/*
+ * Resume a socket.  If a card is present, verify its CIS against
+ * our cached copy.  If they are different, the card has been
+ * replaced, and we need to tell the drivers.
+ */
+static int socket_resume(struct pcmcia_socket *skt)
+{
+	if (!(skt->state & SOCKET_SUSPEND))
+		return -EBUSY;
+
+	socket_start_resume(skt);
+	return socket_late_resume(skt);
+}
+
 static void socket_remove(struct pcmcia_socket *skt)
 {
 	dev_printk(KERN_NOTICE, &skt->dev,
Index: linux-2.6/include/pcmcia/ss.h
===================================================================
--- linux-2.6.orig/include/pcmcia/ss.h
+++ linux-2.6/include/pcmcia/ss.h
@@ -262,6 +262,8 @@ struct pcmcia_socket {
 	struct device			dev;
 	/* data internal to the socket driver */
 	void				*driver_data;
+	/* status of the card during resume from a system sleep state */
+	int				resume_status;
 };
 
 
@@ -280,6 +282,8 @@ extern struct pccard_resource_ops pccard
 
 /* socket drivers are expected to use these callbacks in their .drv struct */
 extern int pcmcia_socket_dev_suspend(struct device *dev);
+extern void pcmcia_socket_dev_early_resume(struct device *dev);
+extern void pcmcia_socket_dev_late_resume(struct device *dev);
 extern int pcmcia_socket_dev_resume(struct device *dev);
 
 /* socket drivers use this callback in their IRQ handler */

Re: Help needed, Re: [Bug #14334] pcmcia suspend regression from 2.6.31.1 to 2.6.31.2 - Dell Inspiron 600m

[Rafael J. Wysocki]

On Sunday 01 November 2009, Rafael J. Wysocki wrote:
> On Sunday 01 November 2009, Rafael J. Wysocki wrote:
> > On Saturday 31 October 2009, Benjamin Herrenschmidt wrote:
> > > On Sat, 2009-10-31 at 22:27 +0100, Rafael J. Wysocki wrote:
> > > 
> > > > In the meantime I invented a patch that works, ie. apparently fixes the problem
> > > > and if there was a card in the socket during the suspend, it's standard config
> > > > space is restored correctly.  I tested it on one of my boxes with two different
> > > > CardBus adapters and Jose says it fixes the problem for him.
> > > > 
> > > > The patch is appended, please have a look.
> > > 
> > > Base idea of the patch sounds good, quick browse through looks good too,
> > > 
> > > I'll try to band on it with various PCMCIA & CB gear tomorrow.
> > 
> > Wait, I made a mistake when testing it, didn't notice that my distro ejected
> > PCMCIA cards automatically before suspend (sigh).
> > 
> > Working on a better patch right now, will hopefully post it in a while.
> 
> Here you go.
> 
> Waiting for feedback from the reporters of BKO #14334, so there's no sign-off
> for now.

Jose confirms that the patch fixes the problem for him, so here it goes again
with full changelog.

If people don't object, I'll push it through the suspend-2.6 tree along with a
few other bug fixes.

---
From: Rafael J. Wysocki <rjw@sisk.pl>
Subject: PM / yenta: Split resume into early and late parts (rev. 2)

Commit 0c570cdeb8fdfcb354a3e9cd81bfc6a09c19de0c
(PM / yenta: Fix cardbus suspend/resume regression) caused resume to
fail on systems with two CardBus bridges.  While the exact nature
of the failure is not known at the moment, it can be worked around by
splitting the yenta resume into an early part, executed during the
early phase of resume, that will only resume the socket and power it
up if there was a card in it during suspend, and a late part,
executed during "regular" resume, that will carry out all of the
remaining yenta resume operations.

Fixes http://bugzilla.kernel.org/show_bug.cgi?id=14334, which is a
listed regression from 2.6.31.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 drivers/pcmcia/cs.c           |   79 ++++++++++++++++++++++++++++--------------
 drivers/pcmcia/yenta_socket.c |   12 +++++-
 include/pcmcia/ss.h           |    4 ++
 3 files changed, 68 insertions(+), 27 deletions(-)

Index: linux-2.6/drivers/pcmcia/yenta_socket.c
===================================================================
--- linux-2.6.orig/drivers/pcmcia/yenta_socket.c
+++ linux-2.6/drivers/pcmcia/yenta_socket.c
@@ -1275,16 +1275,26 @@ static int yenta_dev_resume_noirq(struct
 	if (socket->type && socket->type->restore_state)
 		socket->type->restore_state(socket);
 
-	return pcmcia_socket_dev_resume(dev);
+	pcmcia_socket_dev_early_resume(dev);
+	return 0;
+}
+
+static int yenta_dev_resume(struct device *dev)
+{
+	pcmcia_socket_dev_late_resume(dev);
+	return 0;
 }
 
 static struct dev_pm_ops yenta_pm_ops = {
 	.suspend_noirq = yenta_dev_suspend_noirq,
 	.resume_noirq = yenta_dev_resume_noirq,
+	.resume = yenta_dev_resume,
 	.freeze_noirq = yenta_dev_suspend_noirq,
 	.thaw_noirq = yenta_dev_resume_noirq,
+	.thaw = yenta_dev_resume,
 	.poweroff_noirq = yenta_dev_suspend_noirq,
 	.restore_noirq = yenta_dev_resume_noirq,
+	.restore = yenta_dev_resume,
 };
 
 #define YENTA_PM_OPS	(&yenta_pm_ops)
Index: linux-2.6/drivers/pcmcia/cs.c
===================================================================
--- linux-2.6.orig/drivers/pcmcia/cs.c
+++ linux-2.6/drivers/pcmcia/cs.c
@@ -98,10 +98,13 @@ EXPORT_SYMBOL(pcmcia_socket_list_rwsem);
  * These functions check for the appropriate struct pcmcia_soket arrays,
  * and pass them to the low-level functions pcmcia_{suspend,resume}_socket
  */
+static int socket_early_resume(struct pcmcia_socket *skt);
+static int socket_late_resume(struct pcmcia_socket *skt);
 static int socket_resume(struct pcmcia_socket *skt);
 static int socket_suspend(struct pcmcia_socket *skt);
 
-int pcmcia_socket_dev_suspend(struct device *dev)
+static void pcmcia_socket_dev_run(struct device *dev,
+				  int (*cb)(struct pcmcia_socket *))
 {
 	struct pcmcia_socket *socket;
 
@@ -110,29 +113,34 @@ int pcmcia_socket_dev_suspend(struct dev
 		if (socket->dev.parent != dev)
 			continue;
 		mutex_lock(&socket->skt_mutex);
-		socket_suspend(socket);
+		cb(socket);
 		mutex_unlock(&socket->skt_mutex);
 	}
 	up_read(&pcmcia_socket_list_rwsem);
+}
 
+int pcmcia_socket_dev_suspend(struct device *dev)
+{
+	pcmcia_socket_dev_run(dev, socket_suspend);
 	return 0;
 }
 EXPORT_SYMBOL(pcmcia_socket_dev_suspend);
 
-int pcmcia_socket_dev_resume(struct device *dev)
+void pcmcia_socket_dev_early_resume(struct device *dev)
 {
-	struct pcmcia_socket *socket;
+	pcmcia_socket_dev_run(dev, socket_early_resume);
+}
+EXPORT_SYMBOL(pcmcia_socket_dev_early_resume);
 
-	down_read(&pcmcia_socket_list_rwsem);
-	list_for_each_entry(socket, &pcmcia_socket_list, socket_list) {
-		if (socket->dev.parent != dev)
-			continue;
-		mutex_lock(&socket->skt_mutex);
-		socket_resume(socket);
-		mutex_unlock(&socket->skt_mutex);
-	}
-	up_read(&pcmcia_socket_list_rwsem);
+void pcmcia_socket_dev_late_resume(struct device *dev)
+{
+	pcmcia_socket_dev_run(dev, socket_late_resume);
+}
+EXPORT_SYMBOL(pcmcia_socket_dev_late_resume);
 
+int pcmcia_socket_dev_resume(struct device *dev)
+{
+	pcmcia_socket_dev_run(dev, socket_resume);
 	return 0;
 }
 EXPORT_SYMBOL(pcmcia_socket_dev_resume);
@@ -546,29 +554,34 @@ static int socket_suspend(struct pcmcia_
 	return 0;
 }
 
-/*
- * Resume a socket.  If a card is present, verify its CIS against
- * our cached copy.  If they are different, the card has been
- * replaced, and we need to tell the drivers.
- */
-static int socket_resume(struct pcmcia_socket *skt)
+static void socket_start_resume(struct pcmcia_socket *skt)
 {
-	int ret;
-
-	if (!(skt->state & SOCKET_SUSPEND))
-		return -EBUSY;
-
 	skt->socket = dead_socket;
 	skt->ops->init(skt);
 	skt->ops->set_socket(skt, &skt->socket);
+	if (skt->state & SOCKET_PRESENT)
+		skt->resume_status = socket_setup(skt, resume_delay);
+}
+
+static int socket_early_resume(struct pcmcia_socket *skt)
+{
+	if (skt->state & SOCKET_SUSPEND)
+		socket_start_resume(skt);
+
+	return 0;
+}
+
+static int socket_late_resume(struct pcmcia_socket *skt)
+{
+	if (!(skt->state & SOCKET_SUSPEND))
+		return 0;
 
 	if (!(skt->state & SOCKET_PRESENT)) {
 		skt->state &= ~SOCKET_SUSPEND;
 		return socket_insert(skt);
 	}
 
-	ret = socket_setup(skt, resume_delay);
-	if (ret == 0) {
+	if (skt->resume_status == 0) {
 		/*
 		 * FIXME: need a better check here for cardbus cards.
 		 */
@@ -596,6 +609,20 @@ static int socket_resume(struct pcmcia_s
 	return 0;
 }
 
+/*
+ * Resume a socket.  If a card is present, verify its CIS against
+ * our cached copy.  If they are different, the card has been
+ * replaced, and we need to tell the drivers.
+ */
+static int socket_resume(struct pcmcia_socket *skt)
+{
+	if (!(skt->state & SOCKET_SUSPEND))
+		return -EBUSY;
+
+	socket_start_resume(skt);
+	return socket_late_resume(skt);
+}
+
 static void socket_remove(struct pcmcia_socket *skt)
 {
 	dev_printk(KERN_NOTICE, &skt->dev,
Index: linux-2.6/include/pcmcia/ss.h
===================================================================
--- linux-2.6.orig/include/pcmcia/ss.h
+++ linux-2.6/include/pcmcia/ss.h
@@ -262,6 +262,8 @@ struct pcmcia_socket {
 	struct device			dev;
 	/* data internal to the socket driver */
 	void				*driver_data;
+	/* status of the card during resume from a system sleep state */
+	int				resume_status;
 };
 
 
@@ -280,6 +282,8 @@ extern struct pccard_resource_ops pccard
 
 /* socket drivers are expected to use these callbacks in their .drv struct */
 extern int pcmcia_socket_dev_suspend(struct device *dev);
+extern void pcmcia_socket_dev_early_resume(struct device *dev);
+extern void pcmcia_socket_dev_late_resume(struct device *dev);
 extern int pcmcia_socket_dev_resume(struct device *dev);
 
 /* socket drivers use this callback in their IRQ handler */

Re: Help needed, Re: [Bug #14334] pcmcia suspend regression from 2.6.31.1 to 2.6.31.2 - Dell Inspiron 600m

[Dominik Brodowski]

Hey,

On Sun, Nov 01, 2009 at 09:36:10AM +0100, Rafael J. Wysocki wrote:
> Commit 0c570cdeb8fdfcb354a3e9cd81bfc6a09c19de0c
> (PM / yenta: Fix cardbus suspend/resume regression) caused resume to
> fail on systems with two CardBus bridges.  While the exact nature
> of the failure is not known at the moment, it can be worked around by
> splitting the yenta resume into an early part, executed during the
> early phase of resume, that will only resume the socket and power it
> up if there was a card in it during suspend, and a late part,
> executed during "regular" resume, that will carry out all of the
> remaining yenta resume operations.
> 
> Fixes http://bugzilla.kernel.org/show_bug.cgi?id=14334, which is a
> listed regression from 2.6.31.

The only issue I see is that we now return 0 unconditionally on the resume
callbacks. Otherwise, it's

Acked-by: Dominik Brodowski <linux@dominikbrodowski.net>

> Index: linux-2.6/drivers/pcmcia/yenta_socket.c
> ===================================================================
> --- linux-2.6.orig/drivers/pcmcia/yenta_socket.c
> +++ linux-2.6/drivers/pcmcia/yenta_socket.c
> @@ -1275,16 +1275,26 @@ static int yenta_dev_resume_noirq(struct
>  	if (socket->type && socket->type->restore_state)
>  		socket->type->restore_state(socket);
>  
> -	return pcmcia_socket_dev_resume(dev);
> +	pcmcia_socket_dev_early_resume(dev);
> +	return 0;
> +}
> +
> +static int yenta_dev_resume(struct device *dev)
> +{
> +	pcmcia_socket_dev_late_resume(dev);
> +	return 0;
>  }
>  
>  static struct dev_pm_ops yenta_pm_ops = {
>  	.suspend_noirq = yenta_dev_suspend_noirq,
>  	.resume_noirq = yenta_dev_resume_noirq,
> +	.resume = yenta_dev_resume,
>  	.freeze_noirq = yenta_dev_suspend_noirq,
>  	.thaw_noirq = yenta_dev_resume_noirq,
> +	.thaw = yenta_dev_resume,
>  	.poweroff_noirq = yenta_dev_suspend_noirq,
>  	.restore_noirq = yenta_dev_resume_noirq,
> +	.restore = yenta_dev_resume,
>  };
>  
>  #define YENTA_PM_OPS	(&yenta_pm_ops)
> Index: linux-2.6/drivers/pcmcia/cs.c
> ===================================================================
> --- linux-2.6.orig/drivers/pcmcia/cs.c
> +++ linux-2.6/drivers/pcmcia/cs.c
> @@ -98,10 +98,13 @@ EXPORT_SYMBOL(pcmcia_socket_list_rwsem);
>   * These functions check for the appropriate struct pcmcia_soket arrays,
>   * and pass them to the low-level functions pcmcia_{suspend,resume}_socket
>   */
> +static int socket_early_resume(struct pcmcia_socket *skt);
> +static int socket_late_resume(struct pcmcia_socket *skt);
>  static int socket_resume(struct pcmcia_socket *skt);
>  static int socket_suspend(struct pcmcia_socket *skt);
>  
> -int pcmcia_socket_dev_suspend(struct device *dev)
> +static void pcmcia_socket_dev_run(struct device *dev,
> +				  int (*cb)(struct pcmcia_socket *))
>  {
>  	struct pcmcia_socket *socket;
>  
> @@ -110,29 +113,34 @@ int pcmcia_socket_dev_suspend(struct dev
>  		if (socket->dev.parent != dev)
>  			continue;
>  		mutex_lock(&socket->skt_mutex);
> -		socket_suspend(socket);
> +		cb(socket);
>  		mutex_unlock(&socket->skt_mutex);
>  	}
>  	up_read(&pcmcia_socket_list_rwsem);
> +}
>  
> +int pcmcia_socket_dev_suspend(struct device *dev)
> +{
> +	pcmcia_socket_dev_run(dev, socket_suspend);
>  	return 0;
>  }
>  EXPORT_SYMBOL(pcmcia_socket_dev_suspend);
>  
> -int pcmcia_socket_dev_resume(struct device *dev)
> +void pcmcia_socket_dev_early_resume(struct device *dev)
>  {
> -	struct pcmcia_socket *socket;
> +	pcmcia_socket_dev_run(dev, socket_early_resume);
> +}
> +EXPORT_SYMBOL(pcmcia_socket_dev_early_resume);
>  
> -	down_read(&pcmcia_socket_list_rwsem);
> -	list_for_each_entry(socket, &pcmcia_socket_list, socket_list) {
> -		if (socket->dev.parent != dev)
> -			continue;
> -		mutex_lock(&socket->skt_mutex);
> -		socket_resume(socket);
> -		mutex_unlock(&socket->skt_mutex);
> -	}
> -	up_read(&pcmcia_socket_list_rwsem);
> +void pcmcia_socket_dev_late_resume(struct device *dev)
> +{
> +	pcmcia_socket_dev_run(dev, socket_late_resume);
> +}
> +EXPORT_SYMBOL(pcmcia_socket_dev_late_resume);
>  
> +int pcmcia_socket_dev_resume(struct device *dev)
> +{
> +	pcmcia_socket_dev_run(dev, socket_resume);
>  	return 0;
>  }
>  EXPORT_SYMBOL(pcmcia_socket_dev_resume);
> @@ -546,29 +554,34 @@ static int socket_suspend(struct pcmcia_
>  	return 0;
>  }
>  
> -/*
> - * Resume a socket.  If a card is present, verify its CIS against
> - * our cached copy.  If they are different, the card has been
> - * replaced, and we need to tell the drivers.
> - */
> -static int socket_resume(struct pcmcia_socket *skt)
> +static void socket_start_resume(struct pcmcia_socket *skt)
>  {
> -	int ret;
> -
> -	if (!(skt->state & SOCKET_SUSPEND))
> -		return -EBUSY;
> -
>  	skt->socket = dead_socket;
>  	skt->ops->init(skt);
>  	skt->ops->set_socket(skt, &skt->socket);
> +	if (skt->state & SOCKET_PRESENT)
> +		skt->resume_status = socket_setup(skt, resume_delay);
> +}
> +
> +static int socket_early_resume(struct pcmcia_socket *skt)
> +{
> +	if (skt->state & SOCKET_SUSPEND)
> +		socket_start_resume(skt);
> +
> +	return 0;
> +}
> +
> +static int socket_late_resume(struct pcmcia_socket *skt)
> +{
> +	if (!(skt->state & SOCKET_SUSPEND))
> +		return 0;
>  
>  	if (!(skt->state & SOCKET_PRESENT)) {
>  		skt->state &= ~SOCKET_SUSPEND;
>  		return socket_insert(skt);
>  	}
>  
> -	ret = socket_setup(skt, resume_delay);
> -	if (ret == 0) {
> +	if (skt->resume_status == 0) {
>  		/*
>  		 * FIXME: need a better check here for cardbus cards.
>  		 */
> @@ -596,6 +609,20 @@ static int socket_resume(struct pcmcia_s
>  	return 0;
>  }
>  
> +/*
> + * Resume a socket.  If a card is present, verify its CIS against
> + * our cached copy.  If they are different, the card has been
> + * replaced, and we need to tell the drivers.
> + */
> +static int socket_resume(struct pcmcia_socket *skt)
> +{
> +	if (!(skt->state & SOCKET_SUSPEND))
> +		return -EBUSY;
> +
> +	socket_start_resume(skt);
> +	return socket_late_resume(skt);
> +}
> +
>  static void socket_remove(struct pcmcia_socket *skt)
>  {
>  	dev_printk(KERN_NOTICE, &skt->dev,
> Index: linux-2.6/include/pcmcia/ss.h
> ===================================================================
> --- linux-2.6.orig/include/pcmcia/ss.h
> +++ linux-2.6/include/pcmcia/ss.h
> @@ -262,6 +262,8 @@ struct pcmcia_socket {
>  	struct device			dev;
>  	/* data internal to the socket driver */
>  	void				*driver_data;
> +	/* status of the card during resume from a system sleep state */
> +	int				resume_status;
>  };
>  
>  
> @@ -280,6 +282,8 @@ extern struct pccard_resource_ops pccard
>  
>  /* socket drivers are expected to use these callbacks in their .drv struct */
>  extern int pcmcia_socket_dev_suspend(struct device *dev);
> +extern void pcmcia_socket_dev_early_resume(struct device *dev);
> +extern void pcmcia_socket_dev_late_resume(struct device *dev);
>  extern int pcmcia_socket_dev_resume(struct device *dev);
>  
>  /* socket drivers use this callback in their IRQ handler */

Re: Help needed, Re: [Bug #14334] pcmcia suspend regression from 2.6.31.1 to 2.6.31.2 - Dell Inspiron 600m

[Rafael J. Wysocki]

On Sunday 01 November 2009, Dominik Brodowski wrote:
> Hey,
> 
> On Sun, Nov 01, 2009 at 09:36:10AM +0100, Rafael J. Wysocki wrote:
> > Commit 0c570cdeb8fdfcb354a3e9cd81bfc6a09c19de0c
> > (PM / yenta: Fix cardbus suspend/resume regression) caused resume to
> > fail on systems with two CardBus bridges.  While the exact nature
> > of the failure is not known at the moment, it can be worked around by
> > splitting the yenta resume into an early part, executed during the
> > early phase of resume, that will only resume the socket and power it
> > up if there was a card in it during suspend, and a late part,
> > executed during "regular" resume, that will carry out all of the
> > remaining yenta resume operations.
> > 
> > Fixes http://bugzilla.kernel.org/show_bug.cgi?id=14334, which is a
> > listed regression from 2.6.31.
> 
> The only issue I see is that we now return 0 unconditionally on the resume
> callbacks.

Hmm. pcmcia_socket_dev_suspend() and pcmcia_socket_dev_resume() return 0
unconditionally even without the patch, so it doesn't change that.

> Otherwise, it's
> 
> Acked-by: Dominik Brodowski <linux-X3ehHDuj6sIIGcDfoQAp7OTW4wlIGRCZ@public.gmane.org>

Thanks!

Rafael

Re: Help needed, Re: [Bug #14334] pcmcia suspend regression from 2.6.31.1 to 2.6.31.2 - Dell Inspiron 600m

[Linus Torvalds]

On Sun, 1 Nov 2009, Rafael J. Wysocki wrote:
> 
> If people don't object, I'll push it through the suspend-2.6 tree along 
> with a few other bug fixes.

No objections, but a cleanup request:

> +static int socket_early_resume(struct pcmcia_socket *skt)
> +{
> +	if (skt->state & SOCKET_SUSPEND)
> +		socket_start_resume(skt);
> +
> +	return 0;
> +}
> +
> +static int socket_late_resume(struct pcmcia_socket *skt)
> +{
> +	if (!(skt->state & SOCKET_SUSPEND))
> +		return 0;

As far as I can tell, that "SOCKET_SUSPEND" test is totally pointless. 
That socket _is_ going to be suspended, and testing for it here just seems 
to confuse things. 

So I'd remove it from both early_resume and late_resume, and only keep it 
in the case of the legacy user-requested suspend/resume (do we even do 
that any more?).

The SOCKET_SUSPEND flag itself is still relevant, of course, since the 
state change handling will test it (in order to avoid insert/remove 
handlign while we have the suspend flag set). It's just that the suspend 
code shouldn't _test_ it, since the suspend code is what sets it in the 
first place.

		Linus

Re: Help needed, Re: [Bug #14334] pcmcia suspend regression from 2.6.31.1 to 2.6.31.2 - Dell Inspiron 600m

[Rafael J. Wysocki]

On Sunday 01 November 2009, Linus Torvalds wrote:
> 
> On Sun, 1 Nov 2009, Rafael J. Wysocki wrote:
> > 
> > If people don't object, I'll push it through the suspend-2.6 tree along 
> > with a few other bug fixes.
> 
> No objections, but a cleanup request:
> 
> > +static int socket_early_resume(struct pcmcia_socket *skt)
> > +{
> > +	if (skt->state & SOCKET_SUSPEND)
> > +		socket_start_resume(skt);
> > +
> > +	return 0;
> > +}
> > +
> > +static int socket_late_resume(struct pcmcia_socket *skt)
> > +{
> > +	if (!(skt->state & SOCKET_SUSPEND))
> > +		return 0;
> 
> As far as I can tell, that "SOCKET_SUSPEND" test is totally pointless. 

Right.

> That socket _is_ going to be suspended, and testing for it here just seems 
> to confuse things. 
> 
> So I'd remove it from both early_resume and late_resume, and only keep it 
> in the case of the legacy user-requested suspend/resume (do we even do 
> that any more?).

OK, updated patch is appended.

Thanks,
Rafael

---
From: Rafael J. Wysocki <rjw@sisk.pl>
Subject: PM / yenta: Split resume into early and late parts (rev. 3)

Commit 0c570cdeb8fdfcb354a3e9cd81bfc6a09c19de0c
(PM / yenta: Fix cardbus suspend/resume regression) caused resume to
fail on systems with two CardBus bridges.  While the exact nature
of the failure is not known at the moment, it can be worked around by
splitting the yenta resume into an early part, executed during the
early phase of resume, that will only resume the socket and power it
up if there was a card in it during suspend, and a late part,
executed during "regular" resume, that will carry out all of the
remaining yenta resume operations.

Fixes http://bugzilla.kernel.org/show_bug.cgi?id=14334, which is a
listed regression from 2.6.31.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 drivers/pcmcia/cs.c           |   74 +++++++++++++++++++++++++++---------------
 drivers/pcmcia/yenta_socket.c |   12 ++++++
 include/pcmcia/ss.h           |    4 ++
 3 files changed, 63 insertions(+), 27 deletions(-)

Index: linux-2.6/drivers/pcmcia/yenta_socket.c
===================================================================
--- linux-2.6.orig/drivers/pcmcia/yenta_socket.c
+++ linux-2.6/drivers/pcmcia/yenta_socket.c
@@ -1275,16 +1275,26 @@ static int yenta_dev_resume_noirq(struct
 	if (socket->type && socket->type->restore_state)
 		socket->type->restore_state(socket);
 
-	return pcmcia_socket_dev_resume(dev);
+	pcmcia_socket_dev_early_resume(dev);
+	return 0;
+}
+
+static int yenta_dev_resume(struct device *dev)
+{
+	pcmcia_socket_dev_late_resume(dev);
+	return 0;
 }
 
 static struct dev_pm_ops yenta_pm_ops = {
 	.suspend_noirq = yenta_dev_suspend_noirq,
 	.resume_noirq = yenta_dev_resume_noirq,
+	.resume = yenta_dev_resume,
 	.freeze_noirq = yenta_dev_suspend_noirq,
 	.thaw_noirq = yenta_dev_resume_noirq,
+	.thaw = yenta_dev_resume,
 	.poweroff_noirq = yenta_dev_suspend_noirq,
 	.restore_noirq = yenta_dev_resume_noirq,
+	.restore = yenta_dev_resume,
 };
 
 #define YENTA_PM_OPS	(&yenta_pm_ops)
Index: linux-2.6/drivers/pcmcia/cs.c
===================================================================
--- linux-2.6.orig/drivers/pcmcia/cs.c
+++ linux-2.6/drivers/pcmcia/cs.c
@@ -98,10 +98,13 @@ EXPORT_SYMBOL(pcmcia_socket_list_rwsem);
  * These functions check for the appropriate struct pcmcia_soket arrays,
  * and pass them to the low-level functions pcmcia_{suspend,resume}_socket
  */
+static int socket_early_resume(struct pcmcia_socket *skt);
+static int socket_late_resume(struct pcmcia_socket *skt);
 static int socket_resume(struct pcmcia_socket *skt);
 static int socket_suspend(struct pcmcia_socket *skt);
 
-int pcmcia_socket_dev_suspend(struct device *dev)
+static void pcmcia_socket_dev_run(struct device *dev,
+				  int (*cb)(struct pcmcia_socket *))
 {
 	struct pcmcia_socket *socket;
 
@@ -110,29 +113,34 @@ int pcmcia_socket_dev_suspend(struct dev
 		if (socket->dev.parent != dev)
 			continue;
 		mutex_lock(&socket->skt_mutex);
-		socket_suspend(socket);
+		cb(socket);
 		mutex_unlock(&socket->skt_mutex);
 	}
 	up_read(&pcmcia_socket_list_rwsem);
+}
 
+int pcmcia_socket_dev_suspend(struct device *dev)
+{
+	pcmcia_socket_dev_run(dev, socket_suspend);
 	return 0;
 }
 EXPORT_SYMBOL(pcmcia_socket_dev_suspend);
 
-int pcmcia_socket_dev_resume(struct device *dev)
+void pcmcia_socket_dev_early_resume(struct device *dev)
 {
-	struct pcmcia_socket *socket;
+	pcmcia_socket_dev_run(dev, socket_early_resume);
+}
+EXPORT_SYMBOL(pcmcia_socket_dev_early_resume);
 
-	down_read(&pcmcia_socket_list_rwsem);
-	list_for_each_entry(socket, &pcmcia_socket_list, socket_list) {
-		if (socket->dev.parent != dev)
-			continue;
-		mutex_lock(&socket->skt_mutex);
-		socket_resume(socket);
-		mutex_unlock(&socket->skt_mutex);
-	}
-	up_read(&pcmcia_socket_list_rwsem);
+void pcmcia_socket_dev_late_resume(struct device *dev)
+{
+	pcmcia_socket_dev_run(dev, socket_late_resume);
+}
+EXPORT_SYMBOL(pcmcia_socket_dev_late_resume);
 
+int pcmcia_socket_dev_resume(struct device *dev)
+{
+	pcmcia_socket_dev_run(dev, socket_resume);
 	return 0;
 }
 EXPORT_SYMBOL(pcmcia_socket_dev_resume);
@@ -546,29 +554,29 @@ static int socket_suspend(struct pcmcia_
 	return 0;
 }
 
-/*
- * Resume a socket.  If a card is present, verify its CIS against
- * our cached copy.  If they are different, the card has been
- * replaced, and we need to tell the drivers.
- */
-static int socket_resume(struct pcmcia_socket *skt)
+static void socket_start_resume(struct pcmcia_socket *skt)
 {
-	int ret;
-
-	if (!(skt->state & SOCKET_SUSPEND))
-		return -EBUSY;
-
 	skt->socket = dead_socket;
 	skt->ops->init(skt);
 	skt->ops->set_socket(skt, &skt->socket);
+	if (skt->state & SOCKET_PRESENT)
+		skt->resume_status = socket_setup(skt, resume_delay);
+}
 
+static int socket_early_resume(struct pcmcia_socket *skt)
+{
+	socket_start_resume(skt);
+	return 0;
+}
+
+static int socket_late_resume(struct pcmcia_socket *skt)
+{
 	if (!(skt->state & SOCKET_PRESENT)) {
 		skt->state &= ~SOCKET_SUSPEND;
 		return socket_insert(skt);
 	}
 
-	ret = socket_setup(skt, resume_delay);
-	if (ret == 0) {
+	if (skt->resume_status == 0) {
 		/*
 		 * FIXME: need a better check here for cardbus cards.
 		 */
@@ -596,6 +604,20 @@ static int socket_resume(struct pcmcia_s
 	return 0;
 }
 
+/*
+ * Resume a socket.  If a card is present, verify its CIS against
+ * our cached copy.  If they are different, the card has been
+ * replaced, and we need to tell the drivers.
+ */
+static int socket_resume(struct pcmcia_socket *skt)
+{
+	if (!(skt->state & SOCKET_SUSPEND))
+		return -EBUSY;
+
+	socket_start_resume(skt);
+	return socket_late_resume(skt);
+}
+
 static void socket_remove(struct pcmcia_socket *skt)
 {
 	dev_printk(KERN_NOTICE, &skt->dev,
Index: linux-2.6/include/pcmcia/ss.h
===================================================================
--- linux-2.6.orig/include/pcmcia/ss.h
+++ linux-2.6/include/pcmcia/ss.h
@@ -262,6 +262,8 @@ struct pcmcia_socket {
 	struct device			dev;
 	/* data internal to the socket driver */
 	void				*driver_data;
+	/* status of the card during resume from a system sleep state */
+	int				resume_status;
 };
 
 
@@ -280,6 +282,8 @@ extern struct pccard_resource_ops pccard
 
 /* socket drivers are expected to use these callbacks in their .drv struct */
 extern int pcmcia_socket_dev_suspend(struct device *dev);
+extern void pcmcia_socket_dev_early_resume(struct device *dev);
+extern void pcmcia_socket_dev_late_resume(struct device *dev);
 extern int pcmcia_socket_dev_resume(struct device *dev);
 
 /* socket drivers use this callback in their IRQ handler */

Re: Help needed, Re: [Bug #14334] pcmcia suspend regression from 2.6.31.1 to 2.6.31.2 - Dell Inspiron 600m

[Dominik Brodowski]

Hey,

just two minor nit-pick which we could handle post-2.6.32:

> +++ linux-2.6/drivers/pcmcia/cs.c
> @@ -98,10 +98,13 @@ EXPORT_SYMBOL(pcmcia_socket_list_rwsem);
>   * These functions check for the appropriate struct pcmcia_soket arrays,
>   * and pass them to the low-level functions pcmcia_{suspend,resume}_socket

... some documentation of the new functions, especially whether other socket
drivers should be updated?

> -static int socket_resume(struct pcmcia_socket *skt)
> +static void socket_start_resume(struct pcmcia_socket *skt)
>  {
> -	int ret;
> -
> -	if (!(skt->state & SOCKET_SUSPEND))
> -		return -EBUSY;
> -
>  	skt->socket = dead_socket;
>  	skt->ops->init(skt);
>  	skt->ops->set_socket(skt, &skt->socket);
> +	if (skt->state & SOCKET_PRESENT)
> +		skt->resume_status = socket_setup(skt, resume_delay);
> +}
>  
> +static int socket_early_resume(struct pcmcia_socket *skt)
> +{
> +	socket_start_resume(skt);
> +	return 0;
> +}

Why do we need to have two functions doing the same? Wouldn't

static int socket_early_resume(...)

suffice, with the only call to socket_start_resume() being replaced with
socket_early_resume()?

Best,
	Dominik

Re: Help needed, Re: [Bug #14334] pcmcia suspend regression from 2.6.31.1 to 2.6.31.2 - Dell Inspiron 600m

[Rafael J. Wysocki]

On Monday 02 November 2009, Dominik Brodowski wrote:
> Hey,
> 
> just two minor nit-pick which we could handle post-2.6.32:
> 
> > +++ linux-2.6/drivers/pcmcia/cs.c
> > @@ -98,10 +98,13 @@ EXPORT_SYMBOL(pcmcia_socket_list_rwsem);
> >   * These functions check for the appropriate struct pcmcia_soket arrays,
> >   * and pass them to the low-level functions pcmcia_{suspend,resume}_socket
> 
> ... some documentation of the new functions, especially whether other socket
> drivers should be updated?

OK, I'll post a separate patch for that for .33.

> > -static int socket_resume(struct pcmcia_socket *skt)
> > +static void socket_start_resume(struct pcmcia_socket *skt)
> >  {
> > -	int ret;
> > -
> > -	if (!(skt->state & SOCKET_SUSPEND))
> > -		return -EBUSY;
> > -
> >  	skt->socket = dead_socket;
> >  	skt->ops->init(skt);
> >  	skt->ops->set_socket(skt, &skt->socket);
> > +	if (skt->state & SOCKET_PRESENT)
> > +		skt->resume_status = socket_setup(skt, resume_delay);
> > +}
> >  
> > +static int socket_early_resume(struct pcmcia_socket *skt)
> > +{
> > +	socket_start_resume(skt);
> > +	return 0;
> > +}
> 
> Why do we need to have two functions doing the same? Wouldn't
> 
> static int socket_early_resume(...)
> 
> suffice, with the only call to socket_start_resume() being replaced with
> socket_early_resume()?

Yes, it would.  I'll do that in the final version of the patch.

Thanks,
Rafael

Re: Help needed, Re: [Bug #14334] pcmcia suspend regression from 2.6.31.1 to 2.6.31.2 - Dell Inspiron 600m

[Linus Torvalds]

On Mon, 2 Nov 2009, Rafael J. Wysocki wrote:
> 
> OK, updated patch is appended.

Looks good to me, feel free to push any time (assuming you've gotten 
testing confirmation from the people who reported it). Thanks,

		Linus

Re: Help needed, Re: [Bug #14334] pcmcia suspend regression from 2.6.31.1 to 2.6.31.2 - Dell Inspiron 600m

[Benjamin Herrenschmidt]

On Mon, 2009-11-02 at 14:39 +0100, Rafael J. Wysocki wrote:
> 
> > That socket _is_ going to be suspended, and testing for it here just seems 
> > to confuse things. 
> > 
> > So I'd remove it from both early_resume and late_resume, and only keep it 
> > in the case of the legacy user-requested suspend/resume (do we even do 
> > that any more?).
> 
> OK, updated patch is appended.

Test by me delayed to tomorrow ... hit another suspend/resume bug on
that laptop which took away the time I had to do that test yesterday and
today I'm off :-)

(Bug was simple but took a while to track down: machine was left in
storage for a while, battery ran out, RTC went back to Jan 1, 1904,
which means a negative xtime, and the new timekeeping code will do
horrible things including hanging at resume when that happens. Fix is to
make powerpc read_persistent_clock() to ignore the RTC when it contains
a date older than epoch).

Cheers,
Ben.

Re: Help needed, Re: [Bug #14334] pcmcia suspend regression from 2.6.31.1 to 2.6.31.2 - Dell Inspiron 600m

[Pavel Machek]

Hi!

> (Bug was simple but took a while to track down: machine was left in
> storage for a while, battery ran out, RTC went back to Jan 1, 1904,
> which means a negative xtime, and the new timekeeping code will do
> horrible things including hanging at resume when that happens. Fix is to
> make powerpc read_persistent_clock() to ignore the RTC when it contains
> a date older than epoch).

Additionaly, it would be nice if the machine would not hang, no matter
what RTC says...
									Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html