On Thu, 2016-03-31 at 14:44 +0300, Irina Tirdea wrote: > Add ACPI support for pin controller properties. These are > based on ACPI _DSD properties and follow the device tree > model based on states and node configurations. The states > are defined as _DSD properties and configuration nodes > are defined using the _DSD Hierarchical Properties Extension. > > A configuration node supports the generic device tree properties. > > The implementation is based on device tree code from devicetree.c. > Patch is good to me, though few minor comments below. > +/** > + * struct pinctrl_acpi_map - mapping table chunk parsed from ACPI > + * @node: list node for struct pinctrl's @fw_maps field > + * @pctldev: the pin controller that allocated this struct, and will > free it > + * @maps: the mapping table entries We have @map and @num_maps. > + */ > +struct pinctrl_acpi_map { > + struct list_head node; > + struct pinctrl_dev *pctldev; > + struct pinctrl_map *map; > + unsigned num_maps; > +}; > + > > +static int acpi_remember_or_free_map(struct pinctrl *p, const char > *statename, > + struct pinctrl_dev *pctldev, > + struct pinctrl_map *map, > unsigned num_maps) > +{ > + struct pinctrl_acpi_map *acpi_map; > + struct list_head *acpi_maps; > + unsigned int i; Just unsigned i to be in align with unsigned num_maps. > + > + /* Initialize common mapping table entry fields */ > + for (i = 0; i < num_maps; i++) { > + map[i].dev_name = dev_name(p->dev); > + map[i].name = statename; > + if (pctldev) > + map[i].ctrl_dev_name = dev_name(pctldev- > >dev); > + } > +int pinctrl_acpi_to_map(struct pinctrl *p) > +{ > + const union acpi_object *prop, *statenames, *configs; > + unsigned int state, nstates, nconfigs, config; > + char *statename, *propname, *configname; > + struct fwnode_handle *fw_prop; > + struct acpi_device *adev; > + int ret; > + > + /* We may store pointers to property names within the node > */ > + adev = acpi_bus_get_acpi_device(ACPI_HANDLE(p->dev)); > + if (!adev) > + return -ENODEV; > + > + /* Only allow named states (device must have prop 'pinctrl- > names') */ Does it fit one line? > +err_free_map: Perhaps err_free_maps? > + pinctrl_acpi_free_maps(p); > + return ret; -- Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> Intel Finland Oy -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html