Re: [Qemu-devel] [PATCH v3 10/30] imx_fec: Reserve full 4K page for the register file

[Andrey Smirnov]

On Tue, Nov 21, 2017 at 9:48 AM, Peter Maydell  wrote:
> On 6 November 2017 at 15:47, Andrey Smirnov  wrote:
>> Some i.MX SoCs (e.g. i.MX7) have FEC registers going as far as offset
>> 0x614, so to avoid getting aborts when accessing those on QEMU, extend
>> the register file to cover 4KB of address space instead of just 1K.
>>
>> Cc: Peter Maydell 
>> Cc: Jason Wang 
>> Cc: Philippe Mathieu-Daudé 
>> Cc: qemu-devel@nongnu.org
>> Cc: qemu-...@nongnu.org
>> Cc: yurov...@gmail.com
>> Signed-off-by: Andrey Smirnov 
>> ---
>>  hw/net/imx_fec.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/hw/net/imx_fec.c b/hw/net/imx_fec.c
>> index 48d012cad6..e236bc933c 100644
>> --- a/hw/net/imx_fec.c
>> +++ b/hw/net/imx_fec.c
>> @@ -1252,7 +1252,7 @@ static void imx_eth_realize(DeviceState *dev, Error 
>> **errp)
>>  SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
>>
>>  memory_region_init_io(>iomem, OBJECT(dev), _eth_ops, s,
>> -  TYPE_IMX_FEC, 0x400);
>> +  TYPE_IMX_FEC, 0x1000);
>>  sysbus_init_mmio(sbd, >iomem);
>>  sysbus_init_irq(sbd, >irq[0]);
>>  sysbus_init_irq(sbd, >irq[1]);
>> --
>
> I notice that we have an unused #define FSL_IMX25_FEC_SIZE 0x4000 in
> fsl-imx25.h, and the Linux device trees for the imx25 define the size
> of the FEC register block as 0x4000. Should this be 0x4000 ?
>

I think the size reserved for that register file differs between
differen SoC. E.g. it's 16K on i.MX25, as you pointed out, but 64K on
i.MX7. It's all the same to me as long as it's greater than 0x1000.
I'll change the code to use FSL_IMX25_FEC_SIZE since it gets rid of a
magic number.

Thanks,
Andrey Smirnov

Re: [Qemu-devel] [PATCH v3 10/30] imx_fec: Reserve full 4K page for the register file

[Peter Maydell]

On 6 November 2017 at 15:47, Andrey Smirnov  wrote:
> Some i.MX SoCs (e.g. i.MX7) have FEC registers going as far as offset
> 0x614, so to avoid getting aborts when accessing those on QEMU, extend
> the register file to cover 4KB of address space instead of just 1K.
>
> Cc: Peter Maydell 
> Cc: Jason Wang 
> Cc: Philippe Mathieu-Daudé 
> Cc: qemu-devel@nongnu.org
> Cc: qemu-...@nongnu.org
> Cc: yurov...@gmail.com
> Signed-off-by: Andrey Smirnov 
> ---
>  hw/net/imx_fec.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/net/imx_fec.c b/hw/net/imx_fec.c
> index 48d012cad6..e236bc933c 100644
> --- a/hw/net/imx_fec.c
> +++ b/hw/net/imx_fec.c
> @@ -1252,7 +1252,7 @@ static void imx_eth_realize(DeviceState *dev, Error 
> **errp)
>  SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
>
>  memory_region_init_io(>iomem, OBJECT(dev), _eth_ops, s,
> -  TYPE_IMX_FEC, 0x400);
> +  TYPE_IMX_FEC, 0x1000);
>  sysbus_init_mmio(sbd, >iomem);
>  sysbus_init_irq(sbd, >irq[0]);
>  sysbus_init_irq(sbd, >irq[1]);
> --

I notice that we have an unused #define FSL_IMX25_FEC_SIZE 0x4000 in
fsl-imx25.h, and the Linux device trees for the imx25 define the size
of the FEC register block as 0x4000. Should this be 0x4000 ?

thanks
-- PMM