netdev - Re: [PATCH 11/14] net: ks8851: Implement register and FIFO accessor callbacks

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: <20200324134555.wgtvj4owmkw3jup4@wunner.de>
Date:   Tue, 24 Mar 2020 14:45:55 +0100
From:   Lukas Wunner <lukas@...ner.de>
To:     Marek Vasut <marex@...x.de>
Cc:     netdev@...r.kernel.org, "David S . Miller" <davem@...emloft.net>,
        Petr Stetiar <ynezz@...e.cz>,
        YueHaibing <yuehaibing@...wei.com>, Andrew Lunn <andrew@...n.ch>
Subject: Re: [PATCH 11/14] net: ks8851: Implement register and FIFO accessor
 callbacks

On Tue, Mar 24, 2020 at 12:43:00AM +0100, Marek Vasut wrote:
> The register and FIFO accessors are bus specific. Implement callbacks so
> that each variant of the KS8851 can implement matching accessors and use
> the rest of the common code.
[...]
> +	unsigned int		(*rdreg16)(struct ks8851_net *ks,
> +					   unsigned int reg);
> +	void			(*wrreg16)(struct ks8851_net *ks,
> +					   unsigned int reg, unsigned int val);
> +	void			(*rdfifo)(struct ks8851_net *ks, u8 *buff,
> +					  unsigned int len);
> +	void			(*wrfifo)(struct ks8851_net *ks,
> +					  struct sk_buff *txp, bool irq);

Using callbacks entails a dereference for each invocation.

A cheaper approach is to just declare the function signatures
in ks8851.h and provide non-static implementations in
ks8851_spi.c and ks8851_mll.c, so I'd strongly prefer that.

Even better, since this only concerns the register accessors
(which are inlined anyway by the compiler), it would be best
to have them in header files (e.g. ks8851_spi.h / ks8851_par.h)
which are included by the common ks8851.c based on the target
which is being compiled.  That involves a bit of kbuild magic
though to generate two different .o from the same .c file,
each with specific "-include ..." CFLAGS.

Thanks,

Lukas