Re: [PATCH v2 3/6] riscv: dts: spacemit: k1-musepi-pro: enable QSPI and add SPI NOR

On 12.05.26 4:28 AM, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] Potential overvoltage risk for the W25Q64FW SPI NOR flash via the 'buck4_3v3' supply.
> - [Low] The unit-address in the 'uboot' partition node name does not match the base address in its 'reg' property.
> --
> 
> commit 470f9c74bd3cfec41ed69dd8c4e22535a7e8164f
> Author: Andre Heider <[email protected]>
> 
> riscv: dts: spacemit: k1-musepi-pro: enable QSPI and add SPI NOR
> 
> This commit adds the QSPI controller node and describes the attached
> SPI NOR flash. It also provides a corresponding vendor flash partition
> layout.
> 
>> diff --git a/arch/riscv/boot/dts/spacemit/k1-musepi-pro.dts b/arch/riscv/boot/dts/spacemit/k1-musepi-pro.dts
>> --- a/arch/riscv/boot/dts/spacemit/k1-musepi-pro.dts
>> +++ b/arch/riscv/boot/dts/spacemit/k1-musepi-pro.dts
> 
> [ ... ]
> 
>> @@ -166,7 +166,7 @@ buck3_1v8: buck3 {
>>   				regulator-always-on;
>>   			};
>>   
>> -			buck4 {
>> +			buck4_3v3: buck4 {
>>   				regulator-min-microvolt = <500000>;
>>   				regulator-max-microvolt = <3300000>;
>>   				regulator-ramp-delay = <5000>;
> 
> [ ... ]
> 
>> @@ -249,6 +249,47 @@ dldo7 {
>>   	};
>>   };
>>   
>> +&qspi {
>> +	pinctrl-names = "default";
>> +	pinctrl-0 = <&qspi_cfg>;
>> +	status = "okay";
>> +
>> +	flash@0 {
>> +		compatible = "jedec,spi-nor";
>> +		reg = <0>;
>> +		spi-max-frequency = <26500000>;
>> +		spi-rx-bus-width = <4>;
>> +		spi-tx-bus-width = <4>;
>> +		vcc-supply = <&buck4_3v3>; /* QSPI_VCC1833 */
> 
> The commit message notes this flash is a Winbond W25Q64FWSSAQ, which is a
> strict 1.8V part. Could this vcc-supply assignment cause hardware damage?
> 
> The buck4 regulator has a maximum voltage of 3.3V. If the regulator defaults
> to 3.3V or is configured to 3.3V dynamically, it exceeds the absolute maximum
> ratings for the 1.8V W25Q series flash.

As per the comment, there's a the non controllable regulator QSPI_VCC1833 in between.
k1-milkv-jupiter.dts matches in this regard.
I'll keep it unchanged unless someone has an idea how to express that in a better way.

> 
> Is the board actually using the 3V equivalent (W25Q64JV), or does the
> regulator need stricter constraints if it exclusively powers this flash?
> 
>> +		m25p,fast-read;
>> +
>> +		partitions {
>> +			compatible = "fixed-partitions";
>> +			#address-cells = <1>;
>> +			#size-cells = <1>;
> 
> [ ... ]
> 
>> +			opensbi@70000 {
>> +				reg = <0x70000 0x30000>;
>> +			};
>> +			uboot@a00000 {
>> +				reg = <0xa0000 0x760000>;
> 
> This isn't a regression, but there is a mismatch between the unit-address in
> the node name and the base address in the reg property.
> 
> Should the node be named uboot@a0000 to match the 0xa0000 address and prevent
> a compiler warning?
> 
>> +			};
>> +		};
>> +	};
>> +};
>