Re: [PATCH] sysfs: add filter function to groups
On Mon, 29 Oct 2007 12:24:06 -0500,
James Bottomley [EMAIL PROTECTED] wrote:
Can you determine which subset of the attributes you want just before
actually creating the group? Then you could do something like:
create_group(grp, kobj)
{
grp-update_creation_mask(kobj);
actually_create_attrs();
}
That's actually what we currently do (at least in hand coded form) in
the current transport classes. However, it leads to one separate group
for each attached class. With the filter approach, we only need one
constructed group for every transport class.
I meant doing it in the core. You still have one group for all cases,
but immediately before creating the attributes, the core checks back
which ones it should create. (Of course, that doesn't solve your
problems if you dynamically want to change availability of attributes
later on. You would need a different mechanism for that.)
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] sysfs: add filter function to groups
On Mon, 29 Oct 2007 12:29:27 -0500, James Bottomley [EMAIL PROTECTED] wrote: On Mon, 2007-10-29 at 13:27 -0400, Jeff Garzik wrote: James Bottomley wrote: visibility and creation are the same thing, aren't they? An invisible attribute doesn't appear in the sysfs directory, so it's equivalent to the file for it not being created. What about the case where it's visible at creation time, but then needs to be made selectively invisible later on? That implies either a remove operation or dentry checks on each lookup? Yes, that comes with the bitmap manipulation code. There will be a way to add and remove runtime visibility. I just wanted to get the basic concept agreed to first. But visibility always comes with creation or deletion of attributes? Perhaps what we want is to move knowledge of the attributes to the kobject (when attributes are created/deleted), while we decide on visibilty of the attributes (creation/deletion of sysfs entries) based on a filter (where visibility may change, while the attribute still exists). - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] drivers/scsi/dpt_i2o: Convert to generic boolean
Convert to use the generic boolean.
Signed-off-by: Richard Knutsson [EMAIL PROTECTED]
---
Diffed against linus-git
Checked with script/checkpatch.pl
(warned about long lines, but it is not introduced by this patch)
dpt_i2o.c | 22 +++---
dpti.h|9 ++---
2 files changed, 13 insertions(+), 18 deletions(-)
diff --git a/drivers/scsi/dpt_i2o.c b/drivers/scsi/dpt_i2o.c
index 8258506..bd3a82d 100644
--- a/drivers/scsi/dpt_i2o.c
+++ b/drivers/scsi/dpt_i2o.c
@@ -252,7 +252,7 @@ rebuild_sys_tab:
adpt_i2o_delete_hba(pHba);
continue;
}
- pHba-initialized = TRUE;
+ pHba-initialized = true;
pHba-state = ~DPTI_STATE_RESET;
scsi_scan_host(pHba-host);
}
@@ -519,7 +519,7 @@ static int adpt_proc_info(struct Scsi_Host *host, char
*buffer, char **start, of
int unit;
*start = buffer;
- if (inout == TRUE) {
+ if (inout) {
/*
* The user has done a write and wants us to take the
* data in the buffer and do something with it.
@@ -893,7 +893,7 @@ static int adpt_install_hba(struct pci_dev* pDev)
void __iomem *base_addr_virt = NULL;
void __iomem *msg_addr_virt = NULL;
- int raptorFlag = FALSE;
+ bool raptorFlag = false;
if(pci_enable_device(pDev)) {
return -EINVAL;
@@ -926,7 +926,7 @@ static int adpt_install_hba(struct pci_dev* pDev)
// Use BAR1 in this configuration
base_addr1_phys = pci_resource_start(pDev,1);
hba_map1_area_size = pci_resource_len(pDev,1);
- raptorFlag = TRUE;
+ raptorFlag = true;
}
base_addr_virt = ioremap(base_addr0_phys,hba_map0_area_size);
@@ -936,7 +936,7 @@ static int adpt_install_hba(struct pci_dev* pDev)
return -EINVAL;
}
-if(raptorFlag == TRUE) {
+ if (raptorFlag) {
msg_addr_virt = ioremap(base_addr1_phys, hba_map1_area_size );
if (!msg_addr_virt) {
PERROR(dpti: adpt_config_hba: io remap failed on
BAR1\n);
@@ -996,7 +996,7 @@ static int adpt_install_hba(struct pci_dev* pDev)
spin_lock_init(pHba-state_lock);
spin_lock_init(adpt_post_wait_lock);
- if(raptorFlag == 0){
+ if (!raptorFlag) {
printk(KERN_INFOAdaptec I2O RAID controller %d at %p size=%x
irq=%d\n,
hba_count-1, base_addr_virt, hba_map0_area_size,
pDev-irq);
} else {
@@ -1273,7 +1273,7 @@ static s32 adpt_i2o_reset_hba(adpt_hba* pHba)
u32 m = EMPTY_QUEUE ;
ulong timeout = jiffies + (TMOUT_IOPRESET*HZ);
- if(pHba-initialized == FALSE) { // First time reset should be
quick
+ if (!pHba-initialized) { /* First time reset should be quick */
timeout = jiffies + (25*HZ);
} else {
adpt_i2o_quiesce_hba(pHba);
@@ -1576,7 +1576,7 @@ static int adpt_open(struct inode *inode, struct file
*file)
// return -EBUSY;
// }
- pHba-in_use = 1;
+ pHba-in_use = true;
mutex_unlock(adpt_configuration_lock);
return 0;
@@ -1602,7 +1602,7 @@ static int adpt_close(struct inode *inode, struct file
*file)
return -ENXIO;
}
- pHba-in_use = 0;
+ pHba-in_use = false;
return 0;
}
@@ -2433,8 +2433,8 @@ static s32 adpt_i2o_reparse_lct(adpt_hba* pHba)
pDev-tid = tid;
memcpy(d-lct_data,
lct-lct_entry[i], sizeof(i2o_lct_entry));
if (pDev-pScsi_dev) {
-
pDev-pScsi_dev-changed = TRUE;
-
pDev-pScsi_dev-removable = TRUE;
+
pDev-pScsi_dev-changed = true;
+
pDev-pScsi_dev-removable = true;
}
}
// Found it - mark it scanned
diff --git a/drivers/scsi/dpti.h b/drivers/scsi/dpti.h
index 0892f6c..5eb7274 100644
--- a/drivers/scsi/dpti.h
+++ b/drivers/scsi/dpti.h
@@ -155,11 +155,6 @@ static int adpt_device_reset(struct scsi_cmnd* cmd);
#define I2O_SCSI_DSC_QUEUE_FROZEN 0x4000
-#ifndef TRUE
-#define TRUE 1
-#define FALSE 0
-#endif
-
#define HBA_FLAGS_INSTALLED_B 0x0001 // Adapter Was Installed
#define HBA_FLAGS_BLINKLED_B0x0002 // Adapter In Blink LED State
#define HBA_FLAGS_IN_RESET 0x0040 /* in reset */
@@ -209,8 +204,8 @@ typedef struct _adpt_hba {
spinlock_t state_lock;
[EMAIL PROTECTED]: [EMAIL PROTECTED]:
Hello James Bottomley, This driver has nothing changed since last time submit. However recently we check the 2.6.23.1 kernel and found code is not up-to-date (still remain @2005). Resend this message to submit the same driver again. (.c diff file under atp870u.c.diff.bz2 format) Please help Acard to build in this modified SCSI driver with latest Linux kernel. If any code still volient the documentation, please let us know the reasons and file path. Thanks! James Hsu - Original Message - From: jameshsu To: [EMAIL PROTECTED] Cc: 'David Miller' ; James Bottomley Sent: Thursday, March 29, 2007 3:36 PM Subject: Re: FW: [EMAIL PROTECTED]: [EMAIL PROTECTED]: - - Completed To whom can take care of this driver issues: We, Acard, modified this driver again based on James Bottomley's input 3/16/2007. In the mean time, I attach the source code with another email for James Bottomley. Those changes are: 1.Modify coding style. 2.Add old copy right. 3.CompString, ClearString, CopyString routine replaced with memcmp, memset and memcpy. 4.Remove adapters array form 2.6 kernel. 5.Removed the scsi_data_direction() hack macro.Just pass the direction directly into the functions 6.Use defined values instead of bare numbers (REQUEST_SENSE instead of 0x03) 7.Replace page_address() to get arbitrary userspace memory with kmap_atomic()/kunmap_atomic(). 8.Use SAM_STAT_CHECK_CONDITION/GOOD instead of separate #define 9.Removed home defined READ_6, WRITE_6 etc. 10.Return SCSI_MLQUEUE_HOST_BUSY on out of resources in queuecommand. Please let me know if this submission is proper. Again, relly appreciate for your help! James Hsu, Manager,QA Testing Dept. ACARD TECHNOLOGY CORP. Taipei Hsien,Taiwan 6F No78,Sec 1,Kwang Fu Road,Sang Chung, (02)8512-2290 x 2311(O),(02)8512-2548(FAX) 0968-989-555 (mobile) atp870u.c.diff.bz2 Description: Binary data atp870u.h.diff Description: Binary data
Re: Questions about scsi.c
Randy Dunlap wrote: On Thu, 25 Oct 2007 16:40:35 -0500 Rob Landley wrote: On Thursday 25 October 2007 12:32:41 pm Randy Dunlap wrote: Entirely possible I'm doing something wrong: sect1 id=scsi_device.h titleinclude/scsi/scsi_device.h/title para /para !Einclude/scsi/scsi_device.h /sect1 !E is for exported symbols and that file has none. USe !I instead. So how do I handle a case like drivers/ata/libata-core.c which has EXPORT_SYMBOL() calls for functions that live in (and are documented in) other files, such as ata_scsi_ioctl() in drivers/ata/libata-scsi.c? I don't see ata_scsi_ioctl() documented at all. Are you looking at a newer tree than I am? (i'm using 2.6.24-rc1) Long-term answer is that we prefer EXPORT_SYMBOL() to be used just under the function that is being exported. In this case, the maintainer may be disagreeing with that. [cc-ed] Short-term answer is to use !Isource_filename_where_kernel_doc_is as though it's not EXPORTed. I think. Yeah I tended to prefer that all exports be in one place, rather than scattered around and difficult to evaluate en masse :) Jeff - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [GIT PATCH] final SCSI pieces for the merge window
On Wednesday 24 October 2007 15:27, Matthew Wilcox wrote: On Wed, Oct 24, 2007 at 09:28:10AM -0400, James Bottomley wrote: OK, so it's no secret that I'm the last of the subsystem maintainers whose day job isn't working on the linux kernel. If you want a full time person, who did you have in mind? I'm willing to take on the role of scsi git-monkey. Alternatively, we could split the scsi maintainer role the same way that Dave and Jeff do for net where Dave handles the core and Jeff handles the drivers. Or we can negotiate some other arrangement. That would be great. Maybe my aic7xxx debloating patches which were submitted four times already (IIRC) will be looked at at last. -- vda - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC 0/3] [SCSI/libata] libata EH conversion for ipr SAS
Brian King wrote: The following three patches convert ipr to use the new libata EH APIs. In the process of doing this, I first looked into implementing this in a similar manner to how libata SAS is done today, which is hooking into target_alloc/target_destroy to allocate/delete sata ports. While I was able to get this working after writing my own eh_strategy_handler, I was doubtful this was the best long term solution. One problem with this implementation I didn't solve was the fact that libata now invokes EH for each and every error received. For some devices, such as optical devices, each not ready response received during a media reload would result in all the attached SAS devices getting quiesced as well. My second approach is the attached patch set. In this series I have created a new libata API which can be used by SAS LLDDs. It introduces an ata_sas_rphy device object which is created/destroyed by the following API: ata_sas_rphy_alloc ata_sas_rphy_add ata_sas_rphy_delete When using this API in ipr, I made ipr's scsi_host the parent device of the SATA rphy. The SATA rphy is then the parent of the allocated scsi_hosts. This means that each SATA rphy in the SAS topology will have its own scsi_host, making SAS *much* more like all the SATA LLDDs in how it uses libata. The only issue I ran into with this implementation is that since each SATA port has its own scsi_host, the adapter cannot rely on scsi core to manage any LLDD or adapter imposed queue depth. To solve this I added some code to ipr. Longer term, block layer queue groups might be another way to do this. I'm still polishing this up, but it is up and running and seems to work with what testing I've done so far. Like I said on IRC... thanks for taking care of this! After this we are down to three old-EH users: libsas, sata_qstor (patch exists, waiting on testing) and sata_sx4 (patch exists, waiting on testing). I'll take a good look in the next day or so. Overall the coupling between SAS (ipr and libsas) and libata definitely needs some thought. Jeff - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Questions about scsi.c
On Tue, 2007-10-30 at 08:34 -0400, Jeff Garzik wrote: Randy Dunlap wrote: On Thu, 25 Oct 2007 16:40:35 -0500 Rob Landley wrote: On Thursday 25 October 2007 12:32:41 pm Randy Dunlap wrote: Entirely possible I'm doing something wrong: sect1 id=scsi_device.h titleinclude/scsi/scsi_device.h/title para /para !Einclude/scsi/scsi_device.h /sect1 !E is for exported symbols and that file has none. USe !I instead. So how do I handle a case like drivers/ata/libata-core.c which has EXPORT_SYMBOL() calls for functions that live in (and are documented in) other files, such as ata_scsi_ioctl() in drivers/ata/libata-scsi.c? I don't see ata_scsi_ioctl() documented at all. Are you looking at a newer tree than I am? (i'm using 2.6.24-rc1) Long-term answer is that we prefer EXPORT_SYMBOL() to be used just under the function that is being exported. In this case, the maintainer may be disagreeing with that. [cc-ed] Short-term answer is to use !Isource_filename_where_kernel_doc_is as though it's not EXPORTed. I think. Yeah I tended to prefer that all exports be in one place, rather than scattered around and difficult to evaluate en masse :) My personal preference (and how I code) is export at the bottom of the function. However, it's one of those stylistic things that I'm happy to have people code however they want (either everything at the bottom of the file or all exports at the bottom of the exported function) as long as they follow the current style of whatever file they're patching. James - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] drivers/scsi/dpt_i2o: Convert to generic boolean
ACK
Sincerely -- Mark Salyzyn
-Original Message-
From: [EMAIL PROTECTED]
[mailto:[EMAIL PROTECTED] On Behalf Of
Richard Knutsson
Sent: Tuesday, October 30, 2007 6:54 AM
To: [EMAIL PROTECTED]
Cc: [EMAIL PROTECTED]; linux-scsi@vger.kernel.org;
Richard Knutsson
Subject: [PATCH] drivers/scsi/dpt_i2o: Convert to generic boolean
Convert to use the generic boolean.
Signed-off-by: Richard Knutsson [EMAIL PROTECTED]
---
Diffed against linus-git
Checked with script/checkpatch.pl
(warned about long lines, but it is not introduced by this patch)
dpt_i2o.c | 22 +++---
dpti.h|9 ++---
2 files changed, 13 insertions(+), 18 deletions(-)
diff --git a/drivers/scsi/dpt_i2o.c b/drivers/scsi/dpt_i2o.c
index 8258506..bd3a82d 100644
--- a/drivers/scsi/dpt_i2o.c
+++ b/drivers/scsi/dpt_i2o.c
@@ -252,7 +252,7 @@ rebuild_sys_tab:
adpt_i2o_delete_hba(pHba);
continue;
}
- pHba-initialized = TRUE;
+ pHba-initialized = true;
pHba-state = ~DPTI_STATE_RESET;
scsi_scan_host(pHba-host);
}
@@ -519,7 +519,7 @@ static int adpt_proc_info(struct
Scsi_Host *host, char *buffer, char **start, of
int unit;
*start = buffer;
- if (inout == TRUE) {
+ if (inout) {
/*
* The user has done a write and wants us to take the
* data in the buffer and do something with it.
@@ -893,7 +893,7 @@ static int adpt_install_hba(struct pci_dev* pDev)
void __iomem *base_addr_virt = NULL;
void __iomem *msg_addr_virt = NULL;
- int raptorFlag = FALSE;
+ bool raptorFlag = false;
if(pci_enable_device(pDev)) {
return -EINVAL;
@@ -926,7 +926,7 @@ static int adpt_install_hba(struct pci_dev* pDev)
// Use BAR1 in this configuration
base_addr1_phys = pci_resource_start(pDev,1);
hba_map1_area_size = pci_resource_len(pDev,1);
- raptorFlag = TRUE;
+ raptorFlag = true;
}
base_addr_virt = ioremap(base_addr0_phys,hba_map0_area_size);
@@ -936,7 +936,7 @@ static int adpt_install_hba(struct pci_dev* pDev)
return -EINVAL;
}
-if(raptorFlag == TRUE) {
+ if (raptorFlag) {
msg_addr_virt = ioremap(base_addr1_phys,
hba_map1_area_size );
if (!msg_addr_virt) {
PERROR(dpti: adpt_config_hba: io remap
failed on BAR1\n);
@@ -996,7 +996,7 @@ static int adpt_install_hba(struct pci_dev* pDev)
spin_lock_init(pHba-state_lock);
spin_lock_init(adpt_post_wait_lock);
- if(raptorFlag == 0){
+ if (!raptorFlag) {
printk(KERN_INFOAdaptec I2O RAID controller %d
at %p size=%x irq=%d\n,
hba_count-1, base_addr_virt,
hba_map0_area_size, pDev-irq);
} else {
@@ -1273,7 +1273,7 @@ static s32 adpt_i2o_reset_hba(adpt_hba* pHba)
u32 m = EMPTY_QUEUE ;
ulong timeout = jiffies + (TMOUT_IOPRESET*HZ);
- if(pHba-initialized == FALSE) { // First time
reset should be quick
+ if (!pHba-initialized) { /* First time reset
should be quick */
timeout = jiffies + (25*HZ);
} else {
adpt_i2o_quiesce_hba(pHba);
@@ -1576,7 +1576,7 @@ static int adpt_open(struct inode
*inode, struct file *file)
// return -EBUSY;
// }
- pHba-in_use = 1;
+ pHba-in_use = true;
mutex_unlock(adpt_configuration_lock);
return 0;
@@ -1602,7 +1602,7 @@ static int adpt_close(struct inode
*inode, struct file *file)
return -ENXIO;
}
- pHba-in_use = 0;
+ pHba-in_use = false;
return 0;
}
@@ -2433,8 +2433,8 @@ static s32 adpt_i2o_reparse_lct(adpt_hba* pHba)
pDev-tid = tid;
memcpy(d-lct_data, lct-lct_entry[i], sizeof(i2o_lct_entry));
if (pDev-pScsi_dev) {
-
pDev-pScsi_dev-changed = TRUE;
-
pDev-pScsi_dev-removable = TRUE;
+
pDev-pScsi_dev-changed = true;
+
pDev-pScsi_dev-removable = true;
}
}
// Found it - mark it scanned
diff --git a/drivers/scsi/dpti.h b/drivers/scsi/dpti.h
index 0892f6c..5eb7274 100644
--- a/drivers/scsi/dpti.h
+++ b/drivers/scsi/dpti.h
@@ -155,11 +155,6 @@ static int adpt_device_reset(struct
scsi_cmnd* cmd);
#define I2O_SCSI_DSC_QUEUE_FROZEN 0x4000
-#ifndef TRUE
-#define TRUE
Re: [PATCH] drivers/scsi/dpt_i2o: Convert to generic boolean
On Tue, Oct 30, 2007 at 11:54:22AM +0100, Richard Knutsson wrote: Convert to use the generic boolean. - u8 initialized; - u8 in_use; /* is the management node open*/ + bool initialized:8; + bool in_use:8; /* is the management node open*/ Are you serious? -- Intel are signing my paycheques ... these opinions are still mine Bill, look, we understand that you're interested in selling us this operating system, but compare it to ours. We can't possibly take such a retrograde step. - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/5] qla2xxx: add target mode support
FUJITA Tomonori wrote: On Sun, 30 Sep 2007 03:57:07 -0700 Seokmann Ju [EMAIL PROTECTED] wrote: FUJITA Tomonori wrote: On Thu, 27 Sep 2007 07:34:52 -0700 Seokmann Ju [EMAIL PROTECTED] wrote: FUJITA Tomonori wrote: On Fri, 21 Sep 2007 07:34:18 -0700 Seokmann Ju [EMAIL PROTECTED] wrote: Andrew Vasquez wrote: On Sat, 01 Sep 2007, FUJITA Tomonori wrote: This adds target mode support to qla2xxx. With set ql2enable_target_mode module parameter to 1, the driver runs in target mode. By default, ql2enable_target_mode is set to 0, and the driver should work in initiator mode as before. The driver could support dual-mode in the future but it doesn't at the moment (we need to add dual-mode support tgt first). It is based on scst qla2xxx target mode driver. Mike converted the driver to use tgt long ago. I changed it to use the latest (mainline) version of qla2xxx driver and tgt, and also converted it to use fc transport class. Thanks for doing this. Some initial comments before a full review is complete, As was seen from the initiator updates needed for 24xx support, there are comparable changes needed in the area of target-mode support for 4gb and 8gb parts. Also, which ISPs and firmwares were exercised with this code? The patch is still under reviewing and will get done, soon. Great, thinks! One more question on typical testing setup. I wonder how should I setup the testing environment esp., for the target-mode. Sorry, I should have explained it with the patch. Probabaly, you need to compile scsi-misc with the qla2xxx target patch and the user-space target code. Thank you, I will make it. 1. scsi-misc + the qla2xxx target patch can you please help me to locate 'the qla2xxx target patch'? Just for the future references, should I expect same effect from following repository? git://git.kernel.org/pub/scm/linux/kernel/git/tomo/linux-2.6-target.git Yup, but note that sometimes the tree isn't based on the latest scsi-misc. Thank you, Seokmann - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] drivers/scsi/dpt_i2o: Convert to generic boolean
On Tue, Oct 30, 2007 at 04:02:25PM +0100, Richard Knutsson wrote: Matthew Wilcox wrote: On Tue, Oct 30, 2007 at 11:54:22AM +0100, Richard Knutsson wrote: Convert to use the generic boolean. - u8 initialized; - u8 in_use; /* is the management node open*/ + bool initialized:8; + bool in_use:8; /* is the management node open*/ Are you serious? Well, yes. It is since it was defined to really be 8 bits before, and there is no reason why a boolean would be 8 bits and not 1 or 16. If it is overly cautious/not needed, then I don't mind removing the ':8'... What's wrong with leaving it as 'u8'? -- Intel are signing my paycheques ... these opinions are still mine Bill, look, we understand that you're interested in selling us this operating system, but compare it to ours. We can't possibly take such a retrograde step. - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] drivers/scsi/dpt_i2o: Convert to generic boolean
Matthew Wilcox wrote: On Tue, Oct 30, 2007 at 11:54:22AM +0100, Richard Knutsson wrote: Convert to use the generic boolean. - u8 initialized; - u8 in_use; /* is the management node open*/ + bool initialized:8; + bool in_use:8; /* is the management node open*/ Are you serious? Well, yes. It is since it was defined to really be 8 bits before, and there is no reason why a boolean would be 8 bits and not 1 or 16. If it is overly cautious/not needed, then I don't mind removing the ':8'... Richard Knutsson - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [EMAIL PROTECTED]: [EMAIL PROTECTED]:
On Tue, Oct 30, 2007 at 07:36:05PM +0800, jameshsu wrote: Hello James Bottomley, This driver has nothing changed since last time submit. However recently we check the 2.6.23.1 kernel and found code is not up-to-date (still remain @2005). Resend this message to submit the same driver again. (.c diff file under atp870u.c.diff.bz2 format) Please help Acard to build in this modified SCSI driver with latest Linux kernel. If any code still volient the documentation, please let us know the reasons and file path. Thanks! I tried applying both patches but none of them applies. atp870u.h.diff reverse-applies so we're at the state already after your patch, and atp870u.c.diff doesn't apply at all, partially because the before version you have has dos line endings, while atp870u.c in the kernel doesn't. James Hsu - Original Message - From: jameshsu To: [EMAIL PROTECTED] Cc: 'David Miller' ; James Bottomley Sent: Thursday, March 29, 2007 3:36 PM Subject: Re: FW: [EMAIL PROTECTED]: [EMAIL PROTECTED]: - - Completed To whom can take care of this driver issues: We, Acard, modified this driver again based on James Bottomley's input 3/16/2007. In the mean time, I attach the source code with another email for James Bottomley. Those changes are: 1.Modify coding style. 2.Add old copy right. 3.CompString, ClearString, CopyString routine replaced with memcmp, memset and memcpy. 4.Remove adapters array form 2.6 kernel. 5.Removed the scsi_data_direction() hack macro.Just pass the direction directly into the functions 6.Use defined values instead of bare numbers (REQUEST_SENSE instead of 0x03) 7.Replace page_address() to get arbitrary userspace memory with kmap_atomic()/kunmap_atomic(). 8.Use SAM_STAT_CHECK_CONDITION/GOOD instead of separate #define 9.Removed home defined READ_6, WRITE_6 etc. 10.Return SCSI_MLQUEUE_HOST_BUSY on out of resources in queuecommand. Please let me know if this submission is proper. Again, relly appreciate for your help! James Hsu, Manager,QA Testing Dept. ACARD TECHNOLOGY CORP. Taipei Hsien,Taiwan 6F No78,Sec 1,Kwang Fu Road,Sang Chung, (02)8512-2290 x 2311(O),(02)8512-2548(FAX) 0968-989-555 (mobile) ---end quoted text--- - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] drivers/scsi/lpfc/lpfc_hw.h: Some minor cleanup.
On Tuesday 30 October 2007 10:54, Richard Knutsson wrote:
Signed-off-by: Richard Knutsson [EMAIL PROTECTED]
---
Diffed against linus-git
Checked with script/checkpatch.pl
diff --git a/drivers/scsi/lpfc/lpfc_hw.h b/drivers/scsi/lpfc/lpfc_hw.h
index 451accd..6f56528 100644
--- a/drivers/scsi/lpfc/lpfc_hw.h
+++ b/drivers/scsi/lpfc/lpfc_hw.h
@@ -3158,31 +3158,30 @@ struct lpfc_sli2_slim {
*
* Parameters:
* device : struct pci_dev 's device field
- *
- * return 1 = TRUE
- *0 = FALSE
*/
-static inline int
+static inline bool
lpfc_is_LC_HBA(unsigned short device)
{
- if ((device == PCI_DEVICE_ID_TFLY) ||
- (device == PCI_DEVICE_ID_PFLY) ||
- (device == PCI_DEVICE_ID_LP101) ||
- (device == PCI_DEVICE_ID_BMID) ||
- (device == PCI_DEVICE_ID_BSMB) ||
- (device == PCI_DEVICE_ID_ZMID) ||
- (device == PCI_DEVICE_ID_ZSMB) ||
- (device == PCI_DEVICE_ID_RFLY))
- return 1;
- else
- return 0;
+ switch (device) {
+ case PCI_DEVICE_ID_TFLY:
+ case PCI_DEVICE_ID_PFLY:
+ case PCI_DEVICE_ID_LP101:
+ case PCI_DEVICE_ID_BMID:
+ case PCI_DEVICE_ID_BSMB:
+ case PCI_DEVICE_ID_ZMID:
+ case PCI_DEVICE_ID_ZSMB:
+ case PCI_DEVICE_ID_RFLY:
+ return true;
+ }
+
+ return false;
}
Why is this patch useful? I'd rather do this instead:
-static inline int
+static int
(this function has three callsites, thus de-inlining will
make code smaller)
--
vda
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 1/8] scsi: megaraid_sas - add hibernation support
On Mon, 2007-10-29 at 14:26 -0600, Yang, Bo wrote: James, We submitted the 03.16-rc1 (megaraid_sas) patches about three weeks back. Do you have any feedback or we need to re-submit the patches? Randy Dunlap asked you why the module parameters were invisible ... is there a reason (if not the patch needs to be updated to make them visible and writeable by root). James - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] sysfs: add filter function to groups
James Bottomley wrote: OK, so is this latest revision acceptable to everyone? No complaint from me. (I'm more or less by accident in this thread anyway. Once this feature is available in mainline, I may have use for it in drivers/firewire/ though.) Thanks, -- Stefan Richter -=-=-=== =-=- - http://arcgraph.de/sr/ - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] Wean NCR_Q720 off the dma_declare_coherent_memory interface
By restructuring the ncr53c8xx driver a little, we can provide
the same functionality as the dma_declare_coherent_memory interface
without touching the generic dma paths.
Signed-off-by: Matthew Wilcox [EMAIL PROTECTED]
---
drivers/scsi/NCR_Q720.c | 45 ---
drivers/scsi/ncr53c8xx.c | 113 +-
drivers/scsi/ncr53c8xx.h | 13 +
3 files changed, 124 insertions(+), 47 deletions(-)
diff --git a/drivers/scsi/NCR_Q720.c b/drivers/scsi/NCR_Q720.c
index a8bbdc2..8db20a6 100644
--- a/drivers/scsi/NCR_Q720.c
+++ b/drivers/scsi/NCR_Q720.c
@@ -36,9 +36,10 @@ MODULE_LICENSE(GPL);
#define NCR_Q720_VERSION 0.9
-/* We needs this helper because we have up to four hosts per struct device */
+/* We have up to four hosts per struct device */
struct NCR_Q720_private {
struct device *dev;
+ struct ncr_mem ncr_mem;
void __iomem * mem_base;
__u32 phys_mem_base;
__u32 mem_size;
@@ -105,6 +106,7 @@ NCR_Q720_probe_one(struct NCR_Q720_private *p, int siop,
device.slot.base_v = vaddr;
device.slot.irq = irq;
device.differential = differential ? 2 : 0;
+ device.ncr_mem = p-ncr_mem;
printk(Q720 probe unit %d (siop%d) at 0x%lx, diff = %d, vers = %d\n,
unit, siop,
(unsigned long)paddr, differential, version);
@@ -214,28 +216,21 @@ NCR_Q720_probe(struct device *dev)
(unsigned long)(base_addr + mem_size));
goto out_free;
}
-
- if (dma_declare_coherent_memory(dev, base_addr, base_addr,
- mem_size, DMA_MEMORY_MAP)
- != DMA_MEMORY_MAP) {
- printk(KERN_ERR NCR_Q720: DMA declare memory failed\n);
- goto out_release_region;
- }
- /* The first 1k of the memory buffer is a memory map of the registers
-*/
- mem_base = dma_mark_declared_memory_occupied(dev, base_addr,
- 1024);
- if (IS_ERR(mem_base)) {
- printk(NCR_Q720 failed to reserve memory mapped region\n);
- goto out_release;
- }
+ mem_base = ioremap(base_addr, mem_size);
+ if (!mem_base)
+ goto out_free;
+
+ p-ncr_mem.virt_base = mem_base;
+ p-ncr_mem.device_base = base_addr;
+ ncr_declare_coherent_memory(p-ncr_mem, mem_size);
+
+ /* The first 1k of the memory buffer are the registers */
+ ncr_reserve_declared_memory(p-ncr_mem, 0, 1024);
/* now also enable accesses in asr 2 */
asr2 = inb(io_base + 0x0a);
-
asr2 |= 0x01;
-
outb(asr2, io_base + 0x0a);
/* get the number of SIOPs (this should be 2 or 4) */
@@ -250,7 +245,6 @@ NCR_Q720_probe(struct device *dev)
irq = readb(mem_base + 5) 0x0f;
-
/* now do the bus related transforms */
irq = mca_device_transform_irq(mca_dev, irq);
@@ -297,10 +291,8 @@ NCR_Q720_probe(struct device *dev)
found++;
}
- if (!found) {
- kfree(p);
- return -ENODEV;
- }
+ if (!found)
+ goto out_release;
mca_device_set_claim(mca_dev, 1);
mca_device_set_name(mca_dev, NCR_Q720);
@@ -309,8 +301,8 @@ NCR_Q720_probe(struct device *dev)
return 0;
out_release:
- dma_release_declared_memory(dev);
- out_release_region:
+ ncr_release_declared_memory(p-ncr_mem);
+ iounmap(mem_base);
release_mem_region(base_addr, mem_size);
out_free:
kfree(p);
@@ -335,7 +327,8 @@ NCR_Q720_remove(struct device *dev)
if(p-hosts[i])
NCR_Q720_remove_one(p-hosts[i]);
- dma_release_declared_memory(dev);
+ ncr_release_declared_memory(p-ncr_mem);
+ iounmap(p-ncr_mem.virt_base);
release_mem_region(p-phys_mem_base, p-mem_size);
free_irq(p-irq, p);
kfree(p);
diff --git a/drivers/scsi/ncr53c8xx.c b/drivers/scsi/ncr53c8xx.c
index 016c462..149a42b 100644
--- a/drivers/scsi/ncr53c8xx.c
+++ b/drivers/scsi/ncr53c8xx.c
@@ -128,6 +128,14 @@
#define NAME53C8XX ncr53c8xx
+/* Linux host data structure */
+
+struct host_data {
+ struct ncb *ncb;
+ struct ncr_mem *ncr_mem;
+ struct device *dev;
+};
+
/*==
**
** Debugging tags
@@ -172,6 +180,74 @@ static inline struct list_head *ncr_list_pop(struct
list_head *head)
return NULL;
}
+/*
+ * Support the onboard memory on the NCR Q720
+ *
+ * XXX: This is only coherent because it's only present on x86.
+ */
+
+int ncr_declare_coherent_memory(struct ncr_mem *mem, unsigned size)
+{
+ int pages = size PAGE_SHIFT;
+ int bitmap_size = BITS_TO_LONGS(pages) *
[PATCH 2/2] Remove dma_coherent_mem interface
This interface only had one user (and two implementations).
It complicates other work, so remove it.
Signed-off-by: Matthew Wilcox [EMAIL PROTECTED]
---
Documentation/DMA-API.txt | 78
Documentation/driver-model/devres.txt |1 -
arch/cris/arch-v32/drivers/pci/dma.c | 106 +
arch/x86/kernel/pci-dma_32.c | 108 +
drivers/base/dma-mapping.c| 55 -
include/asm-cris/dma-mapping.h| 14
include/asm-mips/dma-mapping.h| 10 ---
include/asm-x86/dma-mapping_32.h | 12
include/linux/device.h|2 -
include/linux/dma-mapping.h | 39
10 files changed, 2 insertions(+), 423 deletions(-)
diff --git a/Documentation/DMA-API.txt b/Documentation/DMA-API.txt
index b939ebb..9cebf5b 100644
--- a/Documentation/DMA-API.txt
+++ b/Documentation/DMA-API.txt
@@ -469,81 +469,3 @@ Do a partial sync of memory that was allocated by
dma_alloc_noncoherent(), starting at virtual address vaddr and
continuing on for size. Again, you *must* observe the cache line
boundaries when doing this.
-
-int
-dma_declare_coherent_memory(struct device *dev, dma_addr_t bus_addr,
- dma_addr_t device_addr, size_t size, int
- flags)
-
-Declare region of memory to be handed out by dma_alloc_coherent when
-it's asked for coherent memory for this device.
-
-bus_addr is the physical address to which the memory is currently
-assigned in the bus responding region (this will be used by the
-platform to perform the mapping).
-
-device_addr is the physical address the device needs to be programmed
-with actually to address this memory (this will be handed out as the
-dma_addr_t in dma_alloc_coherent()).
-
-size is the size of the area (must be multiples of PAGE_SIZE).
-
-flags can be or'd together and are:
-
-DMA_MEMORY_MAP - request that the memory returned from
-dma_alloc_coherent() be directly writable.
-
-DMA_MEMORY_IO - request that the memory returned from
-dma_alloc_coherent() be addressable using read/write/memcpy_toio etc.
-
-One or both of these flags must be present.
-
-DMA_MEMORY_INCLUDES_CHILDREN - make the declared memory be allocated by
-dma_alloc_coherent of any child devices of this one (for memory residing
-on a bridge).
-
-DMA_MEMORY_EXCLUSIVE - only allocate memory from the declared regions.
-Do not allow dma_alloc_coherent() to fall back to system memory when
-it's out of memory in the declared region.
-
-The return value will be either DMA_MEMORY_MAP or DMA_MEMORY_IO and
-must correspond to a passed in flag (i.e. no returning DMA_MEMORY_IO
-if only DMA_MEMORY_MAP were passed in) for success or zero for
-failure.
-
-Note, for DMA_MEMORY_IO returns, all subsequent memory returned by
-dma_alloc_coherent() may no longer be accessed directly, but instead
-must be accessed using the correct bus functions. If your driver
-isn't prepared to handle this contingency, it should not specify
-DMA_MEMORY_IO in the input flags.
-
-As a simplification for the platforms, only *one* such region of
-memory may be declared per device.
-
-For reasons of efficiency, most platforms choose to track the declared
-region only at the granularity of a page. For smaller allocations,
-you should use the dma_pool() API.
-
-void
-dma_release_declared_memory(struct device *dev)
-
-Remove the memory region previously declared from the system. This
-API performs *no* in-use checking for this region and will return
-unconditionally having removed all the required structures. It is the
-driver's job to ensure that no parts of this memory region are
-currently in use.
-
-void *
-dma_mark_declared_memory_occupied(struct device *dev,
- dma_addr_t device_addr, size_t size)
-
-This is used to occupy specific regions of the declared space
-(dma_alloc_coherent() will hand out the first free region it finds).
-
-device_addr is the *device* address of the region requested.
-
-size is the size (and should be a page-sized multiple).
-
-The return value will be either a pointer to the processor virtual
-address of the memory, or an error (via PTR_ERR()) if any part of the
-region is occupied.
diff --git a/Documentation/driver-model/devres.txt
b/Documentation/driver-model/devres.txt
index 387b8a7..24d74e9 100644
--- a/Documentation/driver-model/devres.txt
+++ b/Documentation/driver-model/devres.txt
@@ -248,7 +248,6 @@ DMA
dmam_free_coherent()
dmam_alloc_noncoherent()
dmam_free_noncoherent()
- dmam_declare_coherent_memory()
dmam_pool_create()
dmam_pool_destroy()
diff --git a/arch/cris/arch-v32/drivers/pci/dma.c
b/arch/cris/arch-v32/drivers/pci/dma.c
index 66f9500..88954fd 100644
--- a/arch/cris/arch-v32/drivers/pci/dma.c
+++ b/arch/cris/arch-v32/drivers/pci/dma.c
@@ -15,36 +15,14 @@
#include linux/pci.h
#include asm/io.h
-struct dma_coherent_mem {
-
Re: [PATCH] sysfs: add filter function to groups
On Tue, 2007-10-30 at 13:25 -0500, James Bottomley wrote:
On Mon, 2007-10-29 at 18:58 +0100, Stefan Richter wrote:
James Bottomley wrote:
struct attribute_group {
const char *name;
+ int (*filter_show)(struct kobject *, int);
Actually, it returns a true/false value indicating whether the given
attribute should be displayed.
How about this:
int (*is_visible)(...);
OK, so is this latest revision acceptable to everyone?
Acked-by: Kay Sievers [EMAIL PROTECTED]
Best,
Kay
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/1] aacraid: forced reset override
Some of our vendors have requested that our adapters ignore the hardware reset attempts during recovery and have enforced this with changes in Adapter Firmware. Some of our customers have requested the option to be able to reset the adapter under adverse adapter failure, we even had a few defects reported here considering it a regression that the Adapter could not be reset. This patch addresses this dichotomy. The user can force the adapter to be reset if it supports the IOP_RESET_ALWAYS command, in cases where the adapter has been programmed to ignore the reset, by setting the aacraid.check_reset parameter to a value of -1. The driver will not reset an Adapter that does not support the reset command(s). This patch also fixes and cleans up some of the logic associated with resetting the adapter. This attached patch is against current scsi-misc-2.6. ObligatoryDisclaimer: Please accept my condolences regarding Outlook's handling of patch attachments. Signed-off-by: Mark Salyzyn [EMAIL PROTECTED] drivers/scsi/aacraid/aachba.c |8 drivers/scsi/aacraid/commsup.c | 13 +++-- drivers/scsi/aacraid/linit.c |7 +-- drivers/scsi/aacraid/rx.c |4 +++- 4 files changed, 19 insertions(+), 13 deletions(-) Sincerely -- Mark Salyzyn aacraid_forced_reset.patch Description: aacraid_forced_reset.patch
Re: [PATCH 2/2] Remove dma_coherent_mem interface
On Tue, 2007-10-30 at 15:43 -0400, Matthew Wilcox wrote: This interface only had one user (and two implementations). It complicates other work, so remove it. Its design was basically to facilitate the use of bus remote memory. There's a long thread somewhere discussing this with the ARM people. They had some type of SoC implementation that needed to allocate local memory for device descriptors. The Q720 is pretty much the same way, so I used it to build the implementation when I created it for the ARM people. This is the original thread: http://marc.info/?t=10875786211 They said they were implementing this system, so I've no idea what happened. However, what are the problems the API is causing? it seems useful, so there should be a preference in its favour of existence unless it's causing a problem. James - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] Remove dma_coherent_mem interface
On Tue, Oct 30, 2007 at 03:26:31PM -0500, James Bottomley wrote: Its design was basically to facilitate the use of bus remote memory. There's a long thread somewhere discussing this with the ARM people. They had some type of SoC implementation that needed to allocate local memory for device descriptors. The Q720 is pretty much the same way, so I used it to build the implementation when I created it for the ARM people. This is the original thread: http://marc.info/?t=10875786211 They said they were implementing this system, so I've no idea what happened. It's been over three years, and it hasn't happened. However, what are the problems the API is causing? it seems useful, so there should be a preference in its favour of existence unless it's causing a problem. What I'm currently looking at is the dmapool allocator. It's not exactly fast (a spinlock for each allocation ... no concept of cpu affinity, etc), and some drivers (eg qla2xxx) use it in the performance path. One of the suggestions in the existing dmapool driver is to share the guts of slab. Well, slab is probably going away, so I took a look at slub. Slub really, *really* needs the struct page associated with the page of memory allocated, so I'm currently working my way through the architectures trying to turn dma_alloc_coherent into dma_alloc_coherent_pages. The dma_coherent_mem API is one of the things which gets in the way of doing this. So I deleted it, then sent the patch out for comments early. -- Intel are signing my paycheques ... these opinions are still mine Bill, look, we understand that you're interested in selling us this operating system, but compare it to ours. We can't possibly take such a retrograde step. - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] drivers/scsi/dpt_i2o: Convert to generic boolean
Matthew Wilcox wrote: On Tue, Oct 30, 2007 at 05:46:08PM +0100, Richard Knutsson wrote: I just don't see the reason why expressing a boolean as an integer. Some advantage? This is C, not Java, or some other highly-typed language. if (int) and if (ptr) are perfectly acceptable in C. Yes, and it has been corrected in the C99-standard (which we are using since the Linux support for the the 2.95 compiler stopped). Is there something wrong in actually typing the variable to the type we want it to be? Or would it be better to regress (becoming like Perl or PHP)[1][2]? Btw, I am all for 'if (ptr)' since it rarely (if ever) makes any sense to do 'if (ptr == 0x1234)', which evaluates to ptr == NULL is invalid and otherwise valid. Is it really the same for integers? ;) (also just want to add: to the this is not a highly-typed language that I have heard many times, it theory, I think it would suffice with just the void-pointer) (also helps us if someone does: 'if (var == true)', even thou we should try to avoid them) I have no idea what you mean. There is some places where things like 'if (var == true)' is done, but what happens when var is not the same number as 'true' (and still != 0)? It is a potential bug if 'var' is an integer and expected to be a boolean in this case. Like in a case of var = some_var some_flag; Best regards Richard Knutsson [1] Nothing wrong with Perl or PHP, they are suited for the tasks they are meant to solve. [2] If I recall correctly, the predecessor 'B' did not have any types. - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Questions about scsi.c
On Tuesday 30 October 2007 9:43:29 am James Bottomley wrote:
On Tue, 2007-10-30 at 08:34 -0400, Jeff Garzik wrote:
Randy Dunlap wrote:
On Thu, 25 Oct 2007 16:40:35 -0500 Rob Landley wrote:
On Thursday 25 October 2007 12:32:41 pm Randy Dunlap wrote:
!E is for exported symbols and that file has none.
USe !I instead.
So how do I handle a case like drivers/ata/libata-core.c which has
EXPORT_SYMBOL() calls for functions that live in (and are documented
in) other files, such as ata_scsi_ioctl() in
drivers/ata/libata-scsi.c?
...
Yeah I tended to prefer that all exports be in one place, rather than
scattered around and difficult to evaluate en masse :)
My personal preference (and how I code) is export at the bottom of the
function. However, it's one of those stylistic things that I'm happy to
have people code however they want (either everything at the bottom of
the file or all exports at the bottom of the exported function) as long
as they follow the current style of whatever file they're patching.
Actually, this one is written up in CodingStyle:
In source files, separate functions with one blank line. If the function
is exported, the EXPORT* macro for it should follow immediately after the
closing function brace line. E.g.:
int system_is_up(void)
{
return system_state == SYSTEM_RUNNING;
}
EXPORT_SYMBOL(system_is_up);
The functional problem is that when the EXPORT_SYMBOL() is in a different file
entirely, the documentation infrastructure doesn't pick up that it's an
exported function.
James
Rob
--
One of my most productive days was throwing away 1000 lines of code.
- Ken Thompson.
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] sysfs: add filter function to groups
Hi James:
* James Bottomley [EMAIL PROTECTED] [2007-10-30 13:25:43 -0500]:
On Mon, 2007-10-29 at 18:58 +0100, Stefan Richter wrote:
James Bottomley wrote:
struct attribute_group {
const char *name;
+ int (*filter_show)(struct kobject *, int);
Actually, it returns a true/false value indicating whether the given
attribute should be displayed.
How about this:
int (*is_visible)(...);
OK, so is this latest revision acceptable to everyone?
I've just been hacking around in this area a bit, for a completely different
reason: there are literally 1000's of attributes in drivers/hwmon/*.c that
really want to be const, but which cannot be due to the current API. I was
about to propose some patches that move in a different direction...
Index: BUILD-2.6/fs/sysfs/group.c
===
--- BUILD-2.6.orig/fs/sysfs/group.c 2007-10-28 17:27:04.0 -0500
+++ BUILD-2.6/fs/sysfs/group.c2007-10-30 12:35:47.0 -0500
@@ -16,25 +16,31 @@
#include sysfs.h
-static void remove_files(struct sysfs_dirent *dir_sd,
+static void remove_files(struct sysfs_dirent *dir_sd, struct kobject *kobj,
const struct attribute_group *grp)
{
struct attribute *const* attr;
+ int i;
- for (attr = grp-attrs; *attr; attr++)
- sysfs_hash_and_remove(dir_sd, (*attr)-name);
+ for (i = 0, attr = grp-attrs; *attr; i++, attr++)
+ if (grp-is_visible
+ grp-is_visible(kobj, *attr, i))
+ sysfs_hash_and_remove(dir_sd, (*attr)-name);
}
-static int create_files(struct sysfs_dirent *dir_sd,
+static int create_files(struct sysfs_dirent *dir_sd, struct kobject *kobj,
const struct attribute_group *grp)
{
struct attribute *const* attr;
- int error = 0;
+ int error = 0, i;
- for (attr = grp-attrs; *attr !error; attr++)
- error = sysfs_add_file(dir_sd, *attr, SYSFS_KOBJ_ATTR);
+ for (i = 0, attr = grp-attrs; *attr !error; i++, attr++)
+ if (grp-is_visible
+ grp-is_visible(kobj, *attr, i))
+ error |=
+ sysfs_add_file(dir_sd, *attr, SYSFS_KOBJ_ATTR);
if (error)
- remove_files(dir_sd, grp);
+ remove_files(dir_sd, kobj, grp);
return error;
}
@@ -54,7 +60,7 @@ int sysfs_create_group(struct kobject *
} else
sd = kobj-sd;
sysfs_get(sd);
- error = create_files(sd, grp);
+ error = create_files(sd, kobj, grp);
if (error) {
if (grp-name)
sysfs_remove_subdir(sd);
@@ -75,7 +81,7 @@ void sysfs_remove_group(struct kobject *
} else
sd = sysfs_get(dir_sd);
- remove_files(sd, grp);
+ remove_files(sd, kobj, grp);
if (grp-name)
sysfs_remove_subdir(sd);
Index: BUILD-2.6/include/linux/sysfs.h
===
--- BUILD-2.6.orig/include/linux/sysfs.h 2007-10-28 17:20:06.0
-0500
+++ BUILD-2.6/include/linux/sysfs.h 2007-10-30 13:24:06.0 -0500
@@ -32,6 +32,8 @@ struct attribute {
struct attribute_group {
const char *name;
+ int (*is_visible)(struct kobject *,
+ struct attribute *, int);
struct attribute**attrs;
};
IMHO the fundamental problem is struct attribute_group itself. This structure
is nothing but a convenience for packaging the arguments to sysfs_create_group
and sysfs_remove_group. Those functions should take the contents of that
struct as direct arguments. I haven't finished the patch series to implement
this, but since I noticed your patch I thought I'd better speak up now. Here's
the first... the idea is to eventually deprecate sysfs_[create|remove]_group()
altogether.
commit 5b5bc08ae31519b7012d7fc23ff73e0c6474de53
Author: Mark M. Hoffman [EMAIL PROTECTED]
Date: Sun Oct 21 11:49:57 2007 -0400
sysfs: allow attribute (group) lists to be const
The current declaration of struct attribute_group prevents long lists of
attributes from being marked const. Ideally, the second argument to the
sysfs_create_group() and sysfs_remove_group() functions would be marked
deep
const, but C has no such construct. This patch provides a parallel set of
functions with the desired decoration.
Signed-off-by: Mark M. Hoffman [EMAIL PROTECTED]
diff --git a/fs/sysfs/group.c b/fs/sysfs/group.c
index d197237..b217a8e 100644
--- a/fs/sysfs/group.c
+++ b/fs/sysfs/group.c
@@ -17,71 +17,84 @@
static void remove_files(struct sysfs_dirent *dir_sd,
-const struct attribute_group *grp)
+const struct
Re: [PATCH] sysfs: add filter function to groups
On Oct 31, 2007 1:40 AM, Mark M. Hoffman [EMAIL PROTECTED] wrote:
* James Bottomley [EMAIL PROTECTED] [2007-10-30 13:25:43 -0500]:
On Mon, 2007-10-29 at 18:58 +0100, Stefan Richter wrote:
James Bottomley wrote:
struct attribute_group {
const char *name;
+ int (*filter_show)(struct kobject *, int);
Actually, it returns a true/false value indicating whether the given
attribute should be displayed.
How about this:
int (*is_visible)(...);
OK, so is this latest revision acceptable to everyone?
I've just been hacking around in this area a bit, for a completely different
reason: there are literally 1000's of attributes in drivers/hwmon/*.c that
really want to be const, but which cannot be due to the current API. I was
about to propose some patches that move in a different direction...
That isn't related to dynamic attributes, right?
IMHO the fundamental problem is struct attribute_group itself. This structure
is nothing but a convenience for packaging the arguments to sysfs_create_group
and sysfs_remove_group.
That problem is actually a good thing. If you look at the change
rate of the internal kernel API, it saves us so much trouble. Like in
this case, James can just add a callback without caring about any
(almost :)) of the current users.
Those functions should take the contents of that
struct as direct arguments.
I think we should move in the opposite direction. You are right, it
isn't neccessarily pretty, but having encapsulations like this saves
us a lot of trouble while interacting with so many other people and
extending API's all the time. It's a trade, and it's a good one, if
you need to maintain code that has so many callers, and so many
architectures, you can't even check that you don't break them.
I haven't finished the patch series to implement
this, but since I noticed your patch I thought I'd better speak up now.
Here's
the first... the idea is to eventually deprecate sysfs_[create|remove]_group()
altogether.
Again, I don't think, that we want to get rid of the struct container
housing all the parameters and beeing open for future extensions
without changing all the callers.
The current declaration of struct attribute_group prevents long lists of
attributes from being marked const. Ideally, the second argument to the
sysfs_create_group() and sysfs_remove_group() functions would be marked
deep
const, but C has no such construct. This patch provides a parallel set of
functions with the desired decoration.
What do we get out of this constification compared to the current code?
Thanks,
Kay
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RESEND] SCSI not showing tray status correctly
On Sat, 2007-10-27 at 22:58 +, Maarten Bressers wrote:
This patch is too simplistic. ide-cd.c:ide_cdrom_drive_status() looks
to be a reasonable implementation. However, the worry is that
GET_EVENT_NOTIFICATION is a MMC command; devices not conforming to MMC
won't support it. In theory, they should just return ILLEGAL_REQUEST,
but USB devices have been known to crash when given commands they don't
understand. How widely tested has this been (if it's been in Gentoo
since 2004, then it's probably widely tested enough)?
James
This patch hasn't been tested at all, I'm sorry if I gave you that
impression, it isn't in Gentoo's patches, it's just been brought to our
attention in the bug I linked too.
I created another patch, based on your recommendations, and with a lot of
help from Daniel Drake ([EMAIL PROTECTED]), that's included below. Some
changes are made to test_unit_ready() to be able to pass the sense data
to sr_drive_status(). Currently, sr_do_ioctl() swallows this data and
makes its own interpretations of sense codes, which isn't what we want
here.
Is this what you had in mind? Do you think the possible problems with
USB drives that you mentioned will prevent this patch from going in?
The patch has only been compile tested right now, we can do some real
testing later, for now I'd just like to get your feedback on it.
In general it looks good ... I suppose the only way to test it is to
stick it in and see what happens. However, there are a few rough edges.
Maarten
---
--- a/drivers/scsi/sr_ioctl.c 2007-10-26 22:40:41.0 +0200
+++ b/drivers/scsi/sr_ioctl.c 2007-10-27 23:56:16.0 +0200
@@ -275,16 +275,21 @@ int sr_do_ioctl(Scsi_CD *cd, struct pack
/* -- */
/* interface to cdrom.c */
-static int test_unit_ready(Scsi_CD *cd)
+static int test_unit_ready(Scsi_CD *cd, struct request_sense *sense)
{
- struct packet_command cgc;
+ struct scsi_device *SDev = cd-device;
+ unsigned char cmd[CDROM_PACKET_SIZE] = { GPCMD_TEST_UNIT_READY };
+ int result;
- memset(cgc, 0, sizeof(struct packet_command));
- cgc.cmd[0] = GPCMD_TEST_UNIT_READY;
- cgc.quiet = 1;
- cgc.data_direction = DMA_NONE;
- cgc.timeout = IOCTL_TIMEOUT;
- return sr_do_ioctl(cd, cgc);
+ if (!scsi_block_when_processing_errors(SDev))
+ return -ENODEV;
+
+ memset(sense, 0, sizeof(*sense));
+ result = scsi_execute(SDev, cmd, DMA_NONE,
+ NULL, 0, (char *)sense,
+ IOCTL_TIMEOUT, IOCTL_RETRIES, 0);
+
+ return driver_byte(result);
This bit isn't quite right ... this will return true if there's a
collected sense code and false otherwise (so it returns 0 on failure
that doesn't produce any sense information).
}
int sr_tray_move(struct cdrom_device_info *cdi, int pos)
@@ -310,14 +315,41 @@ int sr_lock_door(struct cdrom_device_inf
int sr_drive_status(struct cdrom_device_info *cdi, int slot)
{
+ struct request_sense sense;
+ struct media_event_desc med;
+
if (CDSL_CURRENT != slot) {
/* we have no changer support */
return -EINVAL;
}
- if (0 == test_unit_ready(cdi-handle))
+ if ((0 == test_unit_ready(cdi-handle, sense)) ||
+ sense.sense_key == UNIT_ATTENTION)
Unit attention won't necessarily mean the media is OK ... unit attention
can mean the media or tray status has changed.
return CDS_DISC_OK;
- return CDS_TRAY_OPEN;
+ if (!cdrom_get_media_event(cdi, med)) {
+ if (med.media_present)
+ return CDS_DISC_OK;
+ else if (med.door_open)
+ return CDS_TRAY_OPEN;
+ else
+ return CDS_NO_DISC;
+ }
+
+ if (sense.sense_key == NOT_READY sense.asc == 0x04 sense.ascq ==
0x04)
Actually, all asc 0x4 codes are some type of in progress operation,
format doesn't necessarily have to be treated specially.
+ return CDS_DISC_OK;
+
+ /*
+ * If not using Mt Fuji extended media tray reports,
+ * just return TRAY_OPEN since ATAPI doesn't provide
+ * any other way to detect this...
+ */
+ if (sense.sense_key == NOT_READY) {
+ if (sense.asc == 0x3a sense.ascq == 1)
+ return CDS_NO_DISC;
+ else
+ return CDS_TRAY_OPEN;
This is also a bit odd. asc 0x3a ascq 0x2 definitely means tray open,
but there are a lot of other not ready conditions that don't mean this.
+ }
+ return CDS_DRIVE_NOT_READY;
}
int sr_disk_status(struct cdrom_device_info *cdi)
James
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at
Re: [PATCH] sysfs: add filter function to groups
On Tue, Oct 30, 2007 at 01:25:43PM -0500, James Bottomley wrote:
On Mon, 2007-10-29 at 18:58 +0100, Stefan Richter wrote:
James Bottomley wrote:
struct attribute_group {
const char *name;
+ int (*filter_show)(struct kobject *, int);
Actually, it returns a true/false value indicating whether the given
attribute should be displayed.
How about this:
int (*is_visible)(...);
OK, so is this latest revision acceptable to everyone?
James
Index: BUILD-2.6/fs/sysfs/group.c
===
--- BUILD-2.6.orig/fs/sysfs/group.c 2007-10-28 17:27:04.0 -0500
+++ BUILD-2.6/fs/sysfs/group.c2007-10-30 12:35:47.0 -0500
@@ -16,25 +16,31 @@
#include sysfs.h
-static void remove_files(struct sysfs_dirent *dir_sd,
+static void remove_files(struct sysfs_dirent *dir_sd, struct kobject *kobj,
const struct attribute_group *grp)
{
struct attribute *const* attr;
+ int i;
- for (attr = grp-attrs; *attr; attr++)
- sysfs_hash_and_remove(dir_sd, (*attr)-name);
+ for (i = 0, attr = grp-attrs; *attr; i++, attr++)
+ if (grp-is_visible
+ grp-is_visible(kobj, *attr, i))
+ sysfs_hash_and_remove(dir_sd, (*attr)-name);
Hm, doesn't this break for the zillions of attribute groups that do not
have the is_visible function set?
-static int create_files(struct sysfs_dirent *dir_sd,
+static int create_files(struct sysfs_dirent *dir_sd, struct kobject *kobj,
const struct attribute_group *grp)
{
struct attribute *const* attr;
- int error = 0;
+ int error = 0, i;
- for (attr = grp-attrs; *attr !error; attr++)
- error = sysfs_add_file(dir_sd, *attr, SYSFS_KOBJ_ATTR);
+ for (i = 0, attr = grp-attrs; *attr !error; i++, attr++)
+ if (grp-is_visible
+ grp-is_visible(kobj, *attr, i))
+ error |=
+ sysfs_add_file(dir_sd, *attr, SYSFS_KOBJ_ATTR);
Same problem here, if grp-is_visible is not set, sysfs_add_file() would
never be called, right?
Other than the logic problem (I think), I have no issue with this idea
at all. Care to redo this so it works?
thanks,
greg k-h
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] aacraid: don't assign cpu_to_le32(constant) to u8
Noticed on PowerPC allmod config build: drivers/scsi/aacraid/commsup.c:1342: warning: large integer implicitly truncated to unsigned type drivers/scsi/aacraid/commsup.c:1343: warning: large integer implicitly truncated to unsigned type drivers/scsi/aacraid/commsup.c:1344: warning: large integer implicitly truncated to unsigned type Signed-off-by: Stephen Rothwell [EMAIL PROTECTED] --- drivers/scsi/aacraid/commsup.c |6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-) -- Cheers, Stephen Rothwell[EMAIL PROTECTED] diff --git a/drivers/scsi/aacraid/commsup.c b/drivers/scsi/aacraid/commsup.c index 240a0bb..b9682a8 100644 --- a/drivers/scsi/aacraid/commsup.c +++ b/drivers/scsi/aacraid/commsup.c @@ -1339,9 +1339,9 @@ int aac_check_health(struct aac_dev * aac) aif = (struct aac_aifcmd *)hw_fib-data; aif-command = cpu_to_le32(AifCmdEventNotify); aif-seqnum = cpu_to_le32(0x); - aif-data[0] = cpu_to_le32(AifEnExpEvent); - aif-data[1] = cpu_to_le32(AifExeFirmwarePanic); - aif-data[2] = cpu_to_le32(AifHighPriority); + aif-data[0] = AifEnExpEvent; + aif-data[1] = AifExeFirmwarePanic; + aif-data[2] = AifHighPriority; aif-data[3] = cpu_to_le32(BlinkLED); /* -- 1.5.3.4 - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: BUG in: Driver core: convert block from raw kobjects to core devices (fwd)
On Mon, Oct 29, 2007 at 02:47:59PM -0400, Alan Stern wrote: On Mon, 29 Oct 2007, James Bottomley wrote: I'm not sure if we can do this patch. If you kill a device, you see there's a processing state in scsi_prep_fn() for SDEV_DEL, which has a printk message I see quite often when I unplug devices while they're operating. If you NULL out and free the request queue immediately after setting SDEV_DEL, without allowing the devices and filesystems to finish, are you sure we're not going to get a null deref on sdev-request_queue? Let's get at this another way. The patch below probably should be applied in any case. It's essentially a reversion of this patch from 2003: http://git.kernel.org/?p=linux/kernel/git/tglx/history.git;a=commit;h=10921a8f1305b8ec97794941db78b825db5839bc which was written to compensate for problems in the SCSI stack. The idea was to avoid releasing a kobject before all its children were released. However as far as I can see, in general we should be able to release a kobject as soon as all its children are removed and its refcount drops to 0, without waiting for the children to be released. To put it another way, once a child is removed from visibility it should not try (or need) to access its parent at all. If it has to then it should take an explicit reference to the parent. Since the SCSI stack is now in much better shape, there doesn't seem to be any reason to keep the old code. Do you agree that the patch below is worth merging? I submitted it to Greg some time ago, but he didn't want to accept it without some assurance that it is now safe. It would fix the problem Kay and saw with the circular references, because the references would all get dropped when the scsi_device is removed instead of waiting for a release that will never come. Hm, will this really solve the problem properly? I kept that above patch because of the very real problem where a device in the middle of the device tree can go away without the lower devices being gone. The example at the time was usb-serial devices. The physical USB device goes away, but the tty device that userspace has open still sticks around until userspace closes the char device. With your patch, will things break for this usage model? This kobject cleanup path is so fickle, I hate having to touch it :( thanks, greg k-h - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: BUG in: Driver core: convert block from raw kobjects to core devices (fwd)
On Mon, Oct 29, 2007 at 08:13:17PM +0100, Kay Sievers wrote: On Mon, 2007-10-29 at 14:47 -0400, Alan Stern wrote: On Mon, 29 Oct 2007, James Bottomley wrote: I'm not sure if we can do this patch. If you kill a device, you see there's a processing state in scsi_prep_fn() for SDEV_DEL, which has a printk message I see quite often when I unplug devices while they're operating. If you NULL out and free the request queue immediately after setting SDEV_DEL, without allowing the devices and filesystems to finish, are you sure we're not going to get a null deref on sdev-request_queue? Let's get at this another way. The patch below probably should be applied in any case. It's essentially a reversion of this patch from 2003: http://git.kernel.org/?p=linux/kernel/git/tglx/history.git;a=commit;h=10921a8f1305b8ec97794941db78b825db5839bc which was written to compensate for problems in the SCSI stack. The idea was to avoid releasing a kobject before all its children were released. However as far as I can see, in general we should be able to release a kobject as soon as all its children are removed and its refcount drops to 0, without waiting for the children to be released. To put it another way, once a child is removed from visibility it should not try (or need) to access its parent at all. If it has to then it should take an explicit reference to the parent. Since the SCSI stack is now in much better shape, there doesn't seem to be any reason to keep the old code. Do you agree that the patch below is worth merging? I submitted it to Greg some time ago, but he didn't want to accept it without some assurance that it is now safe. It would fix the problem Kay and saw with the circular references, because the references would all get dropped when the scsi_device is removed instead of waiting for a release that will never come. It indeed fixes my problem. And it sounds you are right, the fix from 2003 is probably just a paper-over for a missing explicit reference that time. I'm all for giving the reversion a try, and add explicit parent get/put if needed somewhere. If, for some reason, that will not happen, I'll need to do something like this in the block patch, which will then be a fix for the paper-over solution for an unknown bug. :) --- a/fs/partitions/check.c +++ b/fs/partitions/check.c ... + device_del(disk-dev); + + /* +* disconnect from parent device, so the parent can clean up +* without waiting for us to clean up +* +* the driver core took this reference while we added ourself as +* a child of the parent device +*/ + parent = disk-dev.kobj.parent; Save off the parent before calling device_del() otherwise it could be a stale pointer, right? + disk-dev.kobj.parent = NULL; + disk-dev.parent = NULL; + kobject_put(parent); No, that's just wrong, if this is needed, something else really is broken, and I don't think it's the driver core... thanks, greg k-h - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] Remove dma_coherent_mem interface
On Tue, Oct 30, 2007 at 02:35:14PM -0600, Matthew Wilcox wrote: On Tue, Oct 30, 2007 at 03:26:31PM -0500, James Bottomley wrote: Its design was basically to facilitate the use of bus remote memory. There's a long thread somewhere discussing this with the ARM people. They had some type of SoC implementation that needed to allocate local memory for device descriptors. The Q720 is pretty much the same way, so I used it to build the implementation when I created it for the ARM people. This is the original thread: http://marc.info/?t=10875786211 They said they were implementing this system, so I've no idea what happened. It's been over three years, and it hasn't happened. However, what are the problems the API is causing? it seems useful, so there should be a preference in its favour of existence unless it's causing a problem. What I'm currently looking at is the dmapool allocator. It's not exactly fast (a spinlock for each allocation ... no concept of cpu affinity, etc), and some drivers (eg qla2xxx) use it in the performance path. One of the suggestions in the existing dmapool driver is to share the guts of slab. Well, slab is probably going away, so I took a look at slub. Slub really, *really* needs the struct page associated with the page of memory allocated, so I'm currently working my way through the architectures trying to turn dma_alloc_coherent into dma_alloc_coherent_pages. The dma_coherent_mem API is one of the things which gets in the way of doing this. So I deleted it, then sent the patch out for comments early. We have some out-of-tree users for this on SH as well, particularly the SM501 MFD USB driver which needs to do 8051-local allocations. We have an in-tree hack for this now, I never bothered pushing the dma_declare_coherent_memory() bits due to the fact the driver hadn't been merged yet, but I had planned on merging both for 2.6.25. We can continue using the in-tree hack if there aren't going to be any other users of this API and it's causing problems elsewhere, but I do expect that we will continue to see devices with a need for this sort of API. I would imagine that the ARM case is similar, even if it's been a low priority item. - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
