Re: Hardcoded IDE Controller B/D/F (0/1/1) in BdsPlatform.c?

[Bryan Rosario <bcr@google.com>]

Hi Ken,

That's what I had guessed -- thanks for confirming that it's platform
specific.

In my use case, I actually want the code to work on multiple hardware
configurations, so I can't just replace the hardcoded B/D/F with the one
used by a particular platform. Does the change to SataController.c that I
mentioned in the original email make sense? I've tested it and confirmed
that it works on my platform, but I want to get a sanity check that that is
an appropriate place to put this code.

Thanks,
Bryan

On Wed, Nov 21, 2018 at 4:48 PM Ken Taylor <Ken_Taylor@phoenix.com> wrote:

> Hi Bryan,
>
> That appears to be silicon or platform specific code.  It's intended for
> one specific hardware configuration.  Unless you have an identical hardware
> configuration, those particular register writes are actually nonsensical.
>
> I think what you need to do is consult the silicon specific documentation
> for your particular hardware configuration, and determine what silicon
> specific registers need to be configured to what specific values to support
> your specific hardware design.
>
> Regards,
> -Ken.
>
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Bryan Rosario via edk2-devel
> Sent: Wednesday, November 21, 2018 4:07 PM
> To: edk2-devel@lists.01.org
> Cc: Alain Gefflaut
> Subject: [edk2] Hardcoded IDE Controller B/D/F (0/1/1) in BdsPlatform.c?
>
> Hi all,
>
> I noticed that there is apparently a hardcoded B/D/F for an IDE Controller
> in BdsPlatform.c, when setting bit 15 of a particular register:
>
> https://github.com/tianocore/edk2/blob/14923c1a6bf9940b48feeaf47cb5d6c662b6528c/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c#L1612-L1617
> .
>
> Why is this hardcoded? Is it just old code that has hung around? I noticed
> that this code is from this commit, and the surrounding code at the time
> had lots of hardcoded B/D/Fs:
>
> https://github.com/tianocore/edk2/commit/40f2c454343be84ab3bacf9955cc8d7842c70b5c
> .
>
> The context of this question is that I'm trying to make this work in a
> configuration with an IDE Controller located at a different B/D/F, and so
> the hardcoded value of 0/1/1 doesn't work for me.
>
> If I want to change this so that it's not hardcoded, what is a good
> approach? I've added some code locally to the IdeInitSetTiming function in
> SataController.c to set the bit using the PciIo structure which is opened
> specifically on the controller in question -- does that sound like the
> right approach?
> Link to the function I'm referring to:
>
> https://github.com/tianocore/edk2/blob/f6b0258d25a63ae3d3bc6430abe30fb625abc52a/OvmfPkg/SataControllerDxe/SataController.c#L1091-L1099
>
> Thanks,
> Bryan
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
>
>