Date: Sat, 6 Sep 2014 13:00:35 +0100 From: Ruslan Bukin <br@FreeBSD.org> To: Andrew Turner <andrew@fubar.geek.nz> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r271186 - in head/sys: arm/altera/socfpga boot/fdt/dts/arm Message-ID: <20140906120035.GA62565@bsdpad.com> In-Reply-To: <20140906114514.5c9f7223@bender.lan> References: <201409060848.s868mwgm066917@svn.freebsd.org> <20140906114514.5c9f7223@bender.lan>
next in thread | previous in thread | raw e-mail | index | archive | help
--MGYHOYXEY6WxJCY8
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
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
--MGYHOYXEY6WxJCY8
Content-Type: text/plain; charset=utf-8
Content-Disposition: attachment; filename="patch.socfpga.0"
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]);
--MGYHOYXEY6WxJCY8--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20140906120035.GA62565>