[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-trivial] [PATCH v2 07/14] hw/i2c/omap_i2c: Use qemu_log_mask(U
From: |
Thomas Huth |
Subject: |
Re: [Qemu-trivial] [PATCH v2 07/14] hw/i2c/omap_i2c: Use qemu_log_mask(UNIMP) instead of fprintf |
Date: |
Mon, 25 Jun 2018 17:32:18 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.8.0 |
On 25.06.2018 17:07, Philippe Mathieu-Daudé wrote:
> On 06/25/2018 11:30 AM, Thomas Huth wrote:
>> On 25.06.2018 14:52, Philippe Mathieu-Daudé wrote:
>>> On 06/25/2018 03:08 AM, Thomas Huth wrote:
>>>> On 22.06.2018 22:10, Philippe Mathieu-Daudé wrote:
>>>>> On 06/22/2018 04:38 PM, Thomas Huth wrote:
>>>>>> 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.
>>>>>
>>>>> This is a <tab> and checkpatch complains...
>>>>>
>>>>> I can use 4 spaces for this tab. I tried to align with other tab-aligned
>>>>> comments I didn't modify, but the result is messier. Thus a simple space.
>>>>
>>>> Oh, sorry, I didn't notice that you've replaced a TAB here. I guess it's
>>>> ok then. But why does checkpatch complain if it is just in the context
>>>> of your modification? That's weird.
>>>
>>> The first 2 contexts (MST and XA) are fine, however checkpatch complains
>>> with the last one (ST_EN):
>>>
>>> ERROR: code indent should never use tabs
>>> #38: FILE: hw/i2c/omap_i2c.c:397:
>>> + if (value & (1 << 15)) {^I^I^I^I^I/* ST_EN */$
>>>
>>> Since I replaced this one, I also did with the 2 previous.
>>>
>>> Now I realize I can _not_ add the brackets so I don't have to update the
>>> <tabs>:
>>>
>>> if (value & (1 << 15))^I^I^I^I^I/* ST_EN */
>>> - fprintf(stderr, "%s: System Test not supported\n", __func__);
>>> + qemu_log_mask(LOG_UNIMP,
>>> + "%s: System Test not supported\n", __func__);
>>> break;
>>>
>>> I think if it better to unify the code style when possible, but it is up
>>> to you, if you prefer I can resend with tabs and no brackets.
>>
>> I think it's OK to fix up the coding style here, too. Maybe just mention
>> it in the patch description ("While we're at it, change TABs to spaces
>> and add missing curly braces to the surounding if-statements" or so).
>
> OK. If that's fine with you I won't respin the whole series for this
> comment, but if I have to respin for another reason I'll improve the
> comment.
Fine for me!
Thomas
- Re: [Qemu-trivial] [PATCH v2 04/14] hw/dma/omap_dma: Use qemu_log_mask(GUEST_ERROR) instead of fprintf, (continued)
- [Qemu-trivial] [PATCH v2 05/14] hw/ssi/omap_spi: Use qemu_log_mask(GUEST_ERROR) instead of fprintf, Philippe Mathieu-Daudé, 2018/06/22
- [Qemu-trivial] [PATCH v2 06/14] hw/sd/omap_mmc: Use qemu_log_mask(UNIMP) instead of printf, Philippe Mathieu-Daudé, 2018/06/22
- [Qemu-trivial] [PATCH v2 07/14] hw/i2c/omap_i2c: Use qemu_log_mask(UNIMP) instead of fprintf, Philippe Mathieu-Daudé, 2018/06/22
- Re: [Qemu-trivial] [PATCH v2 07/14] hw/i2c/omap_i2c: Use qemu_log_mask(UNIMP) instead of fprintf, Thomas Huth, 2018/06/22
- Re: [Qemu-trivial] [PATCH v2 07/14] hw/i2c/omap_i2c: Use qemu_log_mask(UNIMP) instead of fprintf, Philippe Mathieu-Daudé, 2018/06/22
- Re: [Qemu-trivial] [PATCH v2 07/14] hw/i2c/omap_i2c: Use qemu_log_mask(UNIMP) instead of fprintf, Thomas Huth, 2018/06/25
- Re: [Qemu-trivial] [PATCH v2 07/14] hw/i2c/omap_i2c: Use qemu_log_mask(UNIMP) instead of fprintf, Philippe Mathieu-Daudé, 2018/06/25
- Re: [Qemu-trivial] [PATCH v2 07/14] hw/i2c/omap_i2c: Use qemu_log_mask(UNIMP) instead of fprintf, Thomas Huth, 2018/06/25
- Re: [Qemu-trivial] [PATCH v2 07/14] hw/i2c/omap_i2c: Use qemu_log_mask(UNIMP) instead of fprintf, Philippe Mathieu-Daudé, 2018/06/25
- Re: [Qemu-trivial] [PATCH v2 07/14] hw/i2c/omap_i2c: Use qemu_log_mask(UNIMP) instead of fprintf,
Thomas Huth <=
[Qemu-trivial] [PATCH v2 08/14] hw/arm/omap1: Use qemu_log_mask(GUEST_ERROR) instead of fprintf, Philippe Mathieu-Daudé, 2018/06/22
[Qemu-trivial] [PATCH v2 10/14] hw/arm/stellaris: Use qemu_log_mask(UNIMP) instead of fprintf, Philippe Mathieu-Daudé, 2018/06/22
[Qemu-trivial] [PATCH v2 12/14] hw/net/smc91c111: Use qemu_log_mask(GUEST_ERROR) instead of hw_error, Philippe Mathieu-Daudé, 2018/06/22
[Qemu-trivial] [PATCH v2 11/14] hw/net/stellaris_enet: Use qemu_log_mask(GUEST_ERROR) instead of hw_error, Philippe Mathieu-Daudé, 2018/06/22
[Qemu-trivial] [PATCH v2 13/14] hw/net/smc91c111: Use qemu_log_mask(UNIMP) instead of fprintf, Philippe Mathieu-Daudé, 2018/06/22