public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
* [U-Boot] [PATCH] Fix checkpatch warnings about externs in *.c
@ 2012-09-16 11:53 Pavel Herrmann
  2012-09-16 12:37 ` Marek Vasut
  2012-09-28  9:18 ` [U-Boot] [PATCH 1/2] remove unnecessary code in ata_piix Pavel Herrmann
  0 siblings, 2 replies; 12+ messages in thread
From: Pavel Herrmann @ 2012-09-16 11:53 UTC (permalink / raw)
  To: u-boot

Move all extern declarations of sata_dev_desc into a single header file.

Signed-off-by: Pavel Herrmann <morpheus.ibis@gmail.com>
---
 drivers/block/ata_piix.c     |  4 +---
 drivers/block/dwc_ahsata.c   |  1 +
 drivers/block/dwc_ahsata.h   |  2 --
 drivers/block/fsl_sata.c     |  3 +--
 drivers/block/pata_bfin.c    |  1 +
 drivers/block/pata_bfin.h    |  2 --
 drivers/block/sata_dwc.c     |  3 +--
 drivers/block/sata_extern.h  | 30 ++++++++++++++++++++++++++++++
 drivers/block/sata_sil.c     |  1 +
 drivers/block/sata_sil.h     |  2 --
 drivers/block/sata_sil3114.c |  2 +-
 11 files changed, 37 insertions(+), 14 deletions(-)
 create mode 100644 drivers/block/sata_extern.h

diff --git a/drivers/block/ata_piix.c b/drivers/block/ata_piix.c
index c81d11a..de8d563 100644
--- a/drivers/block/ata_piix.c
+++ b/drivers/block/ata_piix.c
@@ -34,9 +34,7 @@
 #include <part.h>
 #include <ide.h>
 #include <ata.h>
-
-extern block_dev_desc_t sata_dev_desc[CONFIG_SYS_SATA_MAX_DEVICE];
-extern int sata_curr_device;
+#include "sata_extern.h"
 
 #define DEBUG_SATA 0		/*For debug prints set DEBUG_SATA to 1 */
 
diff --git a/drivers/block/dwc_ahsata.c b/drivers/block/dwc_ahsata.c
index 2703d3d..4b32466 100644
--- a/drivers/block/dwc_ahsata.c
+++ b/drivers/block/dwc_ahsata.c
@@ -33,6 +33,7 @@
 #include <linux/bitops.h>
 #include <asm/arch/clock.h>
 #include "dwc_ahsata.h"
