Re: [PATCH 01/12] fsl/fman: Add the FMan FLIB headers

2015-06-17 Thread Scott Wood
On Wed, 2015-06-17 at 09:59 -0500, Liberman Igal-B31950 wrote:
 
 Regards,
 Igal Liberman.
 
  -Original Message-
  From: Wood Scott-B07421
  Sent: Wednesday, June 10, 2015 9:54 PM
  To: Bucur Madalin-Cristian-B32716
  Cc: net...@vger.kernel.org; linux-ker...@vger.kernel.org; linuxppc-
  d...@lists.ozlabs.org; Liberman Igal-B31950
  Subject: Re: [PATCH 01/12] fsl/fman: Add the FMan FLIB headers
  
  On Wed, 2015-06-10 at 18:21 +0300, Madalin Bucur wrote:
   From: Igal Liberman igal.liber...@freescale.com
   
   This patch presents the FMan Foundation Libraries (FLIB) headers.
   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.liber...@freescale.com
   ---
.../ethernet/freescale/fman/flib/common/general.h  |  41 ++
.../net/ethernet/freescale/fman/flib/fsl_fman.h| 609
   +
2 files changed, 650 insertions(+)
create mode 100644
   drivers/net/ethernet/freescale/fman/flib/common/general.h
create mode 100644
   drivers/net/ethernet/freescale/fman/flib/fsl_fman.h
  
  Why do we need separate patches just for headers?
  
 
 We wanted to make the patches smaller, it's the main reason for this 
 separation.

Patches should be divided by function, not arbitrarily in order to 
decrease size.  Splitting like this makes it harder to see the whole 
picture, to search for identifiers, etc.

The right way to make these patches smaller is to remove unnecessary 
features.


  What does the flib directory mean, in the context of Linux?  If 
  someone were
  to add code to this driver, how do they know if the code should go 
  into the
  flib directory or not?
  
   
   +#define iowrite32be(val, addr)   out_be32((*addr), 
   val)
   +#define ioread32be(addr) in_be32((*addr))
  
  iowrite32be()/ioread32be() are already defined for all relevant 
  architectures.
  Why are you redefining them into something PPC- specific?
  
 
 Removed those. 

Please don't stop at the specific things I'm pointing out.

   +/* do not change! if changed, must be disabled for rev1 ! */ 
   #define
   +DEFAULT_HALT_ON_EXTERNAL_ACTIVATION  false
   +/* do not change! if changed, must be disabled for rev1 ! */ 
   #define
   +DEFAULT_HALT_ON_UNRECOVERABLE_ECC_ERROR false
  
  rev1 of what chip?
  
 
 P4080. I'll update the comments. 

No.  p4080rev1 is not supported, and in any case this is not the right 
way to select different errata for different chips.

-Scott


___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

RE: [PATCH 01/12] fsl/fman: Add the FMan FLIB headers

2015-06-17 Thread Liberman Igal


Regards,
Igal Liberman.

 -Original Message-
 From: Wood Scott-B07421
 Sent: Wednesday, June 10, 2015 9:54 PM
 To: Bucur Madalin-Cristian-B32716
 Cc: net...@vger.kernel.org; linux-ker...@vger.kernel.org; linuxppc-
 d...@lists.ozlabs.org; Liberman Igal-B31950
 Subject: Re: [PATCH 01/12] fsl/fman: Add the FMan FLIB headers
 
 On Wed, 2015-06-10 at 18:21 +0300, Madalin Bucur wrote:
  From: Igal Liberman igal.liber...@freescale.com
 
  This patch presents the FMan Foundation Libraries (FLIB) headers.
  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.liber...@freescale.com
  ---
   .../ethernet/freescale/fman/flib/common/general.h  |  41 ++
   .../net/ethernet/freescale/fman/flib/fsl_fman.h| 609
  +
   2 files changed, 650 insertions(+)
   create mode 100644
  drivers/net/ethernet/freescale/fman/flib/common/general.h
   create mode 100644
  drivers/net/ethernet/freescale/fman/flib/fsl_fman.h
 
 Why do we need separate patches just for headers?
 

We wanted to make the patches smaller, it's the main reason for this separation.

 What does common refer to?
 

I removed ./flib/common.

 What does the flib directory mean, in the context of Linux?  If someone were
 to add code to this driver, how do they know if the code should go into the
 flib directory or not?
 
 
  +#define iowrite32be(val, addr)   out_be32((*addr), val)
  +#define ioread32be(addr) in_be32((*addr))
 
 iowrite32be()/ioread32be() are already defined for all relevant architectures.
 Why are you redefining them into something PPC- specific?
 

Removed those. 

  +/* do not change! if changed, must be disabled for rev1 ! */ #define
  +DEFAULT_HALT_ON_EXTERNAL_ACTIVATION  false
  +/* do not change! if changed, must be disabled for rev1 ! */ #define
  +DEFAULT_HALT_ON_UNRECOVERABLE_ECC_ERROR false
 
 rev1 of what chip?
 

