diff mbox series

[edk2,edk2-platforms,1/5] Silicon/SynQuaver/DeviceTree: add node for SPI controller

Message ID 20180215172054.27452-2-ard.biesheuvel@linaro.org
State New
Headers show
Series Add Secure96 mezzanine support | expand

Commit Message

Ard Biesheuvel Feb. 15, 2018, 5:20 p.m. UTC
Add a node for the SPI controller to the device tree so the OS may
attach to it. This is the SPI controller that is attached to the
96boards mezzanine connector on Developer Box.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

---
 Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

-- 
2.11.0

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Comments

Leif Lindholm Feb. 16, 2018, 5 p.m. UTC | #1
On Thu, Feb 15, 2018 at 05:20:50PM +0000, Ard Biesheuvel wrote:
> Add a node for the SPI controller to the device tree so the OS may

> attach to it. This is the SPI controller that is attached to the

> 96boards mezzanine connector on Developer Box.


Just a generic question (which also applies to the subsequent patch):
Are there any implications here with regards to this bus running in
master or slave mode?

/
    Leif

> Contributed-under: TianoCore Contribution Agreement 1.1

> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

> ---

>  Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi | 18 ++++++++++++++++++

>  1 file changed, 18 insertions(+)

> 

> diff --git a/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi b/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi

> index 9085adb326ab..ba445a50f16f 100644

> --- a/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi

> +++ b/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi

> @@ -538,4 +538,22 @@

>          clock-names = "core", "iface";

>          dma-coherent;

>      };

> +

> +    clk_alw_1_8: spi_ihclk {

> +        compatible = "fixed-clock";

> +        #clock-cells = <0>;

> +        clock-frequency = <125000000>;

> +        clock-output-names = "iHCLK";

> +    };

> +

> +    spi: spi@54810000 {

> +        compatible = "socionext,synquacer-spi";

> +        reg = <0x0 0x54810000 0x0 0x1000>;

> +        clocks = <&clk_alw_1_8>;

> +        clock-names = "iHCLK";

> +        socionext,use-rtm;

> +        socionext,set-aces;

> +        #address-cells = <1>;

> +        #size-cells = <0>;

> +    };

>  };

> -- 

> 2.11.0

> 

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Ard Biesheuvel Feb. 16, 2018, 6:34 p.m. UTC | #2
On 16 February 2018 at 17:00, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> On Thu, Feb 15, 2018 at 05:20:50PM +0000, Ard Biesheuvel wrote:

>> Add a node for the SPI controller to the device tree so the OS may

>> attach to it. This is the SPI controller that is attached to the

>> 96boards mezzanine connector on Developer Box.

>

> Just a generic question (which also applies to the subsequent patch):

> Are there any implications here with regards to this bus running in

> master or slave mode?

>


Not really, since that depends entirely on the OS. We just assert the
presence of a certain IP block at a certain memory offset, and whether
the hardware supports slave mode is left unspecified. Whether the OS
supports slave mode (for this particular IP block) is not a property
of the hardware.


>> Contributed-under: TianoCore Contribution Agreement 1.1

>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

>> ---

>>  Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi | 18 ++++++++++++++++++

>>  1 file changed, 18 insertions(+)

>>

>> diff --git a/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi b/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi

>> index 9085adb326ab..ba445a50f16f 100644

>> --- a/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi

>> +++ b/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi

>> @@ -538,4 +538,22 @@

>>          clock-names = "core", "iface";

>>          dma-coherent;

>>      };

>> +

>> +    clk_alw_1_8: spi_ihclk {

>> +        compatible = "fixed-clock";

>> +        #clock-cells = <0>;

>> +        clock-frequency = <125000000>;

>> +        clock-output-names = "iHCLK";

>> +    };

>> +

>> +    spi: spi@54810000 {

>> +        compatible = "socionext,synquacer-spi";

>> +        reg = <0x0 0x54810000 0x0 0x1000>;

>> +        clocks = <&clk_alw_1_8>;

>> +        clock-names = "iHCLK";

>> +        socionext,use-rtm;

>> +        socionext,set-aces;

>> +        #address-cells = <1>;

>> +        #size-cells = <0>;

>> +    };

>>  };

>> --

>> 2.11.0

>>

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Leif Lindholm Feb. 18, 2018, 11:39 a.m. UTC | #3
On Fri, Feb 16, 2018 at 06:34:30PM +0000, Ard Biesheuvel wrote:
> On 16 February 2018 at 17:00, Leif Lindholm <leif.lindholm@linaro.org> wrote:

> > On Thu, Feb 15, 2018 at 05:20:50PM +0000, Ard Biesheuvel wrote:

> >> Add a node for the SPI controller to the device tree so the OS may

> >> attach to it. This is the SPI controller that is attached to the

> >> 96boards mezzanine connector on Developer Box.

> >

> > Just a generic question (which also applies to the subsequent patch):

> > Are there any implications here with regards to this bus running in

> > master or slave mode?

> >

> 

> Not really, since that depends entirely on the OS. We just assert the

> presence of a certain IP block at a certain memory offset, and whether

> the hardware supports slave mode is left unspecified. Whether the OS

> supports slave mode (for this particular IP block) is not a property

> of the hardware.


I was thinking more along the lines of whether the hardware supports
slave mode or not (perhaps as a synthesis option).

But, fair enough.

If you change SynQuaver -> SynQuacer in 1-2 subject lines, for the series:
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>


> >> Contributed-under: TianoCore Contribution Agreement 1.1

> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

> >> ---

> >>  Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi | 18 ++++++++++++++++++

> >>  1 file changed, 18 insertions(+)

> >>

> >> diff --git a/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi b/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi

