lkml.org 
[lkml]   [2016]   [Apr]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [RFC PATCH 3/4] pinctrl: Add ACPI support
    From
    Date
    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@linux.intel.com>
    Intel Finland Oy

    \
     
     \ /
      Last update: 2016-04-01 16:41    [W:4.378 / U:0.028 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site