+#include "sata_extern.h"
 
 struct sata_port_regs {
 	u32 clb;
diff --git a/drivers/block/dwc_ahsata.h b/drivers/block/dwc_ahsata.h
index 84860ea..4dac5dc 100644
--- a/drivers/block/dwc_ahsata.h
+++ b/drivers/block/dwc_ahsata.h
@@ -330,6 +330,4 @@
 #define READ_CMD	0
 #define WRITE_CMD	1
 
-extern block_dev_desc_t sata_dev_desc[CONFIG_SYS_SATA_MAX_DEVICE];
-
 #endif /* __FSL_SATA_H__ */
diff --git a/drivers/block/fsl_sata.c b/drivers/block/fsl_sata.c
index 3026ade..9691f2e 100644
--- a/drivers/block/fsl_sata.c
+++ b/drivers/block/fsl_sata.c
@@ -27,8 +27,7 @@
 #include <libata.h>
 #include <fis.h>
 #include "fsl_sata.h"
-
-extern block_dev_desc_t sata_dev_desc[CONFIG_SYS_SATA_MAX_DEVICE];
+#include "sata_extern.h"
 
 #ifndef CONFIG_SYS_SATA1_FLAGS
 	#define CONFIG_SYS_SATA1_FLAGS	FLAGS_DMA
diff --git a/drivers/block/pata_bfin.c b/drivers/block/pata_bfin.c
index cce21fb..6aacde0 100644
--- a/drivers/block/pata_bfin.c
+++ b/drivers/block/pata_bfin.c
@@ -19,6 +19,7 @@
 #include <ata.h>
 #include <libata.h>
 #include "pata_bfin.h"
+#include "sata_extern.h"
 
 static struct ata_port port[CONFIG_SYS_SATA_MAX_DEVICE];
 
diff --git a/drivers/block/pata_bfin.h b/drivers/block/pata_bfin.h
index 2b3425b..2093cf0 100644
--- a/drivers/block/pata_bfin.h
+++ b/drivers/block/pata_bfin.h
@@ -41,8 +41,6 @@ struct ata_port {
 	unsigned char dev_mask;
 };
 
-extern block_dev_desc_t sata_dev_desc[CONFIG_SYS_SATA_MAX_DEVICE];
-
 #define DRV_NAME		"pata-bfin"
 #define DRV_VERSION		"0.9"
 #define __iomem
diff --git a/drivers/block/sata_dwc.c b/drivers/block/sata_dwc.c
index 75101b5..cc41316 100644
--- a/drivers/block/sata_dwc.c
+++ b/drivers/block/sata_dwc.c
@@ -38,6 +38,7 @@
 #include <linux/ctype.h>
 
 #include "sata_dwc.h"
+#include "sata_extern.h"
 
 #define DMA_NUM_CHANS			1
 #define DMA_NUM_CHAN_REGS		8
@@ -268,8 +269,6 @@ static int ata_dev_read_id(struct ata_device *dev, unsigned int *p_class,
 		unsigned int flags, u16 *id);
 static int check_sata_dev_state(void);
 
-extern block_dev_desc_t sata_dev_desc[CONFIG_SYS_SATA_MAX_DEVICE];
-
 static const struct ata_port_info sata_dwc_port_info[] = {
 	{
 		.flags		= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY |
diff --git a/drivers/block/sata_extern.h b/drivers/block/sata_extern.h
new file mode 100644
index 0000000..3bf9df9
--- /dev/null
+++ b/drivers/block/sata_extern.h
@@ -0,0 +1,30 @@
+/*
+ * (C) Copyright 2012
+ * Pavel Herrmann <morpheus.ibis@gmail.com>
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#ifndef _BLOCK_SATA_EXTERN_H_
+#define _BLOCK_SATA_EXTERN_H_
+
+extern block_dev_desc_t sata_dev_desc[CONFIG_SYS_SATA_MAX_DEVICE];
+extern int sata_curr_device;
+
+#endif
diff --git a/drivers/block/sata_sil.c b/drivers/block/sata_sil.c
index fb7cd2a..ee9194c 100644
--- a/drivers/block/sata_sil.c
+++ b/drivers/block/sata_sil.c
@@ -27,6 +27,7 @@
 #include <fis.h>
 #include <libata.h>
 #include "sata_sil.h"
+#include "sata_extern.h"
 
 /* Convert sectorsize to wordsize */
 #define ATA_SECTOR_WORDS (ATA_SECT_SIZE/2)
diff --git a/drivers/block/sata_sil.h b/drivers/block/sata_sil.h
index 2dfd4a5..9f3a37f 100644
--- a/drivers/block/sata_sil.h
+++ b/drivers/block/sata_sil.h
@@ -24,8 +24,6 @@
 #define READ_CMD	0
 #define WRITE_CMD	1
 
-extern block_dev_desc_t sata_dev_desc[CONFIG_SYS_SATA_MAX_DEVICE];
-
 /*
  * SATA device driver struct for each dev
  */
diff --git a/drivers/block/sata_sil3114.c b/drivers/block/sata_sil3114.c
index 34fe038..da55385 100644
--- a/drivers/block/sata_sil3114.c
+++ b/drivers/block/sata_sil3114.c
@@ -30,6 +30,7 @@
 #include <ide.h>
 #include <libata.h>
 #include "sata_sil3114.h"
+#include "sata_extern.h"
 
 /* Convert sectorsize to wordsize */
 #define ATA_SECTOR_WORDS (ATA_SECT_SIZE/2)
@@ -48,7 +49,6 @@ static u8 sata_chk_status (struct sata_ioports *ioaddr, u8 usealtstatus);
 static void msleep (int count);
 
 static u32 iobase[6] = { 0, 0, 0, 0, 0, 0};	/* PCI BAR registers for device */
-extern block_dev_desc_t sata_dev_desc[CONFIG_SYS_SATA_MAX_DEVICE];
 
 static struct sata_port port[CONFIG_SYS_SATA_MAX_DEVICE];
 
-- 
1.7.12

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH] Fix checkpatch warnings about externs in *.c
  2012-09-16 11:53 [U-Boot] [PATCH] Fix checkpatch warnings about externs in *.c Pavel Herrmann
@ 2012-09-16 12:37 ` Marek Vasut
  2012-09-16 12:59   ` Pavel Herrmann
  2012-09-28  9:18 ` [U-Boot] [PATCH 1/2] remove unnecessary code in ata_piix Pavel Herrmann
  1 sibling, 1 reply; 12+ messages in thread
From: Marek Vasut @ 2012-09-16 12:37 UTC (permalink / raw)
  To: u-boot

Dear Pavel Herrmann,

> Move all extern declarations of sata_dev_desc into a single header file.
> 
> Signed-off-by: Pavel Herrmann <morpheus.ibis@gmail.com>
> ---
>  drivers/block/ata_piix.c     |  4 +---
>  drivers/block/dwc_ahsata.c   |  1 +
>  drivers/block/dwc_ahsata.h   |  2 --
>  drivers/block/fsl_sata.c     |  3 +--
>  drivers/block/pata_bfin.c    |  1 +
>  drivers/block/pata_bfin.h    |  2 --
>  drivers/block/sata_dwc.c     |  3 +--
>  drivers/block/sata_extern.h  | 30 ++++++++++++++++++++++++++++++
>  drivers/block/sata_sil.c     |  1 +
>  drivers/block/sata_sil.h     |  2 --
>  drivers/block/sata_sil3114.c |  2 +-
>  11 files changed, 37 insertions(+), 14 deletions(-)
>  create mode 100644 drivers/block/sata_extern.h

Won't include/sata.h work just fine ?

> diff --git a/drivers/block/ata_piix.c b/drivers/block/ata_piix.c
> index c81d11a..de8d563 100644
> --- a/drivers/block/ata_piix.c
> +++ b/drivers/block/ata_piix.c
> @@ -34,9 +34,7 @@
>  #include <part.h>
>  #include <ide.h>
>  #include <ata.h>
> -
> -extern block_dev_desc_t sata_dev_desc[CONFIG_SYS_SATA_MAX_DEVICE];
> -extern int sata_curr_device;
> +#include "sata_extern.h"
> 
>  #define DEBUG_SATA 0		/*For debug prints set DEBUG_SATA to 1 */
> 
> diff --git a/drivers/block/dwc_ahsata.c b/drivers/block/dwc_ahsata.c
> index 2703d3d..4b32466 100644
> --- a/drivers/block/dwc_ahsata.c
> +++ b/drivers/block/dwc_ahsata.c
> @@ -33,6 +33,7 @@
>  #include <linux/bitops.h>
>  #include <asm/arch/clock.h>
>  #include "dwc_ahsata.h"
> +#include "sata_extern.h"
> 
>  struct sata_port_regs {
>  	u32 clb;
> diff --git a/drivers/block/dwc_ahsata.h b/drivers/block/dwc_ahsata.h
> index 84860ea..4dac5dc 100644
> --- a/drivers/block/dwc_ahsata.h
> +++ b/drivers/block/dwc_ahsata.h
> @@ -330,6 +330,4 @@
>  #define READ_CMD	0
>  #define WRITE_CMD	1
> 
> -extern block_dev_desc_t sata_dev_desc[CONFIG_SYS_SATA_MAX_DEVICE];
> -
>  #endif /* __FSL_SATA_H__ */
> diff --git a/drivers/block/fsl_sata.c b/drivers/block/fsl_sata.c
> index 3026ade..9691f2e 100644
> --- a/drivers/block/fsl_sata.c
> +++ b/drivers/block/fsl_sata.c
> @@ -27,8 +27,7 @@
>  #include <libata.h>
>  #include <fis.h>
>  #include "fsl_sata.h"
> -
> -extern block_dev_desc_t sata_dev_desc[CONFIG_SYS_SATA_MAX_DEVICE];
> +#include "sata_extern.h"
> 
>  #ifndef CONFIG_SYS_SATA1_FLAGS
>  	#define CONFIG_SYS_SATA1_FLAGS	FLAGS_DMA
> diff --git a/drivers/block/pata_bfin.c b/drivers/block/pata_bfin.c
> index cce21fb..6aacde0 100644
> --- a/drivers/block/pata_bfin.c
> +++ b/drivers/block/pata_bfin.c
> @@ -19,6 +19,7 @@
>  #include <ata.h>
>  #include <libata.h>
>  #include "pata_bfin.h"
> +#include "sata_extern.h"
> 
>  static struct ata_port port[CONFIG_SYS_SATA_MAX_DEVICE];
> 
> diff --git a/drivers/block/pata_bfin.h b/drivers/block/pata_bfin.h
> index 2b3425b..2093cf0 100644
> --- a/drivers/block/pata_bfin.h
> +++ b/drivers/block/pata_bfin.h
> @@ -41,8 +41,6 @@ struct ata_port {
>  	unsigned char dev_mask;
>  };
> 
> -extern block_dev_desc_t sata_dev_desc[CONFIG_SYS_SATA_MAX_DEVICE];
> -
>  #define DRV_NAME		"pata-bfin"
>  #define DRV_VERSION		"0.9"
>  #define __iomem
> diff --git a/drivers/block/sata_dwc.c b/drivers/block/sata_dwc.c
> index 75101b5..cc41316 100644
> --- a/drivers/block/sata_dwc.c
> +++ b/drivers/block/sata_dwc.c
> @@ -38,6 +38,7 @@
>  #include <linux/ctype.h>
> 
>  #include "sata_dwc.h"
> +#include "sata_extern.h"
> 
>  #define DMA_NUM_CHANS			1
>  #define DMA_NUM_CHAN_REGS		8
> @@ -268,8 +269,6 @@ static int ata_dev_read_id(struct ata_device *dev,
> unsigned int *p_class, unsigned int flags, u16 *id);
>  static int check_sata_dev_state(void);
> 
> -extern block_dev_desc_t sata_dev_desc[CONFIG_SYS_SATA_MAX_DEVICE];
> -
>  static const struct ata_port_info sata_dwc_port_info[] = {
>  	{
>  		.flags		= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY |
> diff --git a/drivers/block/sata_extern.h b/drivers/block/sata_extern.h
> new file mode 100644
> index 0000000..3bf9df9
> --- /dev/null
> +++ b/drivers/block/sata_extern.h
> @@ -0,0 +1,30 @@
> +/*
> + * (C) Copyright 2012
> + * Pavel Herrmann <morpheus.ibis@gmail.com>
> + *
> + * See file CREDITS for list of people who contributed to this
> + * project.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation; either version 2 of
> + * the License, or (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
> + * MA 02111-1307 USA
> + */
> +
> +#ifndef _BLOCK_SATA_EXTERN_H_
> +#define _BLOCK_SATA_EXTERN_H_
> +
> +extern block_dev_desc_t sata_dev_desc[CONFIG_SYS_SATA_MAX_DEVICE];
> +extern int sata_curr_device;
> +
> +#endif
> diff --git a/drivers/block/sata_sil.c b/drivers/block/sata_sil.c
> index fb7cd2a..ee9194c 100644
> --- a/drivers/block/sata_sil.c
> +++ b/drivers/block/sata_sil.c
> @@ -27,6 +27,7 @@
>  #include <fis.h>
>  #include <libata.h>
>  #include "sata_sil.h"
> +#include "sata_extern.h"
> 
>  /* Convert sectorsize to wordsize */
>  #define ATA_SECTOR_WORDS (ATA_SECT_SIZE/2)
> diff --git a/drivers/block/sata_sil.h b/drivers/block/sata_sil.h
> index 2dfd4a5..9f3a37f 100644
> --- a/drivers/block/sata_sil.h
> +++ b/drivers/block/sata_sil.h
> @@ -24,8 +24,6 @@
>  #define READ_CMD	0
>  #define WRITE_CMD	1
> 
> -extern block_dev_desc_t sata_dev_desc[CONFIG_SYS_SATA_MAX_DEVICE];
> -
>  /*
>   * SATA device driver struct for each dev
>   */
> diff --git a/drivers/block/sata_sil3114.c b/drivers/block/sata_sil3114.c
> index 34fe038..da55385 100644
> --- a/drivers/block/sata_sil3114.c
> +++ b/drivers/block/sata_sil3114.c
> @@ -30,6 +30,7 @@
>  #include <ide.h>
>  #include <libata.h>
>  #include "sata_sil3114.h"
> +#include "sata_extern.h"
> 
>  /* Convert sectorsize to wordsize */
>  #define ATA_SECTOR_WORDS (ATA_SECT_SIZE/2)
> @@ -48,7 +49,6 @@ static u8 sata_chk_status (struct sata_ioports *ioaddr,
> u8 usealtstatus); static void msleep (int count);
> 
>  static u32 iobase[6] = { 0, 0, 0, 0, 0, 0};	/* PCI BAR registers for
> device */ -extern block_dev_desc_t
> sata_dev_desc[CONFIG_SYS_SATA_MAX_DEVICE];
> 
>  static struct sata_port port[CONFIG_SYS_SATA_MAX_DEVICE];

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH] Fix checkpatch warnings about externs in *.c
  2012-09-16 12:37 ` Marek Vasut
@ 2012-09-16 12:59   ` Pavel Herrmann
  2012-09-16 13:11     ` Marek Vasut
  2012-09-17 19:05     ` Tom Rini
  0 siblings, 2 replies; 12+ messages in thread
From: Pavel Herrmann @ 2012-09-16 12:59 UTC (permalink / raw)
  To: u-boot

On Sunday 16 September 2012 14:37:16 Marek Vasut wrote:
> Dear Pavel Herrmann,
>  ...
> Won't include/sata.h work just fine ?

I feel include/sata.h is a "consumer-facing" header, and implementation 
details such as the array used for all data-retention for command and drivers 
should not be there.

Therefore i created a new header in drivers/block for this purpose, feel free 
to oppose to its name though.

Pavel Herrmann

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH] Fix checkpatch warnings about externs in *.c
  2012-09-16 12:59   ` Pavel Herrmann
@ 2012-09-16 13:11     ` Marek Vasut
  2012-09-17 19:05     ` Tom Rini
  1 sibling, 0 replies; 12+ messages in thread
From: Marek Vasut @ 2012-09-16 13:11 UTC (permalink / raw)
  To: u-boot

Dear Pavel Herrmann,

> On Sunday 16 September 2012 14:37:16 Marek Vasut wrote:
> > Dear Pavel Herrmann,
> > 
> >  ...
> > 
> > Won't include/sata.h work just fine ?
> 
> I feel include/sata.h is a "consumer-facing" header, and implementation
> details such as the array used for all data-retention for command and
> drivers should not be there.
> 
> Therefore i created a new header in drivers/block for this purpose, feel
> free to oppose to its name though.

Just add a comment there, no need to create new file for that.

And using "extern" keyword in header file is useless.

Actually, proper accessors might be even better idea that directly exporting an 
array.

> Pavel Herrmann

Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH] Fix checkpatch warnings about externs in *.c
  2012-09-16 12:59   ` Pavel Herrmann
  2012-09-16 13:11     ` Marek Vasut
@ 2012-09-17 19:05     ` Tom Rini
  1 sibling, 0 replies; 12+ messages in thread
From: Tom Rini @ 2012-09-17 19:05 UTC (permalink / raw)
  To: u-boot

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 09/16/12 05:59, Pavel Herrmann wrote:
> On Sunday 16 September 2012 14:37:16 Marek Vasut wrote:
>> Dear Pavel Herrmann, ... Won't include/sata.h work just fine ?
> 
> I feel include/sata.h is a "consumer-facing" header, and
> implementation details such as the array used for all
> data-retention for command and drivers should not be there.

But it needs to be in sync with common/cmd_sata.c which is where both
that and sata_curr_device are defined.  So please use sata.h.  I will
also state this doesn't seem to be ideal but it's how our sata
"subsystem" is setup currently.

- -- 
Tom

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://www.enigmail.net/

iQIcBAEBAgAGBQJQV3RiAAoJENk4IS6UOR1WszYP/jK3Llu0R1H4a57Me1k56Afu
qniSbmI9pNr8eA6Bf5NxC7clvnK6HOBHWMegX6V42P4Dxy2/Bhod0K24RppsWaFs
Wj604F+3gWWy/fhCojw6kVyzjV2WBlBse51XBHEMjNa5Ftf33lOKxr2mHNG5XrSD
IL9uxRTwlwhKhs/WJ8HwH3C4Eo/zBG2AoIXbqmoyvqr/7gqGcERdo7NWtHPUrLwB
smgJkL6t91MULlZc/RMwGTtX7CoKv0YbM+swWqY3rBQ7jQrP31vH8e6R9CfDr9nV
ynZarYwTC4gy+qC7z7dLbutJHPdHcVv7SPJXh1fKUlpgOssbRKB7bxG/Gw9k52Pf
ZtvnOP91pIKckAZ6EpgFrpYDf7B6fujjg4Z8hbCORBx3l45tmYxAbCnaAeO0Gn/r
fbW78jElfOewr0QDpbTor1yorK9Qidf8Y2lujs9VtR7Gek2jIYIedtL/hPcG7Awa
7yI0qfLXPe/wo6eoQphoARtFf/oM2URlPAzyB1smJB85ytn8AR8J98O1JnzPbY+E
2foQ9g/wcKCeWRZohCjOyVK7cJ2kX618GJOukLXKzQVIxRQBkf2fdDocE2FbD9LW
uAbSL0k6HT7TKugBh8FNolhNfOggyFONK/A4UQ+rQmsgTZ2dWvvDnr53Lyv5KagS
1OJ5YAkwIxUJpiaCQNLl
=0CLe
-----END PGP SIGNATURE-----

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH 1/2] remove unnecessary code in ata_piix
  2012-09-16 11:53 [U-Boot] [PATCH] Fix checkpatch warnings about externs in *.c Pavel Herrmann
  2012-09-16 12:37 ` Marek Vasut
@ 2012-09-28  9:18 ` Pavel Herrmann
  2012-09-28  9:18   ` [U-Boot] [PATCH v2 2/2] Fix checkpatch warnings about externs in *.c Pavel Herrmann
                     ` (2 more replies)
  1 sibling, 3 replies; 12+ messages in thread
From: Pavel Herrmann @ 2012-09-28  9:18 UTC (permalink / raw)
  To: u-boot

We set sata_curr_device to 0 right after returning from init_sata(), so there's
no point in setting it to the last scanned driver at this point.
Note: there are more duplicities with cmd_sata, but those might be required,
as the code seems to reset the entire controller on every scan, ignoring the
requested port number.

Signed-off-by: Pavel Herrmann <morpheus.ibis@gmail.com>
---
 drivers/block/ata_piix.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/block/ata_piix.c b/drivers/block/ata_piix.c
index c81d11a..1c3ab8a 100644
--- a/drivers/block/ata_piix.c
+++ b/drivers/block/ata_piix.c
@@ -204,9 +204,6 @@ init_sata (int dev)
 				dev_print (&sata_dev_desc[devno]);
 				/* initialize partition type */
 				init_part (&sata_dev_desc[devno]);
-				if (sata_curr_device < 0)
-					sata_curr_device =
-					    i * CONFIG_SYS_SATA_DEVS_PER_BUS + j;
 			}
 		}
 	}
-- 
1.7.12

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH v2 2/2] Fix checkpatch warnings about externs in *.c
  2012-09-28  9:18 ` [U-Boot] [PATCH 1/2] remove unnecessary code in ata_piix Pavel Herrmann
@ 2012-09-28  9:18   ` Pavel Herrmann
  2012-09-28 18:13     ` Marek Vasut
  2012-09-28 19:39     ` Tom Rini
  2012-09-28 18:12   ` [U-Boot] [PATCH 1/2] remove unnecessary code in ata_piix Marek Vasut
  2012-09-29 14:52   ` [U-Boot] [U-Boot,1/2] " Tom Rini
  2 siblings, 2 replies; 12+ messages in thread
From: Pavel Herrmann @ 2012-09-28  9:18 UTC (permalink / raw)
  To: u-boot

Move all extern declarations of sata_dev_desc[] into <sata.h> to make checkpatch
happy, inslude <sata.h> in every sata driver, and remove now duplicit
declarations of sata API functions.

Signed-off-by: Pavel Herrmann <morpheus.ibis@gmail.com>
---
Changes for v2:
 use include/sata.h instead of a separate header file

 drivers/block/ata_piix.c     | 4 +---
 drivers/block/ata_piix.h     | 2 --
 drivers/block/dwc_ahsata.c   | 1 +
 drivers/block/dwc_ahsata.h   | 2 --
 drivers/block/fsl_sata.c     | 3 +--
 drivers/block/pata_bfin.c    | 1 +
 drivers/block/pata_bfin.h    | 2 --
 drivers/block/sata_dwc.c     | 3 +--
 drivers/block/sata_sil.c     | 1 +
 drivers/block/sata_sil.h     | 2 --
 drivers/block/sata_sil3114.c | 2 +-
 include/sata.h               | 3 +++
 12 files changed, 10 insertions(+), 16 deletions(-)

diff --git a/drivers/block/ata_piix.c b/drivers/block/ata_piix.c
index 1c3ab8a..d18af73 100644
--- a/drivers/block/ata_piix.c
+++ b/drivers/block/ata_piix.c
@@ -34,9 +34,7 @@
 #include <part.h>
 #include <ide.h>
 #include <ata.h>
-
-extern block_dev_desc_t sata_dev_desc[CONFIG_SYS_SATA_MAX_DEVICE];
-extern int sata_curr_device;
+#include <sata.h>
 
 #define DEBUG_SATA 0		/*For debug prints set DEBUG_SATA to 1 */
 
diff --git a/drivers/block/ata_piix.h b/drivers/block/ata_piix.h
index 9157cf8..6d8988d 100644
--- a/drivers/block/ata_piix.h
+++ b/drivers/block/ata_piix.h
@@ -63,8 +63,6 @@ int sata_devchk (struct sata_ioports *ioaddr, int dev);
 void dev_select (struct sata_ioports *ioaddr, int dev);
 u8 sata_busy_wait (struct sata_ioports *ioaddr, int bits, unsigned int max);
 u8 sata_chk_status (struct sata_ioports *ioaddr);
-ulong sata_read (int device, ulong blknr,lbaint_t blkcnt, void * buffer);
-ulong sata_write (int device,ulong blknr, lbaint_t blkcnt, void * buffer);
 void msleep (int count);
 #endif
 
diff --git a/drivers/block/dwc_ahsata.c b/drivers/block/dwc_ahsata.c
index 2703d3d..7b06c81 100644
--- a/drivers/block/dwc_ahsata.c
+++ b/drivers/block/dwc_ahsata.c
@@ -24,6 +24,7 @@
 #include <libata.h>
 #include <ahci.h>
 #include <fis.h>
+#include <sata.h>
 
 #include <common.h>
 #include <malloc.h>
diff --git a/drivers/block/dwc_ahsata.h b/drivers/block/dwc_ahsata.h
index 84860ea..4dac5dc 100644
--- a/drivers/block/dwc_ahsata.h
+++ b/drivers/block/dwc_ahsata.h
@@ -330,6 +330,4 @@
 #define READ_CMD	0
 #define WRITE_CMD	1
 
-extern block_dev_desc_t sata_dev_desc[CONFIG_SYS_SATA_MAX_DEVICE];
-
 #endif /* __FSL_SATA_H__ */
diff --git a/drivers/block/fsl_sata.c b/drivers/block/fsl_sata.c
index 3026ade..88f3891 100644
--- a/drivers/block/fsl_sata.c
+++ b/drivers/block/fsl_sata.c
@@ -26,10 +26,9 @@
 #include <malloc.h>
 #include <libata.h>
 #include <fis.h>
+#include <sata.h>
 #include "fsl_sata.h"
 
-extern block_dev_desc_t sata_dev_desc[CONFIG_SYS_SATA_MAX_DEVICE];
-
 #ifndef CONFIG_SYS_SATA1_FLAGS
 	#define CONFIG_SYS_SATA1_FLAGS	FLAGS_DMA
 #endif
diff --git a/drivers/block/pata_bfin.c b/drivers/block/pata_bfin.c
index cce21fb..a42a7b8 100644
--- a/drivers/block/pata_bfin.c
+++ b/drivers/block/pata_bfin.c
@@ -17,6 +17,7 @@
 #include <asm/portmux.h>
 #include <asm/mach-common/bits/pata.h>
 #include <ata.h>
+#include <sata.h>
 #include <libata.h>
 #include "pata_bfin.h"
 
diff --git a/drivers/block/pata_bfin.h b/drivers/block/pata_bfin.h
index 2b3425b..2093cf0 100644
--- a/drivers/block/pata_bfin.h
+++ b/drivers/block/pata_bfin.h
@@ -41,8 +41,6 @@ struct ata_port {
 	unsigned char dev_mask;
 };
 
-extern block_dev_desc_t sata_dev_desc[CONFIG_SYS_SATA_MAX_DEVICE];
-
 #define DRV_NAME		"pata-bfin"
 #define DRV_VERSION		"0.9"
 #define __iomem
diff --git a/drivers/block/sata_dwc.c b/drivers/block/sata_dwc.c
index 75101b5..ba98ecd 100644
--- a/drivers/block/sata_dwc.c
+++ b/drivers/block/sata_dwc.c
@@ -35,6 +35,7 @@
 #include <asm/io.h>
 #include <malloc.h>
 #include <ata.h>
+#include <sata.h>
 #include <linux/ctype.h>
 
 #include "sata_dwc.h"
@@ -268,8 +269,6 @@ static int ata_dev_read_id(struct ata_device *dev, unsigned int *p_class,
 		unsigned int flags, u16 *id);
 static int check_sata_dev_state(void);
 
-extern block_dev_desc_t sata_dev_desc[CONFIG_SYS_SATA_MAX_DEVICE];
-
 static const struct ata_port_info sata_dwc_port_info[] = {
 	{
 		.flags		= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY |
diff --git a/drivers/block/sata_sil.c b/drivers/block/sata_sil.c
index fb7cd2a..8e8d4fb 100644
--- a/drivers/block/sata_sil.c
+++ b/drivers/block/sata_sil.c
@@ -25,6 +25,7 @@
 #include <malloc.h>
 #include <asm/io.h>
 #include <fis.h>
+#include <sata.h>
 #include <libata.h>
 #include "sata_sil.h"
 
diff --git a/drivers/block/sata_sil.h b/drivers/block/sata_sil.h
index 2dfd4a5..9f3a37f 100644
--- a/drivers/block/sata_sil.h
+++ b/drivers/block/sata_sil.h
@@ -24,8 +24,6 @@
 #define READ_CMD	0
 #define WRITE_CMD	1
 
-extern block_dev_desc_t sata_dev_desc[CONFIG_SYS_SATA_MAX_DEVICE];
-
 /*
  * SATA device driver struct for each dev
  */
diff --git a/drivers/block/sata_sil3114.c b/drivers/block/sata_sil3114.c
index 34fe038..3a5e032 100644
--- a/drivers/block/sata_sil3114.c
+++ b/drivers/block/sata_sil3114.c
@@ -28,6 +28,7 @@
 #include <asm/byteorder.h>
 #include <asm/io.h>
 #include <ide.h>
+#include <sata.h>
 #include <libata.h>
 #include "sata_sil3114.h"
 
@@ -48,7 +49,6 @@ static u8 sata_chk_status (struct sata_ioports *ioaddr, u8 usealtstatus);
 static void msleep (int count);
 
 static u32 iobase[6] = { 0, 0, 0, 0, 0, 0};	/* PCI BAR registers for device */
-extern block_dev_desc_t sata_dev_desc[CONFIG_SYS_SATA_MAX_DEVICE];
 
 static struct sata_port port[CONFIG_SYS_SATA_MAX_DEVICE];
 
diff --git a/include/sata.h b/include/sata.h
index 37573cf..52c3033 100644
--- a/include/sata.h
+++ b/include/sata.h
@@ -1,5 +1,6 @@
 #ifndef __SATA_H__
 #define __SATA_H__
+#include <part.h>
 
 int init_sata(int dev);
 int scan_sata(int dev);
@@ -9,4 +10,6 @@ ulong sata_write(int dev, ulong blknr, ulong blkcnt, const void *buffer);
 int sata_initialize(void);
 int __sata_initialize(void);
 
+extern block_dev_desc_t sata_dev_desc[CONFIG_SYS_SATA_MAX_DEVICE];
+
 #endif
-- 
1.7.12

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH 1/2] remove unnecessary code in ata_piix
  2012-09-28  9:18 ` [U-Boot] [PATCH 1/2] remove unnecessary code in ata_piix Pavel Herrmann
  2012-09-28  9:18   ` [U-Boot] [PATCH v2 2/2] Fix checkpatch warnings about externs in *.c Pavel Herrmann
@ 2012-09-28 18:12   ` Marek Vasut
  2012-09-28 18:29     ` Tom Rini
  2012-09-29 14:52   ` [U-Boot] [U-Boot,1/2] " Tom Rini
  2 siblings, 1 reply; 12+ messages in thread
From: Marek Vasut @ 2012-09-28 18:12 UTC (permalink / raw)
  To: u-boot

Dear Pavel Herrmann,

> We set sata_curr_device to 0 right after returning from init_sata(), so
> there's no point in setting it to the last scanned driver at this point.
> Note: there are more duplicities with cmd_sata, but those might be
> required, as the code seems to reset the entire controller on every scan,
> ignoring the requested port number.

I think that code is valid. It configures the sata_curr_device to valid value in 
case this was not called from the context of the command. No?

I think it can be pulled from the loops above the return 0 though.

> Signed-off-by: Pavel Herrmann <morpheus.ibis@gmail.com>
> ---
>  drivers/block/ata_piix.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/drivers/block/ata_piix.c b/drivers/block/ata_piix.c
> index c81d11a..1c3ab8a 100644
> --- a/drivers/block/ata_piix.c
> +++ b/drivers/block/ata_piix.c
> @@ -204,9 +204,6 @@ init_sata (int dev)
>  				dev_print (&sata_dev_desc[devno]);
>  				/* initialize partition type */
>  				init_part (&sata_dev_desc[devno]);
> -				if (sata_curr_device < 0)
> -					sata_curr_device =
> -					    i * CONFIG_SYS_SATA_DEVS_PER_BUS + 
j;
>  			}
>  		}
>  	}

Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH v2 2/2] Fix checkpatch warnings about externs in *.c
  2012-09-28  9:18   ` [U-Boot] [PATCH v2 2/2] Fix checkpatch warnings about externs in *.c Pavel Herrmann
@ 2012-09-28 18:13     ` Marek Vasut
  2012-09-28 19:39     ` Tom Rini
  1 sibling, 0 replies; 12+ messages in thread
From: Marek Vasut @ 2012-09-28 18:13 UTC (permalink / raw)
  To: u-boot

Dear Pavel Herrmann,

> Move all extern declarations of sata_dev_desc[] into <sata.h> to make
> checkpatch happy, inslude <sata.h> in every sata driver, and remove now
> duplicit declarations of sata API functions.
> 
> Signed-off-by: Pavel Herrmann <morpheus.ibis@gmail.com>

WFM

Reviewed-by: Marek Vasut <marex@denx.de>
[...]


Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH 1/2] remove unnecessary code in ata_piix
  2012-09-28 18:12   ` [U-Boot] [PATCH 1/2] remove unnecessary code in ata_piix Marek Vasut
