[PATCH] riscv: dts: fix memory size for the SiFive HiFive Unmatched

[Palmer Dabbelt]

On Fri, 30 Jul 2021 04:54:28 PDT (-0700), heinrich.schuchardt at canonical.com wrote:
> On 04.07.21 14:13, Vincent Pelletier wrote:
>> On Sun, 04 Jul 2021 11:15:55 +0200, Andreas Schwab <schwab at linux-m68k.org> wrote:
>>> On Jul 04 2021, Qiu Wenbo wrote:
>>>
>>>> The production version of HiFive Unmatched have 16GB memory.
>>>>
>>>> Signed-off-by: Qiu Wenbo <qiuwenbo at kylinos.com.cn>
>>>> ---
>>>>   arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts | 2 +-
>>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts b/arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts
>>>> index b1c3c596578f..2e4ea84f27e7 100644
>>>> --- a/arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts
>>>> +++ b/arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts
>>>> @@ -24,7 +24,7 @@ cpus {
>>>>
>>>>   	memory at 80000000 {
>>>>   		device_type = "memory";
>>>> -		reg = <0x0 0x80000000 0x2 0x00000000>;
>>>> +		reg = <0x0 0x80000000 0x4 0x00000000>;
>>>>   	};
>>>>
>>>>   	soc {
>>>
>>> https://github.com/sifive/meta-sifive/blob/2021.06/recipes-kernel/linux/files/0003-riscv-sifive-unmatched-update-for-16GB-rev3.patch
>>> contains more changes.
>>
>> Here is what I learned on this topic while poking at the regulator part
>> of this devicetree:
>>
>> While these extra changes match the board's schematics, they are
>> rejected by the da9063-regulator driver:
>> - some channels are merged on the board, but the devicetree does not
>>    show that: they have different names, with different maximum current.
>>    The updated values exceed the single-channel maximum, so the driver
>>    rejects them.
>> - 3 of the regulators are further configurable in overdrive mode on the
>>    board, allowing higher maximum current, but the driver does not
>>    handle this.
>>    Similarly to previous point, the updated values exceed the
>>    non-overdrive maximum, so the driver rejects them.
>>    I've submitted a tentative fix, but its logic is backwards: it
>>    detects the overdrive bits and increases the maximums, whereas it
>>    should see the higher maximums and as a reaction enable the overdrive
>>    bits
>> - also, some voltages fall in-between possible values, which causes the
>>    driver to reject as well (ex: vdd_bperi is at 1.05V, but the chip can
>>    either do 1.04 or 1.06)
>> - ...and likewise for some current values (ex: vdd_bpro is at 2.5A, but
>>    the chip can only detect 2.4 or 2.6)
>>
>> So from a pure system behaviour perspective, these extra changes should
>> not matter either way (at least in my understanding). At least not with
>> the driver in its current state.
>>
>> OTOH:
>> - the changes missing here are more correct than current master
>> - not including them will probably make meta-sifive maintainer's life a
>>    bit more difficult
>>
>
> We should not mix changes that are unrelated in a single patch. Even if
> regulator changes are advised they should be submitted in a separate patch.
>
> The HiFive Unmatched shows the full available memory with this patch alone.
>
> Tested-by: Heinrich Schuchardt <heinrich.schuchardt at canonical.com>
> Reviewed-by: Heinrich Schuchardt <heinrich.schuchardt at canonical.com>

Agreed.  This is on fixes, if there are more DTS fixes I'm happy to take 
those as well.