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

[Sean Anderson]

On 10/26/22 11:58, Sean Anderson wrote:
> 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.

err, it won't break anything, because the existing code doesn't work, but
this patch doesn't address the problem.

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