Re: Bug in Bonito? (mips/fuloong2e)

[Philippe Mathieu-Daudé]

On 12/29/20 2:02 PM, Jiaxun Yang wrote:
> 在 2020/12/29 18:47, Philippe Mathieu-Daudé 写道:
>> On 12/29/20 6:26 AM, Jiaxun Yang wrote:
>>> 在 2020/12/29 上午11:26, BALATON Zoltan 写道:
>>>> Hello,
>>>>
>>>> While continuing with part two of my vt82c686b clean ups I've tried to
>>>> implement SMBus IO base configuration in the vt82c686b-pm part that
>>>> I've already done for vt8231 for pegasos2 and it should be the same
>>>> for 686B. (In short, writing address to pm config 0x90 sets base
>>>> address of smbus regs and bit 0 of 0xd2 enables/disables it.) This is
>>>> what the firmware does first and it would allow removing hard coded
>>>> 0xeee1 value and the property to set it and then I could reuse the
>>>> same PM part in VT8231.
>>>>
>>> [...]
>>>> Any idea what this is trying to do and how to fix it?
>>> It's trying to translate Bonito style PCI config space r/w to
>>> standard PCI
>>> config space R/W.
>>>
>>> A quick galance told me change BONITO_PCICONF_REG_MASK to 0xff
>>> may help.
>> Per the datasheet section "5.7.5. Accessing PCI configuration space"
>> 0xfc is the correct value, but the register number starts at the 2nd
>> bit. So this is not a write access to register 0xd2 but 0x34?
> 
> It seems like Loongson changed defination of the register?

Maybe, I only have the bonito64 specs, not the Loongson2 ones.
I am a bit confused, I thought the Fuloong 2E was based on
bonito64 (which QEMU models).

Do you know if the Loongson2 specs are public?

> There is no shifting in kernel[1] as well.
> 
> ```
> /* Type 1 configuration for offboard PCI bus */
> addr = (busnum << 16) | (device << 11) | (function << 8) | reg;
> ```

OK, this makes sense after looking at Linux kernel commit e2fee5723bbd
("MIPS: Bonito64: Make Loongson independent from Bonito64 code.") [2]

I'm a bit reluctant to modify hw/pci-host/bonito.c to make Loongson2
works without having the specs handy, justifying simply because
"Linux uses it that way".

OTOH it is pointless to maintain a model that has no users (thinking
about not breaking the bonito64 model).

[2]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=e2fee5723bbd

> 
> Thanks.
> 
> [1]:
> https://elixir.bootlin.com/linux/latest/source/arch/mips/pci/ops-loongson2.c
> 
> 
> - Jiaxun
>>
>> If you can test, this is the snippet I plan to send later:
>>
>> -- >8 --
>> diff --git a/hw/pci-host/bonito.c b/hw/pci-host/bonito.c
>> index a99eced0657..65953766dd0 100644
>> --- a/hw/pci-host/bonito.c
>> +++ b/hw/pci-host/bonito.c
>> @@ -189,3 +189,3 @@ FIELD(BONGENCFG, PCIQUEUE,      12, 1)
>>   #define BONITO_PCICONF_REG_MASK        0xFC
>> -#define BONITO_PCICONF_REG_OFFSET      0
>> +#define BONITO_PCICONF_REG_OFFSET      2
>> ---
>>
>>> Thanks.
>>>
>>> - Jiaxun
>>>
>>>> Regards,
>>>> BALATON Zoltan
>>>
> 
>