Patch Detail
get:
Show a patch.
patch:
Update a patch.
put:
Update a patch.
GET /api/1.2/patches/805790/?format=api
{ "id": 805790, "url": "http://patchwork.ozlabs.org/api/1.2/patches/805790/?format=api", "web_url": "http://patchwork.ozlabs.org/project/openbmc/patch/20170825065244.488-4-andrew@aj.id.au/", "project": { "id": 56, "url": "http://patchwork.ozlabs.org/api/1.2/projects/56/?format=api", "name": "OpenBMC development", "link_name": "openbmc", "list_id": "openbmc.lists.ozlabs.org", "list_email": "openbmc@lists.ozlabs.org", "web_url": "http://github.com/openbmc/", "scm_url": "", "webscm_url": "", "list_archive_url": "", "list_archive_url_format": "", "commit_url_format": "" }, "msgid": "<20170825065244.488-4-andrew@aj.id.au>", "list_archive_url": null, "date": "2017-08-25T06:52:42", "name": "[linux,dev-4.10,3/5] leds: pca955x: Don't invert requested value in pca955x_gpio_set_value()", "commit_ref": null, "pull_url": null, "state": "accepted", "archived": true, "hash": "863b23ec122ec74cab03e87c81853cd6a43cfa20", "submitter": { "id": 68332, "url": "http://patchwork.ozlabs.org/api/1.2/people/68332/?format=api", "name": "Andrew Jeffery", "email": "andrew@aj.id.au" }, "delegate": null, "mbox": "http://patchwork.ozlabs.org/project/openbmc/patch/20170825065244.488-4-andrew@aj.id.au/mbox/", "series": [], "comments": "http://patchwork.ozlabs.org/api/patches/805790/comments/", "check": "pending", "checks": "http://patchwork.ozlabs.org/api/patches/805790/checks/", "tags": {}, "related": [], "headers": { "Return-Path": "<openbmc-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org>", "X-Original-To": [ "incoming@patchwork.ozlabs.org", "openbmc@lists.ozlabs.org" ], "Delivered-To": [ "patchwork-incoming@bilbo.ozlabs.org", "openbmc@lists.ozlabs.org" ], "Received": [ "from lists.ozlabs.org (lists.ozlabs.org [103.22.144.68])\n\t(using TLSv1.2 with cipher ADH-AES256-GCM-SHA384 (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xdsNJ4l2Xz9sDB\n\tfor <incoming@patchwork.ozlabs.org>;\n\tFri, 25 Aug 2017 16:54:40 +1000 (AEST)", "from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3])\n\tby lists.ozlabs.org (Postfix) with ESMTP id 3xdsNJ3c7pzDrWX\n\tfor <incoming@patchwork.ozlabs.org>;\n\tFri, 25 Aug 2017 16:54:40 +1000 (AEST)", "from out2-smtp.messagingengine.com (out2-smtp.messagingengine.com\n\t[66.111.4.26])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby lists.ozlabs.org (Postfix) with ESMTPS id 3xdsMB2Cx7zDrKs\n\tfor <openbmc@lists.ozlabs.org>; Fri, 25 Aug 2017 16:53:42 +1000 (AEST)", "from compute4.internal (compute4.nyi.internal [10.202.2.44])\n\tby mailout.nyi.internal (Postfix) with ESMTP id 2C9CF20DE9;\n\tFri, 25 Aug 2017 02:53:40 -0400 (EDT)", "from frontend2 ([10.202.2.161])\n\tby compute4.internal (MEProxy); Fri, 25 Aug 2017 02:53:40 -0400", "from keelia.aj.id.au\n\t(ppp118-210-176-216.bras2.adl6.internode.on.net [118.210.176.216])\n\tby mail.messagingengine.com (Postfix) with ESMTPA id E110A240B1;\n\tFri, 25 Aug 2017 02:53:36 -0400 (EDT)" ], "Authentication-Results": [ "ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=aj.id.au header.i=@aj.id.au header.b=\"bhU5Wrfz\";\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=messagingengine.com\n\theader.i=@messagingengine.com header.b=\"i5MyM5TP\"; \n\tdkim-atps=neutral", "lists.ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=aj.id.au header.i=@aj.id.au header.b=\"bhU5Wrfz\";\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=messagingengine.com\n\theader.i=@messagingengine.com header.b=\"i5MyM5TP\"; \n\tdkim-atps=neutral", "lists.ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=aj.id.au header.i=@aj.id.au header.b=\"bhU5Wrfz\";\n\tdkim=pass (2048-bit key;\n\tunprotected) header.d=messagingengine.com\n\theader.i=@messagingengine.com\n\theader.b=\"i5MyM5TP\"; dkim-atps=neutral" ], "DKIM-Signature": [ "v=1; a=rsa-sha256; c=relaxed/relaxed; d=aj.id.au; h=cc\n\t:content-transfer-encoding:content-type:date:from:in-reply-to\n\t:message-id:mime-version:references:subject:to:x-me-sender\n\t:x-me-sender:x-sasl-enc:x-sasl-enc; s=fm1; bh=RqXDlNpgaSZrmEkaJV\n\tjsiOzdVZc+khWjLDfRhic6flw=; b=bhU5WrfzSsBYK9N+1Y/M3BgnoQvEwbwYQA\n\trdw0PkfPK3YOeNUoszwNXupuxIireGT8U5d2mDMNTsIm1Wm7lLSaKsN4YBN9+2LM\n\td08txQ24H2hw6b+WwLQdhXV+7PSGauDbcWJtLXB2tzpDtJQ0eF6CGpiwFiyt8p93\n\tn+SuU2Bp+JlppJJRORIe/BCmAVv6O4AHg8zu8UVLEcnE9Gn2Hm2hDCFApF/XrnlF\n\tLi2YIeMNaGXb4nSq9h8+3/6jNipE0YdrgRFzDmRy32Ndpa3DMbkVeny+NJEQwgw/\n\t7qjbLwqgZpQo8RqyIlWAPz+usj8LBT0CBn2RatWs+P5wKU93EGZA==", "v=1; a=rsa-sha256; c=relaxed/relaxed; d=\n\tmessagingengine.com; h=cc:content-transfer-encoding:content-type\n\t:date:from:in-reply-to:message-id:mime-version:references\n\t:subject:to:x-me-sender:x-me-sender:x-sasl-enc:x-sasl-enc; s=\n\tfm1; bh=RqXDlNpgaSZrmEkaJVjsiOzdVZc+khWjLDfRhic6flw=; b=i5MyM5TP\n\t2IVwPG5x7fUUYOR2EEnqAksvzj/VW6SncxdnSQy1AmVfdZqni9gHiRs0BZFlemmq\n\tJgGoJweU0easjQqtsg4dyLFVMv5BCCBcXsTECjILnDC1PfgXzwL1BdFyG7RDObeF\n\tkfhBKu4DW6nCetadDGgDATGBbNDV4qUnY4EC8bwTlPssxNagaDnh5ygsHiOEMCKx\n\t8LZ793PHTqpYaeKAvKW6RrOgyMEFUOkfWxqMNL64Hq6kcRF707ynr/OPYvFtV4vt\n\to6JmljT+tugD5UBOt8B+U0h/nMReunJ8SIZymlCEncvP13SRsliz7v1GWCyhzyEU\n\tWw9JH8ajNdccSw==" ], "X-ME-Sender": "<xms:dMmfWUGaIFmiShawYNdWDRhDnjMBkVGg-kygOND1kcYP76PR5Z6tzg>", "X-Sasl-enc": "tHb1aOe2CD1hxfSl7omIIPTLuRlesD2R8b0qZIFCiZTA 1503644019", "From": "Andrew Jeffery <andrew@aj.id.au>", "To": "joel@jms.id.au", "Subject": "[PATCH linux dev-4.10 3/5] leds: pca955x: Don't invert requested\n\tvalue in pca955x_gpio_set_value()", "Date": "Fri, 25 Aug 2017 16:22:42 +0930", "Message-Id": "<20170825065244.488-4-andrew@aj.id.au>", "X-Mailer": "git-send-email 2.11.0", "In-Reply-To": "<20170825065244.488-1-andrew@aj.id.au>", "References": "<20170825065244.488-1-andrew@aj.id.au>", "MIME-Version": "1.0", "Content-Type": "text/plain; charset=UTF-8", "Content-Transfer-Encoding": "8bit", "X-BeenThere": "openbmc@lists.ozlabs.org", "X-Mailman-Version": "2.1.23", "Precedence": "list", "List-Id": "Development list for OpenBMC <openbmc.lists.ozlabs.org>", "List-Unsubscribe": "<https://lists.ozlabs.org/options/openbmc>,\n\t<mailto:openbmc-request@lists.ozlabs.org?subject=unsubscribe>", "List-Archive": "<http://lists.ozlabs.org/pipermail/openbmc/>", "List-Post": "<mailto:openbmc@lists.ozlabs.org>", "List-Help": "<mailto:openbmc-request@lists.ozlabs.org?subject=help>", "List-Subscribe": "<https://lists.ozlabs.org/listinfo/openbmc>,\n\t<mailto:openbmc-request@lists.ozlabs.org?subject=subscribe>", "Cc": "Andrew Jeffery <andrew@aj.id.au>, eajames@linux.vnet.ibm.com,\n\topenbmc@lists.ozlabs.org, clg@kaod.org, bjwyman@gmail.com", "Errors-To": "openbmc-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org", "Sender": "\"openbmc\"\n\t<openbmc-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org>" }, "content": "The PCA9552 lines can be used either for driving LEDs or as GPIOs. The\nmanual states that for LEDs, the operation is open-drain:\n\n The LSn LED select registers determine the source of the LED data.\n\n 00 = output is set LOW (LED on)\n 01 = output is set high-impedance (LED off; default)\n 10 = output blinks at PWM0 rate\n 11 = output blinks at PWM1 rate\n\nFor GPIOs it suggests a pull-up so that the open-case drives the line\nhigh:\n\n For use as output, connect external pull-up resistor to the pin\n and size it according to the DC recommended operating\n characteristics. LED output pin is HIGH when the output is\n programmed as high-impedance, and LOW when the output is\n programmed LOW through the ‘LED selector’ register. The output\n can be pulse-width controlled when PWM0 or PWM1 are used.\n\nNow, I have a hardware design that uses the LED controller to control\nLEDs. However, for $reasons, we're using the leds-gpio driver to drive\nthe LEDs which means wending our way through the gpiochip abstractions.\nSo with that in mind we need to describe an active-low GPIO\nconfiguration to drive the LEDs as though they were GPIOs.\n\nThe set() gpiochip callback in leds-pca955x does the following:\n\n ...\n if (val)\n pca955x_led_set(&led->led_cdev, LED_FULL);\n else\n pca955x_led_set(&led->led_cdev, LED_OFF);\n ...\n\nWhere LED_FULL = 255. pca955x_led_set() in turn does:\n\n ...\n switch (value) {\n case LED_FULL:\n ls = pca955x_ledsel(ls, ls_led, PCA955X_LS_LED_ON);\n break;\n ...\n\nWhere PCA955X_LS_LED_ON is defined as:\n\n #define PCA955X_LS_LED_ON\t0x0\t/* Output LOW */\n\nSo here we have some type confusion: We've crossed domains from GPIO\nbehaviour to LED behaviour without accounting for possible inversions\nin the process.\n\nStepping back to leds-gpio for a moment, during probe() we call\ncreate_gpio_led(), which eventually executes:\n\n if (template->default_state == LEDS_GPIO_DEFSTATE_KEEP) {\n state = gpiod_get_value_cansleep(led_dat->gpiod);\n if (state < 0)\n return state;\n } else {\n state = (template->default_state == LEDS_GPIO_DEFSTATE_ON);\n }\n ...\n ret = gpiod_direction_output(led_dat->gpiod, state);\n\nIn the devicetree the GPIO is annotated as active-low, and\ngpiod_get_value_cansleep() handles this for us:\n\n int gpiod_get_value_cansleep(const struct gpio_desc *desc)\n {\n int value;\n\n might_sleep_if(extra_checks);\n VALIDATE_DESC(desc);\n value = _gpiod_get_raw_value(desc);\n if (value < 0)\n return value;\n\n if (test_bit(FLAG_ACTIVE_LOW, &desc->flags))\n value = !value;\n\n return value;\n }\n\n_gpiod_get_raw_value() in turn calls through the get() callback for the\ngpiochip implementation, so returning to our get() implementation in\nleds-pca955x we find we extract the raw value from hardware:\n\n static int pca955x_gpio_get_value(struct gpio_chip *gc, unsigned int offset)\n {\n struct pca955x *pca955x = gpiochip_get_data(gc);\n struct pca955x_led *led = &pca955x->leds[offset];\n u8 reg = pca955x_read_input(pca955x->client, led->led_num / 8);\n\n return !!(reg & (1 << (led->led_num % 8)));\n }\n\nThis behaviour is not symmetric with that of set(), where the val is\ninverted by the driver.\n\nClosing the loop on the GPIO_ACTIVE_LOW inversions,\ngpiod_direction_output(), like gpiod_get_value_cansleep, handles it for\nus:\n\n int gpiod_direction_output(struct gpio_desc *desc, int value)\n {\n VALIDATE_DESC(desc);\n if (test_bit(FLAG_ACTIVE_LOW, &desc->flags))\n value = !value;\n else\n value = !!value;\n return _gpiod_direction_output_raw(desc, value);\n }\n\nAll-in-all, with a value of 'keep' for default-state property in the\ngpio-leds child node, the current state of the hardware will in-fact be\ninverted; precisely the opposite of what was intended.\n\nRework leds-pca955x so that we avoid the incorrect inversion and clarify\nthe semantics with respect to GPIO.\n\nSigned-off-by: Andrew Jeffery <andrew@aj.id.au>\n---\n drivers/leds/leds-pca955x.c | 7 +++++--\n 1 file changed, 5 insertions(+), 2 deletions(-)", "diff": "diff --git a/drivers/leds/leds-pca955x.c b/drivers/leds/leds-pca955x.c\nindex 0217bac2f47b..a02c1fd5dee9 100644\n--- a/drivers/leds/leds-pca955x.c\n+++ b/drivers/leds/leds-pca955x.c\n@@ -61,6 +61,9 @@\n #define PCA955X_LS_BLINK0\t0x2\t/* Blink at PWM0 rate */\n #define PCA955X_LS_BLINK1\t0x3\t/* Blink at PWM1 rate */\n \n+#define PCA955X_GPIO_HIGH\tLED_OFF\n+#define PCA955X_GPIO_LOW\tLED_FULL\n+\n enum pca955x_type {\n \tpca9550,\n \tpca9551,\n@@ -292,9 +295,9 @@ static void pca955x_gpio_set_value(struct gpio_chip *gc, unsigned int offset,\n \tstruct pca955x_led *led = &pca955x->leds[offset];\n \n \tif (val)\n-\t\tpca955x_led_set(&led->led_cdev, LED_FULL);\n+\t\tpca955x_led_set(&led->led_cdev, PCA955X_GPIO_HIGH);\n \telse\n-\t\tpca955x_led_set(&led->led_cdev, LED_OFF);\n+\t\tpca955x_led_set(&led->led_cdev, PCA955X_GPIO_LOW);\n }\n \n static int pca955x_gpio_get_value(struct gpio_chip *gc, unsigned int offset)\n", "prefixes": [ "linux", "dev-4.10", "3/5" ] }