linux-kernel - Re: [PATCH 1/2] net: dpaa2: move DPAA2 PTP driver out of staging/

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]

Date:   Thu, 27 Sep 2018 15:25:07 +0200
From:   Andrew Lunn <andrew@...n.ch>
To:     Yangbo Lu <yangbo.lu@....com>
Cc:     linux-kernel@...r.kernel.org, devel@...verdev.osuosl.org,
        netdev@...r.kernel.org, Richard Cochran <richardcochran@...il.com>,
        "David S . Miller" <davem@...emloft.net>,
        Ioana Radulescu <ruxandra.radulescu@....com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Subject: Re: [PATCH 1/2] net: dpaa2: move DPAA2 PTP driver out of staging/

On Thu, Sep 27, 2018 at 07:12:27PM +0800, Yangbo Lu wrote:
> This patch is to move DPAA2 PTP driver out of staging/
> since the dpaa2-eth had been moved out.
> 
> Signed-off-by: Yangbo Lu <yangbo.lu@....com>
> ---
>  drivers/net/ethernet/freescale/Kconfig             |    9 +--------
>  drivers/net/ethernet/freescale/dpaa2/Kconfig       |   15 +++++++++++++++
>  drivers/net/ethernet/freescale/dpaa2/Makefile      |    6 ++++--
>  .../ethernet/freescale/dpaa2}/dprtc-cmd.h          |    0
>  .../rtc => net/ethernet/freescale/dpaa2}/dprtc.c   |    0
>  .../rtc => net/ethernet/freescale/dpaa2}/dprtc.h   |    0
>  .../rtc => net/ethernet/freescale/dpaa2}/rtc.c     |    0
>  .../rtc => net/ethernet/freescale/dpaa2}/rtc.h     |    0
>  drivers/staging/fsl-dpaa2/Kconfig                  |    8 --------
>  drivers/staging/fsl-dpaa2/Makefile                 |    1 -
>  drivers/staging/fsl-dpaa2/rtc/Makefile             |    7 -------
>  11 files changed, 20 insertions(+), 26 deletions(-)
>  create mode 100644 drivers/net/ethernet/freescale/dpaa2/Kconfig
>  rename drivers/{staging/fsl-dpaa2/rtc => net/ethernet/freescale/dpaa2}/dprtc-cmd.h (100%)
>  rename drivers/{staging/fsl-dpaa2/rtc => net/ethernet/freescale/dpaa2}/dprtc.c (100%)
>  rename drivers/{staging/fsl-dpaa2/rtc => net/ethernet/freescale/dpaa2}/dprtc.h (100%)
>  rename drivers/{staging/fsl-dpaa2/rtc => net/ethernet/freescale/dpaa2}/rtc.c (100%)
>  rename drivers/{staging/fsl-dpaa2/rtc => net/ethernet/freescale/dpaa2}/rtc.h (100%)

Hi Yangbo

Calling a ptp driver rtc.[ch] seems rather odd. Could you fixup the
name, change it to ptp.[ch]. Also, some of the function names, and
structures, rtc_probe->ptp_probe, rtc_remove->ptp_remove,
rtc_match_id_table-> ptp_match_id_table, etc.

ptp_dpaa2_adjfreq() probably should return err, not 0.
ptp_dpaa2_gettime() again does not return the error.
If fact, it seems like all the main functions ignore errors.

kzalloc() could be changed to devm_kzalloc() to simplify the cleanup
Can ptp_dpaa2_caps be made const?
dpaa2_phc_index does not appear to be used.
dev_set_drvdata(dev, NULL); is not needed.
Can rtc_drv be made const?
Is rtc.h used by anything other than rtc.c? It seems like it can be removed.

It seems like there is a lot of code in dprtc.c which is unused. rtc.c
does nothing with interrupts for example. Do you plan to make use of
this extra code? Or can it be removed leaving just what is needed?

struct dprtc_cmd_get_irq - Putting pad at the beginning of a struct
seems very odd. And it is not the only example.

      Andrew