diff mbox

[v1,01/11] char/cadence_uart: Mark struct fields as public/private

Message ID 99da8f561252a97b53c5ec2e5006c5b44f25c5f6.1387244071.git.peter.crosthwaite@xilinx.com
State New
Headers show

Commit Message

Peter Crosthwaite Dec. 17, 2013, 1:40 a.m. UTC
As per current QOM conventions.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---

 hw/char/cadence_uart.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

Andreas Färber Dec. 18, 2013, 11:15 p.m. UTC | #1
Am 17.12.2013 02:40, schrieb Peter Crosthwaite:
> As per current QOM conventions.
> 
> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
> ---
> 
>  hw/char/cadence_uart.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/hw/char/cadence_uart.c b/hw/char/cadence_uart.c
> index f18db53..2f19a53 100644
> --- a/hw/char/cadence_uart.c
> +++ b/hw/char/cadence_uart.c
> @@ -110,7 +110,9 @@
>  #define CADENCE_UART(obj) OBJECT_CHECK(UartState, (obj), TYPE_CADENCE_UART)
>  
>  typedef struct {
> +    /* <private> */
>      SysBusDevice parent_obj;
> +    /* <public> */

Note that it's /*< ... >*/ if I'm not mistaken.

However since this is outside include/ it won't get parsed by the
patches flying around. This being Cadence IP that is likely to be reused
on other SoCs, I do guess we'll end up with an
include/hw/char/cadence_uart.h at some point.

Cheers,
Andreas

>  
>      MemoryRegion iomem;
>      uint32_t r[R_MAX];
Peter Crosthwaite Dec. 18, 2013, 11:21 p.m. UTC | #2
On Thu, Dec 19, 2013 at 9:15 AM, Andreas Färber <afaerber@suse.de> wrote:
> Am 17.12.2013 02:40, schrieb Peter Crosthwaite:
>> As per current QOM conventions.
>>
>> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>> ---
>>
>>  hw/char/cadence_uart.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/hw/char/cadence_uart.c b/hw/char/cadence_uart.c
>> index f18db53..2f19a53 100644
>> --- a/hw/char/cadence_uart.c
>> +++ b/hw/char/cadence_uart.c
>> @@ -110,7 +110,9 @@
>>  #define CADENCE_UART(obj) OBJECT_CHECK(UartState, (obj), TYPE_CADENCE_UART)
>>
>>  typedef struct {
>> +    /* <private> */
>>      SysBusDevice parent_obj;
>> +    /* <public> */
>
> Note that it's /*< ... >*/ if I'm not mistaken.

Thanks, will fix v2.

>
> However since this is outside include/ it won't get parsed by the
> patches flying around. This being Cadence IP that is likely to be reused
> on other SoCs, I do guess we'll end up with an
> include/hw/char/cadence_uart.h at some point.
>

Well ideally all peripherals are headerified for the sake of
consistency. I dont think we should be making judgement calls on
coding style based on target usages.

I'm patching old style files as I notice issue just in general
browsing/developing hopefully to one day bring them into line.

Regards,
Peter

> Cheers,
> Andreas
>
>>
>>      MemoryRegion iomem;
>>      uint32_t r[R_MAX];
>
> --
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
>
diff mbox

Patch

diff --git a/hw/char/cadence_uart.c b/hw/char/cadence_uart.c
index f18db53..2f19a53 100644
--- a/hw/char/cadence_uart.c
+++ b/hw/char/cadence_uart.c
@@ -110,7 +110,9 @@ 
 #define CADENCE_UART(obj) OBJECT_CHECK(UartState, (obj), TYPE_CADENCE_UART)
 
 typedef struct {
+    /* <private> */
     SysBusDevice parent_obj;
+    /* <public> */
 
     MemoryRegion iomem;
     uint32_t r[R_MAX];