On 02/09/2017 11:31 PM, Joel Stanley wrote:
On Wed, Feb 8, 2017 at 9:40 AM, <eajames@xxxxxxxxxxxxxxxxxx> wrote:
diff --git a/Documentation/hwmon/occ b/Documentation/hwmon/occ
new file mode 100644
index 0000000..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 <eajames@xxxxxxxxxx>
+L: linux-hwmon@xxxxxxxxxxxxxxx
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 <kyungmin.park@xxxxxxxxxxx>
L: linux-mtd@xxxxxxxxxxxxxxxxxxx
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 0000000..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?
Yes. The sensor data shouldn't change, as far as I know. I think
something would be wrong if it did.
+ 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)
+{
+ u8 resp[8];
+
+ cap = cpu_to_be16(cap);
+
+ return occ_send_cmd(occ, 0, OCC_SET_USER_POWR_CAP, 2, (const u8 *)&cap,
+ resp);
+}
+EXPORT_SYMBOL(occ_set_user_powercap);
+
+struct occ *occ_start(struct device *dev, void *bus,
From what I can tell this doesn't start anything. Call it occ_init()
or something.
+ struct occ_bus_ops *bus_ops, const struct occ_ops *ops,
+ const struct occ_config *config)
Create a structure with all of these details in it. Some of them don't
need to be broken out into their own, for instance:
struct occ *occ_start(struct device *dev, const struct occ_init_context *init)
{
driver->dev = dev;
driver->bus = init->bus;
driver->bus_read = init->bus_read;
driver->bus_write = init->bus_write;
driver->config = init->config;
driver->cmd_addr = init->cmd_addr;
etc.
+{
+ struct occ *driver = devm_kzalloc(dev, sizeof(struct occ), GFP_KERNEL);
+
+ if (!driver)
+ return ERR_PTR(-ENOMEM);
+
+ driver->dev = dev;
+ driver->bus = bus;
+ driver->bus_ops = *bus_ops;
+ driver->ops = *ops;
+ driver->config = *config;
+ driver->raw_data = devm_kzalloc(dev, OCC_DATA_MAX, GFP_KERNEL);
+ if (!driver->raw_data)
+ return ERR_PTR(-ENOMEM);
+
+ mutex_init(&driver->update_lock);
+
+ return driver;
+}
+EXPORT_SYMBOL(occ_start);
+
+MODULE_AUTHOR("Eddie James <eajames@xxxxxxxxxx>");
+MODULE_DESCRIPTION("OCC hwmon core driver");
+MODULE_LICENSE("GPL");
diff --git a/drivers/hwmon/occ/scom.h b/drivers/hwmon/occ/scom.h
new file mode 100644
index 0000000..b0691f3
--- /dev/null
+++ b/drivers/hwmon/occ/scom.h
@@ -0,0 +1,47 @@
+/*
+ * scom.h - hwmon OCC driver
+ *
+ * This file contains data structures for scom operations to the OCC
Are these really SCOM operations?
I think they're better described read and write callbacks, as the
operation is may be talking i2c or FSI or in the future some other
kind of access.
They do take scom addresses as parameters, so I can see the argument
for calling them getscom/putscom.
These are scom operations. The only way to talk to the OCC is with a
scom operation as I understand it. The transport mechanism can change
(i2c or fsi/sbe), but it's still a "scom."
+ *
+ * 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.
+ */
+
+#ifndef __SCOM_H__
+#define __SCOM_H__
+
+/*
+ * occ_bus_ops - represent the low-level transfer methods to communicate with
+ * the OCC.
+ *
+ * getscom - OCC scom read
+ * @bus: handle to slave device
+ * @address: address
+ * @data: where to store data read from slave; buffer size must be at least
+ * eight bytes.
Are there situations where it's more than 8 bytes?
Would it be safer to add a length argument so the read call so we
don't put more data in the buffer than the caller expects?
I guess the documentation isn't that clear, but it's always an 8 byte
operation. I said "at least" 8 bytes because the calling code calls this
function repeatedly with one large buffer at different offsets. I'll add
a comment that it's always 8 bytes.
+ *
+ * Returns 0 on success or a negative errno on error
+ *
+ * putscom - OCC scom write
+ * @bus: handle to slave device
+ * @address: address
+ * @data0: first data word to write
+ * @data1: second data word to write
+ *
+ * Returns 0 on success or a negative errno on error
+ */
+struct occ_bus_ops {
+ int (*getscom)(void *bus, u32 address, u64 *data);
+ int (*putscom)(void *bus, u32 address, u32 data0, u32 data1);
+};
+
+#endif /* __SCOM_H__ */
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
