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

From: Jeremy Linton <jeremy.linton@arm.com>
To: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: edk2-devel-01 <edk2-devel@lists.01.org>,
	Steve Capper <steve.capper@arm.com>,
	Leif Lindholm <leif.lindholm@linaro.org>,
	Ryan Harkin <ryan.harkin@linaro.org>,
	linaro-uefi <linaro-uefi@lists.linaro.org>
Subject: Re: [PATCH 0/8] ATAPI support on SiI SATA adapter
Date: Tue, 15 Nov 2016 08:54:34 -0600	[thread overview]
Message-ID: <bde2e447-967f-190b-f76d-e27179dad206@arm.com> (raw)
In-Reply-To: <CAKv+Gu_1FVUD7s7OycLf_bcszn5-DATou1nh6s-p63KbPusxYg@mail.gmail.com>

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?

>
>> 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.

>
>> 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