Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Fri, 7 Aug 2020 09:05:54 -0700
From:   Guenter Roeck <linux@roeck-us.net>
To:     alexandru.tachici@analog.com
Cc:     linux-hwmon@vger.kernel.org, linux-kernel@vger.kernel.org,
        devicetree@vger.kernel.org, robh+dt@kernel.org
Subject: Re: [PATCH v7 3/9] hwmon: pmbus: adm1266: Add support for GPIOs
Message-ID: <20200807160554.GA40857@roeck-us.net>
References: <20200727161928.14122-1-alexandru.tachici@analog.com>
 <20200727161928.14122-4-alexandru.tachici@analog.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20200727161928.14122-4-alexandru.tachici@analog.com>
User-Agent: Mutt/1.9.4 (2018-02-28)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Mon, Jul 27, 2020 at 07:19:22PM +0300, alexandru.tachici@analog.com wrote:
> From: Alexandru Tachici <alexandru.tachici@analog.com>
> 
> Adm1266 exposes 9 GPIOs and 16 PDIOs which are currently read-only. They
> are controlled by the internal sequencing engine.
> 
> This patch makes adm1266 driver expose GPIOs and PDIOs to user-space
> using GPIO provider kernel api.
> 
> Signed-off-by: Alexandru Tachici <alexandru.tachici@analog.com>

This needs to add "select GPIOLIB" to the driver's Kconfig entry.

