Hi Ryan,
Am Mittwoch, den 06.11.2013, 07:43 +1100 schrieb Ryan Mallon:
> On 05/11/13 19:44, Frank Haverkamp wrote:
>
> > Sysfs interfaces for the GenWQE card. There are attributes to query
> > the version of the bitstream as well as some for the driver. For
> > debugging, please also see the debugfs interfaces of this driver.
> >
> > Signed-off-by: Frank Haverkamp <ha...@linux.vnet.ibm.com>
> > Co-authors: Joerg-Stephan Vogt <jsv...@de.ibm.com>,
> > Michael Jung <mij...@de.ibm.com>,
> > Michael Ruettger <mich...@ibmra.de>
>
>
> Handful of small comments below,
>
> ~Ryan
>
> > ---
> > Documentation/ABI/testing/sysfs-driver-genwqe | 55 +++++
> > drivers/misc/genwqe/card_sysfs.c | 290
> > +++++++++++++++++++++++++
> > 2 files changed, 345 insertions(+), 0 deletions(-)
> > create mode 100644 Documentation/ABI/testing/sysfs-driver-genwqe
> > create mode 100644 drivers/misc/genwqe/card_sysfs.c
> >
> > diff --git a/Documentation/ABI/testing/sysfs-driver-genwqe
> > b/Documentation/ABI/testing/sysfs-driver-genwqe
> > new file mode 100644
> > index 0000000..0976901
> > --- /dev/null
> > +++ b/Documentation/ABI/testing/sysfs-driver-genwqe
> > @@ -0,0 +1,55 @@
> > +What: /sys/class/genwqe/genwqe<n>_card/version
> > +Date: Oct 2013
> > +Contact: ha...@linux.vnet.ibm.com
> > +Description: Unique bitstream identification e.g.
> > + '0000000330336283.00000000475a4950'.
> > +
> > +What: /sys/class/genwqe/genwqe<n>_card/appid
> > +Date: Oct 2013
> > +Contact: ha...@linux.vnet.ibm.com
> > +Description: Identifies the currently active card application e.g.
> > 'GZIP'
> > + for compression/decompression.
> > +
> > +What: /sys/class/genwqe/genwqe<n>_card/type
> > +Date: Oct 2013
> > +Contact: ha...@linux.vnet.ibm.com
> > +Description: Type of the card e.g. 'GenWQE5-A7'.
> > +
> > +What: /sys/class/genwqe/genwqe<n>_card/cpld_version
> > +Date: Oct 2013
> > +Contact: ha...@linux.vnet.ibm.com
> > +Description: Version id of the cards CPLD.
> > +
> > +What: /sys/class/genwqe/genwqe<n>_card/curr_bitstream
> > +Date: Oct 2013
> > +Contact: ha...@linux.vnet.ibm.com
> > +Description: Currently active bitstream. 1 is default, 0 is backup.
> > +
> > +What: /sys/class/genwqe/genwqe<n>_card/next_bitstream
> > +Date: Oct 2013
> > +Contact: ha...@linux.vnet.ibm.com
> > +Description: Interface to set the next bitstream to be used.
> > +
> > +What: /sys/class/genwqe/genwqe<n>_card/tempsens
> > +Date: Oct 2013
> > +Contact: ha...@linux.vnet.ibm.com
> > +Description: Interface to read the cards temperature sense register.
> > +
> > +What: /sys/class/genwqe/genwqe<n>_card/state
> > +Date: Oct 2013
> > +Contact: ha...@linux.vnet.ibm.com
> > +Description: State of the card: "unused", "used", "error".
> > +
> > +What: /sys/class/genwqe/genwqe<n>_card/base_clock
> > +Date: Oct 2013
> > +Contact: ha...@linux.vnet.ibm.com
> > +Description: Base clock frequency of the card.
> > +
> > +What: /sys/class/genwqe/genwqe<n>_card/device/sriov_numvfs
> > +Date: Oct 2013
> > +Contact: ha...@linux.vnet.ibm.com
> > +Description: Enable VFs (1..15):
> > + sudo sh -c 'echo 15 > \
> > + /sys/bus/pci/devices/0000\:1b\:00.0/sriov_numvfs'
> > + Disable VFs:
> > + Write a 0 into the same sysfs entry.
> > diff --git a/drivers/misc/genwqe/card_sysfs.c
> > b/drivers/misc/genwqe/card_sysfs.c
> > new file mode 100644
> > index 0000000..f29ce55
> > --- /dev/null
> > +++ b/drivers/misc/genwqe/card_sysfs.c
> > @@ -0,0 +1,290 @@
> > +/**
> > + * IBM Accelerator Family 'GenWQE'
> > + *
> > + * (C) Copyright IBM Corp. 2013
> > + *
> > + * Author: Frank Haverkamp <ha...@linux.vnet.ibm.com>
> > + * Author: Joerg-Stephan Vogt <jsv...@de.ibm.com>
> > + * Author: Michael Jung <mij...@de.ibm.com>
> > + * Author: Michael Ruettger <mich...@ibmra.de>
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License (version 2 only)
> > + * as published by the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > + * GNU General Public License for more details.
> > + */
> > +
> > +/*
> > + * Sysfs interfaces for the GenWQE card. There are attributes to query
> > + * the version of the bitstream as well as some for the driver. For
> > + * debugging, please also see the debugfs interfaces of this driver.
> > + */
> > +
> > +#include <linux/version.h>
> > +#include <linux/kernel.h>
> > +#include <linux/types.h>
> > +#include <linux/module.h>
> > +#include <linux/pci.h>
> > +#include <linux/string.h>
> > +#include <linux/fs.h>
> > +#include <linux/sysfs.h>
> > +#include <linux/ctype.h>
> > +#include <linux/device.h>
> > +
> > +#include "card_base.h"
> > +#include "card_ddcb.h"
> > +
> > +static const char * const genwqe_types[] = {
> > + [GENWQE_TYPE_ALTERA_230] = "GenWQE4-230",
> > + [GENWQE_TYPE_ALTERA_530] = "GenWQE4-530",
> > + [GENWQE_TYPE_ALTERA_A4] = "GenWQE5-A4",
> > + [GENWQE_TYPE_ALTERA_A7] = "GenWQE5-A7",
> > +};
> > +
> > +static ssize_t status_show(struct device *dev, struct device_attribute
> > *attr,
> > + char *buf)
> > +{
> > + struct genwqe_dev *cd = dev_get_drvdata(dev);
> > + const char *cs[GENWQE_CARD_STATE_MAX] = { "unused", "used", "error" };
> > +
> > + return scnprintf(buf, PAGE_SIZE, "%s\n", cs[cd->card_state]);
> > +}
> > +static DEVICE_ATTR_RO(status);
> > +
> > +static ssize_t appid_show(struct device *dev, struct device_attribute
> > *attr,
> > + char *buf)
> > +{
> > + char app_name[5];
> > + struct genwqe_dev *cd = dev_get_drvdata(dev);
> > +
> > + genwqe_read_app_id(cd, app_name, sizeof(app_name));
> > + return scnprintf(buf, PAGE_SIZE, "%s\n", app_name);
> > +}
> > +static DEVICE_ATTR_RO(appid);
> > +
> > +static ssize_t version_show(struct device *dev, struct device_attribute
> > *attr,
> > + char *buf)
> > +{
> > + u64 slu_id, app_id;
> > + struct genwqe_dev *cd = dev_get_drvdata(dev);
> > +
> > + slu_id = __genwqe_readq(cd, IO_SLU_UNITCFG);
> > + app_id = __genwqe_readq(cd, IO_APP_UNITCFG);
> > +
> > + return scnprintf(buf, PAGE_SIZE, "%016llx.%016llx\n", slu_id, app_id);
> > +}
> > +static DEVICE_ATTR_RO(version);
> > +
> > +static ssize_t type_show(struct device *dev, struct device_attribute *attr,
> > + char *buf)
> > +{
> > + u8 card_type;
> > + struct genwqe_dev *cd = dev_get_drvdata(dev);
> > +
> > + card_type = genwqe_card_type(cd);
> > + return scnprintf(buf, PAGE_SIZE, "%s\n",
> > + (card_type >= ARRAY_SIZE(genwqe_types)) ?
> > + "invalid" : genwqe_types[card_type]);
> > +}
> > +static DEVICE_ATTR_RO(type);
> > +
> > +static ssize_t driver_show(struct device *dev, struct device_attribute
> > *attr,
> > + char *buf)
> > +{
> > + return scnprintf(buf, PAGE_SIZE, "%s\n", DRV_VERS_STRING);
> > +}
> > +static DEVICE_ATTR_RO(driver);
> > +
> > +static ssize_t tempsens_show(struct device *dev, struct device_attribute
> > *attr,
> > + char *buf)
> > +{
> > + u64 tempsens;
> > + struct genwqe_dev *cd = dev_get_drvdata(dev);
> > +
> > + tempsens = __genwqe_readq(cd, IO_SLU_TEMPERATURE_SENSOR);
> > + return scnprintf(buf, PAGE_SIZE, "%016llx\n", tempsens);
> > +}
> > +static DEVICE_ATTR_RO(tempsens);
> > +
> > +static ssize_t base_clock_show(struct device *dev,
> > + struct device_attribute *attr,
> > + char *buf)
> > +{
> > + u64 base_clock;
> > + struct genwqe_dev *cd = dev_get_drvdata(dev);
> > +
> > + base_clock = genwqe_base_clock_frequency(cd);
> > + return scnprintf(buf, PAGE_SIZE, "%lld\n", base_clock);
> > +}
> > +static DEVICE_ATTR_RO(base_clock);
> > +
> > +/**
> > + * curr_bitstream_show() - Show the current bitstream id
> > + *
> > + * There is a bug in some old versions of the CPLD which selects the
> > + * bitstream, which causes the IO_SLU_BITSTREAM register to report
> > + * unreliable data in very rare cases. This makes this sysfs
> > + * unreliable up to the point were a new CPLD version is being used.
> > + *
> > + * Unfortunately there is no automatic way yet to query the CPLD
> > + * version (See show_cpld_version() ;-)), such that you need to
> > + * manually ensure via programming tools that you have a recent
> > + * version of the CPLD software.
> > + *
> > + * The proposed circumvention is to use a special recovery bitstream
> > + * on the backup partition (0) to identify problems while loading the
> > + * image.
> > + */
> > +static ssize_t curr_bitstream_show(struct device *dev,
> > + struct device_attribute *attr, char *buf)
> > +{
> > + int curr_bitstream;
> > + struct genwqe_dev *cd = dev_get_drvdata(dev);
> > +
> > + curr_bitstream = __genwqe_readq(cd, IO_SLU_BITSTREAM) & 0x1;
> > + return scnprintf(buf, PAGE_SIZE, "%d\n", curr_bitstream);
> > +}
> > +static DEVICE_ATTR_RO(curr_bitstream);
> > +
> > +/**
> > + * next_bitstream_show() - Show the next activated bitstream
> > + *
> > + * IO_SLC_CFGREG_SOFTRESET: This register can only be accessed by the PF.
> > + */
> > +static ssize_t next_bitstream_show(struct device *dev,
> > + struct device_attribute *attr, char *buf)
> > +{
> > + int next_bitstream;
> > + struct genwqe_dev *cd = dev_get_drvdata(dev);
> > +
> > + switch ((cd->softreset & 0xcull) >> 2) {
>
>
> The ull suffix isn't really necessary here, and can just be removed.
Ok.
>
> > + case 0x2:
> > + next_bitstream = 0;
> > + break;
> > + case 0x3:
> > + next_bitstream = 1;
> > + break;
> > + default:
> > + next_bitstream = -1;
> > + break; /* error */
> > + }
> > + return scnprintf(buf, PAGE_SIZE, "%d\n", next_bitstream);
>
>
> sprintf is fine, a %d is not going to be larger than PAGE_SIZE.
>
Agreed.
> > +}
> > +
> > +static ssize_t next_bitstream_store(struct device *dev,
> > + struct device_attribute *attr,
> > + const char *buf, size_t count)
> > +{
> > + u64 partition;
> > + struct genwqe_dev *cd = dev_get_drvdata(dev);
> > +
> > + if (kstrtoull(buf, 0, &partition) < 0)
> > + return -EINVAL;
>
>
> Why are you using kstrtoull for a value which is only 0 or 1?
>
> > +
> > + switch (partition) {
> > + case 0x0:
> > + cd->softreset = 0x78ull;
> > + break;
> > + case 0x1:
> > + cd->softreset = 0x7cull;
> > + break;
> > + default:
> > + return -EINVAL;
> > + }
> > +
> > + __genwqe_writeq(cd, IO_SLC_CFGREG_SOFTRESET, cd->softreset);
> > + return count;
> > +}
> > +static DEVICE_ATTR_RW(next_bitstream);
> > +
> > +/**
> > + * FIXME Implement me!
> > + */
> > +static ssize_t cpld_version_show(struct device *dev,
> > + struct device_attribute *attr, char *buf)
> > +{
> > + return scnprintf(buf, PAGE_SIZE, "unknown (FIXME)\n");
> > +}
> > +static DEVICE_ATTR_RO(cpld_version);
>
>
> Why provide this if it doesn't work? User-space obviously can't do the
> right thing with it, so just remove it until it gets implemented
> properly.
I removed it.
>
> > +
> > +/*
> > + * Create device_attribute structures / params: name, mode, show, store
> > + * additional flag if valid in VF
> > + */
> > +static struct attribute *genwqe_attributes[] = {
> > + &dev_attr_tempsens.attr,
> > + &dev_attr_next_bitstream.attr,
> > + &dev_attr_curr_bitstream.attr,
> > + &dev_attr_base_clock.attr,
> > + &dev_attr_driver.attr,
> > + &dev_attr_type.attr,
> > + &dev_attr_version.attr,
> > + &dev_attr_appid.attr,
> > + &dev_attr_status.attr,
> > + &dev_attr_cpld_version.attr,
> > + NULL,
> > +};
> > +
> > +static struct attribute *genwqe_normal_attributes[] = {
> > + &dev_attr_driver.attr,
> > + &dev_attr_type.attr,
> > + &dev_attr_version.attr,
> > + &dev_attr_appid.attr,
> > + &dev_attr_status.attr,
> > + NULL,
> > +};
> > +
> > +/**
> > + * genwqe_is_visible() - Determine if sysfs attribute should be visible or
> > not
> > + *
> > + * VFs have restricted mmio capabilities, so not all sysfs entries
> > + * are allowed in VFs.
> > + */
> > +static umode_t genwqe_is_visible(struct kobject *kobj,
> > + struct attribute *attr, int n)
>
> > +{
> > + unsigned int j;
> > + struct device *dev = container_of(kobj, struct device, kobj);
> > + struct genwqe_dev *cd = dev_get_drvdata(dev);
> > + umode_t mode = attr->mode;
> > +
> > + if (genwqe_is_privileged(cd))
> > + return mode;
> > +
> > + for (j = 0; genwqe_normal_attributes[j] != NULL; j++)
> > + if (genwqe_normal_attributes[j] == attr)
> > + return mode;
> > +
> > + return 0;
> > +}
> > +
> > +static struct attribute_group genwqe_attribute_group = {
> > + .is_visible = genwqe_is_visible,
> > + .attrs = genwqe_attributes,
> > +};
> > +
> > +/**
> > + * genwqe_init_sysfs() - Setup sysfs entries of the card device
> > + */
> > +int genwqe_init_sysfs(struct genwqe_dev *cd)
> > +{
> > + int rc;
> > +
> > + rc = sysfs_create_group(&cd->dev->kobj, &genwqe_attribute_group);
> > + if (rc)
> > + return -ENXIO;
> > +
> > + return 0;
> > +}
> > +
> > +/**
> > + * genwqe_exit_sysfs() - Remove sysfs entries
> > + */
> > +void genwqe_exit_sysfs(struct genwqe_dev *cd)
> > +{
> > + sysfs_remove_group(&cd->dev->kobj, &genwqe_attribute_group);
> > +}
>
>
Regards
Frank
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
