| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20180316185418.GA31981@roeck-us.net>
Date: Fri, 16 Mar 2018 11:54:18 -0700
From: Guenter Roeck <linux@...ck-us.net>
To: Eddie James <eajames@...ux.vnet.ibm.com>
Cc: linux-hwmon@...r.kernel.org, linux-kernel@...r.kernel.org,
jdelvare@...e.com, joel@....id.au, andy.shevchenko@...il.com,
Christopher Bostic <cbostic@...ux.vnet.ibm.com>,
Andrew Jeffery <andrew@...id.au>
Subject: Re: [PATCH v5 1/2] hwmon: (ucd9000) Add gpio chip interface
On Fri, Mar 16, 2018 at 01:39:48PM -0500, Eddie James wrote:
>
>
> On 03/16/2018 08:40 AM, Guenter Roeck wrote:
> >On 03/15/2018 03:21 PM, Eddie James wrote:
> >>From: Christopher Bostic <cbostic@...ux.vnet.ibm.com>
> >>
> >>Add a struct gpio_chip and define some methods so that this device's
> >>I/O can be accessed via /sys/class/gpio.
> >>
> >
> >Sorry for not noticing earlier. The 0day reports should be addressed by
> >selecting GPIOLIB
> >in the Kconfig entry.
>
> Getting kbuild recursive dependencies when I select GPIOLIB for ucd9000 :(
>
Sure, it would have been too easy otherwise.
> May have to do "depends on" instead and #ifdef GPIOLIB in ucd9000, unless
> you have another recommendation?
>
Good news is that the code is all in one code block in the driver,
so make it #ifdef and
...
#else
static void ucd9000_probe_gpio(...) { }
#endif
With that, you won't need "depends on ..."
Thanks,
Guenter
> Thanks
> Eddie
>
> >
> >Guenter
> >
> >>Signed-off-by: Christopher Bostic <cbostic@...ux.vnet.ibm.com>
> >>Signed-off-by: Andrew Jeffery <andrew@...id.au>
> >>Signed-off-by: Eddie James <eajames@...ux.vnet.ibm.com>
> >>---
> >> drivers/hwmon/pmbus/ucd9000.c | 201
> >>++++++++++++++++++++++++++++++++++++++++++
> >> 1 file changed, 201 insertions(+)
> >>
> >>diff --git a/drivers/hwmon/pmbus/ucd9000.c
> >>b/drivers/hwmon/pmbus/ucd9000.c
> >>index b74dbec..a34ffc4 100644
> >>--- a/drivers/hwmon/pmbus/ucd9000.c
> >>+++ b/drivers/hwmon/pmbus/ucd9000.c
> >>@@ -27,6 +27,7 @@
> >> #include <linux/slab.h>
> >> #include <linux/i2c.h>
> >> #include <linux/pmbus.h>
> >>+#include <linux/gpio.h>
> >> #include "pmbus.h"
> >> enum chips { ucd9000, ucd90120, ucd90124, ucd90160, ucd9090,
> >>ucd90910 };
> >>@@ -35,8 +36,18 @@
> >> #define UCD9000_NUM_PAGES 0xd6
> >> #define UCD9000_FAN_CONFIG_INDEX 0xe7
> >> #define UCD9000_FAN_CONFIG 0xe8
> >>+#define UCD9000_GPIO_SELECT 0xfa
> >>+#define UCD9000_GPIO_CONFIG 0xfb
> >> #define UCD9000_DEVICE_ID 0xfd
> >> +/* GPIO CONFIG bits */
> >>+#define UCD9000_GPIO_CONFIG_ENABLE BIT(0)
> >>+#define UCD9000_GPIO_CONFIG_OUT_ENABLE BIT(1)
> >>+#define UCD9000_GPIO_CONFIG_OUT_VALUE BIT(2)
> >>+#define UCD9000_GPIO_CONFIG_STATUS BIT(3)
> >>+#define UCD9000_GPIO_INPUT 0
> >>+#define UCD9000_GPIO_OUTPUT 1
> >>+
> >> #define UCD9000_MON_TYPE(x) (((x) >> 5) & 0x07)
> >> #define UCD9000_MON_PAGE(x) ((x) & 0x0f)
> >> @@ -47,9 +58,15 @@
> >> #define UCD9000_NUM_FAN 4
> >> +#define UCD9000_GPIO_NAME_LEN 16
> >>+#define UCD9090_NUM_GPIOS 23
> >>+#define UCD901XX_NUM_GPIOS 26
> >>+#define UCD90910_NUM_GPIOS 26
> >>+
> >> struct ucd9000_data {
> >> u8 fan_data[UCD9000_NUM_FAN][I2C_SMBUS_BLOCK_MAX];
> >> struct pmbus_driver_info info;
> >>+ struct gpio_chip gpio;
> >> };
> >> #define to_ucd9000_data(_info) container_of(_info, struct
> >>ucd9000_data, info)
> >> @@ -149,6 +166,188 @@ static int ucd9000_read_byte_data(struct
> >>i2c_client *client, int page, int reg)
> >> };
> >> MODULE_DEVICE_TABLE(of, ucd9000_of_match);
> >> +static int ucd9000_gpio_read_config(struct i2c_client *client,
> >>+ unsigned int offset)
> >>+{
> >>+ int ret;
> >>+
> >>+ /* No page set required */
> >>+ ret = i2c_smbus_write_byte_data(client, UCD9000_GPIO_SELECT,
> >>offset);
> >>+ if (ret < 0)
> >>+ return ret;
> >>+
> >>+ return i2c_smbus_read_byte_data(client, UCD9000_GPIO_CONFIG);
> >>+}
> >>+
> >>+static int ucd9000_gpio_get(struct gpio_chip *gc, unsigned int offset)
> >>+{
> >>+ struct i2c_client *client = gpiochip_get_data(gc);
> >>+ int ret;
> >>+
> >>+ ret = ucd9000_gpio_read_config(client, offset);
> >>+ if (ret < 0)
> >>+ return ret;
> >>+
> >>+ return !!(ret & UCD9000_GPIO_CONFIG_STATUS);
> >>+}
> >>+
> >>+static void ucd9000_gpio_set(struct gpio_chip *gc, unsigned int offset,
> >>+ int value)
> >>+{
> >>+ struct i2c_client *client = gpiochip_get_data(gc);
> >>+ int ret;
> >>+
> >>+ ret = ucd9000_gpio_read_config(client, offset);
> >>+ if (ret < 0) {
> >>+ dev_dbg(&client->dev, "failed to read GPIO %d config: %d\n",
> >>+ offset, ret);
> >>+ return;
> >>+ }
> >>+
> >>+ if (value) {
> >>+ if (ret & UCD9000_GPIO_CONFIG_STATUS)
> >>+ return;
> >>+
> >>+ ret |= UCD9000_GPIO_CONFIG_STATUS;
> >>+ } else {
> >>+ if (!(ret & UCD9000_GPIO_CONFIG_STATUS))
> >>+ return;
> >>+
> >>+ ret &= ~UCD9000_GPIO_CONFIG_STATUS;
> >>+ }
> >>+
> >>+ ret |= UCD9000_GPIO_CONFIG_ENABLE;
> >>+
> >>+ /* Page set not required */
> >>+ ret = i2c_smbus_write_byte_data(client, UCD9000_GPIO_CONFIG, ret);
> >>+ if (ret < 0) {
> >>+ dev_dbg(&client->dev, "Failed to write GPIO %d config: %d\n",
> >>+ offset, ret);
> >>+ return;
> >>+ }
> >>+
> >>+ ret &= ~UCD9000_GPIO_CONFIG_ENABLE;
> >>+
> >>+ ret = i2c_smbus_write_byte_data(client, UCD9000_GPIO_CONFIG, ret);
> >>+ if (ret < 0)
> >>+ dev_dbg(&client->dev, "Failed to write GPIO %d config: %d\n",
> >>+ offset, ret);
> >>+}
> >>+
> >>+static int ucd9000_gpio_get_direction(struct gpio_chip *gc,
> >>+ unsigned int offset)
> >>+{
> >>+ struct i2c_client *client = gpiochip_get_data(gc);
> >>+ int ret;
> >>+
> >>+ ret = ucd9000_gpio_read_config(client, offset);
> >>+ if (ret < 0)
> >>+ return ret;
> >>+
> >>+ return !(ret & UCD9000_GPIO_CONFIG_OUT_ENABLE);
> >>+}
> >>+
> >>+static int ucd9000_gpio_set_direction(struct gpio_chip *gc,
> >>+ unsigned int offset, bool direction_out,
> >>+ int requested_out)
> >>+{
> >>+ struct i2c_client *client = gpiochip_get_data(gc);
> >>+ int ret, config, out_val;
> >>+
> >>+ ret = ucd9000_gpio_read_config(client, offset);
> >>+ if (ret < 0)
> >>+ return ret;
> >>+
> >>+ if (direction_out) {
> >>+ out_val = requested_out ? UCD9000_GPIO_CONFIG_OUT_VALUE : 0;
> >>+
> >>+ if (ret & UCD9000_GPIO_CONFIG_OUT_ENABLE) {
> >>+ if ((ret & UCD9000_GPIO_CONFIG_OUT_VALUE) == out_val)
> >>+ return 0;
> >>+ } else {
> >>+ ret |= UCD9000_GPIO_CONFIG_OUT_ENABLE;
> >>+ }
> >>+
> >>+ if (out_val)
> >>+ ret |= UCD9000_GPIO_CONFIG_OUT_VALUE;
> >>+ else
> >>+ ret &= ~UCD9000_GPIO_CONFIG_OUT_VALUE;
> >>+
> >>+ } else {
> >>+ if (!(ret & UCD9000_GPIO_CONFIG_OUT_ENABLE))
> >>+ return 0;
> >>+
> >>+ ret &= ~UCD9000_GPIO_CONFIG_OUT_ENABLE;
> >>+ }
> >>+
> >>+ ret |= UCD9000_GPIO_CONFIG_ENABLE;
> >>+ config = ret;
> >>+
> >>+ /* Page set not required */
> >>+ ret = i2c_smbus_write_byte_data(client, UCD9000_GPIO_CONFIG,
> >>config);
> >>+ if (ret < 0)
> >>+ return ret;
> >>+
> >>+ config &= ~UCD9000_GPIO_CONFIG_ENABLE;
> >>+
> >>+ return i2c_smbus_write_byte_data(client, UCD9000_GPIO_CONFIG,
> >>config);
> >>+}
> >>+
> >>+static int ucd9000_gpio_direction_input(struct gpio_chip *gc,
> >>+ unsigned int offset)
> >>+{
> >>+ return ucd9000_gpio_set_direction(gc, offset, UCD9000_GPIO_INPUT,
> >>0);
> >>+}
> >>+
> >>+static int ucd9000_gpio_direction_output(struct gpio_chip *gc,
> >>+ unsigned int offset, int val)
> >>+{
> >>+ return ucd9000_gpio_set_direction(gc, offset, UCD9000_GPIO_OUTPUT,
> >>+ val);
> >>+}
> >>+
> >>+static void ucd9000_probe_gpio(struct i2c_client *client,
> >>+ const struct i2c_device_id *mid,
> >>+ struct ucd9000_data *data)
> >>+{
> >>+ int rc;
> >>+
> >>+ switch (mid->driver_data) {
> >>+ case ucd9090:
> >>+ data->gpio.ngpio = UCD9090_NUM_GPIOS;
> >>+ break;
> >>+ case ucd90120:
> >>+ case ucd90124:
> >>+ case ucd90160:
> >>+ data->gpio.ngpio = UCD901XX_NUM_GPIOS;
> >>+ break;
> >>+ case ucd90910:
> >>+ data->gpio.ngpio = UCD90910_NUM_GPIOS;
> >>+ break;
> >>+ default:
> >>+ return; /* GPIO support is optional. */
> >>+ }
> >>+
> >>+ /*
> >>+ * Pinmux support has not been added to the new gpio_chip.
> >>+ * This support should be added when possible given the mux
> >>+ * behavior of these IO devices.
> >>+ */
> >>+ data->gpio.label = client->name;
> >>+ data->gpio.get_direction = ucd9000_gpio_get_direction;
> >>+ data->gpio.direction_input = ucd9000_gpio_direction_input;
> >>+ data->gpio.direction_output = ucd9000_gpio_direction_output;
> >>+ data->gpio.get = ucd9000_gpio_get;
> >>+ data->gpio.set = ucd9000_gpio_set;
> >>+ data->gpio.can_sleep = true;
> >>+ data->gpio.base = -1;
> >>+ data->gpio.parent = &client->dev;
> >>+
> >>+ rc = devm_gpiochip_add_data(&client->dev, &data->gpio, client);
> >>+ if (rc)
> >>+ dev_warn(&client->dev, "Could not add gpiochip: %d\n", rc);
> >>+}
> >>+
> >> static int ucd9000_probe(struct i2c_client *client,
> >> const struct i2c_device_id *id)
> >> {
> >>@@ -263,6 +462,8 @@ static int ucd9000_probe(struct i2c_client *client,
> >> | PMBUS_HAVE_FAN34 | PMBUS_HAVE_STATUS_FAN34;
> >> }
> >> + ucd9000_probe_gpio(client, mid, data);
> >>+
> >> return pmbus_do_probe(client, mid, info);
> >> }
> >>
> >
>
Powered by blists - more mailing lists