@ 2012-09-28 18:29     ` Tom Rini
  0 siblings, 0 replies; 12+ messages in thread
From: Tom Rini @ 2012-09-28 18:29 UTC (permalink / raw)
  To: u-boot

On Fri, Sep 28, 2012 at 08:12:21PM +0200, Marek Vasut wrote:

> Dear Pavel Herrmann,
> 
> > We set sata_curr_device to 0 right after returning from init_sata(),
> > so there's no point in setting it to the last scanned driver at this
> > point.  Note: there are more duplicities with cmd_sata, but those
> > might be required, as the code seems to reset the entire controller
> > on every scan, ignoring the requested port number.
> 
> I think that code is valid. It configures the sata_curr_device to
> valid value in case this was not called from the context of the
> command. No?
> 
> I think it can be pulled from the loops above the return 0 though.

Pavel has it right.  A further clean-up would be to make
sata_curr_device be static to common/cmd_sata.c as it's unused /
referenced anywhere else.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20120928/a8acc114/attachment.pgp>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH v2 2/2] Fix checkpatch warnings about externs in *.c
  2012-09-28  9:18   ` [U-Boot] [PATCH v2 2/2] Fix checkpatch warnings about externs in *.c Pavel Herrmann
  2012-09-28 18:13     ` Marek Vasut
@ 2012-09-28 19:39     ` Tom Rini
  1 sibling, 0 replies; 12+ messages in thread
