> +.. code-block:: > + > + +-------------+ > + | PSE PI | > + 8 -----+ +-------------+ > + 7 -----+ Rail 1 | > + 6 -----+------+----------------------+ > + 5 -----+ | | > + 4 -----+ / Rail 2 | PSE 1 > + 3 -----+----? +-------------+ > + 2 -----+----+---------? | > + 1 -----+---? +-------------+ > + | > + +-------------+ Is ? a standard markup character? I don't remember seeing it used like this before. Maybe offset the connection for pins 1 and 2 from that of 3. I mean: > + 4 -----+ / Rail 2 | PSE 1 > + 3 -----+----? +-------------+ > + 2 -----+--------+-----? | > + 1 -----+-------? +-------------+ You version is a little ambiguous, pins 1, 2 & 3 could be interconnected at the +. The text does however make it clear they are not, but i don't see any harm in making the diagram clearer. > +static int of_load_single_pse_pi_pairset(struct device_node *node, > + struct pse_pi *pi, > + int pairset_num) > +{ > + struct device_node *pairset_np; > + const char *name; > + int ret; > + > + ret = of_property_read_string_index(node, "pairset-names", > + pairset_num, &name); > + if (ret) > + return ret; > + > + if (!strcmp(name, "alternative-a")) { > + pi->pairset[pairset_num].pinout = ALTERNATIVE_A; > + } else if (!strcmp(name, "alternative-b")) { > + pi->pairset[pairset_num].pinout = ALTERNATIVE_B; > + } else { > + pr_err("pse: wrong pairset-names value %s\n", name); > + return -EINVAL; Maybe include the node path in the error message? For a 24 port switch, it will help find a typo in one of the ports. I would do this for all error messages in this code. Please add my Reviewed-by on the next version. Andrew