svn commit: r271186 - in head/sys: arm/altera/socfpga boot/fdt/dts/arm

[Ruslan Bukin]

On Sat, Sep 06, 2014 at 11:45:14AM +0100, Andrew Turner wrote:
> > +#define	WRITE1(_sc, _reg, _val)	\
> > +	bus_space_write_1(_sc->bst, _sc->bsh, _reg, _val)
> 
> Why are these in a header when the softc is in a .c file? Also why not
> use bus_read_n, e.g. READ4 would become:
> 
> #define READ4(_sc, _reg) bus_read_4((_sc)->res[0], _reg)
> > 

I want to use these defines in other socfpga drivers, so I leaved it in header file. Is that fine ?

Excellent review, thanks. I've prepared the patch

Ruslan
-------------- next part --------------
Index: sys/arm/altera/socfpga/socfpga_common.h
===================================================================
--- sys/arm/altera/socfpga/socfpga_common.h	(revision 271188)
+++ sys/arm/altera/socfpga/socfpga_common.h	(working copy)
@@ -30,15 +30,9 @@
  * $FreeBSD$
  */
 
-#define	READ4(_sc, _reg)	\
-	bus_space_read_4(_sc->bst, _sc->bsh, _reg)
-#define	WRITE4(_sc, _reg, _val)	\
-	bus_space_write_4(_sc->bst, _sc->bsh, _reg, _val)
-#define	READ2(_sc, _reg)	\
-	bus_space_read_2(_sc->bst, _sc->bsh, _reg)
-#define	WRITE2(_sc, _reg, _val)	\
-	bus_space_write_2(_sc->bst, _sc->bsh, _reg, _val)
-#define	READ1(_sc, _reg)	\
-	bus_space_read_1(_sc->bst, _sc->bsh, _reg)
-#define	WRITE1(_sc, _reg, _val)	\
-	bus_space_write_1(_sc->bst, _sc->bsh, _reg, _val)
+#define	READ4(_sc, _reg) bus_read_4((_sc)->res[0], _reg)
+#define	READ2(_sc, _reg) bus_read_2((_sc)->res[0], _reg)
+#define	READ1(_sc, _reg) bus_read_1((_sc)->res[0], _reg)
+#define	WRITE4(_sc, _reg, _val) bus_write_4((_sc)->res[0], _reg, _val)
+#define	WRITE2(_sc, _reg, _val) bus_write_2((_sc)->res[0], _reg, _val)
+#define	WRITE1(_sc, _reg, _val) bus_write_1((_sc)->res[0], _reg, _val)
Index: sys/arm/altera/socfpga/socfpga_manager.c
===================================================================
--- sys/arm/altera/socfpga/socfpga_manager.c	(revision 271188)
+++ sys/arm/altera/socfpga/socfpga_manager.c	(working copy)
@@ -91,6 +91,7 @@
 #define	GPIO_PORTA_EOI		0x84C	/* Clear Interrupt Register */
 #define	 PORTA_EOI_NS		(1 << 0)
 #define	GPIO_EXT_PORTA		0x850	/* External Port A Register */
+#define	 EXT_PORTA_CDP		(1 << 10) /* Configuration done */
 #define	GPIO_LS_SYNC		0x860	/* Synchronization Level Register */
 #define	GPIO_VER_ID_CODE	0x86C	/* GPIO Version Register */
 #define	GPIO_CONFIG_REG2	0x870	/* Configuration Register 2 */
@@ -147,8 +148,6 @@
 
 struct fpgamgr_softc {
 	struct resource		*res[3];
-	bus_space_tag_t		bst;
-	bus_space_handle_t	bsh;
 	bus_space_tag_t		bst_data;
 	bus_space_handle_t	bsh_data;
 	struct cdev		*mgr_cdev;
@@ -163,20 +162,6 @@
 };
 
 static int
-fpgamgr_probe(device_t dev)
-{
-
-	if (!ofw_bus_status_okay(dev))
-		return (ENXIO);
-
-	if (!ofw_bus_is_compatible(dev, "altr,fpga-mgr"))
-		return (ENXIO);
-
-	device_set_desc(dev, "FPGA Manager");
-	return (BUS_PROBE_DEFAULT);
-}
-
-static int
 fpgamgr_state_get(struct fpgamgr_softc *sc)
 {
 	int reg;
@@ -208,7 +193,7 @@
 }
 
 static int
-fpga_open(struct cdev *dev __unused, int flags __unused,
+fpga_open(struct cdev *dev, int flags __unused,
     int fmt __unused, struct thread *td __unused)
 {
 	struct fpgamgr_softc *sc;
@@ -310,7 +295,7 @@
 }
 
 static int
-fpga_close(struct cdev *dev __unused, int flags __unused,
+fpga_close(struct cdev *dev, int flags __unused,
     int fmt __unused, struct thread *td __unused)
 {
 	struct fpgamgr_softc *sc;
@@ -319,7 +304,7 @@
 	sc = dev->si_drv1;
 
 	reg = READ4(sc, GPIO_EXT_PORTA);
-	if ((reg & (1 << 10)) == 0) {
+	if ((reg & EXT_PORTA_CDP) == 0) {
 		device_printf(sc->dev, "Err: configuration failed\n");
 		return (ENXIO);
 	}
@@ -388,6 +373,20 @@
 };
 
 static int
+fpgamgr_probe(device_t dev)
+{
+
+	if (!ofw_bus_status_okay(dev))
+		return (ENXIO);
+
+	if (!ofw_bus_is_compatible(dev, "altr,fpga-mgr"))
+		return (ENXIO);
+
+	device_set_desc(dev, "FPGA Manager");
+	return (BUS_PROBE_DEFAULT);
+}
+
+static int
 fpgamgr_attach(device_t dev)
 {
 	struct fpgamgr_softc *sc;
@@ -401,8 +400,6 @@
 	}
 
 	/* Memory interface */
-	sc->bst = rman_get_bustag(sc->res[0]);
-	sc->bsh = rman_get_bushandle(sc->res[0]);
 	sc->bst_data = rman_get_bustag(sc->res[1]);
 	sc->bsh_data = rman_get_bushandle(sc->res[1]);