Re: [Qemu-devel] [PATCH v1 1/3] i.MX: Add GPIO device

[Peter Crosthwaite]

On Sun, Sep 6, 2015 at 5:55 AM, Jean-Christophe DUBOIS
<address@hidden> wrote:
> Le 05/09/2015 11:03, Peter Crosthwaite a écrit :
>>
>> On Sat, Sep 5, 2015 at 1:17 AM, Jean-Christophe Dubois
>> <address@hidden> wrote:
>>>
>>> Signed-off-by: Jean-Christophe Dubois <address@hidden>
>>> ---
>>>   hw/gpio/Makefile.objs      |   1 +
>>>   hw/gpio/imx_gpio.c         | 358
>>> +++++++++++++++++++++++++++++++++++++++++++++
>>>   include/hw/gpio/imx_gpio.h |  60 ++++++++
>>>   3 files changed, 419 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..8ec1d4c
>>> --- /dev/null
>>> +++ b/hw/gpio/imx_gpio.c
>>> @@ -0,0 +1,358 @@
>>> +/*
>>> + * i.MX processors GPIO emulation.
>>> + *
>>> + * Copyright (C) 2015 Jean-Christophe Dubois <address@hidden>
>>> + *
>>> + * 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 IMX_GPIO_DEBUG
>>> +#define IMX_GPIO_DEBUG                 0
>>> +#endif
>>> +
>>> +#if IMX_GPIO_DEBUG
>>> +#  define DPRINTF(fmt, args...) \
>>> +          do { fprintf(stder, "%s: " fmt , __func__, ##args); } while
>>> (0)
>>> +
>>
>> Use a regular if for debug conditional. This is so the debug code is
>> always compiled.
>
> Not sure what you mean here ... Most files in qemu have DEBUG functions
> compiled out in the normal case ...
>

We are trying to slowly change that.

> Is there a change in "politic" on this topic?
>

No this has been the policy for a while now.

> Could you point to an implementation doing things right?
>

hw/dma/pl330.c

Regards,
Peter

>>
>>> +static char const *imx_gpio_reg_name(uint32_t reg)
>>> +{
>>> +    switch (reg) {
>>> +    case DR_ADDR:
>>> +        return "DR";
>>> +    case GDIR_ADDR: