Sashiko discussions
 help / color / mirror / Atom feed
* Re: [PATCH v2 3/6] riscv: dts: spacemit: k1-musepi-pro: enable QSPI and add SPI NOR
       [not found] ` <20260512022817.879A3C2BCB0@smtp.kernel.org>
@ 2026-05-12  6:51   ` Andre Heider
  2026-05-12 11:55     ` Rob Herring
  0 siblings, 1 reply; 3+ messages in thread
From: Andre Heider @ 2026-05-12  6:51 UTC (permalink / raw)
  To: sashiko; +Cc: krzk+dt, devicetree, conor+dt, robh

On 12.05.26 4:28 AM, sashiko-bot@kernel.org 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 <a.heider@gmail.com>
> 
> 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?
> 
>> +			};
>> +		};
>> +	};
>> +};
> 


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v2 3/6] riscv: dts: spacemit: k1-musepi-pro: enable QSPI and add SPI NOR
  2026-05-12  6:51   ` [PATCH v2 3/6] riscv: dts: spacemit: k1-musepi-pro: enable QSPI and add SPI NOR Andre Heider
@ 2026-05-12 11:55     ` Rob Herring
  2026-05-12 12:18       ` Andre Heider
  0 siblings, 1 reply; 3+ messages in thread
From: Rob Herring @ 2026-05-12 11:55 UTC (permalink / raw)
  To: Andre Heider; +Cc: sashiko, krzk+dt, devicetree, conor+dt

On Tue, May 12, 2026 at 1:51 AM Andre Heider <a.heider@gmail.com> wrote:
>
> On 12.05.26 4:28 AM, sashiko-bot@kernel.org 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 <a.heider@gmail.com>
> >
> > 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.

That's what we have fixed-regulator binding for. It can take an input supply.

Rob

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v2 3/6] riscv: dts: spacemit: k1-musepi-pro: enable QSPI and add SPI NOR
  2026-05-12 11:55     ` Rob Herring
@ 2026-05-12 12:18       ` Andre Heider
  0 siblings, 0 replies; 3+ messages in thread
From: Andre Heider @ 2026-05-12 12:18 UTC (permalink / raw)
  To: Rob Herring; +Cc: sashiko, krzk+dt, devicetree, conor+dt

On 12.05.26 1:55 PM, Rob Herring wrote:
> On Tue, May 12, 2026 at 1:51 AM Andre Heider <a.heider@gmail.com> wrote:
>>
>> On 12.05.26 4:28 AM, sashiko-bot@kernel.org 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 <a.heider@gmail.com>
>>>
>>> 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.
> 
> That's what we have fixed-regulator binding for. It can take an input supply.

Yeah, I was just trying to avoid the "non controllable" comment with a request to get rid of it ;)
But I guess if I have a consumer that's fine.

Note that dtbs_check passes, AFAICT sashiko is the only instance noticing it.

Thanks,
Andre

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-05-12 12:18 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20260511111116.1109643-4-a.heider@gmail.com>
     [not found] ` <20260512022817.879A3C2BCB0@smtp.kernel.org>
2026-05-12  6:51   ` [PATCH v2 3/6] riscv: dts: spacemit: k1-musepi-pro: enable QSPI and add SPI NOR Andre Heider
2026-05-12 11:55     ` Rob Herring
2026-05-12 12:18       ` Andre Heider

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox