Re: [PATCH linux v7 4/6] hwmon: occ: Add callbacks for parsing P8 OCC datastructures
On Wed, Feb 8, 2017 at 9:40 AM, wrote: > From: "Edward A. James" > > Add functions to parse the data structures that are specific to the OCC on > the POWER8 processor. These are the sensor data structures, including > temperature, frequency, power, and "caps." > > Signed-off-by: Edward A. James > Signed-off-by: Andrew Jeffery > --- > Documentation/hwmon/occ| 9 ++ > drivers/hwmon/occ/occ_p8.c | 248 > + > drivers/hwmon/occ/occ_p8.h | 30 ++ > 3 files changed, 287 insertions(+) > create mode 100644 drivers/hwmon/occ/occ_p8.c > create mode 100644 drivers/hwmon/occ/occ_p8.h > > diff --git a/drivers/hwmon/occ/occ_p8.c b/drivers/hwmon/occ/occ_p8.c > new file mode 100644 > index 000..5c61fc4 > --- /dev/null > +++ b/drivers/hwmon/occ/occ_p8.c > +void p8_parse_sensor(u8 *data, void *sensor, int sensor_type, int off, > +int snum) > +{ > + switch (sensor_type) { > + case FREQ: > + case TEMP: > + { > + struct p8_occ_sensor *os = > + &(((struct p8_occ_sensor *)sensor)[snum]); > + > + os->sensor_id = be16_to_cpu(get_unaligned((u16 *)&data[off])); > + os->value = be16_to_cpu(get_unaligned((u16 *)&data[off + 2])); > + } > + break; > + case POWER: > + { > + struct p8_power_sensor *ps = > + &(((struct p8_power_sensor *)sensor)[snum]); > + > + ps->sensor_id = be16_to_cpu(get_unaligned((u16 *)&data[off])); > + ps->update_tag = > + be32_to_cpu(get_unaligned((u32 *)&data[off + 2])); This might be more readable if you wrote a cast_get_unaliged_be32_to_cpu() macro. > + ps->accumulator = > + be32_to_cpu(get_unaligned((u32 *)&data[off + 6])); > + ps->value = be16_to_cpu(get_unaligned((u16 *)&data[off + > 10])); > + } > + break; > + case CAPS: > + { > +const u32 *p8_get_sensor_hwmon_configs() > +{ > + return p8_sensor_hwmon_configs; > +} > +EXPORT_SYMBOL(p8_get_sensor_hwmon_configs); > + > +struct occ *p8_occ_start(struct device *dev, void *bus, > +struct occ_bus_ops *bus_ops) > +{ > + return occ_start(dev, bus, bus_ops, &p8_ops, &p8_config); > +} > +EXPORT_SYMBOL(p8_occ_start); We don't need to export these symbols; they're not used outside of the OCC module. The same goes for all of the exports you've made in this series. I suggest we re-architect the drivers so we build all of the objects and link them into one module for each platform, instead of having an occ module and occ-p8/occ-p9 modules and i2c modules that all depend on each other. The Makefile could look like this: obj-$(CONFIG_SENSORS_PPC_OCC_P8_I2C) += hwmon_occ_p8.o obj-$(CONFIG_SENSORS_PPC_OCC_P9) += hwmon_occ_p9.o hwmon_occ_p8-$(CONFIG_SENSORS_PPC_OCC_P8_I2C) += occ_scom_i2c.o occ_p8.o p8_occ_i2c.o occ_sysfs.o occ.o hwmon_occ_p9-$(CONFIG_SENSORS_PPC_OCC_P9) += occ_p9.o occ_sysfs.o occ.o And the Kbuild like this: menuconfig SENSORS_PPC_OCC bool "PPC On-Chip Controller" if SENSORS_PPC_OCC config SENSORS_PPC_OCC_P8_I2C bool "POWER8 OCC hwmon support" depends on I2C config SENSORS_PPC_OCC_P9 bool "POWER9 OCC hwmon support" endif -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH linux v7 6/6] hwmon: occ: Add callbacks for parsing P9 OCC datastructures
On Wed, Feb 8, 2017 at 9:40 AM, wrote: > From: "Edward A. James" > > Add functions to parse the data structures that are specific to the OCC on > the POWER9 processor. These are the sensor data structures, including > temperature, frequency, power, and "caps." > > Signed-off-by: Edward A. James > Signed-off-by: Andrew Jeffery > --- > Documentation/hwmon/occ| 3 + > drivers/hwmon/occ/occ_p9.c | 309 > + > drivers/hwmon/occ/occ_p9.h | 30 + > 3 files changed, 342 insertions(+) > create mode 100644 drivers/hwmon/occ/occ_p9.c > create mode 100644 drivers/hwmon/occ/occ_p9.h > diff --git a/drivers/hwmon/occ/occ_p9.c b/drivers/hwmon/occ/occ_p9.c > new file mode 100644 > index 000..9c1283c > --- /dev/null > +++ b/drivers/hwmon/occ/occ_p9.c > @@ -0,0 +1,309 @@ > +/* > + * occ_p9.c - OCC hwmon driver > + * > + * This file contains the Power9-specific methods and data structures for > + * the OCC hwmon driver. > + * > + * Copyright 2016 IBM Corp. It's 2017. > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; either version 2 of the License, or > + * (at your option) any later version. > + * > + * 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. We generally just include the first paragraph. Same goes for all of the files. > + > +static const u32 p9_sensor_hwmon_configs[MAX_OCC_SENSOR_TYPE] = { > + HWMON_I_INPUT | HWMON_I_LABEL, /* freq: value | label */ > + /* temp: value | label | fru_type */ > + HWMON_T_INPUT | HWMON_T_LABEL | HWMON_T_TYPE, > + /* power: value | label | accum[0] | accum[1] | update_tag | > +* (function_id | (apss_channel << 8)) > +*/ > + HWMON_P_INPUT | HWMON_P_LABEL | HWMON_P_AVERAGE_MIN | > + HWMON_P_AVERAGE_MAX | HWMON_P_AVERAGE_INTERVAL | > + HWMON_P_RESET_HISTORY, > + /* caps: curr | max | min | norm | user | source */ > + HWMON_P_CAP | HWMON_P_CAP_MAX | HWMON_P_CAP_MIN | HWMON_P_MAX | > + HWMON_P_ALARM | HWMON_P_CAP_ALARM, I find this really hard to read. Perhaps something like this: #define FREQ_CONFIG(HWMON_I_INPUT | HWMON_I_LABEL) #deifne TEMP_CONFIG(HWMON_T_INPUT | HWMON_T_LABEL | HWMON_T_TYPE) #define POWER_CONFIG( HWMON_P_INPUT | HWMON_P_LABEL | HWMON_P_AVERAGE_MIN | \ HWMON_P_AVERAGE_MAX | HWMON_P_AVERAGE_INTERVAL | \ HWMON_P_RESET_HISTORY) etc. Do the same in the p8 driver. > diff --git a/drivers/hwmon/occ/occ_p9.h b/drivers/hwmon/occ/occ_p9.h > new file mode 100644 > index 000..18ca16a > --- /dev/null > +++ b/drivers/hwmon/occ/occ_p9.h > + > +#ifndef __OCC_P9_H__ > +#define __OCC_P9_H__ > + > +#include "scom.h" > + > +struct device; Include the header for struct device instead. Did you consider the one header file for all of your shared functions? I don't think there's much value in having a whole heap of small ones. > + > +const u32 *p9_get_sensor_hwmon_configs(void); > +struct occ *p9_occ_start(struct device *dev, void *bus, > +struct occ_bus_ops *bus_ops); > + > +#endif /* __OCC_P9_H__ */ > -- > 1.8.3.1 > -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH linux v7 1/6] hwmon: Add core On-Chip Controller support for POWER CPUs
On Wed, Feb 8, 2017 at 9:40 AM, wrote: > diff --git a/Documentation/hwmon/occ b/Documentation/hwmon/occ > new file mode 100644 > index 000..79d1642 > --- /dev/null > +++ b/Documentation/hwmon/occ The kernel is using reStructuredText these days. You should consider following suit: https://www.kernel.org/doc/html/latest/doc-guide/sphinx.html#writing-documentation > @@ -0,0 +1,40 @@ > +Kernel driver occ > += > + > +Supported chips: > + * ASPEED AST2400 > + * ASPEED AST2500 Not really - this will run on any chip that's connected to a P8 or P9. > diff --git a/MAINTAINERS b/MAINTAINERS > index 5f10c28..193a13b 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -9128,6 +9128,13 @@ T: git git://linuxtv.org/media_tree.git > S: Maintained > F: drivers/media/i2c/ov7670.c > > +ON-CHIP CONTROLLER HWMON DRIVER Mention P8 and P9? > +M: Eddie James > +L: linux-hw...@vger.kernel.org Have you subscribed to this list? Would you prefer the mail to come to the openbmc list? > +S: Maintained > +F: Documentation/hwmon/occ > +F: drivers/hwmon/occ/ > + > ONENAND FLASH DRIVER > M: Kyungmin Park > L: linux-...@lists.infradead.org > diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig > index 190d270..e80ca81 100644 > --- a/drivers/hwmon/Kconfig > +++ b/drivers/hwmon/Kconfig > @@ -1240,6 +1240,8 @@ config SENSORS_NSA320 > This driver can also be built as a module. If so, the module > will be called nsa320-hwmon. > > +source drivers/hwmon/occ/Kconfig > + > config SENSORS_PCF8591 > tristate "Philips PCF8591 ADC/DAC" > depends on I2C > diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile > index d2cb7e8..c7ec5d4 100644 > --- a/drivers/hwmon/Makefile > +++ b/drivers/hwmon/Makefile > @@ -169,6 +169,7 @@ obj-$(CONFIG_SENSORS_WM831X)+= wm831x-hwmon.o > obj-$(CONFIG_SENSORS_WM8350) += wm8350-hwmon.o > obj-$(CONFIG_SENSORS_XGENE)+= xgene-hwmon.o > > +obj-$(CONFIG_SENSORS_PPC_OCC) += occ/ > obj-$(CONFIG_PMBUS)+= pmbus/ > > ccflags-$(CONFIG_HWMON_DEBUG_CHIP) := -DDEBUG > diff --git a/drivers/hwmon/occ/occ.c b/drivers/hwmon/occ/occ.c > new file mode 100644 > index 000..af077f2 > --- /dev/null > +++ b/drivers/hwmon/occ/occ.c > + sensors = &resp->data.blocks[b].sensors; > + if (!sensors) { > + /* first poll response */ > + sensors = driver->ops.alloc_sensor(dev, sensor_type, > + > block->num_sensors); > + if (!sensors) > + return -ENOMEM; > + > + resp->data.blocks[b].sensors = sensors; > + resp->data.sensor_block_id[sensor_type] = b; > + resp->data.blocks[b].header = *block; > + } else if (block->sensor_length != > +resp->data.blocks[b].header.sensor_length) { > + dev_warn(dev, > +"different sensor length than first poll\n"); The driver has changed behaviour from your previous version; now it discards data if the sensors change. Under what circumstances does the sensor data change? > + continue; > + } > + > + for (s = 0; s < block->num_sensors && > +s < resp->data.blocks[b].header.num_sensors; s++) { > + if (offset + block->sensor_length > num_bytes) { > + dev_warn(dev, "exceeded data length\n"); > + return 0; > + } > + > + driver->ops.parse_sensor(data, sensors, sensor_type, > +offset, s); > + offset += block->sensor_length; > + } > + } > + > + return 0; > +} > + > +static u8 occ_send_cmd(struct occ *driver, u8 seq, u8 type, u16 length, > + const u8 *data, u8 *resp) > +{ > + u32 cmd1, cmd2 = 0; > + u16 checksum = 0; > + bool retry = false; > + int i, rc, tries = 0; > + > + cmd1 = (seq << 24) | (type << 16) | length; > + memcpy(&cmd2, data, length); > + cmd2 <<= ((4 - length) * 8); > + > + /* checksum: sum of every bytes of cmd1, cmd2 */ > + for (i = 0; i < 4; i++) { > + checksum += (cmd1 >> (i * 8)) & 0xFF; > + checksum += (cmd2 >> (i * 8)) & 0xFF; > + } > + > + cmd2 |= checksum << ((2 - length) * 8); > + > + /* Init OCB */ You should mention what OCB means in your documentation. > + rc = driver->bus_ops.putscom(driver->bus, OCB_STATUS_CONTROL_OR, > +OCB_OR_INIT0, OCB_OR_INIT1); > + if (rc) > + goto err; > + > +int occ_set_user_powercap(struct occ *occ, u16 cap) > +{ >
Re: [PATCH linux v7 5/6] hwmon: occ: Add hwmon implementation for the P8 OCC
On Wed, Feb 8, 2017 at 9:40 AM, wrote: > From: "Edward A. James" > > Add code to tie the hwmon sysfs code and the POWER8 OCC code together, as > well as probe the entire driver from the I2C bus. I2C is the communication > method between the BMC and the P8 OCC. > > Signed-off-by: Edward A. James > Signed-off-by: Andrew Jeffery > Acked-by: Rob Herring > --- > Documentation/devicetree/bindings/hwmon/occ.txt | 13 +++ > drivers/hwmon/occ/Kconfig | 14 > drivers/hwmon/occ/Makefile | 1 + > drivers/hwmon/occ/p8_occ_i2c.c | 104 > For consistency, how about we call this occ_p8_i2c.c? The other files have the machine name second. > 4 files changed, 132 insertions(+) > create mode 100644 Documentation/devicetree/bindings/hwmon/occ.txt > create mode 100644 drivers/hwmon/occ/p8_occ_i2c.c > > diff --git a/drivers/hwmon/occ/Kconfig b/drivers/hwmon/occ/Kconfig > index cdb64a7..3a5188f 100644 > --- a/drivers/hwmon/occ/Kconfig > +++ b/drivers/hwmon/occ/Kconfig > @@ -13,3 +13,17 @@ menuconfig SENSORS_PPC_OCC > > This driver can also be built as a module. If so, the module > will be called occ. > + > +if SENSORS_PPC_OCC > + > +config SENSORS_PPC_OCC_P8_I2C > + tristate "POWER8 OCC hwmon support" > + depends on I2C > + help > +Provide a hwmon sysfs interface for the POWER8 On-Chip Controller, > +exposing temperature, frequency and power measurements. > + > +This driver can also be built as a module. If so, the module will be > +called p8-occ-i2c. Mention that this driver is for the BMC (or just "service processor"), and is not useful in the Power8 kernel. I'm trying to think of a better prefix than PPC. PPC means much more than Power8 and Power9, which is what we mean. It's also confusing as we don't run this on any PowerPC machine - it's for an ARM chip. > + > +int p8_i2c_getscom(void *bus, u32 address, u64 *data) > +{ > + /* P8 i2c slave requires address to be shifted by 1 */ > + address = address << 1; I think these are scom addresses? Please indicate that so we don't get them confused with i2c. Or are they scom operations? > + > + return occ_i2c_getscom(bus, address, data); > +} > + > +int p8_i2c_putscom(void *bus, u32 address, u32 data0, u32 data1) The functions can be static. > +{ > + /* P8 i2c slave requires address to be shifted by 1 */ > + address = address << 1; > + > + return occ_i2c_putscom(bus, address, data0, data1); > +} -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH linux v7 3/6] hwmon: occ: Add I2C transport implementation for SCOM operations
On Wed, Feb 8, 2017 at 9:40 AM, wrote: > From: "Edward A. James" > > Add functions to send SCOM operations over I2C bus. The BMC can > communicate with the Power8 host processor over I2C, but needs to use SCOM > operations in order to access the OCC register space. This doesn't need to be separate from the p8_occ_i2c.c file. You can remove a layer of function calls by merging this in and having these be your getscom putscom bus_ops callbacks. > Signed-off-by: Edward A. James > Signed-off-by: Andrew Jeffery > --- > drivers/hwmon/occ/occ_scom_i2c.c | 77 > > drivers/hwmon/occ/occ_scom_i2c.h | 26 ++ > 2 files changed, 103 insertions(+) > create mode 100644 drivers/hwmon/occ/occ_scom_i2c.c > create mode 100644 drivers/hwmon/occ/occ_scom_i2c.h > > diff --git a/drivers/hwmon/occ/occ_scom_i2c.c > b/drivers/hwmon/occ/occ_scom_i2c.c > new file mode 100644 > index 000..74bd6ff > --- /dev/null > +++ b/drivers/hwmon/occ/occ_scom_i2c.c > @@ -0,0 +1,77 @@ > + > +int occ_i2c_getscom(void *bus, u32 address, u64 *data) > +{ > + ssize_t rc; > + u64 buf; If you add endianness annotations sparse can check your types are consistent. The warning looks like this: make C=2 drivers/hwmon/occ/occ_scom_i2c.o drivers/hwmon/occ/occ_scom_i2c.c:48:17: warning: cast to restricted __be64 Which tells you it expects the type you pass to be64_to_cpu to be __be64. > + struct i2c_client *client = bus; > + struct i2c_msg msgs[2]; > + > + msgs[0].addr = client->addr; > + msgs[0].flags = client->flags & I2C_M_TEN; > + msgs[0].len = sizeof(u32); > + msgs[0].buf = (char *)&address; > + > + msgs[1].addr = client->addr; > + msgs[1].flags = client->flags & I2C_M_TEN; > + msgs[1].flags |= I2C_M_RD; I first thought you had made a mistake here. Instead you could do: msgs[1].flags = client->flags & I2C_M_TEN | I2C_M_RD; > + msgs[1].len = sizeof(u64); > + msgs[1].buf = (char *)&buf; > + > + rc = i2c_transfer(client->adapter, msgs, 2); > + if (rc < 0) > + return rc; > + -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH linux v7 2/6] hwmon: occ: Add sysfs interface
On Wed, Feb 8, 2017 at 9:40 AM, wrote: > From: "Edward A. James" > > Add a generic mechanism to expose the sensors provided by the OCC in > sysfs. > > Signed-off-by: Edward A. James > Signed-off-by: Andrew Jeffery > --- > Documentation/hwmon/occ | 62 +++ > drivers/hwmon/occ/Makefile| 2 +- > drivers/hwmon/occ/occ_sysfs.c | 251 > ++ > drivers/hwmon/occ/occ_sysfs.h | 30 + > 4 files changed, 344 insertions(+), 1 deletion(-) > create mode 100644 drivers/hwmon/occ/occ_sysfs.c > create mode 100644 drivers/hwmon/occ/occ_sysfs.h > > diff --git a/Documentation/hwmon/occ b/Documentation/hwmon/occ > index 79d1642..d0bdf06 100644 > --- a/Documentation/hwmon/occ > +++ b/Documentation/hwmon/occ > @@ -25,6 +25,68 @@ Currently, all versions of the OCC support four types of > sensor data: power, > temperature, frequency, and "caps," which indicate limits and thresholds used > internally on the OCC. > > +sysfs Entries > +- > + > +The OCC driver uses the hwmon sysfs framework to provide data to userspace. > + > +The driver exports a number of sysfs files for each type of sensor. The > +sensor-specific files vary depending on the processor type, though many of > the > +attributes are common for both the POWER8 and POWER9. > + > +The hwmon interface cannot define every type of sensor that may be used. > +Therefore, the frequency sensor on the OCC uses the "input" type sensor > defined > +by the hwmon interface, rather than defining a new type of custom sensor. > + > +Below are detailed the names and meaning of each sensor file for both types > of > +processors. All sensors are read-only unless otherwise specified. > indicates > +the hwmon index. sensor id indicates the unique internal OCC identifer. > Please > +see the POWER OCC specification for details on all these sensor values. > + > +frequency: > + all processors: > + in_input - frequency value > + in_label - sensor id > +temperature: > + POWER8: > + temp_input - temperature value > + temp_label - sensor id > + POWER9 (in addition to above): > + temp_type - FRU type > + > +power: > + POWER8: > + power_input - power value > + power_label - sensor id > + power_average - accumulator > + power_average_interval - update tag (number of samples in > + accumulator) > + POWER9: > + power_input - power value > + power_label - sensor id > + power_average_min - accumulator[0] > + power_average_max - accumulator[1] (64 bits total) > + power_average_interval - update tag > + power_reset_history - (function_id | (apss_channel << 8) > + > +caps: > + POWER8: > + power_cap - current powercap > + power_cap_max - max powercap > + power_cap_min - min powercap > + power_max - normal powercap > + power_alarm - user powercap, r/w > + POWER9: > + power_cap_alarm - user powercap source > + > +The driver also provides two sysfs entries through hwmon to better > +control the driver and monitor the master OCC. Though there may be multiple > +OCCs present on the system, these two files are only present for the "master" > +OCC. > + name - read the name of the driver > + update_interval - read or write the minimum interval for polling the > + OCC. > + > BMC - Host Communications > - > > diff --git a/drivers/hwmon/occ/Makefile b/drivers/hwmon/occ/Makefile > index 93cb52f..a6881f9 100644 > --- a/drivers/hwmon/occ/Makefile > +++ b/drivers/hwmon/occ/Makefile > @@ -1 +1 @@ > -obj-$(CONFIG_SENSORS_PPC_OCC) += occ.o > +obj-$(CONFIG_SENSORS_PPC_OCC) += occ.o occ_sysfs.o > diff --git a/drivers/hwmon/occ/occ_sysfs.c b/drivers/hwmon/occ/occ_sysfs.c > new file mode 100644 > index 000..9598f78 > --- /dev/null > +++ b/drivers/hwmon/occ/occ_sysfs.c > @@ -0,0 +1,251 @@ > +/* > + * occ_sysfs.c - OCC sysfs interface > + * > + * This file contains the methods and data structures for implementing the > OCC > + * hwmon sysfs entries. > + * > + * Copyright 2016 IBM Corp. > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; either version 2 of the License, or > + * (at your option) any later version. > + * > + * 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. > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include >
[PATCH 2/2] Documentation: DMA-ISA-LPC.txt
Fixed spelling issue. Signed-off-by: Nathan Howard --- Documentation/DMA-ISA-LPC.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/DMA-ISA-LPC.txt b/Documentation/DMA-ISA-LPC.txt index b1a1983..c413313 100644 --- a/Documentation/DMA-ISA-LPC.txt +++ b/Documentation/DMA-ISA-LPC.txt @@ -42,7 +42,7 @@ requirements you pass the flag GFP_DMA to kmalloc. Unfortunately the memory available for ISA DMA is scarce so unless you allocate the memory during boot-up it's a good idea to also pass -__GFP_REPEAT and __GFP_NOWARN to make the allocater try a bit harder. +__GFP_REPEAT and __GFP_NOWARN to make the allocator try a bit harder. (This scarcity also means that you should allocate the buffer as early as possible and not release it until the driver is unloaded.) -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 0/4] New Microsemi PCI Switch Management Driver
Corrections to the email below. It should be Tested-by: Krishna Dhulipala krish...@fb.com Reviewed-by: Wei Zhang wzh...@fb.com Thanks, -Wei -- wei zhang | software engineer | facebook wzh...@fb.com | (408) 460-4803 On 2/9/17, 3:16 PM, "Wei Zhang" wrote: Hi, The switchtec driver patches [PATCH v2 (0-4)/4] in conjunction with the switchtec userland tool is used to communicate with the Microsemi 8536 PCIe Switch used on Facebook’s Lightning platform. The following essential driver and tool functions were successfully tested with it: · Retrieval of firmware and configuration information along with CRCs · Switch firmware and configuration upgrades · Switch PHY/Link error counter collection and the ability to reset them · Switch upstream and downstream ports’ link status reporting · Switch interface functioning · Retrieval of switch ASIC temperature · Exporting switch firmware log dump · Read the information of firmware and configuration binaries · Extract the firmware and configuration images stored in the switch EEPROM Tested-by: Krishna Dhulipala krish...@fb.com Reviewed-by: Wei Zhang wzh...@fb.com Thanks, -Wei -- wei zhang | software engineer | facebook wzh...@fb.com | (408) 460-4803 On 2/2/17, 10:05 AM, "Logan Gunthorpe" wrote: Changes since v1: * Rebased onto 4.10-rc6 (cleanly) * Split the patch into a few more easily digestible patches (as suggested by Greg Kroah-Hartman) * Folded switchtec.c into switchtec.h (per Greg) * Fixed a bunch of 32bit build warnings caught by the kbuild test robot * Fixed some issues in the documentation so it has a proper reStructredText format (as noted by Jonathan Corbet) * Fixed padding and sizes in the IOCTL structures as noticed by Emil Velikov and used pahole to verify their consistency across 32 and 64 bit builds * Reworked one of the IOCTL interfaces to be more future proof (per Emil). Changes since RFC: * Fixed incorrect use of the drive model as pointed out by Greg Kroah-Hartman * Used devm functions as suggested by Keith Busch * Added a handful of sysfs attributes to the switchtec class * Added a handful of IOCTLs to the switchtec device * A number of miscellaneous bug fixes -- Hi, This is a continuation of the RFC we posted lasted month [1] which proposes a management driver for Microsemi's Switchtec line of PCI switches. This hardware is still looking to be used in the Open Compute Platform To make this entirely clear: the Switchtec products are compliant with the PCI specifications and are supported today with the standard in-kernel driver. However, these devices also expose a management endpoint on a separate PCI function address which can be used to perform some advanced operations. This is a driver for that function. See the patch for more information. Since the RFC, we've made the changes requested by Greg Kroah-Hartman and Keith Busch, and we've also fleshed out a number of features. We've added a couple of IOCTLs and sysfs attributes which are documented in the patch. Significant work has also been done on the userspace tool which is available under a GPL license at [2]. We've also had testing done by some of the interested parties. We hope to see this work included in either 4.11 or 4.12 assuming a smooth review process. The patch is based off of the v4.10-rc6 release. Thanks for your review, Logan [1] https://urldefense.proofpoint.com/v2/url?u=https-3A__www.spinics.net_lists_linux-2Dpci_msg56897.html&d=DwIBAg&c=5VD0RTtNlTh3ycd41b3MUw&r=LRFoLl92zWj5mkgkc_hRcg&m=VLDBJqzotzGkTj8-xjlfT-J0k2uFq6FcWg2nA_oKYJo&s=OkigHoSqH1Z3dnmLqN76lIQ_WxRJDj1uqIDl35SI58A&e= [2] https://github.com/sbates130272/switchtec-user -- Logan Gunthorpe (4): MicroSemi Switchtec management interface driver switchtec: Add user interface documentation switchtec: Add sysfs attributes to the Switchtec driver switchtec: Add IOCTLs to the Switchtec driver Documentation/ABI/testing/sysfs-class-switchtec | 96 ++ Documentation/ioctl/ioctl-number.txt|1 + Documentation/switchtec.txt | 80 ++ MAINTAINERS | 11 + drivers/pci/Kconfig |1 + drivers/pci/Makefile
Re: [PATCH v2 0/4] New Microsemi PCI Switch Management Driver
Hi, The switchtec driver patches [PATCH v2 (0-4)/4] in conjunction with the switchtec userland tool is used to communicate with the Microsemi 8536 PCIe Switch used on Facebook’s Lightning platform. The following essential driver and tool functions were successfully tested with it: · Retrieval of firmware and configuration information along with CRCs · Switch firmware and configuration upgrades · Switch PHY/Link error counter collection and the ability to reset them · Switch upstream and downstream ports’ link status reporting · Switch interface functioning · Retrieval of switch ASIC temperature · Exporting switch firmware log dump · Read the information of firmware and configuration binaries · Extract the firmware and configuration images stored in the switch EEPROM Tested-by: Krishna Dhulipala krish...@fb.com Tested-by: Wei Zhang wzh...@fb.com Thanks, -Wei -- wei zhang | software engineer | facebook wzh...@fb.com | (408) 460-4803 On 2/2/17, 10:05 AM, "Logan Gunthorpe" wrote: Changes since v1: * Rebased onto 4.10-rc6 (cleanly) * Split the patch into a few more easily digestible patches (as suggested by Greg Kroah-Hartman) * Folded switchtec.c into switchtec.h (per Greg) * Fixed a bunch of 32bit build warnings caught by the kbuild test robot * Fixed some issues in the documentation so it has a proper reStructredText format (as noted by Jonathan Corbet) * Fixed padding and sizes in the IOCTL structures as noticed by Emil Velikov and used pahole to verify their consistency across 32 and 64 bit builds * Reworked one of the IOCTL interfaces to be more future proof (per Emil). Changes since RFC: * Fixed incorrect use of the drive model as pointed out by Greg Kroah-Hartman * Used devm functions as suggested by Keith Busch * Added a handful of sysfs attributes to the switchtec class * Added a handful of IOCTLs to the switchtec device * A number of miscellaneous bug fixes -- Hi, This is a continuation of the RFC we posted lasted month [1] which proposes a management driver for Microsemi's Switchtec line of PCI switches. This hardware is still looking to be used in the Open Compute Platform To make this entirely clear: the Switchtec products are compliant with the PCI specifications and are supported today with the standard in-kernel driver. However, these devices also expose a management endpoint on a separate PCI function address which can be used to perform some advanced operations. This is a driver for that function. See the patch for more information. Since the RFC, we've made the changes requested by Greg Kroah-Hartman and Keith Busch, and we've also fleshed out a number of features. We've added a couple of IOCTLs and sysfs attributes which are documented in the patch. Significant work has also been done on the userspace tool which is available under a GPL license at [2]. We've also had testing done by some of the interested parties. We hope to see this work included in either 4.11 or 4.12 assuming a smooth review process. The patch is based off of the v4.10-rc6 release. Thanks for your review, Logan [1] https://urldefense.proofpoint.com/v2/url?u=https-3A__www.spinics.net_lists_linux-2Dpci_msg56897.html&d=DwIBAg&c=5VD0RTtNlTh3ycd41b3MUw&r=LRFoLl92zWj5mkgkc_hRcg&m=VLDBJqzotzGkTj8-xjlfT-J0k2uFq6FcWg2nA_oKYJo&s=OkigHoSqH1Z3dnmLqN76lIQ_WxRJDj1uqIDl35SI58A&e= [2] https://github.com/sbates130272/switchtec-user -- Logan Gunthorpe (4): MicroSemi Switchtec management interface driver switchtec: Add user interface documentation switchtec: Add sysfs attributes to the Switchtec driver switchtec: Add IOCTLs to the Switchtec driver Documentation/ABI/testing/sysfs-class-switchtec | 96 ++ Documentation/ioctl/ioctl-number.txt|1 + Documentation/switchtec.txt | 80 ++ MAINTAINERS | 11 + drivers/pci/Kconfig |1 + drivers/pci/Makefile|1 + drivers/pci/switch/Kconfig | 13 + drivers/pci/switch/Makefile |1 + drivers/pci/switch/switchtec.c | 1608 +++ include/uapi/linux/switchtec_ioctl.h| 132 ++ 10 files changed, 1944 insertions(+) create mode 100644 Documentation/ABI/testing/sysfs-class-switchtec create mode 100644 Documentation/switchtec.txt create mode 100644 drivers/pci/switch/Kconfig create mode 100644 drivers/pci/switch/Makefile create mode 100644 drivers/pci/switch/switchtec.c create mode 100644 include/uapi/linux/switchtec_ioct
Re: [PATCH v2] pci: drop link_reset
On Tue, Jan 24, 2017 at 07:35:56PM +0200, Michael S. Tsirkin wrote: > No hardware seems to actually call link_reset, and > no driver implements it as more than a nop stub. > > This drops the mentions of the callback from everywhere. > It's dropped from the documentation as well, but > the doc really needs to be updated to reflect > reality better (e.g. on pcie slot reset is the link reset). > > This will be done in a later patch. > > Signed-off-by: Michael S. Tsirkin Applied to pci/aer for v4.11, thanks. > --- > > changes from v2: > - drop from genwqe as well > > Note: Doug has patches dropping the implementation from infiniband card > drivers in his tree already. This is unlikely to cause conflicts though. > > Documentation/PCI/pci-error-recovery.txt | 24 +++- > drivers/infiniband/hw/hfi1/pcie.c| 10 -- > drivers/infiniband/hw/qib/qib_pcie.c | 8 > drivers/media/pci/ngene/ngene-cards.c| 7 --- > drivers/misc/genwqe/card_base.c | 1 - > include/linux/pci.h | 3 --- > 6 files changed, 3 insertions(+), 50 deletions(-) > > diff --git a/Documentation/PCI/pci-error-recovery.txt > b/Documentation/PCI/pci-error-recovery.txt > index ac26869..da3b217 100644 > --- a/Documentation/PCI/pci-error-recovery.txt > +++ b/Documentation/PCI/pci-error-recovery.txt > @@ -78,7 +78,6 @@ struct pci_error_handlers > { > int (*error_detected)(struct pci_dev *dev, enum pci_channel_state); > int (*mmio_enabled)(struct pci_dev *dev); > - int (*link_reset)(struct pci_dev *dev); > int (*slot_reset)(struct pci_dev *dev); > void (*resume)(struct pci_dev *dev); > }; > @@ -104,8 +103,7 @@ if it implements any, it must implement error_detected(). > If a callback > is not implemented, the corresponding feature is considered unsupported. > For example, if mmio_enabled() and resume() aren't there, then it > is assumed that the driver is not doing any direct recovery and requires > -a slot reset. If link_reset() is not implemented, the card is assumed to > -not care about link resets. Typically a driver will want to know about > +a slot reset. Typically a driver will want to know about > a slot_reset(). > > The actual steps taken by a platform to recover from a PCI error > @@ -232,25 +230,9 @@ proceeds to STEP 4 (Slot Reset) > > STEP 3: Link Reset > -- > -The platform resets the link, and then calls the link_reset() callback > -on all affected device drivers. This is a PCI-Express specific state > +The platform resets the link. This is a PCI-Express specific step > and is done whenever a non-fatal error has been detected that can be > -"solved" by resetting the link. This call informs the driver of the > -reset and the driver should check to see if the device appears to be > -in working condition. > - > -The driver is not supposed to restart normal driver I/O operations > -at this point. It should limit itself to "probing" the device to > -check its recoverability status. If all is right, then the platform > -will call resume() once all drivers have ack'd link_reset(). > - > - Result codes: > - (identical to STEP 3 (MMIO Enabled) > - > -The platform then proceeds to either STEP 4 (Slot Reset) or STEP 5 > -(Resume Operations). > - > ->>> The current powerpc implementation does not implement this callback. > +"solved" by resetting the link. > > STEP 4: Slot Reset > -- > diff --git a/drivers/infiniband/hw/hfi1/pcie.c > b/drivers/infiniband/hw/hfi1/pcie.c > index 4ac8f33..ebd941f 100644 > --- a/drivers/infiniband/hw/hfi1/pcie.c > +++ b/drivers/infiniband/hw/hfi1/pcie.c > @@ -598,15 +598,6 @@ pci_slot_reset(struct pci_dev *pdev) > return PCI_ERS_RESULT_CAN_RECOVER; > } > > -static pci_ers_result_t > -pci_link_reset(struct pci_dev *pdev) > -{ > - struct hfi1_devdata *dd = pci_get_drvdata(pdev); > - > - dd_dev_info(dd, "HFI1 link_reset function called, ignored\n"); > - return PCI_ERS_RESULT_CAN_RECOVER; > -} > - > static void > pci_resume(struct pci_dev *pdev) > { > @@ -625,7 +616,6 @@ pci_resume(struct pci_dev *pdev) > const struct pci_error_handlers hfi1_pci_err_handler = { > .error_detected = pci_error_detected, > .mmio_enabled = pci_mmio_enabled, > - .link_reset = pci_link_reset, > .slot_reset = pci_slot_reset, > .resume = pci_resume, > }; > diff --git a/drivers/infiniband/hw/qib/qib_pcie.c > b/drivers/infiniband/hw/qib/qib_pcie.c > index 6abe1c6..c379b83 100644 > --- a/drivers/infiniband/hw/qib/qib_pcie.c > +++ b/drivers/infiniband/hw/qib/qib_pcie.c > @@ -682,13 +682,6 @@ qib_pci_slot_reset(struct pci_dev *pdev) > return PCI_ERS_RESULT_CAN_RECOVER; > } > > -static pci_ers_result_t > -qib_pci_link_reset(struct pci_dev *pdev) > -{ > - qib_devinfo(pdev, "QIB link_reset function called, ignored\n"); > - return PCI_ERS_RESULT_CAN_RECOVER; > -} > - > static void > qib_pci_re
Re: make pdfdoc errors with 4.10-rc7
On Thu, 09 Feb 2017, Jim Davis wrote: > On a Fedora 25 system, make pdfdocs is failing with > > [jim@krebstar linux-rc]$ grep -v -i 'warning:' /tmp/make-pdfdocs.err > /data/linux-rc/Documentation/doc-guide/sphinx.rst:110: ERROR: Unknown > target name: "sphinx c domain". This one is caused by 1dc4bbf0b268 ("docs-rst: doc-guide: split the kernel-documentation.rst contents"). Both kernel-doc.rst and sphinx.rst reference Sphinx C Domain, but only kernel-doc.rst copied it over from the source document. sphinx.rst lacks: .. _Sphinx C Domain: http://www.sphinx-doc.org/en/stable/domains.html Not sure if this one is able to eventually cause the pdfdocs failure. BR, Jani. > ./include/net/cfg80211.h:3154: ERROR: Unexpected indentation. > ./include/net/mac80211.h:3214: ERROR: Unexpected indentation. > ./include/net/mac80211.h:3219: ERROR: Unexpected indentation. > ./include/net/mac80211.h:1773: ERROR: Unexpected indentation. > ./kernel/time/timer.c:1240: ERROR: Unexpected indentation. > ./kernel/time/timer.c:1242: ERROR: Unexpected indentation. > ./include/linux/wait.h:124: ERROR: Unexpected indentation. > ./include/linux/spi/spi.h:369: ERROR: Unexpected indentation. > ./drivers/usb/core/message.c:481: ERROR: Unexpected indentation. > /data/linux-rc/Documentation/driver-api/usb.rst:623: ERROR: Unknown > target name: "usb_type". > /data/linux-rc/Documentation/driver-api/usb.rst:623: ERROR: Unknown > target name: "usb_dir". > /data/linux-rc/Documentation/driver-api/usb.rst:623: ERROR: Unknown > target name: "usb_recip". > /data/linux-rc/Documentation/driver-api/usb.rst:689: ERROR: Unknown > target name: "usbdevfs_urb_type". > ./sound/soc/soc-core.c:2508: ERROR: Unknown target name: "snd_soc_daifmt". > ./sound/core/jack.c:312: ERROR: Unknown target name: "snd_jack_btn". > make[2]: *** [linux-user.pdf] Error 1 > make[1]: *** [pdfdocs] Error 2 > make: *** [pdfdocs] Error 2 -- Jani Nikula, Intel Open Source Technology Center -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
make pdfdoc errors with 4.10-rc7
On a Fedora 25 system, make pdfdocs is failing with [jim@krebstar linux-rc]$ grep -v -i 'warning:' /tmp/make-pdfdocs.err /data/linux-rc/Documentation/doc-guide/sphinx.rst:110: ERROR: Unknown target name: "sphinx c domain". ./include/net/cfg80211.h:3154: ERROR: Unexpected indentation. ./include/net/mac80211.h:3214: ERROR: Unexpected indentation. ./include/net/mac80211.h:3219: ERROR: Unexpected indentation. ./include/net/mac80211.h:1773: ERROR: Unexpected indentation. ./kernel/time/timer.c:1240: ERROR: Unexpected indentation. ./kernel/time/timer.c:1242: ERROR: Unexpected indentation. ./include/linux/wait.h:124: ERROR: Unexpected indentation. ./include/linux/spi/spi.h:369: ERROR: Unexpected indentation. ./drivers/usb/core/message.c:481: ERROR: Unexpected indentation. /data/linux-rc/Documentation/driver-api/usb.rst:623: ERROR: Unknown target name: "usb_type". /data/linux-rc/Documentation/driver-api/usb.rst:623: ERROR: Unknown target name: "usb_dir". /data/linux-rc/Documentation/driver-api/usb.rst:623: ERROR: Unknown target name: "usb_recip". /data/linux-rc/Documentation/driver-api/usb.rst:689: ERROR: Unknown target name: "usbdevfs_urb_type". ./sound/soc/soc-core.c:2508: ERROR: Unknown target name: "snd_soc_daifmt". ./sound/core/jack.c:312: ERROR: Unknown target name: "snd_jack_btn". make[2]: *** [linux-user.pdf] Error 1 make[1]: *** [pdfdocs] Error 2 make: *** [pdfdocs] Error 2 -- Jim -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] i2c: i2c-mux-gpio: rename i2c-gpio-mux to i2c-mux-gpio
On Tue, Feb 07, 2017 at 10:41:55PM +0100, Peter Rosin wrote: > The rename did the wrong thing for this documentation file all those > years ago. Fix that as well as the neglected rename of the platform > data structure. > > Fixes: e7065e20d9a6 ("i2c: Rename last mux driver to standard pattern") > Signed-off-by: Peter Rosin Applied to for-next, thanks! signature.asc Description: PGP signature
Re: [PATCH v2] time: Remove CONFIG_TIMER_STATS
On Wed, 8 Feb 2017, Dave Jones wrote: > On Wed, Feb 08, 2017 at 11:54:30AM -0800, Linus Torvalds wrote: > > On Wed, Feb 8, 2017 at 11:26 AM, Kees Cook wrote: > > > > > > Given that the tracer can give the same information, this patch entirely > > > removes CONFIG_TIMER_STATS. > > > > > > Suggested-by: Thomas Gleixner > > > Signed-off-by: Kees Cook > > > Acked-by: John Stultz > > > > Looks good to me. Let's wait for the 4.11 merge window though. I'm > > assuming I'll get this through the tip timer tree.. > > >From a quick look at the source, powertop uses this file, and appears to > handle it gracefully if it fails to open, but it will lose functionality > to determine if a timer is deferred. The lookup happens when evaluating a timer_expire_entry tracing event. With current kernels the same information can be retrieved from the timer_start tracing event (flags field). And that's way more sensible than scanning timer_stats because when the trace is evaluated there is no guarantee at all that the timer is still queued and exposed there. That should be trivial to fix in powertop. Thanks, tglx -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [kernel-hardening] [PATCH v2] time: Remove CONFIG_TIMER_STATS
Hi, Don't forget to send to linux-...@vger.kernel.org Le mercredi 08 février 2017 à 11:26 -0800, Kees Cook a écrit : > Currently CONFIG_TIMER_STATS exposes process information across > namespaces: > > kernel/time/timer_list.c print_timer(): > > SEQ_printf(m, ", %s/%d", tmp, timer->start_pid); > > /proc/timer_list: > > #11: <>, hrtimer_wakeup, S:01, do_nanosleep, > cron/2570 > > Given that the tracer can give the same information, this patch > entirely > removes CONFIG_TIMER_STATS. > > Suggested-by: Thomas Gleixner > Signed-off-by: Kees Cook > Acked-by: John Stultz > --- > v2: > - dropped doc comments for removed structure elements; thx 0-day > builder. > --- > Documentation/timers/timer_stats.txt | 73 -- > include/linux/hrtimer.h | 11 - > include/linux/timer.h| 45 > kernel/kthread.c | 1 - > kernel/time/Makefile | 1 - > kernel/time/hrtimer.c| 38 > kernel/time/timer.c | 48 +--- > kernel/time/timer_list.c | 10 - > kernel/time/timer_stats.c| 425 --- > > kernel/workqueue.c | 2 - > lib/Kconfig.debug| 14 -- > 11 files changed, 2 insertions(+), 666 deletions(-) > delete mode 100644 Documentation/timers/timer_stats.txt > delete mode 100644 kernel/time/timer_stats.c > > diff --git a/Documentation/timers/timer_stats.txt > b/Documentation/timers/timer_stats.txt > deleted file mode 100644 > index de835ee97455.. > --- a/Documentation/timers/timer_stats.txt > +++ /dev/null > @@ -1,73 +0,0 @@ > -timer_stats - timer usage statistics > - > - > -timer_stats is a debugging facility to make the timer (ab)usage in a > Linux > -system visible to kernel and userspace developers. If enabled in the > config > -but not used it has almost zero runtime overhead, and a relatively > small > -data structure overhead. Even if collection is enabled runtime all > the > -locking is per-CPU and lookup is hashed. > - > -timer_stats should be used by kernel and userspace developers to > verify that > -their code does not make unduly use of timers. This helps to avoid > unnecessary > -wakeups, which should be avoided to optimize power consumption. > - > -It can be enabled by CONFIG_TIMER_STATS in the "Kernel hacking" > configuration > -section. > - > -timer_stats collects information about the timer events which are > fired in a > -Linux system over a sample period: > - > -- the pid of the task(process) which initialized the timer > -- the name of the process which initialized the timer > -- the function where the timer was initialized > -- the callback function which is associated to the timer > -- the number of events (callbacks) > - > -timer_stats adds an entry to /proc: /proc/timer_stats > - > -This entry is used to control the statistics functionality and to > read out the > -sampled information. > - > -The timer_stats functionality is inactive on bootup. > - > -To activate a sample period issue: > -# echo 1 >/proc/timer_stats > - > -To stop a sample period issue: > -# echo 0 >/proc/timer_stats > - > -The statistics can be retrieved by: > -# cat /proc/timer_stats > - > -While sampling is enabled, each readout from /proc/timer_stats will > see > -newly updated statistics. Once sampling is disabled, the sampled > information > -is kept until a new sample period is started. This allows multiple > readouts. > - > -Sample output of /proc/timer_stats: > - > -Timerstats sample period: 3.888770 s > - 12, 0 swapper hrtimer_stop_sched_tick > (hrtimer_sched_tick) > - 15, 1 swapper hcd_submit_urb (rh_timer_func) > - 4, 959 kedacschedule_timeout (process_timeout) > - 1, 0 swapper page_writeback_init (wb_timer_fn) > - 28, 0 swapper hrtimer_stop_sched_tick > (hrtimer_sched_tick) > - 22, 2948 IRQ 4tty_flip_buffer_push > (delayed_work_timer_fn) > - 3, 3100 bash schedule_timeout (process_timeout) > - 1, 1 swapper queue_delayed_work_on > (delayed_work_timer_fn) > - 1, 1 swapper queue_delayed_work_on > (delayed_work_timer_fn) > - 1, 1 swapper neigh_table_init_no_netlink > (neigh_periodic_timer) > - 1, 2292 ip __netdev_watchdog_up (dev_watchdog) > - 1,23 events/1 do_cache_clean (delayed_work_timer_fn) > -90 total events, 30.0 events/sec > - > -The first column is the number of events, the second column the pid, > the third > -column is the name of the process. The forth column shows the > function which > -initialized the timer and in parenthesis the callback function which > was > -executed on expiry. > - > -Thomas, Ingo > - > -Added flag to indicate 'deferrable timer' in /proc/timer_stats. A > deferrable > -timer will appear as follows > - 10D, 1 swapper