Re: [RFC PATCH 3/6] dp8393x: Restrict bus access to 16/32-bit operations

[Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>]

On 03/07/2021 15:19, Philippe Mathieu-Daudé wrote:

> Per the DP83932C datasheet from July 1995:
> 
>    1. Functional Description
>    1.3 DATA WIDTH AND BYTE ORDERING
> 
>      The SONIC can be programmed to operate with
>      either 32-bit or 16-bit wide memory.
> 
> Restrict the memory bus to reject 8/64-bit accesses.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>   hw/net/dp8393x.c | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
> index d16ade2b198..c9b478c127c 100644
> --- a/hw/net/dp8393x.c
> +++ b/hw/net/dp8393x.c
> @@ -695,6 +695,8 @@ static const MemoryRegionOps dp8393x_ops = {
>       .write = dp8393x_write,
>       .impl.min_access_size = 2,
>       .impl.max_access_size = 2,
> +    .valid.min_access_size = 2,
> +    .valid.max_access_size = 4,
>       .endianness = DEVICE_NATIVE_ENDIAN,
>   };

The changes to .valid are correct, but I don't think the above reference to the 
datasheet is. My reading of the sentence in section 1.3 "The SONIC can be programmed 
to operate with either 32-bit or 16-bit wide memory" (along with subsequent 
references to the DW bit in the DCR) is that this section is describing bus master 
transfers as per figure 1-5 instead of register accesses which is what this patch 
changes.

I think you could simply squash this patch into the previous one since there will 
already be a behaviour change there.


ATB,

Mark.