On 10/26/22 12:01, Sean Anderson wrote:
> 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 <s...@chromium.org> wrote:
>>>>
>>>> Hi,
>>>>
>>>> On Mon, 24 Oct 2022 at 22:57, Stefan Roese <s...@denx.de> wrote:
>>>> >
>>>> > On 24.10.22 18:49, Bin Meng wrote:
>>>> > > On Mon, Oct 24, 2022 at 11:42 PM Sean Anderson
>>>> > > <sean.ander...@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").
ping?
Bin, do you know which of the above scenarios is correct?
--Sean
>> --Sean
>>
>>>> > >> Signed-off-by: Sean Anderson <sean.ander...@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 <s...@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
>
