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:   Sat, 24 Aug 2019 22:52:16 +0200
From:   Marek Behun <marek.behun@....cz>
To:     Vivien Didelot <vivien.didelot@...il.com>
Cc:     netdev@...r.kernel.org, Andrew Lunn <andrew@...n.ch>,
        Florian Fainelli <f.fainelli@...il.com>,
        Vladimir Oltean <olteanv@...il.com>
Subject: Re: [PATCH net-next v2 8/9] net: dsa: mv88e6xxx: support Block
 Address setting in hidden registers

On Sat, 24 Aug 2019 16:13:28 -0400
Vivien Didelot <vivien.didelot@...il.com> wrote:

> Hi Marek,
> 
> On Fri, 23 Aug 2019 23:26:02 +0200, Marek BehĂșn <marek.behun@....cz> wrote:
> > -int mv88e6xxx_port_hidden_write(struct mv88e6xxx_chip *chip, int port, int reg,
> > -				u16 val);
> > +int mv88e6xxx_port_hidden_write(struct mv88e6xxx_chip *chip, int block, int port,
> > +				int reg, u16 val);
> >  int mv88e6xxx_port_hidden_wait(struct mv88e6xxx_chip *chip);
> > -int mv88e6xxx_port_hidden_read(struct mv88e6xxx_chip *chip, int port, int reg,
> > -			       u16 *val);
> > +int mv88e6xxx_port_hidden_read(struct mv88e6xxx_chip *chip, int block, int port,
> > +			       int reg, u16 *val);  
> 
> 
> There's something I'm having trouble to follow here. This series keeps
> adding and modifying its own code. Wouldn't it be simpler for everyone
> if you directly implement the final mv88e6xxx_port_hidden_{read,write}
> functions taking this block argument, and update the code to switch to it?

I wanted the commits to be atomic, in the sense that one commit does
not do three different things at once. Renaming macros is cosmetic
change, and moving functions to another file is a not a semantic
change, while adding additional argument to functions is a semantic
change. I can of course do all in one patch, but I though it would be
better not to.

> While at it, I don't really mind the "hidden" name, but is this the name
> used in the documentation, if any?

Yes, the registers are indeed named Hidden Registers in documentation.

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