Re: [PATCH v2 38/51] IB/qib: Add qib_sysfs.c

2010-04-12 Thread Ralph Campbell
On Fri, 2010-04-09 at 17:27 -0700, Jason Gunthorpe wrote: > On Fri, Apr 09, 2010 at 05:13:24PM -0700, Ralph Campbell wrote: > > > For the QSFP data, I hope I can leave it as is since it is > > related to the link state that the other files contain. > > It is a read-only file so no issue with tryin

Re: [PATCH v2 38/51] IB/qib: Add qib_sysfs.c

2010-04-09 Thread Jason Gunthorpe
On Fri, Apr 09, 2010 at 05:13:24PM -0700, Ralph Campbell wrote: > For the QSFP data, I hope I can leave it as is since it is > related to the link state that the other files contain. > It is a read-only file so no issue with trying to set a value. There was some flack for other stuff like this a

Re: [PATCH v2 38/51] IB/qib: Add qib_sysfs.c

2010-04-09 Thread Jason Gunthorpe
On Fri, Apr 09, 2010 at 04:31:26PM -0700, Ralph Campbell wrote: > I would be very reluctant to change this code. > One device we have doesn't have an i2c address, the "address" is the > address of memory within the chip, not the address of the chip on the > bus. Fair enough.. But mixing not-i2c +

Re: [PATCH v2 38/51] IB/qib: Add qib_sysfs.c

2010-04-09 Thread Ralph Campbell
On Thu, 2010-04-08 at 14:33 -0700, Roland Dreier wrote: > > This was for debugging and clearly could use a better method. > > Some stats are definitely useful and having files per value > > would make scripting easier too. > > I could add /sys/class/infiniband/qib0/ports/1/diag_counters/* > >

Re: [PATCH v2 38/51] IB/qib: Add qib_sysfs.c

2010-04-09 Thread Ralph Campbell
On Thu, 2010-04-08 at 15:50 -0700, Jason Gunthorpe wrote: > On Thu, Apr 08, 2010 at 03:26:41PM -0700, Ralph Campbell wrote: > > On Thu, 2010-04-08 at 15:08 -0700, Jason Gunthorpe wrote: > > > On Thu, Apr 08, 2010 at 02:29:53PM -0700, Ralph Campbell wrote: > > > > > > > One other place that has mul

Re: [PATCH v2 38/51] IB/qib: Add qib_sysfs.c

2010-04-08 Thread Ira Weiny
On Thu, 08 Apr 2010 16:29:06 -0700 Roland Dreier wrote: > > I for one would rather not see this die. We have debugged some critical > > issues using this data. The sysfs entries above are what Mellanox uses. > > Should those also be changed? > > Which sysfs entries? I don't see any likely

Re: [PATCH v2 38/51] IB/qib: Add qib_sysfs.c

2010-04-08 Thread Ralph Campbell
On Thu, 2010-04-08 at 16:29 -0700, Roland Dreier wrote: > > I for one would rather not see this die. We have debugged some critical > > issues using this data. The sysfs entries above are what Mellanox uses. > > Should those also be changed? > > Which sysfs entries? I don't see any likely loo

Re: [PATCH v2 38/51] IB/qib: Add qib_sysfs.c

2010-04-08 Thread Roland Dreier
> I for one would rather not see this die. We have debugged some critical > issues using this data. The sysfs entries above are what Mellanox uses. > Should those also be changed? Which sysfs entries? I don't see any likely looking code in either of the Mellanox drivers. - R. -- Roland D

Re: [PATCH v2 38/51] IB/qib: Add qib_sysfs.c

2010-04-08 Thread Ralph Campbell
On Thu, 2010-04-08 at 15:50 -0700, Jason Gunthorpe wrote: > On Thu, Apr 08, 2010 at 03:26:41PM -0700, Ralph Campbell wrote: > > On Thu, 2010-04-08 at 15:08 -0700, Jason Gunthorpe wrote: > > > On Thu, Apr 08, 2010 at 02:29:53PM -0700, Ralph Campbell wrote: > > > > > > > One other place that has mul

Re: [PATCH v2 38/51] IB/qib: Add qib_sysfs.c

2010-04-08 Thread Jason Gunthorpe
On Thu, Apr 08, 2010 at 03:26:41PM -0700, Ralph Campbell wrote: > On Thu, 2010-04-08 at 15:08 -0700, Jason Gunthorpe wrote: > > On Thu, Apr 08, 2010 at 02:29:53PM -0700, Ralph Campbell wrote: > > > > > One other place that has multiple values is dumping the QSFP > > > data from the IB cable. > >

Re: [PATCH v2 38/51] IB/qib: Add qib_sysfs.c

2010-04-08 Thread Ralph Campbell
On Thu, 2010-04-08 at 15:08 -0700, Jason Gunthorpe wrote: > On Thu, Apr 08, 2010 at 02:29:53PM -0700, Ralph Campbell wrote: > > > One other place that has multiple values is dumping the QSFP > > data from the IB cable. > > I was going to comment that building your own I2C subsystem is kinda > str

Re: [PATCH v2 38/51] IB/qib: Add qib_sysfs.c

2010-04-08 Thread Ira Weiny
On Thu, 08 Apr 2010 14:33:59 -0700 Roland Dreier wrote: > > This was for debugging and clearly could use a better method. > > Some stats are definitely useful and having files per value > > would make scripting easier too. > > I could add /sys/class/infiniband/qib0/ports/1/diag_counters/* >

Re: [PATCH v2 38/51] IB/qib: Add qib_sysfs.c

2010-04-08 Thread Jason Gunthorpe
On Thu, Apr 08, 2010 at 02:29:53PM -0700, Ralph Campbell wrote: > One other place that has multiple values is dumping the QSFP > data from the IB cable. I was going to comment that building your own I2C subsystem is kinda strange, can't the in-kernel stuff be used? Jason -- To unsubscribe from t

Re: [PATCH v2 38/51] IB/qib: Add qib_sysfs.c

2010-04-08 Thread Roland Dreier
> This was for debugging and clearly could use a better method. > Some stats are definitely useful and having files per value > would make scripting easier too. > I could add /sys/class/infiniband/qib0/ports/1/diag_counters/* > > One other place that has multiple values is dumping the QSFP

Re: [PATCH v2 38/51] IB/qib: Add qib_sysfs.c

2010-04-08 Thread Ralph Campbell
This was for debugging and clearly could use a better method. Some stats are definitely useful and having files per value would make scripting easier too. I could add /sys/class/infiniband/qib0/ports/1/diag_counters/* One other place that has multiple values is dumping the QSFP data from the IB ca

Re: [PATCH v2 38/51] IB/qib: Add qib_sysfs.c

2010-04-08 Thread Roland Dreier
I didn't audit everything (so there may be more problems here too), but certainly this violates the "one value per sysfs file" rule rather egregiously: > +static ssize_t show_stats(struct device *device, struct device_attribute > *attr, > + char *buf) > +{ > +struct

[PATCH v2 38/51] IB/qib: Add qib_sysfs.c

2009-12-03 Thread Ralph Campbell
creates the qib_sysfs.c file. Signed-off-by: Ralph Campbell --- drivers/infiniband/hw/qib/qib_sysfs.c | 736 + 1 files changed, 736 insertions(+), 0 deletions(-) create mode 100644 drivers/infiniband/hw/qib/qib_sysfs.c diff --git a/drivers/infiniband/hw/qib/qi