From: Tom Rini @ 2012-09-28 19:39 UTC (permalink / raw)
  To: u-boot

On Fri, Sep 28, 2012 at 11:18:04AM +0200, Pavel Herrmann wrote:

> Move all extern declarations of sata_dev_desc[] into <sata.h> to make
> checkpatch happy, inslude <sata.h> in every sata driver, and remove
> now duplicit declarations of sata API functions.
> 
> Signed-off-by: Pavel Herrmann <morpheus.ibis@gmail.com>

Not a problem in your patch, but a problem exposed by your patch is
that the prototypes in <ide.h> for ide_{read,write} and in <sata.h> for
sata_{read,write} do not match what <part.h> says block_dev_desc_t says
block_{read,write} should have.  I'm correcting these, and then the
driver fall-out.  I also see that DM corrects this problem as well as
the logical problem we have today (block_{read,write} uses different
types for starting point and number to read).  However, I don't feel
it's good to take these changes in for v2012.10 so it'll be put into
-next.  Thanks for your patience and re-working of things.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20120928/68d73b9b/attachment.pgp>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [U-Boot] [U-Boot,1/2] remove unnecessary code in ata_piix
  2012-09-28  9:18 ` [U-Boot] [PATCH 1/2] remove unnecessary code in ata_piix Pavel Herrmann
  2012-09-28  9:18   ` [U-Boot] [PATCH v2 2/2] Fix checkpatch warnings about externs in *.c Pavel Herrmann
  2012-09-28 18:12   ` [U-Boot] [PATCH 1/2] remove unnecessary code in ata_piix Marek Vasut
@ 2012-09-29 14:52   ` Tom Rini
  2 siblings, 0 replies; 12+ messages in thread
