netdev - Re: [v2,1/9] fsl/fman: Add the FMan FLIB

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] [day] [month] [year] [list]

Date:	Thu, 25 Jun 2015 20:26:31 -0500
From:	Scott Wood <scottwood@...escale.com>
To:	igal.liberman@...escale.com
CC:	netdev@...r.kernel.org, linuxppc-dev@...ts.ozlabs.org,
	madalin.bucur@...escale.com, pebolle@...cali.nl
Subject: Re: [v2,1/9] fsl/fman: Add the FMan FLIB

On Wed, 2015-06-24 at 22:33 +0300, igal.liberman@...escale.com wrote:
> From: Igal Liberman <Igal.Liberman@...escale.com>
> 
> The FMan FLib provides the basic API used by the FMan drivers to
> > configure and control the FMan hardware.
> 
> Signed-off-by: Igal Liberman <Igal.Liberman@...escale.com>

Again, what is an FLib?  What determines whether content should go in 
the "flib" directory?

The patch title says "Add the FMan FLIB", but there's more code added 
outside the "flib" directory than inside.
 
"FMan drivers"?  There's more than one?  What is 
"drivers/net/ethernet/freescale/fman/fman.c" if not "the FMan driver"? 
 What is "the FMan driver" if not the code "to configure and control 
the FMan hardware"?  If this is a public API, where's the 
documentation?

---
>  drivers/net/ethernet/freescale/Kconfig             |    1 +
>  drivers/net/ethernet/freescale/Makefile            |    2 +
>  drivers/net/ethernet/freescale/fman/Kconfig        |    7 +
>  drivers/net/ethernet/freescale/fman/Makefile       |    5 +
>  .../net/ethernet/freescale/fman/flib/fsl_fman.h    |  608 
> ++++++++++++
>  drivers/net/ethernet/freescale/fman/fman.c         |  975 
> ++++++++++++++++++++
>  6 files changed, 1598 insertions(+)
>  create mode 100644 drivers/net/ethernet/freescale/fman/Kconfig
>  create mode 100644 drivers/net/ethernet/freescale/fman/Makefile
>  create mode 100644 
> drivers/net/ethernet/freescale/fman/flib/fsl_fman.h
>  create mode 100644 drivers/net/ethernet/freescale/fman/fman.c
> 
> diff --git a/drivers/net/ethernet/freescale/Kconfig 
> b/drivers/net/ethernet/freescale/Kconfig
> index 25e3425..24e938d 100644
> --- a/drivers/net/ethernet/freescale/Kconfig
> +++ b/drivers/net/ethernet/freescale/Kconfig
> @@ -55,6 +55,7 @@ config FEC_MPC52xx_MDIO
>         If compiled as module, it will be called fec_mpc52xx_phy.
>  
>  source "drivers/net/ethernet/freescale/fs_enet/Kconfig"
> +source "drivers/net/ethernet/freescale/fman/Kconfig"
>  
>  config FSL_PQ_MDIO
>       tristate "Freescale PQ MDIO"
> diff --git a/drivers/net/ethernet/freescale/Makefile 
> b/drivers/net/ethernet/freescale/Makefile
> index 71debd1..4097c58 100644
> --- a/drivers/net/ethernet/freescale/Makefile
> +++ b/drivers/net/ethernet/freescale/Makefile
> @@ -17,3 +17,5 @@ gianfar_driver-objs := gianfar.o \
>               gianfar_ethtool.o
>  obj-$(CONFIG_UCC_GETH) += ucc_geth_driver.o
>  ucc_geth_driver-objs := ucc_geth.o ucc_geth_ethtool.o
> +
> +obj-$(CONFIG_FSL_FMAN) += fman/
> diff --git a/drivers/net/ethernet/freescale/fman/Kconfig 
> b/drivers/net/ethernet/freescale/fman/Kconfig
> new file mode 100644
> index 0000000..8aeae29
> --- /dev/null
> +++ b/drivers/net/ethernet/freescale/fman/Kconfig
> @@ -0,0 +1,7 @@
> +config FSL_FMAN
> +     bool "FMan support"
> +     depends on FSL_SOC || COMPILE_TEST
> +     default n
> +     help
> +             Freescale Data-Path Acceleration Architecture Frame Manager
> +             (FMan) support

"default n" is a no-op.

What does enabling this option actually do, in terms of user-visible 
features?

> +typedef struct fm_prs_result_t fm_prs_result;
> +typedef enum e_enet_mode enet_mode_t;

This use of typedef is contrary to kernel coding style.

-Scott
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html