P4080. I'll update the comments. 

 -Scott

Igal.

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH 01/12] fsl/fman: Add the FMan FLIB headers

2015-06-10 Thread Igal . Liberman
From: Igal Liberman igal.liber...@freescale.com

This patch presents the FMan Foundation Libraries (FLIB) headers.
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.liber...@freescale.com
---
 .../ethernet/freescale/fman/flib/common/general.h  |   41 ++
 .../net/ethernet/freescale/fman/flib/fsl_fman.h|  609 
 2 files changed, 650 insertions(+)
 create mode 100644 drivers/net/ethernet/freescale/fman/flib/common/general.h
 create mode 100644 drivers/net/ethernet/freescale/fman/flib/fsl_fman.h

diff --git a/drivers/net/ethernet/freescale/fman/flib/common/general.h 
b/drivers/net/ethernet/freescale/fman/flib/common/general.h
new file mode 100644
index 000..0501f01
--- /dev/null
+++ b/drivers/net/ethernet/freescale/fman/flib/common/general.h
@@ -0,0 +1,41 @@
+/*
+ * Copyright 2008 - 2015 Freescale Semiconductor Inc.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are met:
+ * * Redistributions of source code must retain the above copyright
+ *  notice, this list of conditions and the following disclaimer.
+ * * Redistributions in binary form must reproduce the above copyright
+ *  notice, this list of conditions and the following disclaimer in the
+ *  documentation and/or other materials provided with the distribution.
+ * * Neither the name of Freescale Semiconductor nor the
+ *  names of its contributors may be used to endorse or promote products
+ *  derived from this software without specific prior written permission.
+ *
+ * ALTERNATIVELY, this software may be distributed under the terms of the
+ * GNU General Public License (GPL) as published by the Free Software
+ * Foundation, either version 2 of that License or (at your option) any
+ * later version.
+ *
+ * THIS SOFTWARE IS PROVIDED BY Freescale Semiconductor ``AS IS'' AND ANY
+ * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
+ * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+ * DISCLAIMED. IN NO EVENT SHALL Freescale Semiconductor BE LIABLE FOR ANY
+ * DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
+ * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
+ * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF 
THIS
+ * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#ifndef __GENERAL_H
+#define __GENERAL_H
+
+#include linux/types.h
+#include linux/io.h
+
+#define iowrite32be(val, addr) out_be32((*addr), val)
+#define ioread32be(addr)   in_be32((*addr))
+
+#endif /* __GENERAL_H */
diff --git a/drivers/net/ethernet/freescale/fman/flib/fsl_fman.h 
b/drivers/net/ethernet/freescale/fman/flib/fsl_fman.h
new file mode 100644
index 000..95eef30
--- /dev/null
+++ b/drivers/net/ethernet/freescale/fman/flib/fsl_fman.h
@@ -0,0 +1,609 @@
+/*
+ * Copyright 2008 - 2015 Freescale Semiconductor Inc.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are met:
+ * * Redistributions of source code must retain the above copyright
+ *  notice, this list of conditions and the following disclaimer.
+ * * Redistributions in binary form must reproduce the above copyright
+ *  notice, this list of conditions and the following disclaimer in the
+ *  documentation and/or other materials provided with the distribution.
+ * * Neither the name of Freescale Semiconductor nor the
+ *  names of its contributors may be used to endorse or promote products
+ *  derived from this software without specific prior written permission.
+ *
+ * ALTERNATIVELY, this software may be distributed under the terms of the
+ * GNU General Public License (GPL) as published by the Free Software
+ * Foundation, either version 2 of that License or (at your option) any
+ * later version.
+ *
+ * THIS SOFTWARE IS PROVIDED BY Freescale Semiconductor ``AS IS'' AND ANY
+ * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
+ * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+ * DISCLAIMED. IN NO EVENT SHALL Freescale Semiconductor BE LIABLE FOR ANY
+ * DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
+ * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
+ * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF 
THIS
+ * SOFTWARE, EVEN IF ADVISED 

Re: [PATCH 01/12] fsl/fman: Add the FMan FLIB headers

2015-06-10 Thread Scott Wood
On Wed, 2015-06-10 at 18:21 +0300, Madalin Bucur wrote:
 From: Igal Liberman igal.liber...@freescale.com
 
 This patch presents the FMan Foundation Libraries (FLIB) headers.
 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.liber...@freescale.com
 ---
  .../ethernet/freescale/fman/flib/common/general.h  |  41 ++
  .../net/ethernet/freescale/fman/flib/fsl_fman.h| 609 
 +
  2 files changed, 650 insertions(+)
  create mode 100644 
 drivers/net/ethernet/freescale/fman/flib/common/general.h
  create mode 100644 
 drivers/net/ethernet/freescale/fman/flib/fsl_fman.h

Why do we need separate patches just for headers?

What does common refer to?

What does the flib directory mean, in the context of Linux?  If 
someone were to add code to this driver, how do they know if the code 
should go into the flib directory or not?

 
 +#define iowrite32be(val, addr)   out_be32((*addr), val)
 +#define ioread32be(addr) in_be32((*addr))

iowrite32be()/ioread32be() are already defined for all relevant 
architectures.  Why are you redefining them into something PPC-
specific?

 +/* do not change! if changed, must be disabled for rev1 ! */
 +#define DEFAULT_HALT_ON_EXTERNAL_ACTIVATION  false
 +/* do not change! if changed, must be disabled for rev1 ! */
 +#define DEFAULT_HALT_ON_UNRECOVERABLE_ECC_ERROR false

