Patchwork [arm-devs,v1,07/15] xilinx_spips: Trash LQ page cache on mode change

login
register
mail settings
Submitter Peter Crosthwaite
Date April 3, 2013, 4:32 a.m.
Message ID <2a5d5374ee57d7025f718b55b1f33891115fcfad.1364962908.git.peter.crosthwaite@xilinx.com>
Download mbox | patch
Permalink /patch/233225/
State New
Headers show

Comments

Peter Crosthwaite - April 3, 2013, 4:32 a.m.
Invalidate the LQSPI cached page when transitioning into LQSPI mode.
Otherwise there is a possibility that the controller will return stale
data to the guest when transitioning back to LQ_MODE after a page
program.

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

 hw/xilinx_spips.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)
Peter Maydell - April 5, 2013, 6:53 p.m.
On 3 April 2013 05:32, Peter Crosthwaite <peter.crosthwaite@xilinx.com> wrote:
> Invalidate the LQSPI cached page when transitioning into LQSPI mode.
> Otherwise there is a possibility that the controller will return stale
> data to the guest when transitioning back to LQ_MODE after a page
> program.
>
> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
> ---
>
>  hw/xilinx_spips.c |    8 ++++++++
>  1 files changed, 8 insertions(+), 0 deletions(-)
>
> diff --git a/hw/xilinx_spips.c b/hw/xilinx_spips.c
> index 78a3fec..4f921f8 100644
> --- a/hw/xilinx_spips.c
> +++ b/hw/xilinx_spips.c
> @@ -390,6 +390,9 @@ static void xilinx_spips_write(void *opaque, hwaddr addr,
>      int mask = ~0;
>      int man_start_com = 0;
>      XilinxSPIPS *s = opaque;
> +    /* FIXME: abstract away somehow */
> +    XilinxQSPIPS *q = (XilinxQSPIPS *)object_dynamic_cast(OBJECT(s),
> +                       TYPE_XILINX_QSPIPS);
>

Hmm, a C cast of the result of an object_dynamic_cast()?
Would be nice to do this some other way... Maybe a member
function (ie fn ptr in the class struct) which does nothing
on SPIPS and clears the cached page on QSPIPS?

>      DB_PRINT("addr=" TARGET_FMT_plx " = %x\n", addr, (unsigned)value);
>      addr >>= 2;
> @@ -435,6 +438,11 @@ static void xilinx_spips_write(void *opaque, hwaddr addr,
>      case R_TXD3:
>          tx_data_bytes(s, (uint32_t)value, 3);
>          goto no_reg_update;
> +    case R_LQSPI_CFG:
> +        if (q) {
> +            q->lqspi_cached_addr = ~0ULL;
> +        }
> +        break;
>      }
>      s->regs[addr] = (s->regs[addr] & ~mask) | (value & mask);
>  no_reg_update:
> --
> 1.7.0.4
>

-- PMM
Peter Crosthwaite - April 8, 2013, 8:19 a.m.
Hi Peter,

On Sat, Apr 6, 2013 at 4:53 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 3 April 2013 05:32, Peter Crosthwaite <peter.crosthwaite@xilinx.com> wrote:
>> Invalidate the LQSPI cached page when transitioning into LQSPI mode.
>> Otherwise there is a possibility that the controller will return stale
>> data to the guest when transitioning back to LQ_MODE after a page
>> program.
>>
>> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>> ---
>>
>>  hw/xilinx_spips.c |    8 ++++++++
>>  1 files changed, 8 insertions(+), 0 deletions(-)
>>
>> diff --git a/hw/xilinx_spips.c b/hw/xilinx_spips.c
>> index 78a3fec..4f921f8 100644
>> --- a/hw/xilinx_spips.c
>> +++ b/hw/xilinx_spips.c
>> @@ -390,6 +390,9 @@ static void xilinx_spips_write(void *opaque, hwaddr addr,
>>      int mask = ~0;
>>      int man_start_com = 0;
>>      XilinxSPIPS *s = opaque;
>> +    /* FIXME: abstract away somehow */
>> +    XilinxQSPIPS *q = (XilinxQSPIPS *)object_dynamic_cast(OBJECT(s),
>> +                       TYPE_XILINX_QSPIPS);
>>
>
> Hmm, a C cast of the result of an object_dynamic_cast()?
> Would be nice to do this some other way... Maybe a member
> function (ie fn ptr in the class struct) which does nothing
> on SPIPS and clears the cached page on QSPIPS?
>

Went a different route. Added the MemoryRegionOps to the class instead
and switched that out on class init. qspis alternate write handler
just calls spis then does its side effects on top. This way, spi has
no awareness of qspi which is more ideal the child class specific
abstract fns in the base class def.

Regards,
Peter

>>      DB_PRINT("addr=" TARGET_FMT_plx " = %x\n", addr, (unsigned)value);
>>      addr >>= 2;
>> @@ -435,6 +438,11 @@ static void xilinx_spips_write(void *opaque, hwaddr addr,
>>      case R_TXD3:
>>          tx_data_bytes(s, (uint32_t)value, 3);
>>          goto no_reg_update;
>> +    case R_LQSPI_CFG:
>> +        if (q) {
>> +            q->lqspi_cached_addr = ~0ULL;
>> +        }
>> +        break;
>>      }
>>      s->regs[addr] = (s->regs[addr] & ~mask) | (value & mask);
>>  no_reg_update:
>> --
>> 1.7.0.4
>>
>
> -- PMM
>

Patch

diff --git a/hw/xilinx_spips.c b/hw/xilinx_spips.c
index 78a3fec..4f921f8 100644
--- a/hw/xilinx_spips.c
+++ b/hw/xilinx_spips.c
@@ -390,6 +390,9 @@  static void xilinx_spips_write(void *opaque, hwaddr addr,
     int mask = ~0;
     int man_start_com = 0;
     XilinxSPIPS *s = opaque;
+    /* FIXME: abstract away somehow */
+    XilinxQSPIPS *q = (XilinxQSPIPS *)object_dynamic_cast(OBJECT(s),
+                       TYPE_XILINX_QSPIPS);
 
     DB_PRINT("addr=" TARGET_FMT_plx " = %x\n", addr, (unsigned)value);
     addr >>= 2;
@@ -435,6 +438,11 @@  static void xilinx_spips_write(void *opaque, hwaddr addr,
     case R_TXD3:
         tx_data_bytes(s, (uint32_t)value, 3);
         goto no_reg_update;
+    case R_LQSPI_CFG:
+        if (q) {
+            q->lqspi_cached_addr = ~0ULL;
+        }
+        break;
     }
     s->regs[addr] = (s->regs[addr] & ~mask) | (value & mask);
 no_reg_update: