Re: [Qemu-arm] [PATCH v2 07/14] hw/i2c/omap_i2c: Use qemu_log_mask(UNIMP) instead of fprintf

[Thomas Huth]

On 22.06.2018 15:40, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé <address@hidden>
> ---
>  hw/i2c/omap_i2c.c | 20 ++++++++++++--------
>  1 file changed, 12 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/i2c/omap_i2c.c b/hw/i2c/omap_i2c.c
> index 26e3e5ebf6..690876e43e 100644
> --- a/hw/i2c/omap_i2c.c
> +++ b/hw/i2c/omap_i2c.c
> @@ -17,6 +17,7 @@
>   * with this program; if not, see <http://www.gnu.org/licenses/>.
>   */
>  #include "qemu/osdep.h"
> +#include "qemu/log.h"
>  #include "hw/hw.h"
>  #include "hw/i2c/i2c.h"
>  #include "hw/arm/omap.h"
> @@ -339,14 +340,15 @@ static void omap_i2c_write(void *opaque, hwaddr addr,
>              }
>              break;
>          }
> -        if ((value & (1 << 15)) && !(value & (1 << 10))) {   /* MST */
> -            fprintf(stderr, "%s: I^2C slave mode not supported\n",
> -                            __func__);
> +        if ((value & (1 << 15)) && !(value & (1 << 10))) { /* MST */

Please keep the white spaces before the comment if you don't change
anything else.

> +            qemu_log_mask(LOG_UNIMP, "%s: I^2C slave mode not supported\n",
> +                          __func__);
>              break;
>          }
> -        if ((value & (1 << 15)) && value & (1 << 8)) {               /* XA */
> -            fprintf(stderr, "%s: 10-bit addressing mode not supported\n",
> -                            __func__);
> +        if ((value & (1 << 15)) && value & (1 << 8)) { /* XA */

dito.

> +            qemu_log_mask(LOG_UNIMP,
> +                          "%s: 10-bit addressing mode not supported\n",
> +                          __func__);

Apart from the nits, looks fine to me, so once you've fixed that:

Reviewed-by: Thomas Huth <address@hidden>