Re: [Qemu-ppc] [Qemu-devel] [PATCH v4 02/11] ppc4xx_i2c: Implement directcntl register

[Thomas Huth]

On 2018-11-28 12:26, BALATON Zoltan wrote:
> On Wed, 28 Nov 2018, Thomas Huth wrote:
>> On 2018-06-19 10:52, BALATON Zoltan wrote:
>>> As well as being able to generate its own i2c transactions, the ppc4xx
>>> i2c controller has a DIRECTCNTL register which allows explicit control
>>> of the i2c lines.
>>>
>>> Using this register an OS can directly bitbang i2c operations. In
>>> order to let emulated i2c devices respond to this, we need to wire up
>>> the DIRECTCNTL register to qemu's bitbanged i2c handling code.
>>>
>>> Signed-off-by: BALATON Zoltan <address@hidden>
>>> ---
>> [...]
>>> diff --git a/include/hw/i2c/ppc4xx_i2c.h b/include/hw/i2c/ppc4xx_i2c.h
>>> index e4b6ded..ea6c8e1 100644
>>> --- a/include/hw/i2c/ppc4xx_i2c.h
>>> +++ b/include/hw/i2c/ppc4xx_i2c.h
>>> @@ -31,6 +31,9 @@
>>>  #include "hw/sysbus.h"
>>>  #include "hw/i2c/i2c.h"
>>>
>>> +/* from hw/i2c/bitbang_i2c.h */
>>> +typedef struct bitbang_i2c_interface bitbang_i2c_interface;
>>
>> This breaks compilation with clang 3.4:
>>
>> In file included from /home/thuth/devel/qemu/hw/i2c/ppc4xx_i2c.c:33:
>> hw/i2c/bitbang_i2c.h:6:38: error: redefinition of typedef
>> 'bitbang_i2c_interface'
>>      is a C11 feature [-Werror,-Wtypedef-redefinition]
>> typedef struct bitbang_i2c_interface bitbang_i2c_interface;
>>                                     ^
>> include/hw/i2c/ppc4xx_i2c.h:35:38: note: previous definition is here
>> typedef struct bitbang_i2c_interface bitbang_i2c_interface;
>>                                     ^
>> 1 error generated.
>> make[1]: *** [hw/i2c/ppc4xx_i2c.o] Error 1
>>
>> Not sure about the best way to fix this ... move the typedef to
>> include/qemu/typedefs.h maybe? Or include the "hw/i2c/bitbang_i2c.h"
>> header instead of "i2c.h" here?
> 
> Not sure either but I could not include bitbang_i2c.h here because
> that's a private header in hw/i2c, whereas ppc4xx_i2c.h is a public
> header in include/hw/i2c. That's why it had to be duplicated here. I had
> it differently in first version but this was thought to be simpler way,
> see http://lists.nongnu.org/archive/html/qemu-ppc/2018-06/msg00374.html
> 
> Maybe we could move this typedef to include/hw/i2c/i2c.h which both
> ppc4xx_i2c.h and bitbang_i2c.h include?

Fine for me, too. Do you have some spare time to create a patch?

 Thanks,
  Thomas