netdev - Re: [PATCH net-next v2 02/10] net: dsa: lan9303: Do not disable/enable switch fabric port 0 at startup

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]

Date:   Thu, 27 Jul 2017 12:39:10 +0200
From:   Egil Hjelmeland <privat@...l-hjelmeland.no>
To:     Andrew Lunn <andrew@...n.ch>
Cc:     corbet@....net, vivien.didelot@...oirfairelinux.com,
        f.fainelli@...il.com, davem@...emloft.net, kernel@...gutronix.de,
        linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org,
        netdev@...r.kernel.org
Subject: Re: [PATCH net-next v2 02/10] net: dsa: lan9303: Do not
 disable/enable switch fabric port 0 at startup

On 26. juli 2017 18:58, Andrew Lunn wrote:
> On Tue, Jul 25, 2017 at 06:15:45PM +0200, Egil Hjelmeland wrote:
>> For some mysterious reason enable switch fabric port 0 TX fails to
>> work, when the TX has previous been disabled. Resolved by not
>> disable/enable switch fabric port 0 at startup. Port 1 and 2 are
>> still disabled in early init.
>>
>> Signed-off-by: Egil Hjelmeland <privat@...l-hjelmeland.no>
>> ---
>>   drivers/net/dsa/lan9303-core.c | 7 -------
>>   1 file changed, 7 deletions(-)
>>
>> diff --git a/drivers/net/dsa/lan9303-core.c b/drivers/net/dsa/lan9303-core.c
>> index e622db586c3d..c2b53659f58f 100644
>> --- a/drivers/net/dsa/lan9303-core.c
>> +++ b/drivers/net/dsa/lan9303-core.c
>> @@ -557,9 +557,6 @@ static int lan9303_disable_processing(struct lan9303 *chip)
>>   {
>>   	int ret;
>>   
>> -	ret = lan9303_disable_packet_processing(chip, LAN9303_PORT_0_OFFSET);
>> -	if (ret)
>> -		return ret;
>>   	ret = lan9303_disable_packet_processing(chip, LAN9303_PORT_1_OFFSET);
>>   	if (ret)
>>   		return ret;
>> @@ -633,10 +630,6 @@ static int lan9303_setup(struct dsa_switch *ds)
>>   	if (ret)
>>   		dev_err(chip->dev, "failed to separate ports %d\n", ret);
>>   
>> -	ret = lan9303_enable_packet_processing(chip, LAN9303_PORT_0_OFFSET);
>> -	if (ret)
>> -		dev_err(chip->dev, "failed to re-enable switching %d\n", ret);
>> -
> 
> Does this mean you are relying on something else enabling port 0? The
> bootloader?
> 
> I'm wondering if it is better to keep the enable, but remove the
> disable?
> 
> 	Andrew
> 

The (switch engine) ports are enabled by default. The only thing our
bootloader does is to set gpo so the lan9303 is kept in reset until
the linux driver starts. When I test with the next-next kernel I just
specify the reset-gpo in DTS and the driver pulls it out of reset.

Keeping the enable does no harm, as far as I recall, but I can
double check that when I get time. I have no idea why the original
mainline code does not work for me. Maybe it is a timing issue?