Re: [PATCH 0/2] [libata] sata_mv: Add support for Marvell's integrated SATA controller

["saeed bishara" <saeed.bishara@gmail.com>]

>
> I really think that a lot of the new variable/macro/enum names are overly long,
> making the code a bit harder to read.
> The patch is all about "System On Chip (SOC)" support,
> yet the names all say "INTEGRATED".
well, many socs have SATA or Ethernet controllers as pci controller,
but in this case, the sata controller has been integrated into the soc
by cutting the pci interface and connecting the sata core directly to
the soc's bus. so I though that the "integrated" well show this fact.

>
> How about changing "INTEGRATED" to "SOC", and "integrated" to "soc" everywhere ?
>
> The mv_integrated_reset_hc_port() (or mv_soc_reset_hc_port()) function
> seems to be a duplicate of the existing mv5_reset_hc_port() function.
except the line that writes to the EDMA_CFG_OFS register (101f vs
11f). also , I think that in the future the the integrated/soc variant
will have more register that does not exist in mv5 to reset.

BTW, the mv5_sht and mv6_sht are identical, are there any plans to
modify any or them?

> The new fields in the mv_host_priv struct are __iomem pointers
> rather than offsets as was done for similar fields in the past
> (offsets take up less space on 64-bit machines).
> We should be consistent there, one way or the other.
well, as those registers get access in the main flow (data commands),
preparing the final address will save some runtime calculations. so,
it's the speed vs memory tradeoff. I think speed should be preferred.
agree?
>
> Cheers
>
> Mark