qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Thomas Huth <thuth@redhat.com>
To: Santiago Monserrat Campanello <santimonserr@gmail.com>,
	qemu-devel@nongnu.org
Cc: qemu-arm@nongnu.org, peter.maydell@linaro.org
Subject: Re: [PATCH] hw/arm: Replace TABs for spaces
Date: Mon, 5 May 2025 12:29:28 +0200	[thread overview]
Message-ID: <08323a18-a480-430a-8ba3-90617c319aa7@redhat.com> (raw)
In-Reply-To: <20250503201322.141948-1-santimonserr@gmail.com>

On 03/05/2025 22.13, Santiago Monserrat Campanello wrote:
> replaced for arm code
> 
> Signed-off-by: Santiago Monserrat Campanello <santimonserr@gmail.com>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/373

  Hi!

Thanks for tackling this! Some comments below...

> 
> diff --git a/hw/arm/omap1.c b/hw/arm/omap1.c
> index 91d7e3f04b..58b2a27533 100644
> --- a/hw/arm/omap1.c
> +++ b/hw/arm/omap1.c
...
> @@ -1592,22 +1592,22 @@ static inline void omap_clkm_idlect2_update(struct omap_mpu_state_s *s,
>   {
>       omap_clk clk;
>   
> -#define SET_ONOFF(clock, bit)				\
> -    if (diff & (1 << bit)) {				\
> -        clk = omap_findclk(s, clock);			\
> -        omap_clk_onoff(clk, (value >> bit) & 1);	\
> +#define SET_ONOFF(clock, bit)               \
> +    if (diff & (1 << bit)) {                \
> +        clk = omap_findclk(s, clock);           \
> +        omap_clk_onoff(clk, (value >> bit) & 1);    \

Could you pease align the backslashes here?

...
> @@ -2244,39 +2244,39 @@ static void omap_uwire_write(void *opaque, hwaddr addr,
>       }
>   
>       switch (offset) {
> -    case 0x00:	/* TDR */
> -        s->txbuf = value;				/* TD */
> -        if ((s->setup[4] & (1 << 2)) &&			/* AUTO_TX_EN */
> -                        ((s->setup[4] & (1 << 3)) ||	/* CS_TOGGLE_TX_EN */
> -                         (s->control & (1 << 12)))) {	/* CS_CMD */
> -            s->control |= 1 << 14;			/* CSRB */
> +    case 0x00:  /* TDR */
> +        s->txbuf = value;               /* TD */
> +        if ((s->setup[4] & (1 << 2)) &&         /* AUTO_TX_EN */
> +                        ((s->setup[4] & (1 << 3)) ||    /* CS_TOGGLE_TX_EN */
> +                         (s->control & (1 << 12)))) {   /* CS_CMD */
> +            s->control |= 1 << 14;          /* CSRB */

Please also align the comments here.

> @@ -2962,9 +2962,9 @@ static void omap_mcbsp_intr_update(struct omap_mcbsp_s *s)
>   
>   static void omap_mcbsp_rx_newdata(struct omap_mcbsp_s *s)
>   {
> -    if ((s->spcr[0] >> 1) & 1)				/* RRDY */
> -        s->spcr[0] |= 1 << 2;				/* RFULL */
> -    s->spcr[0] |= 1 << 1;				/* RRDY */
> +    if ((s->spcr[0] >> 1) & 1)              /* RRDY */
> +        s->spcr[0] |= 1 << 2;               /* RFULL */
> +    s->spcr[0] |= 1 << 1;               /* RRDY */

dito.

...
> @@ -3064,27 +3064,27 @@ static void omap_mcbsp_req_update(struct omap_mcbsp_s *s)
>   {
>       int prev_rx_rate, prev_tx_rate;
>       int rx_rate = 0, tx_rate = 0;
> -    int cpu_rate = 1500000;	/* XXX */
> +    int cpu_rate = 1500000; /* XXX */
>   
>       /* TODO: check CLKSTP bit */
> -    if (s->spcr[1] & (1 << 6)) {			/* GRST */
> -        if (s->spcr[0] & (1 << 0)) {			/* RRST */
> -            if ((s->srgr[1] & (1 << 13)) &&		/* CLKSM */
> -                            (s->pcr & (1 << 8))) {	/* CLKRM */
> -                if (~s->pcr & (1 << 7))			/* SCLKME */
> +    if (s->spcr[1] & (1 << 6)) {            /* GRST */
> +        if (s->spcr[0] & (1 << 0)) {            /* RRST */
> +            if ((s->srgr[1] & (1 << 13)) &&     /* CLKSM */
> +                            (s->pcr & (1 << 8))) {  /* CLKRM */
> +                if (~s->pcr & (1 << 7))         /* SCLKME */
>                       rx_rate = cpu_rate /
> -                            ((s->srgr[0] & 0xff) + 1);	/* CLKGDV */
> +                            ((s->srgr[0] & 0xff) + 1);  /* CLKGDV */
>               } else
>                   if (s->codec)
>                       rx_rate = s->codec->rx_rate;
>           }
>   
> -        if (s->spcr[1] & (1 << 0)) {			/* XRST */
> -            if ((s->srgr[1] & (1 << 13)) &&		/* CLKSM */
> -                            (s->pcr & (1 << 9))) {	/* CLKXM */
> -                if (~s->pcr & (1 << 7))			/* SCLKME */
> +        if (s->spcr[1] & (1 << 0)) {            /* XRST */
> +            if ((s->srgr[1] & (1 << 13)) &&     /* CLKSM */
> +                            (s->pcr & (1 << 9))) {  /* CLKXM */
> +                if (~s->pcr & (1 << 7))         /* SCLKME */
>                       tx_rate = cpu_rate /
> -                            ((s->srgr[0] & 0xff) + 1);	/* CLKGDV */
> +                            ((s->srgr[0] & 0xff) + 1);  /* CLKGDV */

dito

> @@ -3703,25 +3703,25 @@ static const struct omap_map_s {
>       const char *name;
>   } omap15xx_dsp_mm[] = {
>       /* Strobe 0 */
> -    { 0xe1010000, 0xfffb0000, 0x800, "UART1 BT" },		/* CS0 */
> -    { 0xe1010800, 0xfffb0800, 0x800, "UART2 COM" },		/* CS1 */
> -    { 0xe1011800, 0xfffb1800, 0x800, "McBSP1 audio" },		/* CS3 */
> -    { 0xe1012000, 0xfffb2000, 0x800, "MCSI2 communication" },	/* CS4 */
> -    { 0xe1012800, 0xfffb2800, 0x800, "MCSI1 BT u-Law" },	/* CS5 */
> -    { 0xe1013000, 0xfffb3000, 0x800, "uWire" },			/* CS6 */
> -    { 0xe1013800, 0xfffb3800, 0x800, "I^2C" },			/* CS7 */
> -    { 0xe1014000, 0xfffb4000, 0x800, "USB W2FC" },		/* CS8 */
> -    { 0xe1014800, 0xfffb4800, 0x800, "RTC" },			/* CS9 */
> -    { 0xe1015000, 0xfffb5000, 0x800, "MPUIO" },			/* CS10 */
> -    { 0xe1015800, 0xfffb5800, 0x800, "PWL" },			/* CS11 */
> -    { 0xe1016000, 0xfffb6000, 0x800, "PWT" },			/* CS12 */
> -    { 0xe1017000, 0xfffb7000, 0x800, "McBSP3" },		/* CS14 */
> -    { 0xe1017800, 0xfffb7800, 0x800, "MMC" },			/* CS15 */
> -    { 0xe1019000, 0xfffb9000, 0x800, "32-kHz timer" },		/* CS18 */
> -    { 0xe1019800, 0xfffb9800, 0x800, "UART3" },			/* CS19 */
> -    { 0xe101c800, 0xfffbc800, 0x800, "TIPB switches" },		/* CS25 */
> +    { 0xe1010000, 0xfffb0000, 0x800, "UART1 BT" },      /* CS0 */
> +    { 0xe1010800, 0xfffb0800, 0x800, "UART2 COM" },     /* CS1 */
> +    { 0xe1011800, 0xfffb1800, 0x800, "McBSP1 audio" },      /* CS3 */
> +    { 0xe1012000, 0xfffb2000, 0x800, "MCSI2 communication" },   /* CS4 */
> +    { 0xe1012800, 0xfffb2800, 0x800, "MCSI1 BT u-Law" },    /* CS5 */
> +    { 0xe1013000, 0xfffb3000, 0x800, "uWire" },         /* CS6 */
> +    { 0xe1013800, 0xfffb3800, 0x800, "I^2C" },          /* CS7 */
> +    { 0xe1014000, 0xfffb4000, 0x800, "USB W2FC" },      /* CS8 */
> +    { 0xe1014800, 0xfffb4800, 0x800, "RTC" },           /* CS9 */
> +    { 0xe1015000, 0xfffb5000, 0x800, "MPUIO" },         /* CS10 */
> +    { 0xe1015800, 0xfffb5800, 0x800, "PWL" },           /* CS11 */
> +    { 0xe1016000, 0xfffb6000, 0x800, "PWT" },           /* CS12 */
> +    { 0xe1017000, 0xfffb7000, 0x800, "McBSP3" },        /* CS14 */
> +    { 0xe1017800, 0xfffb7800, 0x800, "MMC" },           /* CS15 */
> +    { 0xe1019000, 0xfffb9000, 0x800, "32-kHz timer" },      /* CS18 */
> +    { 0xe1019800, 0xfffb9800, 0x800, "UART3" },         /* CS19 */
> +    { 0xe101c800, 0xfffbc800, 0x800, "TIPB switches" },     /* CS25 */
>       /* Strobe 1 */
> -    { 0xe101e000, 0xfffce000, 0x800, "GPIOs" },			/* CS28 */
> +    { 0xe101e000, 0xfffce000, 0x800, "GPIOs" },         /* CS28 */

dito

>       { 0 }
>   };
> @@ -4025,18 +4025,18 @@ struct omap_mpu_state_s *omap310_mpu_init(MemoryRegion *dram,
>                                 0xfffbd800, omap_findclk(s, "clk32-kHz"));
>   
>       /* Register mappings not currently implemented:
> -     * MCSI2 Comm	fffb2000 - fffb27ff (not mapped on OMAP310)
> -     * MCSI1 Bluetooth	fffb2800 - fffb2fff (not mapped on OMAP310)
> -     * USB W2FC		fffb4000 - fffb47ff
> -     * Camera Interface	fffb6800 - fffb6fff
> -     * USB Host		fffba000 - fffba7ff
> -     * FAC		fffba800 - fffbafff
> -     * HDQ/1-Wire	fffbc000 - fffbc7ff
> -     * TIPB switches	fffbc800 - fffbcfff
> -     * Mailbox		fffcf000 - fffcf7ff
> -     * Local bus IF	fffec100 - fffec1ff
> -     * Local bus MMU	fffec200 - fffec2ff
> -     * DSP MMU		fffed200 - fffed2ff
> +     * MCSI2 Comm   fffb2000 - fffb27ff (not mapped on OMAP310)
> +     * MCSI1 Bluetooth  fffb2800 - fffb2fff (not mapped on OMAP310)
> +     * USB W2FC     fffb4000 - fffb47ff
> +     * Camera Interface fffb6800 - fffb6fff
> +     * USB Host     fffba000 - fffba7ff
> +     * FAC      fffba800 - fffbafff
> +     * HDQ/1-Wire   fffbc000 - fffbc7ff
> +     * TIPB switches    fffbc800 - fffbcfff
> +     * Mailbox      fffcf000 - fffcf7ff
> +     * Local bus IF fffec100 - fffec1ff
> +     * Local bus MMU    fffec200 - fffec2ff
> +     * DSP MMU      fffed200 - fffed2ff

Could you please align the addresses here?

...
> diff --git a/hw/i2c/omap_i2c.c b/hw/i2c/omap_i2c.c
> index 2e45266e74..9333bf7cdb 100644
> --- a/hw/i2c/omap_i2c.c
> +++ b/hw/i2c/omap_i2c.c
> @@ -55,16 +55,16 @@ struct OMAPI2CState {
>       uint16_t test;
>   };
>   
> -#define OMAP2_INTR_REV	0x34
> -#define OMAP2_GC_REV	0x34
> +#define OMAP2_INTR_REV  0x34
> +#define OMAP2_GC_REV    0x34
>   
>   static void omap_i2c_interrupts_update(OMAPI2CState *s)
>   {
>       qemu_set_irq(s->irq, s->stat & s->mask);
> -    if ((s->dma >> 15) & 1)					/* RDMA_EN */
> -        qemu_set_irq(s->drq[0], (s->stat >> 3) & 1);		/* RRDY */
> -    if ((s->dma >> 7) & 1)					/* XDMA_EN */
> -        qemu_set_irq(s->drq[1], (s->stat >> 4) & 1);		/* XRDY */
> +    if ((s->dma >> 15) & 1)                 /* RDMA_EN */
> +        qemu_set_irq(s->drq[0], (s->stat >> 3) & 1);        /* RRDY */
> +    if ((s->dma >> 7) & 1)                  /* XDMA_EN */
> +        qemu_set_irq(s->drq[1], (s->stat >> 4) & 1);        /* XRDY */
>   }
>   
>   static void omap_i2c_fifo_run(OMAPI2CState *s)
> @@ -74,25 +74,25 @@ static void omap_i2c_fifo_run(OMAPI2CState *s)
>       if (!i2c_bus_busy(s->bus))
>           return;
>   
> -    if ((s->control >> 2) & 1) {				/* RM */
> -        if ((s->control >> 1) & 1) {				/* STP */
> +    if ((s->control >> 2) & 1) {                /* RM */
> +        if ((s->control >> 1) & 1) {                /* STP */
>               i2c_end_transfer(s->bus);
> -            s->control &= ~(1 << 1);				/* STP */
> +            s->control &= ~(1 << 1);                /* STP */
>               s->count_cur = s->count;
>               s->txlen = 0;
> -        } else if ((s->control >> 9) & 1) {			/* TRX */
> +        } else if ((s->control >> 9) & 1) {         /* TRX */
>               while (ack && s->txlen)
>                   ack = (i2c_send(s->bus,
>                                           (s->fifo >> ((-- s->txlen) << 3)) &
>                                           0xff) >= 0);
> -            s->stat |= 1 << 4;					/* XRDY */
> +            s->stat |= 1 << 4;                  /* XRDY */
>           } else {
>               while (s->rxlen < 4)
>                   s->fifo |= i2c_recv(s->bus) << ((s->rxlen ++) << 3);
> -            s->stat |= 1 << 3;					/* RRDY */
> +            s->stat |= 1 << 3;                  /* RRDY */
>           }
>       } else {
> -        if ((s->control >> 9) & 1) {				/* TRX */
> +        if ((s->control >> 9) & 1) {                /* TRX */
>               while (ack && s->count_cur && s->txlen) {
>                   ack = (i2c_send(s->bus,
>                                           (s->fifo >> ((-- s->txlen) << 3)) &
> @@ -100,12 +100,12 @@ static void omap_i2c_fifo_run(OMAPI2CState *s)
>                   s->count_cur --;
>               }
>               if (ack && s->count_cur)
> -                s->stat |= 1 << 4;				/* XRDY */
> +                s->stat |= 1 << 4;              /* XRDY */
>               else
> -                s->stat &= ~(1 << 4);				/* XRDY */
> +                s->stat &= ~(1 << 4);               /* XRDY */
>               if (!s->count_cur) {
> -                s->stat |= 1 << 2;				/* ARDY */
> -                s->control &= ~(1 << 10);			/* MST */
> +                s->stat |= 1 << 2;              /* ARDY */
> +                s->control &= ~(1 << 10);           /* MST */
>               }
>           } else {
>               while (s->count_cur && s->rxlen < 4) {
> @@ -113,26 +113,26 @@ static void omap_i2c_fifo_run(OMAPI2CState *s)
>                   s->count_cur --;
>               }
>               if (s->rxlen)
> -                s->stat |= 1 << 3;				/* RRDY */
> +                s->stat |= 1 << 3;              /* RRDY */
>               else
> -                s->stat &= ~(1 << 3);				/* RRDY */
> +                s->stat &= ~(1 << 3);               /* RRDY */
>           }
>           if (!s->count_cur) {
> -            if ((s->control >> 1) & 1) {			/* STP */
> +            if ((s->control >> 1) & 1) {            /* STP */
>                   i2c_end_transfer(s->bus);
> -                s->control &= ~(1 << 1);			/* STP */
> +                s->control &= ~(1 << 1);            /* STP */
>                   s->count_cur = s->count;
>                   s->txlen = 0;
>               } else {
> -                s->stat |= 1 << 2;				/* ARDY */
> -                s->control &= ~(1 << 10);			/* MST */
> +                s->stat |= 1 << 2;              /* ARDY */
> +                s->control &= ~(1 << 10);           /* MST */
>               }
>           }
>       }
>   
> -    s->stat |= (!ack) << 1;					/* NACK */
> +    s->stat |= (!ack) << 1;                 /* NACK */
>       if (!ack)
> -        s->control &= ~(1 << 1);				/* STP */
> +        s->control &= ~(1 << 1);                /* STP */
>   }

Could you please align the comments in this function?

...
> @@ -214,41 +214,41 @@ static uint32_t omap_i2c_read(void *opaque, hwaddr addr)
>               /* XXX: remote access (qualifier) error - what's that?  */
>           }
>           if (!s->rxlen) {
> -            s->stat &= ~(1 << 3);				/* RRDY */
> -            if (((s->control >> 10) & 1) &&			/* MST */
> -                            ((~s->control >> 9) & 1)) {		/* TRX */
> -                s->stat |= 1 << 2;				/* ARDY */
> -                s->control &= ~(1 << 10);			/* MST */
> +            s->stat &= ~(1 << 3);               /* RRDY */
> +            if (((s->control >> 10) & 1) &&         /* MST */
> +                            ((~s->control >> 9) & 1)) {     /* TRX */
> +                s->stat |= 1 << 2;              /* ARDY */
> +                s->control &= ~(1 << 10);           /* MST */
>               }
>           }
> -        s->stat &= ~(1 << 11);					/* ROVR */
> +        s->stat &= ~(1 << 11);                  /* ROVR */

Please align in this hunk, too.

> @@ -351,14 +351,14 @@ static void omap_i2c_write(void *opaque, hwaddr addr,
>                             __func__);
>               break;
>           }
> -        if ((value & (1 << 15)) && value & (1 << 0)) {		/* STT */
> -            nack = !!i2c_start_transfer(s->bus, s->addr[1],	/* SA */
> -                            (~value >> 9) & 1);			/* TRX */
> -            s->stat |= nack << 1;				/* NACK */
> -            s->control &= ~(1 << 0);				/* STT */
> +        if ((value & (1 << 15)) && value & (1 << 0)) {      /* STT */
> +            nack = !!i2c_start_transfer(s->bus, s->addr[1], /* SA */
> +                            (~value >> 9) & 1);         /* TRX */
> +            s->stat |= nack << 1;               /* NACK */
> +            s->control &= ~(1 << 0);                /* STT */
>               s->fifo = 0;
>               if (nack)
> -                s->control &= ~(1 << 1);			/* STP */
> +                s->control &= ~(1 << 1);            /* STP */
>               else {
>                   s->count_cur = s->count;
>                   omap_i2c_fifo_run(s);

dito

...
> diff --git a/hw/misc/omap_clk.c b/hw/misc/omap_clk.c
> index 0157c9be75..b32d55dc54 100644
> --- a/hw/misc/omap_clk.c
> +++ b/hw/misc/omap_clk.c
> @@ -30,170 +30,170 @@ struct clk {
>       struct clk *parent;
>       struct clk *child1;
>       struct clk *sibling;
> -#define ALWAYS_ENABLED		(1 << 0)
> -#define CLOCK_IN_OMAP310	(1 << 10)
> -#define CLOCK_IN_OMAP730	(1 << 11)
> -#define CLOCK_IN_OMAP1510	(1 << 12)
> -#define CLOCK_IN_OMAP16XX	(1 << 13)
> +#define ALWAYS_ENABLED      (1 << 0)
> +#define CLOCK_IN_OMAP310    (1 << 10)
> +#define CLOCK_IN_OMAP730    (1 << 11)
> +#define CLOCK_IN_OMAP1510   (1 << 12)
> +#define CLOCK_IN_OMAP16XX   (1 << 13)
>       uint32_t flags;
>       int id;
>   
> -    int running;		/* Is currently ticking */
> -    int enabled;		/* Is enabled, regardless of its input clk */
> -    unsigned long rate;		/* Current rate (if .running) */
> -    unsigned int divisor;	/* Rate relative to input (if .enabled) */
> -    unsigned int multiplier;	/* Rate relative to input (if .enabled) */
> -    qemu_irq users[16];		/* Who to notify on change */
> -    int usecount;		/* Automatically idle when unused */
> +    int running;        /* Is currently ticking */
> +    int enabled;        /* Is enabled, regardless of its input clk */
> +    unsigned long rate;     /* Current rate (if .running) */
> +    unsigned int divisor;   /* Rate relative to input (if .enabled) */
> +    unsigned int multiplier;    /* Rate relative to input (if .enabled) */
> +    qemu_irq users[16];     /* Who to notify on change */
> +    int usecount;       /* Automatically idle when unused */

dito

  Thanks,
   Thomas



  reply	other threads:[~2025-05-05 10:30 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-05-03 20:13 [PATCH] hw/arm: Replace TABs for spaces Santiago Monserrat Campanello
2025-05-05 10:29 ` Thomas Huth [this message]
2025-05-05 11:03 ` Stefan Weil via

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=08323a18-a480-430a-8ba3-90617c319aa7@redhat.com \
    --to=thuth@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-arm@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=santimonserr@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).