netdev - Re: [PATCH net] net: dsa: mv88e6xxx: Reset mv88e6393x watchdog register

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 for Android: free password hash cracker in your pocket

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

Message-ID: <9ba1722a-8dd7-4d6d-bade-b4c702c8387f@lunn.ch>
Date:   Tue, 28 Mar 2023 14:30:37 +0200
From:   Andrew Lunn <andrew@...n.ch>
To:     Marek Behún <marek.behun@....cz>
Cc:     Gustav Ekelund <gustav.ekelund@...s.com>,
        Florian Fainelli <f.fainelli@...il.com>,
        Vladimir Oltean <olteanv@...il.com>,
        "David S. Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>, kernel@...s.com,
        Gustav Ekelund <gustaek@...s.com>, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH net] net: dsa: mv88e6xxx: Reset mv88e6393x watchdog
 register

> > +static int mv88e6393x_watchdog_action(struct mv88e6xxx_chip *chip, int irq)
> > +{
> > +	mv88e6390_watchdog_action(chip, irq);
> > +
> > +	mv88e6xxx_g2_write(chip, MV88E6390_G2_WDOG_CTL,
> > +			   MV88E6390_G2_WDOG_CTL_UPDATE |
> > +			   MV88E6390_G2_WDOG_CTL_PTR_EVENT);
> > +
> > +	return IRQ_HANDLED;
> > +}
> 
> Shouldn't this update be in .irq_setup() method? In the commit message
> you're saying that the problem is that bits aren't cleared with SW
> reset. So I would guess that the change should be in the setup of
> watchdog IRQ, not in IRQ action?

I think it is a bit more complex than that. At least for the 6352,
which i just looked at the data sheet, the interrupt bits are listed
as "ROC". Which is missing from the list of definitions, but seems to
mean Read Only, Clear on read. So even if it was not cleared on
software reset, it would only fire once, and then be cleared.

The problem description here is that it does not clear on read, it
needs an explicit write. Which suggests Marvell changed it for the
6393.

So i have a couple of questions:

1) Is this new behaviour limited to the 6393, or does the 6390 also
need this write?

2) What about other interrupts? Is this the only one which has changed
behaviour?

	Andrew