On Wed, Jul 5, 2017 at 10:52 AM, Rob Lippert <rob...@gmail.com> wrote:
> On Wed, Jul 5, 2017 at 10:48 AM, Patrick Venture <vent...@google.com> wrote:
>> On Wed, Jul 5, 2017 at 10:33 AM, Rob Lippert <rob...@gmail.com> wrote:
>>> I checked the datasheets when I wrote this and ast2400 does not have
>>> the (undocumented) HICRB register bits 14,15 that enables the BMC to
>>> actually respond to the snoop'ed address.
>>
>> You're right, it is marked as "reserved" in the datasheet for the ast2400.
>>
>>>
>>> Without setting that bit in the ast2500 the transactions to that I/O
>>> port would timeout on the host side (although the BMC snoop logic
>>> would still see it and log it).
>>> Probably not an issue for x86 systems that don't have any LPC I/O
>>> error handling anyways but LPC timeouts causes issues with POWER
>>> systems since it sets a hardware FIR bit which can cause boot
>>> failures.
>>
>> Interesting. I've been running experiments on x86 and I haven't seen
>> any errors, so that adds more credence to your point. If a device
>> doesn't respond within X time, three times in a row, you get a triple
>> fault. But, on x86, I don't think I've seen any mechanism with an
>> expectation that a port IO write will have a guaranteed response.
>>
>> For the use-case I'm chasing, my goal being to snoop PoST codes from
>> the host, there is in the datasheet a post-code control register set,
>> but I haven't explored configuring them or whether someone has written
>> the fifo driver for them.
>>
>>>
>>> -Rob
>>>
>>> On Tue, Jul 4, 2017 at 8:45 AM, Patrick Venture <vent...@google.com> wrote:
>>>> This driver can be used on the aspeed ast2400.
>>>>
>>>> Tested: ast2400 on quanta-q71l
>>>>
>>>> Signed-off-by: Patrick Venture <vent...@google.com>
>>>> ---
>>>> drivers/misc/aspeed-lpc-snoop.c | 1 +
>>>> 1 file changed, 1 insertion(+)
>>>>
>>>> diff --git a/drivers/misc/aspeed-lpc-snoop.c
>>>> b/drivers/misc/aspeed-lpc-snoop.c
>>>> index 593905565b74..0647cff6280a 100644
>>>> --- a/drivers/misc/aspeed-lpc-snoop.c
>>>> +++ b/drivers/misc/aspeed-lpc-snoop.c
>>>> @@ -241,6 +241,7 @@ static int aspeed_lpc_snoop_remove(struct
>>>> platform_device *pdev)
>>>>
>>>> static const struct of_device_id aspeed_lpc_snoop_match[] = {
>>>> { .compatible = "aspeed,ast2500-lpc-snoop" },
>>>> + { .compatible = "aspeed,ast2400-lpc-snoop" },
>>>> { },
>>
>> An approach would be to ditch this change and instead refer to the
>> ast2500-lpc-snoop in my device-tree to avoid anyone non-x86 from
>> running this configuration and hitting issues.
>
> This change is probably fine since the driver does still work but you
> should also guard the setting of the HICRB bits with #ifdef
> MACH_ASPEED_G5 or similar to avoid setting reserved bits on the G4
> hardware.
Will update patch forthwith.
>
> -Rob
