| Message ID | 20251020165152.666221-3-elder@riscstar.com |
|---|---|
| State | New |
| Headers | show |
| Series | spi: enable the SpacemiT K1 SoC QSPI | expand |
On Mon, Oct 20, 2025 at 11:51:45AM -0500, Alex Elder wrote: > Add the SpacemiT K1 SoC QSPI IP to the list of supported hardware. > > Signed-off-by: Alex Elder <elder@riscstar.com> > --- > Documentation/devicetree/bindings/spi/fsl,spi-fsl-qspi.yaml | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/Documentation/devicetree/bindings/spi/fsl,spi-fsl-qspi.yaml b/Documentation/devicetree/bindings/spi/fsl,spi-fsl-qspi.yaml > index 0315a13fe319a..5bbda4bc33350 100644 > --- a/Documentation/devicetree/bindings/spi/fsl,spi-fsl-qspi.yaml > +++ b/Documentation/devicetree/bindings/spi/fsl,spi-fsl-qspi.yaml > @@ -22,6 +22,7 @@ properties: > - fsl,imx6ul-qspi > - fsl,ls1021a-qspi > - fsl,ls2080a-qspi > + - spacemit,k1-qspi Are the newly added resets mandatory for the spacemit platform? > - items: > - enum: > - fsl,ls1043a-qspi > -- > 2.48.1 >
On Mon, Oct 20, 2025 at 11:51:45AM -0500, Alex Elder wrote: > Add the SpacemiT K1 SoC QSPI IP to the list of supported hardware. Also, you should really explain why this spacemit device is the first one to be in what's been an fsl-specific binding for now in the commit message. pw-bot: changes-requested > > Signed-off-by: Alex Elder <elder@riscstar.com> > --- > Documentation/devicetree/bindings/spi/fsl,spi-fsl-qspi.yaml | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/Documentation/devicetree/bindings/spi/fsl,spi-fsl-qspi.yaml b/Documentation/devicetree/bindings/spi/fsl,spi-fsl-qspi.yaml > index 0315a13fe319a..5bbda4bc33350 100644 > --- a/Documentation/devicetree/bindings/spi/fsl,spi-fsl-qspi.yaml > +++ b/Documentation/devicetree/bindings/spi/fsl,spi-fsl-qspi.yaml > @@ -22,6 +22,7 @@ properties: > - fsl,imx6ul-qspi > - fsl,ls1021a-qspi > - fsl,ls2080a-qspi > + - spacemit,k1-qspi > - items: > - enum: > - fsl,ls1043a-qspi > -- > 2.48.1 >
On 10/20/25 12:39 PM, Conor Dooley wrote: > On Mon, Oct 20, 2025 at 11:51:45AM -0500, Alex Elder wrote: >> Add the SpacemiT K1 SoC QSPI IP to the list of supported hardware. >> >> Signed-off-by: Alex Elder <elder@riscstar.com> >> --- >> Documentation/devicetree/bindings/spi/fsl,spi-fsl-qspi.yaml | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/Documentation/devicetree/bindings/spi/fsl,spi-fsl-qspi.yaml b/Documentation/devicetree/bindings/spi/fsl,spi-fsl-qspi.yaml >> index 0315a13fe319a..5bbda4bc33350 100644 >> --- a/Documentation/devicetree/bindings/spi/fsl,spi-fsl-qspi.yaml >> +++ b/Documentation/devicetree/bindings/spi/fsl,spi-fsl-qspi.yaml >> @@ -22,6 +22,7 @@ properties: >> - fsl,imx6ul-qspi >> - fsl,ls1021a-qspi >> - fsl,ls2080a-qspi >> + - spacemit,k1-qspi > > Are the newly added resets mandatory for the spacemit platform? This is interesting. I never even tried it without specifying them. I just tried it, and at least on my system QSPI functioned without defining these resets. I will ask SpacemiT about this. If they are not needed I will omit the first patch (which added optional resets), and won't use them. Thanks for pointing this out. -Alex > >> - items: >> - enum: >> - fsl,ls1043a-qspi >> -- >> 2.48.1 >>
On 10/20/25 12:41 PM, Conor Dooley wrote: > On Mon, Oct 20, 2025 at 11:51:45AM -0500, Alex Elder wrote: >> Add the SpacemiT K1 SoC QSPI IP to the list of supported hardware. > > Also, you should really explain why this spacemit device is the first > one to be in what's been an fsl-specific binding for now in the commit > message. I'm not sure how much of an explanation you're looking for, but yes, I agree with you, it stands out that it's the first one, so I at least should have acknowledged that. I'll add something here in the next version. -Alex > pw-bot: changes-requested > >> >> Signed-off-by: Alex Elder <elder@riscstar.com> >> --- >> Documentation/devicetree/bindings/spi/fsl,spi-fsl-qspi.yaml | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/Documentation/devicetree/bindings/spi/fsl,spi-fsl-qspi.yaml b/Documentation/devicetree/bindings/spi/fsl,spi-fsl-qspi.yaml >> index 0315a13fe319a..5bbda4bc33350 100644 >> --- a/Documentation/devicetree/bindings/spi/fsl,spi-fsl-qspi.yaml >> +++ b/Documentation/devicetree/bindings/spi/fsl,spi-fsl-qspi.yaml >> @@ -22,6 +22,7 @@ properties: >> - fsl,imx6ul-qspi >> - fsl,ls1021a-qspi >> - fsl,ls2080a-qspi >> + - spacemit,k1-qspi >> - items: >> - enum: >> - fsl,ls1043a-qspi >> -- >> 2.48.1 >>
On Mon, Oct 20, 2025 at 01:06:46PM -0500, Alex Elder wrote: > On 10/20/25 12:39 PM, Conor Dooley wrote: > > > + - spacemit,k1-qspi > > Are the newly added resets mandatory for the spacemit platform? > This is interesting. I never even tried it without specifying them. > I just tried it, and at least on my system QSPI functioned without > defining these resets. I will ask SpacemiT about this. If they are > not needed I will omit the first patch (which added optional resets), > and won't use them. It might be safer to describe them, otherwise things are vulnerable to issues like the bootloader not leaving things in a predictable state.
On 10/20/25 1:26 PM, Mark Brown wrote: > On Mon, Oct 20, 2025 at 01:06:46PM -0500, Alex Elder wrote: >> On 10/20/25 12:39 PM, Conor Dooley wrote: > >>>> + - spacemit,k1-qspi > >>> Are the newly added resets mandatory for the spacemit platform? > >> This is interesting. I never even tried it without specifying them. > >> I just tried it, and at least on my system QSPI functioned without >> defining these resets. I will ask SpacemiT about this. If they are >> not needed I will omit the first patch (which added optional resets), >> and won't use them. > > It might be safer to describe them, otherwise things are vulnerable to > issues like the bootloader not leaving things in a predictable state. I mentioned exactly this in my message to SpacemiT just now. And yes, regardless of their answer, you're probably right. It is *possible* that these resets must be de-asserted, so it's safest to describe them. Conor please if you disagree with this, please say so. Otherwise I think I'll keep them in the next version Thanks. -Alex
On Mon, Oct 20, 2025 at 07:26:17PM +0100, Mark Brown wrote: > On Mon, Oct 20, 2025 at 01:06:46PM -0500, Alex Elder wrote: > > On 10/20/25 12:39 PM, Conor Dooley wrote: > > > > > + - spacemit,k1-qspi > > > > Are the newly added resets mandatory for the spacemit platform? > > > This is interesting. I never even tried it without specifying them. > > > I just tried it, and at least on my system QSPI functioned without > > defining these resets. I will ask SpacemiT about this. If they are > > not needed I will omit the first patch (which added optional resets), > > and won't use them. > > It might be safer to describe them, otherwise things are vulnerable to > issues like the bootloader not leaving things in a predictable state. Yeah, if a linux driver requires that a bootloader set up a clock or de-assert a reset etc, then the binding should mark them required since, as you say, a bootloader change might do away with that de-assertion. Additionally, the stage doing that de-assertion etc could be U-Boot or barebox, which import devicetrees from Linux, so making sure that the resets are present has that benefit too.
On Mon, Oct 20, 2025 at 01:06:50PM -0500, Alex Elder wrote: > On 10/20/25 12:41 PM, Conor Dooley wrote: > > On Mon, Oct 20, 2025 at 11:51:45AM -0500, Alex Elder wrote: > > > Add the SpacemiT K1 SoC QSPI IP to the list of supported hardware. > > > > Also, you should really explain why this spacemit device is the first > > one to be in what's been an fsl-specific binding for now in the commit > > message. > > I'm not sure how much of an explanation you're looking for, but > yes, I agree with you, it stands out that it's the first one, so > I at least should have acknowledged that. I'll add something > here in the next version. Even just mentioning that the register interface etc is nigh identical. Otherwise this just looks like picking a random file to put the compatible in. Remember, this is independent from the driver change and must be justified in its own commit message. > > -Alex > > > pw-bot: changes-requested > > > > > > > > Signed-off-by: Alex Elder <elder@riscstar.com> > > > --- > > > Documentation/devicetree/bindings/spi/fsl,spi-fsl-qspi.yaml | 1 + > > > 1 file changed, 1 insertion(+) > > > > > > diff --git a/Documentation/devicetree/bindings/spi/fsl,spi-fsl-qspi.yaml b/Documentation/devicetree/bindings/spi/fsl,spi-fsl-qspi.yaml > > > index 0315a13fe319a..5bbda4bc33350 100644 > > > --- a/Documentation/devicetree/bindings/spi/fsl,spi-fsl-qspi.yaml > > > +++ b/Documentation/devicetree/bindings/spi/fsl,spi-fsl-qspi.yaml > > > @@ -22,6 +22,7 @@ properties: > > > - fsl,imx6ul-qspi > > > - fsl,ls1021a-qspi > > > - fsl,ls2080a-qspi > > > + - spacemit,k1-qspi > > > - items: > > > - enum: > > > - fsl,ls1043a-qspi > > > -- > > > 2.48.1 > > > >
On 10/20/25 1:39 PM, Conor Dooley wrote: > On Mon, Oct 20, 2025 at 07:26:17PM +0100, Mark Brown wrote: >> On Mon, Oct 20, 2025 at 01:06:46PM -0500, Alex Elder wrote: >>> On 10/20/25 12:39 PM, Conor Dooley wrote: >> >>>>> + - spacemit,k1-qspi >> >>>> Are the newly added resets mandatory for the spacemit platform? >> >>> This is interesting. I never even tried it without specifying them. >> >>> I just tried it, and at least on my system QSPI functioned without >>> defining these resets. I will ask SpacemiT about this. If they are >>> not needed I will omit the first patch (which added optional resets), >>> and won't use them. >> >> It might be safer to describe them, otherwise things are vulnerable to >> issues like the bootloader not leaving things in a predictable state. > > Yeah, if a linux driver requires that a bootloader set up a clock or > de-assert a reset etc, then the binding should mark them required since, > as you say, a bootloader change might do away with that de-assertion. > Additionally, the stage doing that de-assertion etc could be U-Boot > or barebox, which import devicetrees from Linux, so making sure that > the resets are present has that benefit too. OK, so the resets property (added in patch 1) will stay. It will be defined such that it is an optional property, and only when the compatible string includes "spacemit,k1-qspi". -Alex
diff --git a/Documentation/devicetree/bindings/spi/fsl,spi-fsl-qspi.yaml b/Documentation/devicetree/bindings/spi/fsl,spi-fsl-qspi.yaml index 0315a13fe319a..5bbda4bc33350 100644 --- a/Documentation/devicetree/bindings/spi/fsl,spi-fsl-qspi.yaml +++ b/Documentation/devicetree/bindings/spi/fsl,spi-fsl-qspi.yaml @@ -22,6 +22,7 @@ properties: - fsl,imx6ul-qspi - fsl,ls1021a-qspi - fsl,ls2080a-qspi + - spacemit,k1-qspi - items: - enum: - fsl,ls1043a-qspi
Add the SpacemiT K1 SoC QSPI IP to the list of supported hardware. Signed-off-by: Alex Elder <elder@riscstar.com> --- Documentation/devicetree/bindings/spi/fsl,spi-fsl-qspi.yaml | 1 + 1 file changed, 1 insertion(+)