On Wed, Oct 13, 2021 at 2:51 PM Vegard Nossum <vegard.nossum@xxxxxxxxxx> wrote:
> On 10/13/21 2:26 PM, Greg Kroah-Hartman wrote:
> > On Mon, Oct 11, 2021 at 05:29:41PM +0200, Vegard Nossum wrote:
> >> Fix the following build/link errors:
> >>
> >> ld: drivers/staging/ks7010/ks_hostif.o: in function `michael_mic.constprop.0':
> >> ks_hostif.c:(.text+0x95b): undefined reference to `crypto_alloc_shash'
> >> ld: ks_hostif.c:(.text+0x97a): undefined reference to `crypto_shash_setkey'
> >> ld: ks_hostif.c:(.text+0xa13): undefined reference to `crypto_shash_update'
> >> ld: ks_hostif.c:(.text+0xa28): undefined reference to `crypto_shash_update'
> >> ld: ks_hostif.c:(.text+0xa48): undefined reference to `crypto_shash_finup'
> >> ld: ks_hostif.c:(.text+0xa6d): undefined reference to `crypto_destroy_tfm'
> >>
> >> Signed-off-by: Vegard Nossum <vegard.nossum@xxxxxxxxxx>
> >> ---
> >> drivers/staging/ks7010/Kconfig | 3 +++
> >> 1 file changed, 3 insertions(+)
> >>
> >> diff --git a/drivers/staging/ks7010/Kconfig b/drivers/staging/ks7010/Kconfig
> >> index 0987fdc2f70db..8ea6c09286798 100644
> >> --- a/drivers/staging/ks7010/Kconfig
> >> +++ b/drivers/staging/ks7010/Kconfig
> >> @@ -5,6 +5,9 @@ config KS7010
> >> select WIRELESS_EXT
> >> select WEXT_PRIV
> >> select FW_LOADER
> >> + select CRYPTO
> >> + select CRYPTO_HASH
> >> + select CRYPTO_MICHAEL_MIC
> >
> > Let's try to rely on 'depend' and not 'select' please.
>
> I used 'select' because it seemed to be the established pattern for
> these options.
Yes, this is the correct way to do it here. In general, using "depends on"
is better than "select", especially when crossing subsystem boundaries,
however mixing the two is what really hurts because of the circular
dependencies you'd get.
The only way we could use 'depends on' here would be to change all
the other drivers to do the same.
> Compare:
>
> $ find -name '*Kconfig*' | xargs git grep 'depends on CRYPTO$' | wc --lines
> 1
>
> $ find -name '*Kconfig*' | xargs git grep 'select CRYPTO$' | wc --lines
> 66
>
> $ find -name '*Kconfig*' | xargs git grep 'depends on CRYPTO' | wc --lines
> 87
>
> $ find -name '*Kconfig*' | xargs git grep 'select CRYPTO' | wc --lines
> 1005
>
> That said, I have found several other cases where CRYPTO_* algorithms
> are getting 'select'-ed without also selecting CRYPTO/CRYPTO_HASH, so I
> definitely see the problem you're trying to address.
>
> I've added some more people on Cc to see if there is a consensus on the
> best way to do this for the CRYPTO* options going forwards. Thoughts,
> anybody?
I don't think there is much point in trying to change the
existing pattern for crypto. We might want to change some of those
that use 'depends on' at the moment for consistency, though most of
those look correct as well.
Arnd
