Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755149Ab0LIPoh (ORCPT ); Thu, 9 Dec 2010 10:44:37 -0500 Received: from cantor.suse.de ([195.135.220.2]:58888 "EHLO mx1.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752166Ab0LIPog (ORCPT ); Thu, 9 Dec 2010 10:44:36 -0500 Date: Thu, 09 Dec 2010 16:44:35 +0100 Message-ID: From: Takashi Iwai To: Maxim Levitsky Cc: linux-kernel , Alex Dubov , Andrew Morton , Aries Lee Subject: Re: [PATCH 1/2] memstick: add support for legacy memorysticks In-Reply-To: References: <1291862363.7792.28.camel@maxim-laptop> <1291862546-10064-1-git-send-email-maximlevitsky@gmail.com> User-Agent: Wanderlust/2.15.6 (Almost Unreal) SEMI/1.14.6 (Maruoka) FLIM/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL/10.7 Emacs/23.2 (x86_64-suse-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 6470 Lines: 145 At Thu, 09 Dec 2010 12:56:44 +0100, Takashi Iwai wrote: > > At Thu, 09 Dec 2010 08:19:47 +0100, > Takashi Iwai wrote: > > > > At Thu, 9 Dec 2010 04:42:25 +0200, > > Maxim Levitsky wrote: > > > > > > Based partialy on spec quotes from Alex Dubov. > > > > > > As any code that works with user data this driver isn't recommened to use > > > with valuable data. > > > > > > It tries its best though to avoid data corruption and possible damage to > > > the card. > > > > > > Tested with MS DUO 64 MB card on Ricoh and Jmicron reader. > > > > > > Signed-off-by: Maxim Levitsky > > > > Oh, this is interesting. Just now I looked for a solution for the old > > MS with new JMicron controllers. (I should have checked mm tree > > beforehand...) > > > > I'll give it a try and report back later. > > I tried your patch now. > But got a WARNING and errors without working properly: > > [ 86.894497] WARNING: at drivers/memstick/core/ms_block.c:255 msb_run_state_machine+0x90/0xb0 [ms_block]() > [ 86.894502] Hardware name: > [ 86.894504] Modules linked in: ms_block(+) nls_iso8859_1 nls_cp437 vfat fat mmc_block netconsole configfs nfs lockd nfs_acl auth_rpcgss sunrpc af_packet i915 drm_kms_helper drm i2c_algo_bit snd_pcm_oss cpufreq_conservative snd_mixer_oss cpufreq_userspace snd_seq cpufreq_powersave acpi_cpufreq mperf binfmt_misc snd_seq_device edd ipv6 fuse loop dm_mod acpiphp pci_hotplug snd_hda_codec_hdmi snd_hda_codec_idt processor rtc_cmos snd_hda_intel snd_hda_codec snd_hwdep snd_pcm snd_timer uvcvideo video thermal thermal_sys snd rtc_core sr_mod soundcore snd_page_alloc sdhci_pci hwmon videodev cdrom r8169 rtc_lib v4l1_compat tpm_tis tpm intel_agp tpm_bios pcspkr hp_accel intel_gtt iTCO_wdt jmb38x_ms memstick sdhci mmc_core mii iTCO_vendor_support sg joydev lis3lv02d serio_raw i2c_core input_polldev output wmi container button battery ac ata_piix sd_mod crc_t10dif ehci_hcd ahci libahci libata usbcore scsi_mod ext3 jbd mbcache > [ 86.895023] Pid: 4606, comm: modprobe Not tainted 2.6.37-rc5-test #2 > [ 86.895037] Call Trace: > [ 86.895048] [] ? msb_run_state_machine+0x90/0xb0 [ms_block] > [ 86.895062] [] ? msb_run_state_machine+0x90/0xb0 [ms_block] > [ 86.895080] [] warn_slowpath_common+0x6c/0xa0 > [ 86.895092] [] ? msb_run_state_machine+0x90/0xb0 [ms_block] > [ 86.895105] [] ? h_msb_reset+0x0/0x90 [ms_block] > [ 86.895118] [] warn_slowpath_null+0x1d/0x20 > [ 86.895132] [] msb_run_state_machine+0x90/0xb0 [ms_block] > [ 86.895151] [] msb_reset+0xac/0x130 [ms_block] > [ 86.895169] [] msb_probe+0x6b/0xcc8 [ms_block] > [ 86.895175] [] ? sysfs_addrm_finish+0x15/0xc0 > [ 86.895187] [] ? sysfs_do_create_link+0xb5/0x1b0 > [ 86.895191] [] ? sysfs_addrm_finish+0x15/0xc0 > [ 86.895212] [] memstick_device_probe+0x34/0x50 [memstick] > [ 86.895228] [] driver_probe_device+0x95/0x280 > [ 86.895233] [] ? kobject_add_internal+0xca/0x210 > [ 86.895249] [] ? kvasprintf+0x43/0x60 > [ 86.895252] [] __driver_attach+0x79/0x80 > [ 86.895256] [] bus_for_each_dev+0x53/0x80 > [ 86.895274] [] ? memstick_device_remove+0x0/0x50 [memstick] > [ 86.895278] [] driver_attach+0x19/0x20 > [ 86.895290] [] ? __driver_attach+0x0/0x80 > [ 86.895292] [] bus_add_driver+0x197/0x280 > [ 86.895297] [] ? memstick_device_remove+0x0/0x50 [memstick] > [ 86.895308] [] driver_register+0x75/0x160 > [ 86.895313] [] ? register_blkdev+0xf8/0x130 > [ 86.895326] [] ? msb_init+0x0/0x6d [ms_block] > [ 86.895332] [] memstick_register_driver+0x12/0x20 [memstick] > [ 86.895337] [] msb_init+0x35/0x6d [ms_block] > [ 86.895351] [] do_one_initcall+0x2b/0x160 > [ 86.895365] [] sys_init_module+0x93/0x1d0 > [ 86.895377] [] sysenter_do_call+0x12/0x28 > [ 86.895381] ---[ end trace 0f1b7248462dd657 ]--- This turned to be an uninitialized state field. Trivial to fix. > [ 86.895593] ms_block: Switch to parallel failed > [ 86.908121] ms_block: Switch to parallel failed > > And "switch to parallel failed" continues endlessly until you unplug. > Also, at another time, the machine got frozen with plugging. This was because I'm using the new JMicron chip which doesn't like the parallel mode. Reverting your commented-out code for checking CAP_PAR4 bit makes it working. I attach the patch below. Maybe it's safer to "parallel-mode-as-default" only for the old JMicron chip using some quirk bits, etc. thanks, Takashi --- drivers/memstick/core/ms_block.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) --- a/drivers/memstick/core/ms_block.c +++ b/drivers/memstick/core/ms_block.c @@ -726,7 +726,7 @@ static int msb_reset(struct msb_data *msb, bool full) { - /*bool was_parallel = msb->regs.param.system & MEMSTICK_SYS_PAM; */ + bool was_parallel = msb->regs.param.system & MEMSTICK_SYS_PAM; struct memstick_dev *card = msb->card; struct memstick_host *host = card->host; int error; @@ -762,8 +762,8 @@ } /* Set parallel mode */ - /* if (was_parallel) */ - msb_switch_to_parallel(msb); + if (was_parallel) + msb_switch_to_parallel(msb); return 0; } @@ -1673,7 +1673,7 @@ static int msb_init_card(struct memstick_dev *card) { struct msb_data *msb = memstick_get_drvdata(card); - /*struct memstick_host *host = card->host;*/ + struct memstick_host *host = card->host; struct ms_boot_page *boot_block; int error = 0, i, raw_size_in_megs; @@ -1740,7 +1740,7 @@ /* Due to a bug in Jmicron driver, its serial mode barely works so we switch to parallel mode right away */ -#if 0 +#if 1 /* Now we can switch the interface */ if (host->caps & msb->caps & MEMSTICK_CAP_PAR4) msb_switch_to_parallel(msb); @@ -2184,6 +2184,7 @@ msb = kzalloc(sizeof(struct msb_data), GFP_KERNEL); if (!msb) return -ENOMEM; + msb->state = -1; memstick_set_drvdata(card, msb); msb->card = card; spin_lock_init(&msb->q_lock); -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/