linux-kernel - Re: [PATCH 2/9] hwrng: arm-smccc-trng - transition to the faux device interface

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: <2025031709-unmoved-carton-c130@gregkh>
Date: Mon, 17 Mar 2025 15:30:15 +0100
From: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To: Sudeep Holla <sudeep.holla@....com>
Cc: linux-kernel@...r.kernel.org, Andre Przywara <andre.przywara@....com>,
	Herbert Xu <herbert@...dor.apana.org.au>,
	Jeff Johnson <jeff.johnson@....qualcomm.com>,
	linux-crypto@...r.kernel.org
Subject: Re: [PATCH 2/9] hwrng: arm-smccc-trng - transition to the faux
 device interface

On Mon, Mar 17, 2025 at 02:22:45PM +0000, Sudeep Holla wrote:
> On Mon, Mar 17, 2025 at 02:04:27PM +0100, Greg Kroah-Hartman wrote:
> > On Mon, Mar 17, 2025 at 10:13:14AM +0000, Sudeep Holla wrote:
> > > +MODULE_ALIAS("faux:smccc_trng");
> > 
> > Why do you need a branch new alias you just made up?  Please don't add
> > that for these types of devices, that's not going to work at all (just
> > like the platform alias really doesn't work well.
> > 
> 
> Sure I will drop all of those alias. One question I have if the idea of
> creating a macro for this is good or bad ? I need this initial condition
> flag to make use of such a macro, so I didn't go for it, but it does
> remove some boiler-plate code.
> 
> Let me know what do you think of it ?
> 
> Regards,
> Sudeep
> 
> -->8 
> diff --git i/include/linux/device/faux.h w/include/linux/device/faux.h
> index 9f43c0e46aa4..8af3eaef281a 100644
> --- i/include/linux/device/faux.h
> +++ w/include/linux/device/faux.h
> @@ -66,4 +66,30 @@ static inline void faux_device_set_drvdata(struct faux_device *faux_dev, void *d
>  	dev_set_drvdata(&faux_dev->dev, data);
>  }
>  
> +#define module_faux_driver(name, tag, init_cond)			\
> +static struct faux_device_ops tag##_ops = {				\
> +	.probe = tag##_probe,						\
> +	.remove = tag##_remove,						\
> +};									\
> +									\
> +static struct faux_device *tag##_dev;					\
> +									\
> +static int __init tag##_init(void)					\
> +{									\
> +	if (!(init_cond))						\
> +		return 0;						\
> +	tag##_dev = faux_device_create(name, NULL, &tag##_ops);		\
> +	if (!tag##_dev)							\
> +		return -ENODEV;						\
> +									\
> +	return 0;							\
> +}									\
> +module_init(tag##_init);						\
> +									\
> +static void __exit tag##_exit(void)					\
> +{									\
> +	faux_device_destroy(tag##_dev);					\
> +}									\
> +module_exit(tag##_exit);						\

Yes, I see that some of your changes could be moved to use this, so I
think it is worth it.

But why can't you use module_driver() here?  Ah, that init_cond?  And
the device.  Hm, why not put the init_cond in the probe callback?  That
should make this macro simpler.

And don't use "tag", that's an odd term here, just copy what
module_driver() does instead please.

thanks,

greg k-h