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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20f8c457-0f61-641e-a12a-7c0bcf2fba21@egil-hjelmeland.no>
Date:   Wed, 11 Oct 2017 10:16:30 +0200
From:   Egil Hjelmeland <privat@...l-hjelmeland.no>
To:     Woojung.Huh@...rochip.com, andrew@...n.ch,
        vivien.didelot@...oirfairelinux.com, f.fainelli@...il.com,
        netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: 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@...rochip.com 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