On 6/16/20 11:11 PM, Simon Glass wrote:
Hi Sean,
On Sun, 7 Jun 2020 at 19:27, Sean Anderson seanga2@gmail.com wrote:
The pinmux property allows for smaller and more compact device trees, especially when there are many pins which need to be assigned individually. Instead of specifying an array of strings to be parsed as pins and a function property, the pinmux property contains an array of integers representing pinmux groups. A pinmux group consists of the pin identifier and mux settings represented as a single integer or an array of integers. Each individual pin controller driver specifies the exact format of a pinmux group. As specified in the Linux documentation, a pinmux group may be multiple integers long. However, no existing drivers use multi-integer pinmux groups, so I have chosen to omit this feature. This makes the implementation easier, since there is no need to allocate a buffer to do endian conversions.
Support for the pinmux property is done differently than in Linux. As far as I can tell, inversion of control is used when implementing support for the pins and groups properties to avoid allocating. This results in some duplication of effort; every property in a config node is parsed once for each pin in that node. This is not such an overhead with pins and groups properties, since having multiple pins in one config node does not occur especially often. However, the semantics of the pinmux property make such a configuration much more appealing. A future patch could parse all config properties at once and store them in an array. This would make it easier to create drivers which do not function solely as callbacks from pinctrl-generic.
This commit increases the size of the sandbox build by approximately 48 bytes. However, it also decreases the size of the K210 device tree by 2 KiB from the previous version of this series.
The documentation has been updated from the last Linux commit before it was split off into yaml files.
Signed-off-by: Sean Anderson seanga2@gmail.com
Changes in v2:
- New
.../pinctrl/pinctrl-bindings.txt | 65 ++++++++- drivers/pinctrl/pinctrl-generic.c | 125 ++++++++++++++---- include/dm/pinctrl.h | 21 +-- 3 files changed, 168 insertions(+), 43 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
[..]
diff --git a/include/dm/pinctrl.h b/include/dm/pinctrl.h index 692e5fc8cb..d50af1ce38 100644 --- a/include/dm/pinctrl.h +++ b/include/dm/pinctrl.h @@ -34,29 +34,33 @@ struct pinconf_param {
- depending on your necessity.
- @get_pins_count: return number of selectable named pins available
in this driver. (necessary to parse "pins" property in DTS)
in this driver. (necessary to parse "pins" property in DTS)What is happening here? I don't think we want the period.
I'm just removing the double-spaces. The period here matches how the rest of the comments also have a period before their parentheses (despite that not being grammatically correct).
- @get_pin_name: return the pin name of the pin selector,
called by the core to figure out which pin it shall do
operations to. (necessary to parse "pins" property in DTS)
operations to. (necessary to parse "pins" property in DTS)- @get_groups_count: return number of selectable named groups available
in this driver. (necessary to parse "groups" property in DTS)
in this driver. (necessary to parse "groups" property in DTS)- @get_group_name: return the group name of the group selector,
called by the core to figure out which pin group it shall do
operations to. (necessary to parse "groups" property in DTS)
operations to. (necessary to parse "groups" property in DTS)- @get_functions_count: return number of selectable named functions available
in this driver. (necessary for pin-muxing)
in this driver. (necessary for pin-muxing)- @get_function_name: return the function name of the muxing selector,
called by the core to figure out which mux setting it shall map a
certain device to. (necessary for pin-muxing)
certain device to. (necessary for pin-muxing)- @pinmux_set: enable a certain muxing function with a certain pin.
The @func_selector selects a certain function whereas @pin_selectorselects a certain pin to be used. On simple controllers one of them
may be ignored. (necessary for pin-muxing against a single pin)
may be ignored. (necessary for pin-muxing against a single pin)- @pinmux_group_set: enable a certain muxing function with a certain pin
group. The @func_selector selects a certain function whereas
group. The @func_selector selects a certain function whereas@group_selector selects a certain set of pins to be used. On simplecontrollers one of them may be ignored.(necessary for pin-muxing against a pin group)
- @pinmux_property_set: enable a pinmux group. @pinmux_group should specify the
pin identifier and mux settings. The exact format of a pinmux group is
left up to the driver. The pin selector for the mux-ed pin should be
returned on success. (necessary to parse the "pinmux" property in DTS)- @pinconf_num_params: number of driver-specific parameters to be parsed
from device trees (necessary for pin-configuration)- @pinconf_params: list of driver_specific parameters to be parsed from
@@ -90,6 +94,7 @@ struct pinctrl_ops { unsigned func_selector); int (*pinmux_group_set)(struct udevice *dev, unsigned group_selector, unsigned func_selector);
int (*pinmux_property_set)(struct udevice *dev, u32 pinmux_group);I am not a fan of how the comments work in this file and would prefer that each function is commented individually. If you have the energy, you could do a follow-up patch.
Sure, I can have a look at that.
unsigned int pinconf_num_params; const struct pinconf_param *pinconf_params; int (*pinconf_set)(struct udevice *dev, unsigned pin_selector,-- 2.26.2
Regards, Simon
--Sean