* Re: [PATCH 4/4] [v4] pinctrl: qcom: qdf2xxx: add support for new ACPI HID QCOM8002 [not found] ` <1512170904-4749-5-git-send-email-timur-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> @ 2017-12-12 10:42 ` Linus Walleij 2017-12-12 11:07 ` Andy Shevchenko 0 siblings, 1 reply; 5+ messages in thread From: Linus Walleij @ 2017-12-12 10:42 UTC (permalink / raw) To: Timur Tabi, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS Cc: linux-arm-msm-u79uwXL29TY76Z2rM5mHXA, Linux ARM, linux-gpio-u79uwXL29TY76Z2rM5mHXA, Andy Shevchenko, Mika Westerberg, thierry.reding-Re5JQEeQqe8AvxtiuMwx3w, Stephen Boyd, David Brown, Andy Gross, Bjorn Andersson, Varadarajan Narayanan, Archit Taneja On Sat, Dec 2, 2017 at 12:28 AM, Timur Tabi <timur-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> wrote: > /* Query the number of GPIOs from ACPI */ > ret = device_property_read_u32(&pdev->dev, "num-gpios", &num_gpios); > if (ret < 0) { > - dev_warn(&pdev->dev, "missing num-gpios property\n"); > + dev_err(&pdev->dev, "missing 'num-gpios' property\n"); > return ret; > } It's unfortunate that this driver uses the undocumented "num-gpios" when the device tree bindings already has standardized "ngpios" as the name for this. Maybe it was not standardized back in 2015 when this driver was merged. Or we were all sloppy :/ > + /* The number of GPIOs in the approved list */ > + ret = device_property_read_u16_array(&pdev->dev, "gpios", > + NULL, 0); > + if (ret < 0) { > + dev_err(&pdev->dev, "missing 'gpios' property\n"); > + return ret; > + } This is in direct conflict with the existing "gpios" binding in device tree. Where is this name coming from? ACPI standards? If device tree and ACPI start defining things which are in direct conflict we can just shut down this device_property() business altogether, it will never work that way. I would try to merge a DT bindings doc defining "valid-gpios" or something like this, can we proceed like that? Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 4/4] [v4] pinctrl: qcom: qdf2xxx: add support for new ACPI HID QCOM8002 2017-12-12 10:42 ` [PATCH 4/4] [v4] pinctrl: qcom: qdf2xxx: add support for new ACPI HID QCOM8002 Linus Walleij @ 2017-12-12 11:07 ` Andy Shevchenko 2017-12-12 20:27 ` Timur Tabi 0 siblings, 1 reply; 5+ messages in thread From: Andy Shevchenko @ 2017-12-12 11:07 UTC (permalink / raw) To: Linus Walleij, Timur Tabi, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS Cc: Archit Taneja, David Brown, linux-arm-msm, Stephen Boyd, Bjorn Andersson, linux-gpio, thierry.reding, Andy Gross, Mika Westerberg, Varadarajan Narayanan, Linux ARM On Tue, 2017-12-12 at 11:42 +0100, Linus Walleij wrote: > On Sat, Dec 2, 2017 at 12:28 AM, Timur Tabi <timur@codeaurora.org> > wrote: > > + /* The number of GPIOs in the approved list */ > > + ret = device_property_read_u16_array(&pdev->dev, > > "gpios", > > + NULL, 0); > > + if (ret < 0) { > > + dev_err(&pdev->dev, "missing 'gpios' > > property\n"); > > + return ret; > > + } > > This is in direct conflict with the existing "gpios" binding in device > tree. > > Where is this name coming from? ACPI standards? Not ACPI standards as of my knowledge. ACPI standard defines a common scheme how to define properties, it doesn't tell anything about property names or any mappings between names to values or names to "OS subsystem"). As for GPIO we just follow *de facto* what DT has right now, i.e. "xxx- gpio" or "xxx-gpios" pattern is used to map ACPI standard resource to a GPIO name. That's how GPIO ACPI lib is being developed. > If device tree and ACPI start defining things which are in direct > conflict > we can just shut down this device_property() business altogether, > it will never work that way. This is fully understandable. Also it works in other direction, i.e. if DT will break the established thing it will break also ACPI and built-in device properties. We are keeping an eye on this not to happen as much as we can in any direction. So, summarize above, I don't see any impediments (except maybe very broken ARM64 firmware that is already on devices on market) to make it properly from the beginning. -- Andy Shevchenko <andriy.shevchenko@linux.intel.com> Intel Finland Oy ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 4/4] [v4] pinctrl: qcom: qdf2xxx: add support for new ACPI HID QCOM8002 2017-12-12 11:07 ` Andy Shevchenko @ 2017-12-12 20:27 ` Timur Tabi 2017-12-13 14:36 ` Andy Shevchenko 0 siblings, 1 reply; 5+ messages in thread From: Timur Tabi @ 2017-12-12 20:27 UTC (permalink / raw) To: Andy Shevchenko, Linus Walleij, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS Cc: linux-arm-msm, Linux ARM, linux-gpio, Mika Westerberg, thierry.reding, Stephen Boyd, David Brown, Andy Gross, Bjorn Andersson, Varadarajan Narayanan, Archit Taneja On 12/12/2017 05:07 AM, Andy Shevchenko wrote: > Not ACPI standards as of my knowledge. ACPI standard defines a common > scheme how to define properties, it doesn't tell anything about property > names or any mappings between names to values or names to "OS > subsystem"). There was an attempt a while back to standardize this like we do for device tree, but it fell apart. Device-specific ACPI-only properties are not standarized. This driver is initialized only on ACPI systems. It has no device tree binding. > As for GPIO we just follow *de facto* what DT has right now, i.e. "xxx- > gpio" or "xxx-gpios" pattern is used to map ACPI standard resource to a > GPIO name. That's how GPIO ACPI lib is being developed. GPIOs in device tree are defined completely differently than in ACPI. On DT, the kernel controls the pin muxing. On ACPI, pins are muxed by firmware and never re-muxed by the operating system. So all this driver does is expose a few pins as simple GPIOs. -- Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project. ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 4/4] [v4] pinctrl: qcom: qdf2xxx: add support for new ACPI HID QCOM8002 2017-12-12 20:27 ` Timur Tabi @ 2017-12-13 14:36 ` Andy Shevchenko 2017-12-13 14:47 ` Timur Tabi 0 siblings, 1 reply; 5+ messages in thread From: Andy Shevchenko @ 2017-12-13 14:36 UTC (permalink / raw) To: Timur Tabi, Linus Walleij, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS Cc: linux-arm-msm, Linux ARM, linux-gpio, Mika Westerberg, thierry.reding, Stephen Boyd, David Brown, Andy Gross, Bjorn Andersson, Varadarajan Narayanan, Archit Taneja On Tue, 2017-12-12 at 14:27 -0600, Timur Tabi wrote: > On 12/12/2017 05:07 AM, Andy Shevchenko wrote: > > > Not ACPI standards as of my knowledge. ACPI standard defines a > > common > > scheme how to define properties, it doesn't tell anything about > > property > > names or any mappings between names to values or names to "OS > > subsystem"). > > There was an attempt a while back to standardize this like we do for > device tree, but it fell apart. Device-specific ACPI-only properties > are not standarized. This driver is initialized only on ACPI > systems. > It has no device tree binding. It should follow DT *de facto* standard bindings like "ngpios" (though it's not needed in ACPI case IIRC) and other properties. > > As for GPIO we just follow *de facto* what DT has right now, i.e. > > "xxx- > > gpio" or "xxx-gpios" pattern is used to map ACPI standard resource > > to a > > GPIO name. That's how GPIO ACPI lib is being developed. > > GPIOs in device tree are defined completely differently than in ACPI. > On DT, the kernel controls the pin muxing. On ACPI, pins are muxed > by > firmware and never re-muxed by the operating system. So all this > driver > does is expose a few pins as simple GPIOs. Wait, runtime muxing is a matter of requesting another function (usually GPIO) and putting it back afterwards. Do you really need anything like this at *runtime*? Pin control design is not compatible with hardware (too abstract), but that is the problem of DT as well: I'm referring here to not carefully designed so called "pin states". This is another story and has nothing specific for ACPI. -- Andy Shevchenko <andriy.shevchenko@linux.intel.com> Intel Finland Oy ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 4/4] [v4] pinctrl: qcom: qdf2xxx: add support for new ACPI HID QCOM8002 2017-12-13 14:36 ` Andy Shevchenko @ 2017-12-13 14:47 ` Timur Tabi 0 siblings, 0 replies; 5+ messages in thread From: Timur Tabi @ 2017-12-13 14:47 UTC (permalink / raw) To: Andy Shevchenko, Linus Walleij, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS Cc: linux-arm-msm, Linux ARM, linux-gpio, Mika Westerberg, thierry.reding, Stephen Boyd, David Brown, Andy Gross, Bjorn Andersson, Varadarajan Narayanan, Archit Taneja On 12/13/2017 08:36 AM, Andy Shevchenko wrote: > Wait, runtime muxing is a matter of requesting another function (usually > GPIO) and putting it back afterwards. Do you really need anything like > this at*runtime*? No, there is no runtime muxing on ACPI platforms. -- Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project. ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-12-13 14:47 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <1512170904-4749-1-git-send-email-timur@codeaurora.org> [not found] ` <1512170904-4749-5-git-send-email-timur@codeaurora.org> [not found] ` <1512170904-4749-5-git-send-email-timur-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> 2017-12-12 10:42 ` [PATCH 4/4] [v4] pinctrl: qcom: qdf2xxx: add support for new ACPI HID QCOM8002 Linus Walleij 2017-12-12 11:07 ` Andy Shevchenko 2017-12-12 20:27 ` Timur Tabi 2017-12-13 14:36 ` Andy Shevchenko 2017-12-13 14:47 ` Timur Tabi
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).