Patchwork [2/3] hw/omap_gpio.c: Don't complain about some writes to r/o registers

login
register
mail settings
Submitter Peter Maydell
Date June 29, 2011, 6:53 p.m.
Message ID <1309373591-30380-3-git-send-email-peter.maydell@linaro.org>
Download mbox | patch
Permalink /patch/102655/
State New
Headers show

Comments

Peter Maydell - June 29, 2011, 6:53 p.m.
Don't complain about some writes to r/o OMAP2 GPIO registers, because the
kernel will do them anyway.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/omap_gpio.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
Edgar Iglesias - June 29, 2011, 8:07 p.m.
On Wed, Jun 29, 2011 at 07:53:10PM +0100, Peter Maydell wrote:
> Don't complain about some writes to r/o OMAP2 GPIO registers, because the
> kernel will do them anyway.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Hi Peter,

I usually find this kind of logs useful.
Maybe we should turn them into _log_mask(~0, xxx) so they only come out
when -d is enabled?

Cheers


> ---
>  hw/omap_gpio.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/omap_gpio.c b/hw/omap_gpio.c
> index 478f7d9..b53b13b 100644
> --- a/hw/omap_gpio.c
> +++ b/hw/omap_gpio.c
> @@ -385,7 +385,7 @@ static void omap2_gpio_module_write(void *opaque, target_phys_addr_t addr,
>      case 0x00:	/* GPIO_REVISION */
>      case 0x14:	/* GPIO_SYSSTATUS */
>      case 0x38:	/* GPIO_DATAIN */
> -        OMAP_RO_REG(addr);
> +        /* read-only, ignore quietly */
>          break;
>  
>      case 0x10:	/* GPIO_SYSCONFIG */
> @@ -531,7 +531,7 @@ static void omap2_gpio_module_writep(void *opaque, target_phys_addr_t addr,
>      case 0x00:	/* GPIO_REVISION */
>      case 0x14:	/* GPIO_SYSSTATUS */
>      case 0x38:	/* GPIO_DATAIN */
> -        OMAP_RO_REG(addr);
> +        /* read-only, ignore quietly */
>          break;
>  
>      case 0x10:	/* GPIO_SYSCONFIG */
> -- 
> 1.7.1
> 
>
Peter Maydell - June 29, 2011, 8:39 p.m.
On 29 June 2011 21:07, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> On Wed, Jun 29, 2011 at 07:53:10PM +0100, Peter Maydell wrote:
>> Don't complain about some writes to r/o OMAP2 GPIO registers, because the
>> kernel will do them anyway.
>>
>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>
> Hi Peter,
>
> I usually find this kind of logs useful.
> Maybe we should turn them into _log_mask(~0, xxx) so they only come out
> when -d is enabled?

I think the ideal would be to have all of this kind of message go
through some consistent path (ie not just printf) so you could
tell all of qemu "I'm doing OS development, warn about things which
indicate driver bugs" vs the more common "I'm just a user and I
don't really care".

You'd also want to be able to trace the guest program counter
and to do things like drop into gdb if running with the gdb stub
enabled.

In the absence of that kind of infrastructure I tend to default
to removing this kind of printf or relegating it to DPRINTF.

-- PMM
Edgar Iglesias - June 29, 2011, 8:49 p.m.
On Wed, Jun 29, 2011 at 09:39:52PM +0100, Peter Maydell wrote:
> On 29 June 2011 21:07, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> > On Wed, Jun 29, 2011 at 07:53:10PM +0100, Peter Maydell wrote:
> >> Don't complain about some writes to r/o OMAP2 GPIO registers, because the
> >> kernel will do them anyway.
> >>
> >> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> >
> > Hi Peter,
> >
> > I usually find this kind of logs useful.
> > Maybe we should turn them into _log_mask(~0, xxx) so they only come out
> > when -d is enabled?
> 
> I think the ideal would be to have all of this kind of message go
> through some consistent path (ie not just printf) so you could
> tell all of qemu "I'm doing OS development, warn about things which
> indicate driver bugs" vs the more common "I'm just a user and I
> don't really care".
> 
> You'd also want to be able to trace the guest program counter
> and to do things like drop into gdb if running with the gdb stub
> enabled.
> 
> In the absence of that kind of infrastructure I tend to default
> to removing this kind of printf or relegating it to DPRINTF.

Yes. I usually find -d exec good enough for this simple errors.
The GDB connection I find useful for more complex hw-errors where
PC location isn't enough to figure out what caused the hw-errror.

I dont feel very strongly about it though.

Cheers

Patch

diff --git a/hw/omap_gpio.c b/hw/omap_gpio.c
index 478f7d9..b53b13b 100644
--- a/hw/omap_gpio.c
+++ b/hw/omap_gpio.c
@@ -385,7 +385,7 @@  static void omap2_gpio_module_write(void *opaque, target_phys_addr_t addr,
     case 0x00:	/* GPIO_REVISION */
     case 0x14:	/* GPIO_SYSSTATUS */
     case 0x38:	/* GPIO_DATAIN */
-        OMAP_RO_REG(addr);
+        /* read-only, ignore quietly */
         break;
 
     case 0x10:	/* GPIO_SYSCONFIG */
@@ -531,7 +531,7 @@  static void omap2_gpio_module_writep(void *opaque, target_phys_addr_t addr,
     case 0x00:	/* GPIO_REVISION */
     case 0x14:	/* GPIO_SYSSTATUS */
     case 0x38:	/* GPIO_DATAIN */
-        OMAP_RO_REG(addr);
+        /* read-only, ignore quietly */
         break;
 
     case 0x10:	/* GPIO_SYSCONFIG */