> ---
>  drivers/hwmon/pmbus/adm1266.c | 204 ++++++++++++++++++++++++++++++++++
>  1 file changed, 204 insertions(+)
> 
> diff --git a/drivers/hwmon/pmbus/adm1266.c b/drivers/hwmon/pmbus/adm1266.c
> index 63975eba34ad..be911de02cf6 100644
> --- a/drivers/hwmon/pmbus/adm1266.c
> +++ b/drivers/hwmon/pmbus/adm1266.c
> @@ -6,18 +6,42 @@
>   * Copyright 2020 Analog Devices Inc.
>   */
>  
> +#include <linux/bitfield.h>
>  #include <linux/crc8.h>
> +#include <linux/debugfs.h>
> +#include <linux/gpio/driver.h>
>  #include <linux/i2c.h>
> +#include <linux/i2c-smbus.h>
>  #include <linux/init.h>
>  #include <linux/kernel.h>
>  #include <linux/module.h>
>  #include "pmbus.h"
>  #include <linux/slab.h>
>  
> +#define ADM1266_PDIO_CONFIG	0xD4
> +#define ADM1266_GPIO_CONFIG	0xE1
> +#define ADM1266_PDIO_STATUS	0xE9
> +#define ADM1266_GPIO_STATUS	0xEA
> +
> +/* ADM1266 GPIO defines */
> +#define ADM1266_GPIO_NR			9
> +#define ADM1266_GPIO_FUNCTIONS(x)	FIELD_GET(BIT(0), x)
> +#define ADM1266_GPIO_INPUT_EN(x)	FIELD_GET(BIT(2), x)
> +#define ADM1266_GPIO_OUTPUT_EN(x)	FIELD_GET(BIT(3), x)
> +#define ADM1266_GPIO_OPEN_DRAIN(x)	FIELD_GET(BIT(4), x)
> +
> +/* ADM1266 PDIO defines */
> +#define ADM1266_PDIO_NR			16
> +#define ADM1266_PDIO_PIN_CFG(x)		FIELD_GET(GENMASK(15, 13), x)
> +#define ADM1266_PDIO_GLITCH_FILT(x)	FIELD_GET(GENMASK(12, 9), x)
> +#define ADM1266_PDIO_OUT_CFG(x)		FIELD_GET(GENMASK(2, 0), x)
> +
>  #define ADM1266_PMBUS_BLOCK_MAX		255
>  
>  struct adm1266_data {
>  	struct pmbus_driver_info info;
> +	struct gpio_chip gc;
> +	const char *gpio_names[ADM1266_GPIO_NR + ADM1266_PDIO_NR];
>  	struct i2c_client *client;
>  	struct mutex buf_mutex;
>  	u8 write_buf[ADM1266_PMBUS_BLOCK_MAX + 1] ____cacheline_aligned;
> @@ -88,9 +112,185 @@ static int adm1266_pmbus_block_xfer(struct adm1266_data *data, u8 cmd, u8 w_len,
>  	return ret;
>  }
>  
> +static const unsigned int adm1266_gpio_mapping[ADM1266_GPIO_NR][2] = {
> +	{1, 0},
> +	{2, 1},
> +	{3, 2},
> +	{4, 8},
> +	{5, 9},
> +	{6, 10},
> +	{7, 11},
> +	{8, 6},
> +	{9, 7},
> +};
> +
> +static const char *adm1266_names[ADM1266_GPIO_NR + ADM1266_PDIO_NR] = {
> +	"GPIO1", "GPIO2", "GPIO3", "GPIO4", "GPIO5", "GPIO6", "GPIO7", "GPIO8",
> +	"GPIO9", "PDIO1", "PDIO2", "PDIO3", "PDIO4", "PDIO5", "PDIO6",
> +	"PDIO7", "PDIO8", "PDIO9", "PDIO10", "PDIO11", "PDIO12", "PDIO13",
> +	"PDIO14", "PDIO15", "PDIO16",
> +};
> +
> +static int adm1266_gpio_get(struct gpio_chip *chip, unsigned int offset)
> +{
> +	struct adm1266_data *data = gpiochip_get_data(chip);
> +	u8 read_buf[I2C_SMBUS_BLOCK_MAX + 1];
> +	unsigned long pins_status;
> +	unsigned int pmbus_cmd;
> +	int ret;
> +
> +	if (offset < ADM1266_GPIO_NR)
> +		pmbus_cmd = ADM1266_GPIO_STATUS;
> +	else
> +		pmbus_cmd = ADM1266_PDIO_STATUS;
> +
> +	ret = i2c_smbus_read_block_data(data->client, pmbus_cmd, read_buf);
> +	if (ret < 0)
> +		return ret;
> +
> +	pins_status = read_buf[0] + (read_buf[1] << 8);
> +	if (offset < ADM1266_GPIO_NR)
> +		return test_bit(adm1266_gpio_mapping[offset][1], &pins_status);
> +
> +	return test_bit(offset - ADM1266_GPIO_NR, &pins_status);
> +}
> +
> +static int adm1266_gpio_get_multiple(struct gpio_chip *chip, unsigned long *mask,
> +				     unsigned long *bits)
> +{
> +	struct adm1266_data *data = gpiochip_get_data(chip);
> +	u8 read_buf[ADM1266_PMBUS_BLOCK_MAX + 1];
> +	unsigned long status;
> +	unsigned int gpio_nr;
> +	int ret;
> +
> +	ret = i2c_smbus_read_block_data(data->client, ADM1266_GPIO_STATUS, read_buf);
> +	if (ret < 0)
> +		return ret;
> +
> +	status = read_buf[0] + (read_buf[1] << 8);
> +
> +	*bits = 0;
> +	for_each_set_bit(gpio_nr, mask, ADM1266_GPIO_NR) {
> +		if (test_bit(adm1266_gpio_mapping[gpio_nr][1], &status))
> +			set_bit(gpio_nr, bits);
> +	}
> +
> +	ret = i2c_smbus_read_block_data(data->client, ADM1266_PDIO_STATUS, read_buf);
> +	if (ret < 0)
> +		return ret;
> +
> +	status = read_buf[0] + (read_buf[1] << 8);
> +
> +	*bits = 0;
> +	for_each_set_bit_from(gpio_nr, mask, ADM1266_GPIO_NR + ADM1266_PDIO_STATUS) {
> +		if (test_bit(gpio_nr - ADM1266_GPIO_NR, &status))
> +			set_bit(gpio_nr, bits);
> +	}
> +
> +	return 0;
> +}
> +
> +static void adm1266_gpio_dbg_show(struct seq_file *s, struct gpio_chip *chip)
> +{
> +	struct adm1266_data *data = gpiochip_get_data(chip);
> +	u8 read_buf[ADM1266_PMBUS_BLOCK_MAX + 1];
> +	unsigned long gpio_config;
> +	unsigned long pdio_config;
> +	unsigned long pin_cfg;
> +	u8 write_cmd;
> +	int ret;
> +	int i;
> +
> +	for (i = 0; i < ADM1266_GPIO_NR; i++) {
> +		write_cmd = adm1266_gpio_mapping[i][1];
> +		ret = adm1266_pmbus_block_xfer(data, ADM1266_GPIO_CONFIG, 1, &write_cmd, read_buf);
> +		if (ret != 2)
> +			return;
> +
> +		gpio_config = read_buf[0];
> +		seq_puts(s, adm1266_names[i]);
> +
> +		seq_puts(s, " ( ");
> +		if (!ADM1266_GPIO_FUNCTIONS(gpio_config)) {
> +			seq_puts(s, "high-Z )\n");
> +			continue;
> +		}
> +		if (ADM1266_GPIO_INPUT_EN(gpio_config))
> +			seq_puts(s, "input ");
> +		if (ADM1266_GPIO_OUTPUT_EN(gpio_config))
> +			seq_puts(s, "output ");
> +		if (ADM1266_GPIO_OPEN_DRAIN(gpio_config))
> +			seq_puts(s, "open-drain )\n");
> +		else
> +			seq_puts(s, "push-pull )\n");
> +	}
> +
> +	write_cmd = 0xFF;
> +	ret = adm1266_pmbus_block_xfer(data, ADM1266_PDIO_CONFIG, 1, &write_cmd, read_buf);
> +	if (ret != 32)
> +		return;
> +
> +	for (i = 0; i < ADM1266_PDIO_NR; i++) {
> +		seq_puts(s, adm1266_names[ADM1266_GPIO_NR + i]);
> +
> +		pdio_config = read_buf[2 * i];
> +		pdio_config += (read_buf[2 * i + 1] << 8);
> +		pin_cfg = ADM1266_PDIO_PIN_CFG(pdio_config);
> +
> +		seq_puts(s, " ( ");
> +		if (!pin_cfg || pin_cfg > 5) {
> +			seq_puts(s, "high-Z )\n");
> +			continue;
> +		}
> +
> +		if (pin_cfg & BIT(0))
> +			seq_puts(s, "output ");
> +
> +		if (pin_cfg & BIT(1))
> +			seq_puts(s, "input ");
> +
> +		seq_puts(s, ")\n");
> +	}
> +}
> +
> +static int adm1266_config_gpio(struct adm1266_data *data)
> +{
> +	const char *name = dev_name(&data->client->dev);
> +	char *gpio_name;
> +	int ret;
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(data->gpio_names); i++) {
> +		gpio_name = devm_kasprintf(&data->client->dev, GFP_KERNEL, "adm1266-%x-%s",
> +					   data->client->addr, adm1266_names[i]);
> +		if (!gpio_name)
> +			return -ENOMEM;
> +
> +		data->gpio_names[i] = gpio_name;
> +	}
> +
> +	data->gc.label = name;
> +	data->gc.parent = &data->client->dev;
> +	data->gc.owner = THIS_MODULE;
> +	data->gc.base = -1;
> +	data->gc.names = data->gpio_names;
> +	data->gc.ngpio = ARRAY_SIZE(data->gpio_names);
> +	data->gc.get = adm1266_gpio_get;
> +	data->gc.get_multiple = adm1266_gpio_get_multiple;
> +	data->gc.dbg_show = adm1266_gpio_dbg_show;
> +
> +	ret = devm_gpiochip_add_data(&data->client->dev, &data->gc, data);
> +	if (ret)
> +		dev_err(&data->client->dev, "GPIO registering failed (%d)\n", ret);
> +
> +	return ret;
> +}
> +
>  static int adm1266_probe(struct i2c_client *client, const struct i2c_device_id *id)
>  {
>  	struct adm1266_data *data;
> +	int ret;
>  	int i;
>  
>  	data = devm_kzalloc(&client->dev, sizeof(struct adm1266_data), GFP_KERNEL);
> @@ -106,6 +306,10 @@ static int adm1266_probe(struct i2c_client *client, const struct i2c_device_id *
>  	crc8_populate_msb(pmbus_crc_table, 0x7);
>  	mutex_init(&data->buf_mutex);
>  
> +	ret = adm1266_config_gpio(data);
> +	if (ret < 0)
> +		return ret;
> +
>  	return pmbus_do_probe(client, id, &data->info);
>  }
>