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é <[email protected]> >>>>>>> --- >>>>>>> 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
