linux-kernel - Re: [PATCH net-next v2 2/2] misc: Add Renesas Synchronization Management Unit (SMU) support

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: <YG3vu9XQ94w5dlbp@kroah.com>
Date:   Wed, 7 Apr 2021 19:45:31 +0200
From:   Greg KH <gregkh@...uxfoundation.org>
To:     min.li.xe@...esas.com
Cc:     derek.kiernan@...inx.com, dragan.cvetic@...inx.com, arnd@...db.de,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH net-next v2 2/2] misc: Add Renesas Synchronization
 Management Unit (SMU) support

On Wed, Apr 07, 2021 at 01:33:35PM -0400, min.li.xe@...esas.com wrote:
> From: Min Li <min.li.xe@...esas.com>
> 
> This driver is developed for the IDT ClockMatrix(TM) and 82P33xxx families
> of timing and synchronization devices.It will be used by Renesas PTP Clock
> Manager for Linux (pcm4l) software to provide support to GNSS assisted
> partial timing support (APTS) and other networking timing functions.
> 
> Current version provides kernel API's to support the following functions
> -set combomode to enable SYNCE clock support
> -read dpll's state to determine if the dpll is locked to the GNSS channel
> -read dpll's ffo (fractional frequency offset) in ppqt
> 
> Signed-off-by: Min Li <min.li.xe@...esas.com>
> ---
> Change log
> -rebase change to linux-next tree
> -remove uncessary condition checks suggested by Greg
> -fix compile error for x86_64
> -register device through misc_register suggested by Greg
> 
>  drivers/misc/Kconfig      |   9 ++
>  drivers/misc/Makefile     |   2 +
>  drivers/misc/rsmu_cdev.c  | 266 ++++++++++++++++++++++++++++++++++++++++++++++
>  drivers/misc/rsmu_cdev.h  |  74 +++++++++++++
>  drivers/misc/rsmu_cm.c    | 166 +++++++++++++++++++++++++++++
>  drivers/misc/rsmu_sabre.c | 133 +++++++++++++++++++++++

Why do you need 4 files here?  Can't you do this all in one?  There's no
need for such a small driver to be split up, that just causes added
complexity and makes things harder to review and understand.

>  include/uapi/linux/rsmu.h |  64 +++++++++++

Where are you documenting these new custom ioctls?  And why do you even
need them?

thanks,

greg k-h