From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.1 required=3.0 tests=DKIM_INVALID,DKIM_SIGNED, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 4F461C3A5A6 for ; Fri, 30 Aug 2019 17:36:10 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 1311021721 for ; Fri, 30 Aug 2019 17:36:10 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="f/XzB+Se" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727924AbfH3RgH (ORCPT ); Fri, 30 Aug 2019 13:36:07 -0400 Received: from mail-pl1-f195.google.com ([209.85.214.195]:45461 "EHLO mail-pl1-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727914AbfH3RgH (ORCPT ); Fri, 30 Aug 2019 13:36:07 -0400 Received: by mail-pl1-f195.google.com with SMTP id y8so3664416plr.12; Fri, 30 Aug 2019 10:36:06 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=sender:date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=Pafo73I1UbBRX8XwIZQylqQ4gK9txbwYzG4pKvHI3F8=; b=f/XzB+SeEsVRxFEknXHKYgibFNLpjaG/LE0MjGm3VLZ/Zx4lss9JzcDr/2nlg03iq2 xpFRuoFL+ArKTHR2Ic3szIPLegOknO4mWolM+GdCzrVU4OBEsGiqW4rFjkIXH4cqyGDL ieLiEgR0SqVHWNkpusRL/q6UdVr5P8WNpNH0eVDZZyHnCdk1u9X/9GuPzEvkieTf4yzs AHtSS8HpN+o5mADp30NlUtg0hxe39Cin6Lyou6n4LWNAAHXNcpsSo61E+yLN2NQO6G21 S3GIG4u2zdTtA9BZ8BWj3nWx1F1wvP5WXoWl5xWMVcFtLRwexcy+0ecGQVY91OXPZjof c93Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:sender:date:from:to:cc:subject:message-id :references:mime-version:content-disposition:in-reply-to:user-agent; bh=Pafo73I1UbBRX8XwIZQylqQ4gK9txbwYzG4pKvHI3F8=; b=X3xEBBSGOsyNw8Roupfh9SB/BMH7iEkN0+qOYUSBFBQedSx0LIAZo1njt4UupFPft+ QkGMT/4iKI7Re86JsNv/QRZ0V7neGBJgneLbxw56ryT06GeJEUCXTVotw4fOT0/ar/QE GTM6CmNKlD5h++XkjY1xoy2Xq6VDkQxRB9drtWvPsalT5VWITOOZBCGfnwGdmxquLOLN ku4eOrBeT08smHY9EjmF2+ozR/iBsBeS1J/Av3HBDXCLY7LFwTjwG36ZQb7/insT1CGI ozJTBnT+D+DiqU6lutWUlgoRjBC0wG/5slT34UYF6ixkb9/Ndr28AUIDfvN4bW6T0ssk FrmQ== X-Gm-Message-State: APjAAAVhTaHh+qS/VZ3xanelkuGHydQbRltilqRNfoN0cEFWj/v1BtXQ eyActNkKxPnkFDR4ye8tCQE= X-Google-Smtp-Source: APXvYqxpGugk/VQ+2edUg60V5g3B6e7BsglHIs5QrhYmn1FKo4djBUFftIpEWoFU+ny6R7+GZQawzg== X-Received: by 2002:a17:902:f204:: with SMTP id gn4mr16870589plb.23.1567186566135; Fri, 30 Aug 2019 10:36:06 -0700 (PDT) Received: from localhost ([2600:1700:e321:62f0:329c:23ff:fee3:9d7c]) by smtp.gmail.com with ESMTPSA id r23sm8887772pfg.10.2019.08.30.10.36.04 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 30 Aug 2019 10:36:04 -0700 (PDT) Date: Fri, 30 Aug 2019 10:36:03 -0700 From: Guenter Roeck To: Eddie James Cc: linux-hwmon@vger.kernel.org, linux-kernel@vger.kernel.org, linux-aspeed@lists.ozlabs.org, devicetree@vger.kernel.org, andrew@aj.id.au, joel@jms.id.au, mark.rutland@arm.com, robh+dt@kernel.org, jdelvare@suse.com Subject: Re: [PATCH 3/3] pmbus: ibm-cffps: Add support for version 2 of the PSU Message-ID: <20190830173603.GA10472@roeck-us.net> References: <1567181385-22129-1-git-send-email-eajames@linux.ibm.com> <1567181385-22129-4-git-send-email-eajames@linux.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1567181385-22129-4-git-send-email-eajames@linux.ibm.com> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-hwmon-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-hwmon@vger.kernel.org On Fri, Aug 30, 2019 at 11:09:45AM -0500, Eddie James wrote: > Version 2 of the PSU supports a second page of data and changes the > format of the FW version. Use the devicetree binding to differentiate > between the version the driver should use. > > Signed-off-by: Eddie James > --- > drivers/hwmon/pmbus/ibm-cffps.c | 109 ++++++++++++++++++++++++++++++++-------- > 1 file changed, 87 insertions(+), 22 deletions(-) > > diff --git a/drivers/hwmon/pmbus/ibm-cffps.c b/drivers/hwmon/pmbus/ibm-cffps.c > index ee2ee9e..ca26fbd 100644 > --- a/drivers/hwmon/pmbus/ibm-cffps.c > +++ b/drivers/hwmon/pmbus/ibm-cffps.c > @@ -12,16 +12,20 @@ > #include > #include > #include > +#include > #include > > #include "pmbus.h" > > +#define CFFPS_VERSIONS 2 > + Any chance you can use an enum for the versions ? Using version numbers 1/2 combined with array indices 0/1 is confusing, error prone, and seems unnecessary. Thanks, Guenter > #define CFFPS_FRU_CMD 0x9A > #define CFFPS_PN_CMD 0x9B > #define CFFPS_SN_CMD 0x9E > #define CFFPS_CCIN_CMD 0xBD > -#define CFFPS_FW_CMD_START 0xFA > -#define CFFPS_FW_NUM_BYTES 4 > +#define CFFPS_FW_CMD 0xFA > +#define CFFPS1_FW_NUM_BYTES 4 > +#define CFFPS2_FW_NUM_WORDS 3 > #define CFFPS_SYS_CONFIG_CMD 0xDA > > #define CFFPS_INPUT_HISTORY_CMD 0xD6 > @@ -61,6 +65,7 @@ struct ibm_cffps_input_history { > }; > > struct ibm_cffps { > + int version; > struct i2c_client *client; > > struct ibm_cffps_input_history input_history; > @@ -132,6 +137,8 @@ static ssize_t ibm_cffps_debugfs_op(struct file *file, char __user *buf, > struct ibm_cffps *psu = to_psu(idxp, idx); > char data[I2C_SMBUS_BLOCK_MAX] = { 0 }; > > + pmbus_set_page(psu->client, 0); > + > switch (idx) { > case CFFPS_DEBUGFS_INPUT_HISTORY: > return ibm_cffps_read_input_history(psu, buf, count, ppos); > @@ -152,16 +159,36 @@ static ssize_t ibm_cffps_debugfs_op(struct file *file, char __user *buf, > rc = snprintf(data, 5, "%04X", rc); > goto done; > case CFFPS_DEBUGFS_FW: > - for (i = 0; i < CFFPS_FW_NUM_BYTES; ++i) { > - rc = i2c_smbus_read_byte_data(psu->client, > - CFFPS_FW_CMD_START + i); > - if (rc < 0) > - return rc; > + switch (psu->version) { > + case 1: > + for (i = 0; i < CFFPS1_FW_NUM_BYTES; ++i) { > + rc = i2c_smbus_read_byte_data(psu->client, > + CFFPS_FW_CMD + > + i); > + if (rc < 0) > + return rc; > + > + snprintf(&data[i * 2], 3, "%02X", rc); > + } > > - snprintf(&data[i * 2], 3, "%02X", rc); > - } > + rc = i * 2; > + break; > + case 2: > + for (i = 0; i < CFFPS2_FW_NUM_WORDS; ++i) { > + rc = i2c_smbus_read_word_data(psu->client, > + CFFPS_FW_CMD + > + i); > + if (rc < 0) > + return rc; > + > + snprintf(&data[i * 4], 5, "%04X", rc); > + } > > - rc = i * 2; > + rc = i * 4; > + break; > + default: > + return -EOPNOTSUPP; > + } > goto done; > default: > return -EINVAL; > @@ -279,6 +306,8 @@ static void ibm_cffps_led_brightness_set(struct led_classdev *led_cdev, > psu->led_state = CFFPS_LED_ON; > } > > + pmbus_set_page(psu->client, 0); > + > rc = i2c_smbus_write_byte_data(psu->client, CFFPS_SYS_CONFIG_CMD, > psu->led_state); > if (rc < 0) > @@ -299,6 +328,8 @@ static int ibm_cffps_led_blink_set(struct led_classdev *led_cdev, > if (led_cdev->brightness == LED_OFF) > return 0; > > + pmbus_set_page(psu->client, 0); > + > rc = i2c_smbus_write_byte_data(psu->client, CFFPS_SYS_CONFIG_CMD, > CFFPS_LED_BLINK); > if (rc < 0) > @@ -328,15 +359,32 @@ static void ibm_cffps_create_led_class(struct ibm_cffps *psu) > dev_warn(dev, "failed to register led class: %d\n", rc); > } > > -static struct pmbus_driver_info ibm_cffps_info = { > - .pages = 1, > - .func[0] = PMBUS_HAVE_VIN | PMBUS_HAVE_VOUT | PMBUS_HAVE_IOUT | > - PMBUS_HAVE_PIN | PMBUS_HAVE_FAN12 | PMBUS_HAVE_TEMP | > - PMBUS_HAVE_TEMP2 | PMBUS_HAVE_TEMP3 | PMBUS_HAVE_STATUS_VOUT | > - PMBUS_HAVE_STATUS_IOUT | PMBUS_HAVE_STATUS_INPUT | > - PMBUS_HAVE_STATUS_TEMP | PMBUS_HAVE_STATUS_FAN12, > - .read_byte_data = ibm_cffps_read_byte_data, > - .read_word_data = ibm_cffps_read_word_data, > +static struct pmbus_driver_info ibm_cffps_info[CFFPS_VERSIONS] = { > + [0] = { > + .pages = 1, > + .func[0] = PMBUS_HAVE_VIN | PMBUS_HAVE_VOUT | PMBUS_HAVE_IOUT | > + PMBUS_HAVE_PIN | PMBUS_HAVE_FAN12 | PMBUS_HAVE_TEMP | > + PMBUS_HAVE_TEMP2 | PMBUS_HAVE_TEMP3 | > + PMBUS_HAVE_STATUS_VOUT | PMBUS_HAVE_STATUS_IOUT | > + PMBUS_HAVE_STATUS_INPUT | PMBUS_HAVE_STATUS_TEMP | > + PMBUS_HAVE_STATUS_FAN12, > + .read_byte_data = ibm_cffps_read_byte_data, > + .read_word_data = ibm_cffps_read_word_data, > + }, > + [1] = { > + .pages = 2, > + .func[0] = PMBUS_HAVE_VIN | PMBUS_HAVE_VOUT | PMBUS_HAVE_IOUT | > + PMBUS_HAVE_PIN | PMBUS_HAVE_FAN12 | PMBUS_HAVE_TEMP | > + PMBUS_HAVE_TEMP2 | PMBUS_HAVE_TEMP3 | > + PMBUS_HAVE_STATUS_VOUT | PMBUS_HAVE_STATUS_IOUT | > + PMBUS_HAVE_STATUS_INPUT | PMBUS_HAVE_STATUS_TEMP | > + PMBUS_HAVE_STATUS_FAN12, > + .func[1] = PMBUS_HAVE_VOUT | PMBUS_HAVE_IOUT | > + PMBUS_HAVE_TEMP | PMBUS_HAVE_TEMP2 | PMBUS_HAVE_TEMP3 | > + PMBUS_HAVE_STATUS_VOUT | PMBUS_HAVE_STATUS_IOUT, > + .read_byte_data = ibm_cffps_read_byte_data, > + .read_word_data = ibm_cffps_read_word_data, > + }, > }; > > static struct pmbus_platform_data ibm_cffps_pdata = { > @@ -346,13 +394,21 @@ static void ibm_cffps_create_led_class(struct ibm_cffps *psu) > static int ibm_cffps_probe(struct i2c_client *client, > const struct i2c_device_id *id) > { > - int i, rc; > + int i, rc, vs; > struct dentry *debugfs; > struct dentry *ibm_cffps_dir; > struct ibm_cffps *psu; > + const void *md = of_device_get_match_data(&client->dev); > + > + if (md) > + vs = (int)md; > + else if (id) > + vs = (int)id->driver_data; > + else > + vs = 1; > > client->dev.platform_data = &ibm_cffps_pdata; > - rc = pmbus_do_probe(client, id, &ibm_cffps_info); > + rc = pmbus_do_probe(client, id, &ibm_cffps_info[vs - 1]); > if (rc) > return rc; > > @@ -364,6 +420,7 @@ static int ibm_cffps_probe(struct i2c_client *client, > if (!psu) > return 0; > > + psu->version = vs; > psu->client = client; > mutex_init(&psu->input_history.update_lock); > psu->input_history.last_update = jiffies - HZ; > @@ -406,12 +463,20 @@ static int ibm_cffps_probe(struct i2c_client *client, > > static const struct i2c_device_id ibm_cffps_id[] = { > { "ibm_cffps1", 1 }, > + { "ibm_cffps2", 2 }, > {} > }; > MODULE_DEVICE_TABLE(i2c, ibm_cffps_id); > > static const struct of_device_id ibm_cffps_of_match[] = { > - { .compatible = "ibm,cffps1" }, > + { > + .compatible = "ibm,cffps1", > + .data = (void *)1 > + }, > + { > + .compatible = "ibm,cffps2", > + .data = (void *)2 > + }, > {} > }; > MODULE_DEVICE_TABLE(of, ibm_cffps_of_match); > -- > 1.8.3.1 >