rev1 of what chip?

-Scott

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH 01/12] fsl/fman: Add the FMan FLIB headers

2015-06-10 Thread Madalin Bucur
From: Igal Liberman igal.liber...@freescale.com

This patch presents the FMan Foundation Libraries (FLIB) headers.
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.liber...@freescale.com
---
 .../ethernet/freescale/fman/flib/common/general.h  |  41 ++
 .../net/ethernet/freescale/fman/flib/fsl_fman.h| 609 +
 2 files changed, 650 insertions(+)
 create mode 100644 drivers/net/ethernet/freescale/fman/flib/common/general.h
 create mode 100644 drivers/net/ethernet/freescale/fman/flib/fsl_fman.h

diff --git a/drivers/net/ethernet/freescale/fman/flib/common/general.h 
b/drivers/net/ethernet/freescale/fman/flib/common/general.h
new file mode 100644
index 000..0501f01
--- /dev/null
+++ b/drivers/net/ethernet/freescale/fman/flib/common/general.h
@@ -0,0 +1,41 @@
+/*
+ * Copyright 2008 - 2015 Freescale Semiconductor Inc.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are met:
+ * * Redistributions of source code must retain the above copyright
+ *  notice, this list of conditions and the following disclaimer.
+ * * Redistributions in binary form must reproduce the above copyright
+ *  notice, this list of conditions and the following disclaimer in the
+ *  documentation and/or other materials provided with the distribution.
+ * * Neither the name of Freescale Semiconductor nor the
+ *  names of its contributors may be used to endorse or promote products
+ *  derived from this software without specific prior written permission.
+ *
+ * ALTERNATIVELY, this software may be distributed under the terms of the
+ * GNU General Public License (GPL) as published by the Free Software
+ * Foundation, either version 2 of that License or (at your option) any
+ * later version.
+ *
+ * THIS SOFTWARE IS PROVIDED BY Freescale Semiconductor ``AS IS'' AND ANY
+ * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
+ * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+ * DISCLAIMED. IN NO EVENT SHALL Freescale Semiconductor BE LIABLE FOR ANY
+ * DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
+ * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
+ * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF 
THIS
+ * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#ifndef __GENERAL_H
+#define __GENERAL_H
+
+#include linux/types.h
+#include linux/io.h
+
+#define iowrite32be(val, addr) out_be32((*addr), val)
+#define ioread32be(addr)   in_be32((*addr))
+
+#endif /* __GENERAL_H */
diff --git a/drivers/net/ethernet/freescale/fman/flib/fsl_fman.h 
b/drivers/net/ethernet/freescale/fman/flib/fsl_fman.h
new file mode 100644
index 000..95eef30
--- /dev/null
+++ b/drivers/net/ethernet/freescale/fman/flib/fsl_fman.h
@@ -0,0 +1,609 @@
+/*
+ * Copyright 2008 - 2015 Freescale Semiconductor Inc.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are met:
+ * * Redistributions of source code must retain the above copyright
+ *  notice, this list of conditions and the following disclaimer.
+ * * Redistributions in binary form must reproduce the above copyright
+ *  notice, this list of conditions and the following disclaimer in the
+ *  documentation and/or other materials provided with the distribution.
+ * * Neither the name of Freescale Semiconductor nor the
+ *  names of its contributors may be used to endorse or promote products
+ *  derived from this software without specific prior written permission.
+ *
+ * ALTERNATIVELY, this software may be distributed under the terms of the
+ * GNU General Public License (GPL) as published by the Free Software
+ * Foundation, either version 2 of that License or (at your option) any
+ * later version.
+ *
+ * THIS SOFTWARE IS PROVIDED BY Freescale Semiconductor ``AS IS'' AND ANY
+ * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
+ * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+ * DISCLAIMED. IN NO EVENT SHALL Freescale Semiconductor BE LIABLE FOR ANY
+ * DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
+ * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
+ * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF 
THIS
+ * SOFTWARE, EVEN IF ADVISED