From owner-svn-src-head@FreeBSD.ORG Sat Sep 6 12:01:37 2014 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 93209C0B; Sat, 6 Sep 2014 12:01:37 +0000 (UTC) Received: from bsdpad.com (xc1.bsdpad.com [195.154.136.64]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 535EE1DD3; Sat, 6 Sep 2014 12:01:36 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=bsdpad.com) by bsdpad.com with smtp (Exim 4.83 (FreeBSD)) (envelope-from ) id 1XQEfU-000GS5-1G; Sat, 06 Sep 2014 13:00:36 +0100 Received: by bsdpad.com (nbSMTP-1.00) for uid 1001 br@bsdpad.com; Sat, 6 Sep 2014 13:00:36 +0100 (BST) Date: Sat, 6 Sep 2014 13:00:35 +0100 From: Ruslan Bukin To: Andrew Turner Subject: Re: svn commit: r271186 - in head/sys: arm/altera/socfpga boot/fdt/dts/arm Message-ID: <20140906120035.GA62565@bsdpad.com> References: <201409060848.s868mwgm066917@svn.freebsd.org> <20140906114514.5c9f7223@bender.lan> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="MGYHOYXEY6WxJCY8" Content-Disposition: inline In-Reply-To: <20140906114514.5c9f7223@bender.lan> User-Agent: Mutt/1.5.23 (2014-03-12) Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 06 Sep 2014 12:01:37 -0000 --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--