Re: [PATCH v1 1/1] pps: generators: pps_gen_parport: Switch to use module_parport_driver()

[Greg Kroah-Hartman]

On Wed, Jun 16, 2021 at 05:31:21PM +0300, Andy Shevchenko wrote:
> Switch to use module_parport_driver() to reduce boilerplate code.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
> ---
>  drivers/pps/generators/pps_gen_parport.c | 44 +++++-------------------
>  1 file changed, 9 insertions(+), 35 deletions(-)
> 
> diff --git a/drivers/pps/generators/pps_gen_parport.c b/drivers/pps/generators/pps_gen_parport.c
> index 6a1af7664f3b..565f99782402 100644
> --- a/drivers/pps/generators/pps_gen_parport.c
> +++ b/drivers/pps/generators/pps_gen_parport.c
> @@ -20,8 +20,6 @@
>  #include <linux/hrtimer.h>
>  #include <linux/parport.h>
>  
> -#define DRVDESC "parallel port PPS signal generator"
> -
>  #define SIGNAL		0
>  #define NO_SIGNAL	PARPORT_CONTROL_STROBE
>  
> @@ -180,6 +178,11 @@ static void parport_attach(struct parport *port)
>  {
>  	struct pardev_cb pps_cb;
>  
> +	if (send_delay > SEND_DELAY_MAX) {
> +		pr_err("delay value should be not greater then %d\n", SEND_DELAY_MAX);
> +		return -EINVAL;

Note this is at a later point in time than before, are you sure this is
ok?

> +	}
> +
>  	if (attached) {
>  		/* we already have a port */
>  		return;
> @@ -223,6 +226,8 @@ static void parport_detach(struct parport *port)
>  	hrtimer_cancel(&device.timer);
>  	parport_release(device.pardev);
>  	parport_unregister_device(device.pardev);
> +
> +	pr_info("hrtimer avg error is %ldns\n", hrtimer_error);

Why is this line needed?  When drivers work properly, they are quiet.

I know the existing code has it, no reason it needs to stay here, that's
why we created these macros, to remove the ability for drivers to be
printing junk like this that they do not need to be printing.

thanks,

greg k-h