On Tue, Sep 8, 2015 at 5:38 PM, Jean-Christophe Dubois
<j...@tribudubois.net> wrote:
> Signed-off-by: Jean-Christophe Dubois <j...@tribudubois.net>
> ---
>
> Changes since V1:
> * added "has-edge-sel" property
> * use extract64() and deposit64() in read/write icr access
> * set "number of GPIO pin" as a #define
> * reorganize functions in file to group them
> * various coding style cleanup
> * rename state handler array in output array.
>
> Changes since v2:
> * always compile DEBUG functions
> * Fix imx_gpio_update_int to use isr, imr and gdir
> * Fix imx_gpio_set_int_line to check gdir instead of imr
> * Fix imx_gpio_set to update psr even if line is output
> * Fix PSR read to use gdir.
>
> hw/gpio/Makefile.objs | 1 +
> hw/gpio/imx_gpio.c | 340
> +++++++++++++++++++++++++++++++++++++++++++++
> include/hw/gpio/imx_gpio.h | 63 +++++++++
> 3 files changed, 404 insertions(+)
> create mode 100644 hw/gpio/imx_gpio.c
> create mode 100644 include/hw/gpio/imx_gpio.h
>
> diff --git a/hw/gpio/Makefile.objs b/hw/gpio/Makefile.objs
> index 1abcf17..52233f7 100644
> --- a/hw/gpio/Makefile.objs
> +++ b/hw/gpio/Makefile.objs
> @@ -5,3 +5,4 @@ common-obj-$(CONFIG_ZAURUS) += zaurus.o
> common-obj-$(CONFIG_E500) += mpc8xxx.o
>
> obj-$(CONFIG_OMAP) += omap_gpio.o
> +obj-$(CONFIG_IMX) += imx_gpio.o
> diff --git a/hw/gpio/imx_gpio.c b/hw/gpio/imx_gpio.c
> new file mode 100644
> index 0000000..f8d7ef8
> --- /dev/null
> +++ b/hw/gpio/imx_gpio.c
> @@ -0,0 +1,340 @@
> +/*
> + * i.MX processors GPIO emulation.
> + *
> + * Copyright (C) 2015 Jean-Christophe Dubois <j...@tribudubois.net>
> + *
> + * 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 or
> + * (at your option) version 3 of the License.
> + *
> + * 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, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include "hw/gpio/imx_gpio.h"
> +
> +#ifndef DEBUG_IMX_GPIO
> +#define DEBUG_IMX_GPIO 0
> +#endif
> +
> +typedef enum IMXGPIOLevel {
> + IMX_GPIO_LEVEL_LOW = 0,
> + IMX_GPIO_LEVEL_HIGH = 1,
> +} IMXGPIOLevel;
> +
> +#define DPRINTF(fmt, args...) \
> + do { \
> + if (DEBUG_IMX_GPIO) { \
> + fprintf(stderr, "%s: " fmt , __func__, ##args); \
> + } \
> + } while (0)
> +
> +static const char *imx_gpio_reg_name(uint32_t reg)
> +{
> + switch (reg) {
> + case DR_ADDR:
> + return "DR";
> + case GDIR_ADDR:
> + return "GDIR";
> + case PSR_ADDR:
> + return "PSR";
> + case ICR1_ADDR:
> + return "ICR1";
> + case ICR2_ADDR:
> + return "ICR2";
> + case IMR_ADDR:
> + return "IMR";
> + case ISR_ADDR:
> + return "ISR";
> + case EDGE_SEL_ADDR:
> + return "EDGE_SEL";
> + default:
> + return "[?]";
> + }
> +}
> +
> +static void imx_gpio_update_int(IMXGPIOState *s)
> +{
> + qemu_set_irq(s->irq, (s->isr & s->imr & ~s->gdir) ? 1 : 0);
This doesn't look right, having it conditional on the gdir. I think
you already implement the gdir masking in set_int_line, why do you
need this extra mask?
> +}
> +
> +static void imx_gpio_set_int_line(IMXGPIOState *s, int line, IMXGPIOLevel
> level)
> +{
> + /* if this signal isn't configured as an input signal, nothing to do */
>
> +/* i.MX GPIO memory map */
> +#define DR_ADDR 0x00 /* DATA REGISTER */
> +#define GDIR_ADDR 0x04 /* DIRECTION REGISTER */
> +#define PSR_ADDR 0x08 /* PAD STATUS REGISTER */
> +#define ICR1_ADDR 0x0c /* INTERRUPT CONFIGURATION REGISTER 1 */
> +#define ICR2_ADDR 0x10 /* INTERRUPT CONFIGURATION REGISTER 2 */
> +#define IMR_ADDR 0x14 /* INTERRUPT MASK REGISTER */
> +#define ISR_ADDR 0x18 /* INTERRUPT STATUS REGISTER */
> +#define EDGE_SEL_ADDR 0x1c /* EDGE SEL REGISTER */
> +
> +#define IMX_GPIO_PIN_COUNT 32
> +
> +typedef struct IMXGPIOState {
> + /*< private >*/
> + SysBusDevice parent_obj;
> +
> + /*< public >*/
> + MemoryRegion iomem;
> +
> + uint32_t dr;
> + uint32_t gdir;
> + uint32_t psr;
> + uint64_t icr;
> + uint32_t imr;
> + uint32_t isr;
> + bool has_edge_sel;
> + /* This register is not present in i.MX31 */
Drop comment.
Otherwise,
Reviewed-by: Peter Crosthwaite <crosthwaite.pe...@gmail.com>
Regards,
Peter
> + uint32_t edge_sel;
> +
> + qemu_irq irq;
> + qemu_irq output[IMX_GPIO_PIN_COUNT];
> +} IMXGPIOState;
> +
> +#endif /* __IMX_GPIO_H_ */
> --
> 2.1.4
>
