Re: [RFC] SATA : Implemented NXP errata A008402

[Udit Kumar <udit.kumar@nxp.com>]

> -----Original Message-----
> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> Sent: Wednesday, January 10, 2018 3:23 PM
> To: Udit Kumar <udit.kumar@nxp.com>
> Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Meenakshi Aggarwal
> <meenakshi.aggarwal@nxp.com>; edk2-devel@lists.01.org; Zeng, Star
> <star.zeng@intel.com>; leif.lindholm@linaro.org
> Subject: Re: [edk2] [RFC] SATA : Implemented NXP errata A008402
> 
> On 10 January 2018 at 09:43, Udit Kumar <udit.kumar@nxp.com> wrote:
> > Hi Ruiyu,
> >
> >> -----Original Message-----
> >> >
> >> > And this change will not impact any other hardware so no one is
> >> > basically
> >> impacted by this change.
> >>
> >> Your buggy HW only need the value zero. But the addition of PCD
> >> exposes an interface that can use any size of PRD.
> >> I am not sure the AtaAtapiPassThru can work if some platform sets the
> >> PCD value to others than 0 or 3F_FFFFh.
> >
> > I don't see someone using this driver will set Pcd randomly, but I
> > agree on this point other value should be handled.
> > Error or Assert could be added, if Pcd value is not 0 or 3F_FFFFh.
> >
> >> Can you please
> >>   just duplicate the AtaAtapiPassThru in your platform?
> >> Because the driver is very stable today, not much code sync effort
> >> will be needed if core version is changed.
> >
> > Duplicating is always a possibility :), but When we will push this
> > duplicated driver (just for one line change) for upstreaming.
> > will this be acceptable ??
> 
> My main concern with this (and with using a PCD) is that the setting affects
> all SATA controllers in the system, including ones the you stick into a PCIe
> slot.

Agreed,  If two types of Sata controllers are used then this patch 
may break one controller. 
At present, we don’t have anyone asking SATA controller behind PCIe
but we shouldn’t ignore this. This is valid use case. 

> So I think forking the driver is the only possible solution, but it will not be a
> one-line change: you need to ensure that you apply the workaround only to
> the SATA controllers in the SoC.

Yes, Considering unrelated SATA controllers this will be more than copy 

> --
> Ard.

Thanks 
Udit