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

[Egil Hjelmeland]

On 10. okt. 2017 17:51, Woojung.Huh@xxxxxxxxxxxxx wrote:
> > > Specific reason to use val then using
> > LAN9303_BM_EGRSS_PORT_TYPE_SPECIAL_TAG_PORT0
> > > 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.
>
> > > > @@ -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