From: Tom Rini @ 2012-09-29 14:52 UTC (permalink / raw)
  To: u-boot

On Thu, Sep 27, 2012 at 11:18:03PM -0000, Pavel Herrmann wrote:

> We set sata_curr_device to 0 right after returning from init_sata(), so there's
> no point in setting it to the last scanned driver at this point.
> Note: there are more duplicities with cmd_sata, but those might be required,
> as the code seems to reset the entire controller on every scan, ignoring the
> requested port number.
> 
> Signed-off-by: Pavel Herrmann <morpheus.ibis@gmail.com>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20120929/e9e45038/attachment.pgp>

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2012-09-29 14:52 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-09-16 11:53 [U-Boot] [PATCH] Fix checkpatch warnings about externs in *.c Pavel Herrmann
2012-09-16 12:37 ` Marek Vasut
2012-09-16 12:59   ` Pavel Herrmann
2012-09-16 13:11     ` Marek Vasut
2012-09-17 19:05     ` Tom Rini
2012-09-28  9:18 ` [U-Boot] [PATCH 1/2] remove unnecessary code in ata_piix Pavel Herrmann
2012-09-28  9:18   ` [U-Boot] [PATCH v2 2/2] Fix checkpatch warnings about externs in *.c Pavel Herrmann
2012-09-28 18:13     ` Marek Vasut
2012-09-28 19:39     ` Tom Rini
2012-09-28 18:12   ` [U-Boot] [PATCH 1/2] remove unnecessary code in ata_piix Marek Vasut
2012-09-28 18:29     ` Tom Rini
2012-09-29 14:52   ` [U-Boot] [U-Boot,1/2] " Tom Rini

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox