Re: [PATCH 0/8] ATAPI support on SiI SATA adapter

[Ard Biesheuvel <ard.biesheuvel@linaro.org>]

On 15 November 2016 at 14:54, Jeremy Linton <jeremy.linton@arm.com> wrote:
> On 11/15/2016 01:43 AM, Ard Biesheuvel wrote:
>>
>> Hi Jeremy,
>>
>> On 14 November 2016 at 21:09, Jeremy Linton <jeremy.linton@arm.com> wrote:
>>>
>>> The SiI isn't an AHCI compatible adapter so it implements the EFI ATA
>>> pass-through protocol directly. This works for fixed hard drives, but
>>> not ATAPI attached devices (CDROM, DVDROM, TAPE, etc).
>>>
>>> This patch adds read only ATAPI support via the EFI SCSI pass-through
>>> protocol, allowing boot from attached CD/DVD. This patch also cleans
>>> up, and tweaks recovery paths/etc in the original driver.
>>
>>
>> Very nice! Thanks for getting to the bottom of this.
>>
>> However, looking at the patches, they are riddled with coding style
>> violations. I am usually less strict than Leif when it comes to
>> upholding those, but these patches really need to be cleaned up to be
>> considered for merging.
>
>
>
> Is there a tool which can correct or at least point out the formatting
> errors?
>

Yes, BaseTools/Scripts/PatchCheck.py

>
>>
>>> When
>>> combined with the ARM/PCI dma lib changes this allows us to relax the
>>> IO alignment requirement that caused grub failures.
>>>
>>
>> What changes are you referring to here?
>
>
> I believe on juno the PCI changed from the ArmDmaLib to the null lib or some
> such, which removed the bounce buffering on unaligned map/unmap.
>

Indeed, it removed the use of uncached memory, which does not work for
coherent masters.
I only realize now that the Sil3132 is a coherent PCI device, and that
you were using 4 KB IoAlign to work around the non-coherent nature of
ArmDmaLib


>
>>
>>> Finally, the OpenPlatformPkg/Juno must be updated, with another patch
>>> to avoid build breaks now that the SiI has a dependency on the SCSI
>>> libraries.
>>>
>>> Contributed-under: TianoCore Contribution Agreement 1.0
>>> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
>>>
>>> Jeremy Linton (7):
>>>   MdePkg IndustryStandard/Scsi.h: Add sense code macro
>>>   EmbeddedPkg: SiI3132: Add ScsiProtocol callbacks
>>>   EmbeddedPkg: SiI3132: Add SCSI protocol support to header
>>>   EmbeddedPkg: SiI3132: Break out FIS command submission
>>>   EmbeddedPkg: SiI3132: Cleanup device node creation
>>>   EmbeddedPkg: SiI3132: Enable SCSI pass-through protocol
>>>   EmbeddedPkg: SiI3132: Correct the IoAlign
>>>
>>>  EmbeddedPkg/Drivers/SataSiI3132Dxe/SataSiI3132.c   |  48 ++-
>>>  EmbeddedPkg/Drivers/SataSiI3132Dxe/SataSiI3132.h   |  89 ++++-
>>>  .../Drivers/SataSiI3132Dxe/SataSiI3132Dxe.inf      |   2 +
>>>  .../Drivers/SataSiI3132Dxe/SiI3132AtaPassThru.c    | 270 ++++++++------
>>>  .../Drivers/SataSiI3132Dxe/SiI3132ScsiPassThru.c   | 401
>>> +++++++++++++++++++++
>>>  MdePkg/Include/IndustryStandard/Scsi.h             |   2 +
>>>  OpenPlatformPkg                                    |   2 +-
>>>  7 files changed, 688 insertions(+), 126 deletions(-)
>>>  create mode 100644
>>> EmbeddedPkg/Drivers/SataSiI3132Dxe/SiI3132ScsiPassThru.c
>>>
>>> --
>>> 2.5.5
>>>
>>> _______________________________________________
>>> edk2-devel mailing list
>>> edk2-devel@lists.01.org
>>> https://lists.01.org/mailman/listinfo/edk2-devel
>
>