linux-kernel - Re: [PATCH net-next V3] net: lan743x: Add support to SGMII register dump for PCI11010/PCI11414 chips

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] [day] [month] [year] [list]

Message-ID: <20221012050252.GA70729@raju-project-pc>
Date:   Wed, 12 Oct 2022 10:32:52 +0530
From:   Raju Lakkaraju <Raju.Lakkaraju@...rochip.com>
To:     Andrew Lunn <andrew@...n.ch>
CC:     <netdev@...r.kernel.org>, <davem@...emloft.net>, <kuba@...nel.org>,
        <linux-kernel@...r.kernel.org>, <bryan.whitehead@...rochip.com>,
        <edumazet@...gle.com>, <pabeni@...hat.com>,
        <UNGLinuxDriver@...rochip.com>
Subject: Re: [PATCH net-next V3] net: lan743x: Add support to SGMII register
 dump for PCI11010/PCI11414 chips

Hi Andrew,

Thank you for review comments.

The 10/10/2022 21:35, Andrew Lunn wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> >  static int lan743x_get_regs_len(struct net_device *dev)
> >  {
> > -     return MAX_LAN743X_ETH_REGS * sizeof(u32);
> > +     struct lan743x_adapter *adapter = netdev_priv(dev);
> > +     u32 num_regs = MAX_LAN743X_ETH_COMMON_REGS;
> > +
> > +     if (adapter->is_sgmii_en)
> > +             num_regs += MAX_LAN743X_ETH_SGMII_REGS;
> > +
> > +     return num_regs * sizeof(u32);
> >  }
> >
> >  static void lan743x_get_regs(struct net_device *dev,
> >                            struct ethtool_regs *regs, void *p)
> >  {
> > +     struct lan743x_adapter *adapter = netdev_priv(dev);
> > +     int regs_len;
> > +
> > +     regs_len = lan743x_get_regs_len(dev);
> > +     memset(p, 0, regs_len);
> > +
> >       regs->version = LAN743X_ETH_REG_VERSION;
> > +     regs->len = regs_len;
> > +
> > +     lan743x_common_regs(dev, p);
> > +     p = (u32 *)p + MAX_LAN743X_ETH_COMMON_REGS;
> >
> > -     lan743x_common_regs(dev, regs, p);
> > +     if (adapter->is_sgmii_en) {
> > +             lan743x_sgmii_regs(dev, p);
> > +             p = (u32 *)p + MAX_LAN743X_ETH_SGMII_REGS;
> > +     }
> 
> This seems O.K. for the moment, but how does it work when you add the
> next set of optional registers? Say you want to add the PTP registers?

Yes. 
For other modules (i.e. PTP etc) register dumps, i would like
implement ethtool -w/-W option to configure dump flag.
Existing private flag use for EEPROM/OTP access which we don't use same
flag for register dumps to avoid accidental changes which may damage
EEPROM/OTP sensitive data.

> 
> One idea might be to use the LAN743X_ETH_REG_VERSION as a
> bitfield. Bit 0 indicates the common registers are present. Bit 1
> indicates the SGMII registers are present. Bit 2 is for whatever next
> set of optional registers you add, say PTP.

OK. 
Otherwise, I will create LAN743X_ETH_REG_FLAGS and follow your
suggestion.

> 
>     Andrew

--------
Thanks,
Raju