Re: [PATCH v2 net-next 1/2] net: dsa: lan9303: Move tag setup to new lan9303_setup_tagging

On 10. okt. 2017 17:51, Woojung.Huh@microchip.com wrote:

quoted

quoted

Specific reason to use val then using

LAN9303_BM_EGRSS_PORT_TYPE_SPECIAL_TAG_PORT0

quoted

like previous line?

Specific reason was to please a reviewer that did not like my
indenting in first version. I did not agree with him, but since
nobody else spoke up, I changed the code.

Got it. Missed previous patch/comment.

quoted

quoted

quoted

@@ -644,6 +648,10 @@ static int lan9303_setup(struct dsa_switch *ds)
   		return -EINVAL;
   	}

+	ret = lan9303_setup_tagging(chip);
+	if (ret)
+		dev_err(chip->dev, "failed to setup port tagging %d\n", ret);
+

Still move on when error happens?

Good question. I just followed the pattern from the original function,
which was not made by me. Actually I did once reflect on whether this
was the correct way. Perhaps it could be argued that it is better to
allow the device to come up, so the problem can be investigated?

Maybe depends on severity of setting?
BTW, lan9303_setup() still returns ZERO at the end?

I did quick survey of the _setup functions of the other dsa drivers.
Some return on error, some ignore errors.
If you think so, I can make a v3 series that return on error. Otherwise
I leave it as it is.

Thanks.
Woojung

Thank you for polite review.

Egil