netdev - Re: Help needed: supporting new device with unique register bitfields

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: <ZEuIGbUz3x4r8KTj@DEN-LT-70577>
Date:   Fri, 28 Apr 2023 08:47:22 +0000
From:   <Daniel.Machon@...rochip.com>
To:     <kuba@...nel.org>
CC:     <chris.chenfeiyang@...il.com>, <peppe.cavallaro@...com>,
        <alexandre.torgue@...s.st.com>, <joabreu@...opsys.com>,
        <netdev@...r.kernel.org>
Subject: Re: Help needed: supporting new device with unique register bitfields

Den Mon, Apr 24, 2023 at 12:53:57PM -0700 skrev Jakub Kicinski:
> On Sun, 23 Apr 2023 16:19:11 +0800 Feiyang Chen wrote:
> > We are hoping to add support for a new device which shares almost
> > identical logic with dwmac1000 (dwmac_lib.c, dwmac1000_core.c, and
> > dwmac1000_dma.c), but with significant differences in the register
> > bitfields (dwmac_dma.h and dwmac1000.h).
> > 
> > We are seeking guidance on the best approach to support this new
> > device. Any advice on how to proceed would be greatly appreciated.
> > 
> > Thank you for your time and expertise.
> 
> There's no recipe on how to support devices with different register
> layout :(  You'll need to find the right balance of (1) indirect calls,
> (2) if conditions and (3) static description data that's right for you.
> 
> Static description data (e.g. putting register addresses in a struct
> and using the members of that struct rather than #defines) is probably
> the best but the least flexible.
> 
> Adding the stmmac maintainers.

Hi,

I thought I would chip in, as we are facing a similar case as described
here.

I am working on adding support for a new chip, that shares most of the
same logic as Sparx5, yet with differences in register layout. Our
approach has basically been what Jakub describes.

- We use a macro-based approach for accessing registers (see
  sparx5_main_regs.h). We have added a layer of indirection, so that any
  _differences_ between the two chips (offset, count, width etc.) have
  been moved into respective structs, which is then consulted when
  accessing registers. This allows us to reuse most of the Sparx5 code.

- Any unique chip features or similar are ops'ed out. In a few cases
  handled by if's.

- Additionally, chip-specific constants like port count, buffer sizes
  etc.  are statically described for the driver.

I think this has worked out pretty well so far. But again, this is
balance, as Jakub said. If the differences are too great, it might not
be the best solution for you.

/Daniel