> >> index 9085adb326ab..ba445a50f16f 100644

> >> --- a/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi

> >> +++ b/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi

> >> @@ -538,4 +538,22 @@

> >>          clock-names = "core", "iface";

> >>          dma-coherent;

> >>      };

> >> +

> >> +    clk_alw_1_8: spi_ihclk {

> >> +        compatible = "fixed-clock";

> >> +        #clock-cells = <0>;

> >> +        clock-frequency = <125000000>;

> >> +        clock-output-names = "iHCLK";

> >> +    };

> >> +

> >> +    spi: spi@54810000 {

> >> +        compatible = "socionext,synquacer-spi";

> >> +        reg = <0x0 0x54810000 0x0 0x1000>;

> >> +        clocks = <&clk_alw_1_8>;

> >> +        clock-names = "iHCLK";

> >> +        socionext,use-rtm;

> >> +        socionext,set-aces;

> >> +        #address-cells = <1>;

> >> +        #size-cells = <0>;

> >> +    };

> >>  };

> >> --

> >> 2.11.0

> >>

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Ard Biesheuvel Feb. 19, 2018, 8:20 a.m. UTC | #4
On 18 February 2018 at 11:39, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> On Fri, Feb 16, 2018 at 06:34:30PM +0000, Ard Biesheuvel wrote:

>> On 16 February 2018 at 17:00, Leif Lindholm <leif.lindholm@linaro.org> wrote:

>> > On Thu, Feb 15, 2018 at 05:20:50PM +0000, Ard Biesheuvel wrote:

>> >> Add a node for the SPI controller to the device tree so the OS may

>> >> attach to it. This is the SPI controller that is attached to the

>> >> 96boards mezzanine connector on Developer Box.

>> >

>> > Just a generic question (which also applies to the subsequent patch):

>> > Are there any implications here with regards to this bus running in

>> > master or slave mode?

>> >

>>

>> Not really, since that depends entirely on the OS. We just assert the

>> presence of a certain IP block at a certain memory offset, and whether

>> the hardware supports slave mode is left unspecified. Whether the OS

>> supports slave mode (for this particular IP block) is not a property

>> of the hardware.

>

> I was thinking more along the lines of whether the hardware supports

> slave mode or not (perhaps as a synthesis option).

>

> But, fair enough.

>

> If you change SynQuaver -> SynQuacer in 1-2 subject lines, for the series:

> Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

>


Excellent, thanks. However, I am going to respin this and make it much
more generic:

- create a separate, generic MezzanineDxe driver (with its own HII menu option)
- redefine all GPIO, I2C and SPI references in terms of the 96boards
spec, e.g., GPIO-A, GPIO-B, GPIO-C

That way, you can basically specify how the LS connector has been
integrated (which I2C/SPI/GPIO), and support anything that the generic
driver supports.

This is only up to a point, of course. Using the Secure96 RNG in UEFI
requires a UEFI driver, and some I2C plumbing, but I am trying to make
that generic as well (which is feasible if the I2C bus on the LS
connector does not contain anything else that UEFI cares about)
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Leif Lindholm Feb. 19, 2018, 11:48 a.m. UTC | #5
On Mon, Feb 19, 2018 at 08:20:28AM +0000, Ard Biesheuvel wrote:
> >> Not really, since that depends entirely on the OS. We just assert the

> >> presence of a certain IP block at a certain memory offset, and whether

> >> the hardware supports slave mode is left unspecified. Whether the OS

> >> supports slave mode (for this particular IP block) is not a property

> >> of the hardware.

> >

> > I was thinking more along the lines of whether the hardware supports

> > slave mode or not (perhaps as a synthesis option).

> >

> > But, fair enough.

> >

> > If you change SynQuaver -> SynQuacer in 1-2 subject lines, for the series:

> > Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

> >

> 

> Excellent, thanks. However, I am going to respin this and make it much

> more generic:

> 

> - create a separate, generic MezzanineDxe driver (with its own HII menu option)

> - redefine all GPIO, I2C and SPI references in terms of the 96boards

> spec, e.g., GPIO-A, GPIO-B, GPIO-C

>

> That way, you can basically specify how the LS connector has been

> integrated (which I2C/SPI/GPIO), and support anything that the generic

> driver supports.


Excellent - that sounds exactly like what I had been hoping for
initially.

> This is only up to a point, of course. Using the Secure96 RNG in UEFI

> requires a UEFI driver, and some I2C plumbing, but I am trying to make

> that generic as well (which is feasible if the I2C bus on the LS

> connector does not contain anything else that UEFI cares about)


I would hope that's generally the case, and otherwise we'd need to
create some protocol that lets you mux?

I.e., even if we do somethign crazy like support stacking mezzanine
boards, an abstracted mezzanine driver should be able to deal with it
- the only issue would come if there were on-board devices on the same
bus?

/
    Leif
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
diff mbox series

Patch

diff --git a/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi b/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi
index 9085adb326ab..ba445a50f16f 100644
--- a/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi
+++ b/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi
@@ -538,4 +538,22 @@ 
         clock-names = "core", "iface";
         dma-coherent;
     };
+
+    clk_alw_1_8: spi_ihclk {
+        compatible = "fixed-clock";
+        #clock-cells = <0>;
+        clock-frequency = <125000000>;
+        clock-output-names = "iHCLK";
+    };
+
+    spi: spi@54810000 {
+        compatible = "socionext,synquacer-spi";
+        reg = <0x0 0x54810000 0x0 0x1000>;
+        clocks = <&clk_alw_1_8>;
+        clock-names = "iHCLK";
+        socionext,use-rtm;
+        socionext,set-aces;
+        #address-cells = <1>;
+        #size-cells = <0>;
+    };
 };