> > 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 - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html