lkml.org 
[lkml]   [2024]   [Mar]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH net-next v6 10/17] net: pse-pd: Add support for PSE PIs
> +.. 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

\
 
 \ /
  Last update: 2024-03-28 13:24    [W:0.255 / U:0.524 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site