[PATCH v3 1/2] x86: fsp: Depend on DM_RTC

[Sean Anderson]

On 10/25/22 20:52, Bin Meng wrote:
> Hi Sean,
> 
> On Wed, Oct 26, 2022 at 7:35 AM Simon Glass <sjg at chromium.org> wrote:
>>
>> Hi,
>>
>> On Mon, 24 Oct 2022 at 22:57, Stefan Roese <sr at denx.de> wrote:
>> >
>> > On 24.10.22 18:49, Bin Meng wrote:
>> > > On Mon, Oct 24, 2022 at 11:42 PM Sean Anderson <sean.anderson at seco.com> wrote:
>> > >>
>> > >> FSP support requires DM_RTC for rtc_write32. Select it.
>> > >>
>> > >> Fixes: ba65808e7d0 ("x86: fsp: Save stack address to CMOS for next S3 boot")
> 
> Please drop this "Fixes" tag.

IMO the existing driver is buggy. I still have not gotten an answer
about where the bug lies, but either

- fsp_save_s3_stack is not called by SPL, in which case it should not be
  compiled in. this indicates a bug in ba65808e7d0 ("x86: fsp: save
  stack address to cmos for next s3 boot").
- fsp_save_s3_stack *is* called by SPL, but it is missing support for
  non-DM RTC. This indicates a bug in ba65808e7d0 ("x86: fsp: save stack
  address to cmos for next s3 boot"). If this is the case, this patch
  will break things.
- fsp_save_s3_stack *is* called by SPL, but it is expected that
  SPL_DM_RTC is selected by the defconfig. This indicates a bug in
  a1d6dc3f840 ("x86: Add chromebook_coral").

--Sean

>> > >> Signed-off-by: Sean Anderson <sean.anderson at seco.com>
>> > >> ---
>> > >> This seems like it would never have worked. Does fsp_save_s3_stack even
>> > >
>> > > This was working before. Did you test it on x86 that now it is broken?
>>
>> I think it is better to select these options rather than rely on
>> boards to do so. I suspect that 'moveconfig.py -s' will remove some
>> things from defconfigs.
>>
>> Reviewed-by: Simon Glass <sjg at chromium.org>
>>
>> > >
>> > > +Stefan
>> >
>> > I don't have access to this FSP x86 target any more, so can't test
>> > anything any more.
>> >
>> > Thanks,
>> > Stefan
>> >
> 
> Regards,
> Bin