Linux-Aspeed Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] ARM: dts: aspeed: Add Power9 and Power9 CFAM description
From: Joel Stanley @ 2019-04-04  4:43 UTC (permalink / raw)
  To: linux-aspeed

From: Benjamin Herrenschmidt <benh@kernel.crashing.org>

To be used by the OpenPower BMC machines.

This provides proper chip IDs but also adds the various sub-devices
necessary for the future OCC driver among other. All the added nodes
comply with the existing upstream FSI bindings.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Signed-off-by: Joel Stanley <joel@jms.id.au>
---
v2: The first version of this used a bit more magic and was nak'd by
Olof. I've reworked it to not use macros. It still needs to be included
in the parent device tree after the fsi node is created.

 arch/arm/boot/dts/aspeed-bmc-opp-lanyang.dts  |   2 +
 arch/arm/boot/dts/aspeed-bmc-opp-palmetto.dts |  22 ++
 arch/arm/boot/dts/aspeed-bmc-opp-romulus.dts  |   8 +
 .../boot/dts/aspeed-bmc-opp-witherspoon.dts   |   2 +
 arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts    |   2 +
 arch/arm/boot/dts/ibm-power9-dual.dtsi        | 248 ++++++++++++++++++
 6 files changed, 284 insertions(+)
 create mode 100644 arch/arm/boot/dts/ibm-power9-dual.dtsi

diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-lanyang.dts b/arch/arm/boot/dts/aspeed-bmc-opp-lanyang.dts
index 024e52a6cd0f..de95112e2a04 100644
--- a/arch/arm/boot/dts/aspeed-bmc-opp-lanyang.dts
+++ b/arch/arm/boot/dts/aspeed-bmc-opp-lanyang.dts
@@ -322,3 +322,5 @@
 &adc {
 	status = "okay";
 };
+
+#include "ibm-power9-dual.dtsi"
diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-palmetto.dts b/arch/arm/boot/dts/aspeed-bmc-opp-palmetto.dts
index b249da80fb83..b0cb34ccb135 100644
--- a/arch/arm/boot/dts/aspeed-bmc-opp-palmetto.dts
+++ b/arch/arm/boot/dts/aspeed-bmc-opp-palmetto.dts
@@ -347,3 +347,25 @@
 		line-name = "BMC_TPM_INT_N";
 	};
 };
+
+&fsi {
+	cfam at 0,0 {
+		reg = <0 0>;
+		#address-cells = <1>;
+		#size-cells = <1>;
+		chip-id = <0>;
+
+		scom at 1000 {
+			compatible = "ibm,fsi2pib";
+			reg = <0x1000 0x400>;
+		};
+
+		fsi_hub0: hub at 3400 {
+			compatible = "ibm,fsi-master-hub";
+			reg = <0x3400 0x400>;
+			#address-cells = <2>;
+			#size-cells = <0>;
+			no-scan-on-init;
+		};
+	};
+};
diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-romulus.dts b/arch/arm/boot/dts/aspeed-bmc-opp-romulus.dts
index 76fe994f2ba4..5a6bbb3b6640 100644
--- a/arch/arm/boot/dts/aspeed-bmc-opp-romulus.dts
+++ b/arch/arm/boot/dts/aspeed-bmc-opp-romulus.dts
@@ -296,3 +296,11 @@
 &adc {
 	status = "okay";
 };
+
+&gfx {
+     status = "okay";
+     memory-region = <&gfx_memory>;
+};
+
+#include "ibm-power9-dual.dtsi"
+
diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts b/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
index ad54117c075e..a0a4a0c6bc2a 100644
--- a/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
+++ b/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
@@ -592,3 +592,5 @@
 &adc {
 	status = "okay";
 };
+
+#include "ibm-power9-dual.dtsi"
diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts b/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts
index 2c5aa90a546d..05df11cacb21 100644
--- a/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts
+++ b/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts
@@ -435,3 +435,5 @@
 &ibt {
 	status = "okay";
 };
+
+#include "ibm-power9-dual.dtsi"
diff --git a/arch/arm/boot/dts/ibm-power9-dual.dtsi b/arch/arm/boot/dts/ibm-power9-dual.dtsi
new file mode 100644
index 000000000000..2abc42eda7b0
--- /dev/null
+++ b/arch/arm/boot/dts/ibm-power9-dual.dtsi
@@ -0,0 +1,248 @@
+// SPDX-License-Identifier: GPL-2.0+
+// Copyright 2018 IBM Corp
+
+&fsi {
+	cfam at 0,0 {
+		reg = <0 0>;
+		#address-cells = <1>;
+		#size-cells = <1>;
+		chip-id = <0>;
+
+		scom at 1000 {
+			compatible = "ibm,fsi2pib";
+			reg = <0x1000 0x400>;
+		};
+
+		i2c at 1800 {
+			compatible = "ibm,fsi-i2c-master";
+			reg = <0x1800 0x400>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			cfam0_i2c0: i2c-bus at 0 {
+				reg = <0>;
+			};
+
+			cfam0_i2c1: i2c-bus at 1 {
+				reg = <1>;
+			};
+
+			cfam0_i2c2: i2c-bus at 2 {
+				reg = <2>;
+			};
+
+			cfam0_i2c3: i2c-bus at 3 {
+				reg = <3>;
+			};
+
+			cfam0_i2c4: i2c-bus at 4 {
+				reg = <4>;
+			};
+
+			cfam0_i2c5: i2c-bus at 5 {
+				reg = <5>;
+			};
+
+			cfam0_i2c6: i2c-bus at 6 {
+				reg = <6>;
+			};
+
+			cfam0_i2c7: i2c-bus at 7 {
+				reg = <7>;
+			};
+
+			cfam0_i2c8: i2c-bus at 8 {
+				reg = <8>;
+			};
+
+			cfam0_i2c9: i2c-bus at 9 {
+				reg = <9>;
+			};
+
+			cfam0_i2c10: i2c-bus at a {
+				reg = <10>;
+			};
+
+			cfam0_i2c11: i2c-bus at b {
+				reg = <11>;
+			};
+
+			cfam0_i2c12: i2c-bus at c {
+				reg = <12>;
+			};
+
+			cfam0_i2c13: i2c-bus at d {
+				reg = <13>;
+			};
+
+			cfam0_i2c14: i2c-bus at e {
+				reg = <14>;
+			};
+		};
+
+		sbefifo at 2400 {
+			compatible = "ibm,p9-sbefifo";
+			reg = <0x2400 0x400>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			fsi_occ0: occ {
+				compatible = "ibm,p9-occ";
+			};
+		};
+
+		fsi_hub0: hub at 3400 {
+			compatible = "fsi-master-hub";
+			reg = <0x3400 0x400>;
+			#address-cells = <2>;
+			#size-cells = <0>;
+
+			no-scan-on-init;
+		};
+	};
+};
+
+&fsi_hub0 {
+	cfam at 1,0 {
+		reg = <1 0>;
+		#address-cells = <1>;
+		#size-cells = <1>;
+		chip-id = <1>;
+
+		scom at 1000 {
+			compatible = "ibm,fsi2pib";
+			reg = <0x1000 0x400>;
+		};
+
+		i2c at 1800 {
+			compatible = "ibm,fsi-i2c-master";
+			reg = <0x1800 0x400>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			cfam1_i2c0: i2c-bus at 0 {
+				reg = <0>;
+			};
+
+			cfam1_i2c1: i2c-bus at 1 {
+				reg = <1>;
+			};
+
+			cfam1_i2c2: i2c-bus at 2 {
+				reg = <2>;
+			};
+
+			cfam1_i2c3: i2c-bus at 3 {
+				reg = <3>;
+			};
+
+			cfam1_i2c4: i2c-bus at 4 {
+				reg = <4>;
+			};
+
+			cfam1_i2c5: i2c-bus at 5 {
+				reg = <5>;
+			};
+
+			cfam1_i2c6: i2c-bus at 6 {
+				reg = <6>;
+			};
+
+			cfam1_i2c7: i2c-bus at 7 {
+				reg = <7>;
+			};
+
+			cfam1_i2c8: i2c-bus at 8 {
+				reg = <8>;
+			};
+
+			cfam1_i2c9: i2c-bus at 9 {
+				reg = <9>;
+			};
+
+			cfam1_i2c10: i2c-bus at a {
+				reg = <10>;
+			};
+
+			cfam1_i2c11: i2c-bus at b {
+				reg = <11>;
+			};
+
+			cfam1_i2c12: i2c-bus at c {
+				reg = <12>;
+			};
+
+			cfam1_i2c13: i2c-bus at d {
+				reg = <13>;
+			};
+
+			cfam1_i2c14: i2c-bus at e {
+				reg = <14>;
+			};
+		};
+
+		sbefifo at 2400 {
+			compatible = "ibm,p9-sbefifo";
+			reg = <0x2400 0x400>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			fsi_occ1: occ {
+				compatible = "ibm,p9-occ";
+			};
+		};
+
+		fsi_hub1: hub at 3400 {
+			compatible = "fsi-master-hub";
+			reg = <0x3400 0x400>;
+			#address-cells = <2>;
+			#size-cells = <0>;
+
+			no-scan-on-init;
+		};
+	};
+};
+
+/* Legacy OCC numbering (to get rid of when userspace is fixed) */
+&fsi_occ0 {
+	reg = <1>;
+};
+
+&fsi_occ1 {
+	reg = <2>;
+};
+
+/ {
+	aliases {
+		i2c100 = &cfam0_i2c0;
+		i2c101 = &cfam0_i2c1;
+		i2c102 = &cfam0_i2c2;
+		i2c103 = &cfam0_i2c3;
+		i2c104 = &cfam0_i2c4;
+		i2c105 = &cfam0_i2c5;
+		i2c106 = &cfam0_i2c6;
+		i2c107 = &cfam0_i2c7;
+		i2c108 = &cfam0_i2c8;
+		i2c109 = &cfam0_i2c9;
+		i2c110 = &cfam0_i2c10;
+		i2c111 = &cfam0_i2c11;
+		i2c112 = &cfam0_i2c12;
+		i2c113 = &cfam0_i2c13;
+		i2c114 = &cfam0_i2c14;
+		i2c200 = &cfam1_i2c0;
+		i2c201 = &cfam1_i2c1;
+		i2c202 = &cfam1_i2c2;
+		i2c203 = &cfam1_i2c3;
+		i2c204 = &cfam1_i2c4;
+		i2c205 = &cfam1_i2c5;
+		i2c206 = &cfam1_i2c6;
+		i2c207 = &cfam1_i2c7;
+		i2c208 = &cfam1_i2c8;
+		i2c209 = &cfam1_i2c9;
+		i2c210 = &cfam1_i2c10;
+		i2c211 = &cfam1_i2c11;
+		i2c212 = &cfam1_i2c12;
+		i2c213 = &cfam1_i2c13;
+		i2c214 = &cfam1_i2c14;
+	};
+};
-- 
2.20.1


^ permalink raw reply related

* Re: [PATCH 2/5] media: platform: Aspeed: Make reserved memory optional
From: Andrew Jeffery @ 2019-04-04  2:16 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <dd017f44-5d1d-d657-b159-c9dbaadd795f@linux.vnet.ibm.com>



On Thu, 4 Apr 2019, at 01:05, Eddie James wrote:
> 
> On 4/3/19 1:01 AM, Andrew Jeffery wrote:
> >
> > On Wed, 3 Apr 2019, at 04:55, Eddie James wrote:
> >> Reserved memory doesn't need to be required; system memory would work
> >> fine.
> > I had to do a bit of legwork to understand what you were doing here. My
> > understanding is that we allocate out of the default CMA region if the
> > memory-region property isn't specified. Is that what you're expecting?
> > Could be helpful to be a little less terse in the commit message.
> 
> 
> Correct.
> 
> 
> >
> >> Signed-off-by: Eddie James <eajames@linux.ibm.com>
> >> ---
> >>   drivers/media/platform/aspeed-video.c | 6 +-----
> >>   1 file changed, 1 insertion(+), 5 deletions(-)
> >>
> >> diff --git a/drivers/media/platform/aspeed-video.c
> >> b/drivers/media/platform/aspeed-video.c
> >> index 55c55a6..8144fe3 100644
> >> --- a/drivers/media/platform/aspeed-video.c
> >> +++ b/drivers/media/platform/aspeed-video.c
> >> @@ -1608,11 +1608,7 @@ static int aspeed_video_init(struct aspeed_video
> >> *video)
> >>   		return PTR_ERR(video->vclk);
> >>   	}
> >>   
> >> -	rc = of_reserved_mem_device_init(dev);
> >> -	if (rc) {
> >> -		dev_err(dev, "Unable to reserve memory\n");
> >> -		return rc;
> >> -	}
> >> +	of_reserved_mem_device_init(dev);
> > You're ignoring *all* errors here with the expectation that the cause is the
> > missing memory-region property. However, other errors can propagate
> > out of of_reserved_mem_device_init() - e.g. ENOMEM. Rather than remove
> > error checking, I think you should explicitly test for ENODEV, which is what is
> > returned if the memory-region property is absent.
> 
> 
> But it doesn't matter if it fails for any reason, any DMA allocation 
> should fall back to default CMA memory. In the case of ENOMEM or other 
> errors, then the later calls to allocate DMA may fail and we can deal 
> with it then.

Fair enough then. I think it deserves a comment, but up to you.

Andrew

^ permalink raw reply

* [PATCH v3 3/3] MAINTAINERS: Add ASPEED BMC GFX DRM driver entry
From: Andrew Jeffery @ 2019-04-04  1:04 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <20190403001909.31637-4-joel@jms.id.au>

On Wed, 3 Apr 2019, at 10:49, Joel Stanley wrote:
> This hardware is found inside ASPEED Baseboard Management Controller
> (BMC) system on chips. It is called the 'SOC Display Controller' or 'GFX'.
> 
> Signed-off-by: Joel Stanley <joel@jms.id.au>

Acked-by: Andrew Jeffery <andrew@aj.id.au>

> ---
>  MAINTAINERS | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index c18f5f10cf91..c3ad730e26f3 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -4909,6 +4909,14 @@ M:	Dave Airlie <airlied@redhat.com>
>  S:	Odd Fixes
>  F:	drivers/gpu/drm/ast/
>  
> +DRM DRIVER FOR ASPEED BMC GFX
> +M: 	Joel Stanley <joel@jms.id.au>
> +L:	linux-aspeed at lists.ozlabs.org
> +T:	git git://anongit.freedesktop.org/drm/drm-misc
> +S:	Supported
> +F:	drivers/gpu/drm/aspeed/
> +F:	Documentation/devicetree/bindings/gpu/aspeed-gfx.txt
> +
>  DRM DRIVER FOR BOCHS VIRTUAL GPU
>  M:	Gerd Hoffmann <kraxel@redhat.com>
>  L:	virtualization at lists.linux-foundation.org
> -- 
> 2.20.1
> 
>

^ permalink raw reply

* [PATCH v2] ARM: dts: Aspeed: Witherspoon: Update BMC partitioning
From: Andrew Jeffery @ 2019-04-04  0:34 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <599ff977-9e23-d1de-24c9-a5f0f1585495@kaod.org>



On Thu, 4 Apr 2019, at 04:09, C?dric Le Goater wrote:
> On 4/3/19 6:59 PM, Adriana Kobylak wrote:
> > From: "Edward A. James" <eajames@us.ibm.com>
> > 
> > Add simplified partitions for BMC and alternate flash. Include these by
> > default in Witherspoon.
> 
> at last ! :
> 
> Reviewed-by: C?dric Le Goater <clg@kaod.org>

Piling on:

Reviewed-by: Andrew Jeffery <andrew@aj.id.au>

> 
> Thanks,
> 
> C.
> 
> > 
> > Signed-off-by: Edward A. James <eajames@us.ibm.com>
> > Signed-off-by: Joel Stanley <joel@jms.id.au>
> > Signed-off-by: Adriana Kobylak <anoo@us.ibm.com>
> > ---
> >  arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts | 37 ++++++++++++++++++++++--
> >  1 file changed, 35 insertions(+), 2 deletions(-)
> > 
> > diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts b/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
> > index 1cdc96d..82f63aa 100644
> > --- a/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
> > +++ b/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
> > @@ -194,14 +194,47 @@
> >  		label = "bmc";
> >  		m25p,fast-read;
> >  		spi-max-frequency = <50000000>;
> > -#include "openbmc-flash-layout.dtsi"
> > +		partitions {
> > +			#address-cells = < 1 >;
> > +			#size-cells = < 1 >;
> > +			compatible = "fixed-partitions";
> > +			u-boot at 0 {
> > +				reg = < 0 0x60000 >;
> > +				label = "u-boot";
> > +			};
> > +			u-boot-env at 60000 {
> > +				reg = < 0x60000 0x20000 >;
> > +				label = "u-boot-env";
> > +			};
> > +			obmc-ubi at 80000 {
> > +				reg = < 0x80000 0x1F80000 >;
> > +				label = "obmc-ubi";
> > +			};
> > +		};
> >  	};
> >  
> >  	flash at 1 {
> >  		status = "okay";
> > -		label = "alt";
> > +		label = "alt-bmc";
> >  		m25p,fast-read;
> >  		spi-max-frequency = <50000000>;
> > +		partitions {
> > +			#address-cells = < 1 >;
> > +			#size-cells = < 1 >;
> > +			compatible = "fixed-partitions";
> > +			u-boot at 0 {
> > +				reg = < 0 0x60000 >;
> > +				label = "alt-u-boot";
> > +			};
> > +			u-boot-env at 60000 {
> > +				reg = < 0x60000 0x20000 >;
> > +				label = "alt-u-boot-env";
> > +			};
> > +			obmc-ubi at 80000 {
> > +				reg = < 0x80000 0x1F80000 >;
> > +				label = "alt-obmc-ubi";
> > +			};
> > +		};
> >  	};
> >  };
> >  
> > 
> 
>

^ permalink raw reply

* [PATCH] ARM: dts: aspeed: tiogapass: Enable VUART
From: Vijay Khemka @ 2019-04-03 22:32 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <24375A52-CC8E-4CFE-A683-0720B60D1201@fb.com>

Hi Joel,
Please apply this patch to kernel 5.0.

Regards
-Vijay

?On 3/18/19, 12:46 PM, "openbmc on behalf of Vijay Khemka" <openbmc-bounces+vijaykhemka=fb.com at lists.ozlabs.org on behalf of vijaykhemka@fb.com> wrote:

    Hi Joel,
    Please apply this patch.

    Regards
    -Vijay

    On 3/5/19, 12:06 PM, "openbmc on behalf of Vijay Khemka" <openbmc-bounces+vijaykhemka=fb.com at lists.ozlabs.org on behalf of vijaykhemka@fb.com> wrote:

        Please review below patch.

        Regards
        -Vijay

        On 1/30/19, 10:14 AM, "Vijay Khemka" <vijaykhemka@fb.com> wrote:

            Enabling vuart for Facebook tiogapass

            Signed-off-by: Vijay Khemka <vijaykhemka@fb.com>
            ---
             arch/arm/boot/dts/aspeed-bmc-facebook-tiogapass.dts | 5 +++++
             1 file changed, 5 insertions(+)

            diff --git a/arch/arm/boot/dts/aspeed-bmc-facebook-tiogapass.dts b/arch/arm/boot/dts/aspeed-bmc-facebook-tiogapass.dts
            index 42e0d7a8e8d0..a058fb2985f7 100644
            --- a/arch/arm/boot/dts/aspeed-bmc-facebook-tiogapass.dts
            +++ b/arch/arm/boot/dts/aspeed-bmc-facebook-tiogapass.dts
            @@ -64,6 +64,11 @@
                status = "okay";
             };

            +&vuart {
            +   // VUART Host Console
            +   status = "okay";
            +};
            +
             &uart1 {
                // Host Console
                status = "okay";
            --
            2.17.1







-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.ozlabs.org/pipermail/linux-aspeed/attachments/20190403/4f4dbf0a/attachment.htm>

^ permalink raw reply

* [PATCH v2] misc: aspeed-lpc-ctrl: make parameter optional
From: Vijay Khemka @ 2019-04-03 22:29 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <4D1E0DC7-038D-46A5-9626-0D6C68476565@fb.com>

Hi Joel,
Can you please apply this below patch to kernel 5.0.

Regards
-Vijay

?On 3/18/19, 12:46 PM, "openbmc on behalf of Vijay Khemka" <openbmc-bounces+vijaykhemka=fb.com at lists.ozlabs.org on behalf of vijaykhemka@fb.com> wrote:

    Hi Joel,
    Can you please apply this patch as " Documentation/devicetree/bindings/mfd/aspeed-lpc.txt" has already been applied

    Regards
    -Vijay

    On 3/5/19, 4:15 PM, "Linux-aspeed on behalf of Vijay Khemka" <linux-aspeed-bounces+vijaykhemka=fb.com at lists.ozlabs.org on behalf of vijaykhemka@fb.com> wrote:

        Joel,
        Did this patch apply upstream. Somehow I can't find this patch in linux or linux-next or our obmc dev4.19.

        Regards
        -Vijay

        On 1/17/19, 10:53 AM, "Linux-aspeed on behalf of Vijay Khemka" <linux-aspeed-bounces+vijaykhemka=fb.com at lists.ozlabs.org on behalf of vijaykhemka@fb.com> wrote:



            On 1/16/19, 10:17 PM, "Joel Stanley" <joel@jms.id.au> wrote:

                On Thu, 17 Jan 2019 at 09:02, Vijay Khemka <vijaykhemka@fb.com> wrote:
                >
                > Makiing memory-region and flash as optional parameter in device
                > tree if user needs to use these parameter through ioctl then
                > need to define in devicetree.
                >
                > Signed-off-by: Vijay Khemka <vijaykhemka@fb.com>

                Thanks! This looks okay to me. I tested it on one of our systems which
                uses both flash and reserved memory and it was fine.

                Reviewed-by: Joel Stanley <joel@jms.id.au>

                Can you also send a patch to update the bindings at
                Documentation/devicetree/bindings/mfd/aspeed-lpc.txt ? I think the
                only change you need to make is to move the memory region and flash
                properties to optional (instead of required).

            Sure I will do this.

                Cheers,

                Joel

                > ---
                >  drivers/misc/aspeed-lpc-ctrl.c | 58 +++++++++++++++++++++-------------
                >  1 file changed, 36 insertions(+), 22 deletions(-)
                >
                > diff --git a/drivers/misc/aspeed-lpc-ctrl.c b/drivers/misc/aspeed-lpc-ctrl.c
                > index a024f8042259..332210e06e98 100644
                > --- a/drivers/misc/aspeed-lpc-ctrl.c
                > +++ b/drivers/misc/aspeed-lpc-ctrl.c
                > @@ -68,6 +68,7 @@ static long aspeed_lpc_ctrl_ioctl(struct file *file, unsigned int cmd,
                >                 unsigned long param)
                >  {
                >         struct aspeed_lpc_ctrl *lpc_ctrl = file_aspeed_lpc_ctrl(file);
                > +       struct device *dev = file->private_data;
                >         void __user *p = (void __user *)param;
                >         struct aspeed_lpc_ctrl_mapping map;
                >         u32 addr;
                > @@ -90,6 +91,12 @@ static long aspeed_lpc_ctrl_ioctl(struct file *file, unsigned int cmd,
                >                 if (map.window_id != 0)
                >                         return -EINVAL;
                >
                > +               /* If memory-region is not described in device tree */
                > +               if (!lpc_ctrl->mem_size) {
                > +                       dev_err(dev, "Didn't find reserved memory\n");
                > +                       return -EINVAL;
                > +               }
                > +
                >                 map.size = lpc_ctrl->mem_size;
                >
                >                 return copy_to_user(p, &map, sizeof(map)) ? -EFAULT : 0;
                > @@ -126,9 +133,18 @@ static long aspeed_lpc_ctrl_ioctl(struct file *file, unsigned int cmd,
                >                         return -EINVAL;
                >
                >                 if (map.window_type == ASPEED_LPC_CTRL_WINDOW_FLASH) {
                > +                       if (!lpc_ctrl->pnor_size) {
                > +                               dev_err(dev, "Didn't find host pnor flash\n");
                > +                               return -EINVAL;
                > +                       }
                >                         addr = lpc_ctrl->pnor_base;
                >                         size = lpc_ctrl->pnor_size;
                >                 } else if (map.window_type == ASPEED_LPC_CTRL_WINDOW_MEMORY) {
                > +                       /* If memory-region is not described in device tree */
                > +                       if (!lpc_ctrl->mem_size) {
                > +                               dev_err(dev, "Didn't find reserved memory\n");
                > +                               return -EINVAL;
                > +                       }
                >                         addr = lpc_ctrl->mem_base;
                >                         size = lpc_ctrl->mem_size;
                >                 } else {
                > @@ -196,17 +212,17 @@ static int aspeed_lpc_ctrl_probe(struct platform_device *pdev)
                >         if (!lpc_ctrl)
                >                 return -ENOMEM;
                >
                > +       /* If flash is described in device tree then store */
                >         node = of_parse_phandle(dev->of_node, "flash", 0);
                >         if (!node) {
                > -               dev_err(dev, "Didn't find host pnor flash node\n");
                > -               return -ENODEV;
                > -       }
                > -
                > -       rc = of_address_to_resource(node, 1, &resm);
                > -       of_node_put(node);
                > -       if (rc) {
                > -               dev_err(dev, "Couldn't address to resource for flash\n");
                > -               return rc;
                > +               dev_dbg(dev, "Didn't find host pnor flash node\n");
                > +       } else {
                > +               rc = of_address_to_resource(node, 1, &resm);
                > +               of_node_put(node);
                > +               if (rc) {
                > +                       dev_err(dev, "Couldn't address to resource for flash\n");
                > +                       return rc;
                > +               }
                >         }
                >
                >         lpc_ctrl->pnor_size = resource_size(&resm);
                > @@ -214,22 +230,22 @@ static int aspeed_lpc_ctrl_probe(struct platform_device *pdev)
                >
                >         dev_set_drvdata(&pdev->dev, lpc_ctrl);
                >
                > +       /* If memory-region is described in device tree then store */
                >         node = of_parse_phandle(dev->of_node, "memory-region", 0);
                >         if (!node) {
                > -               dev_err(dev, "Didn't find reserved memory\n");
                > -               return -EINVAL;
                > -       }
                > +               dev_dbg(dev, "Didn't find reserved memory\n");
                > +       } else {
                > +               rc = of_address_to_resource(node, 0, &resm);
                > +               of_node_put(node);
                > +               if (rc) {
                > +                       dev_err(dev, "Couldn't address to resource for reserved memory\n");
                > +                       return -ENOMEM;
                > +               }
                >
                > -       rc = of_address_to_resource(node, 0, &resm);
                > -       of_node_put(node);
                > -       if (rc) {
                > -               dev_err(dev, "Couldn't address to resource for reserved memory\n");
                > -               return -ENOMEM;
                > +               lpc_ctrl->mem_size = resource_size(&resm);
                > +               lpc_ctrl->mem_base = resm.start;
                >         }
                >
                > -       lpc_ctrl->mem_size = resource_size(&resm);
                > -       lpc_ctrl->mem_base = resm.start;
                > -
                >         lpc_ctrl->regmap = syscon_node_to_regmap(
                >                         pdev->dev.parent->of_node);
                >         if (IS_ERR(lpc_ctrl->regmap)) {
                > @@ -258,8 +274,6 @@ static int aspeed_lpc_ctrl_probe(struct platform_device *pdev)
                >                 goto err;
                >         }
                >
                > -       dev_info(dev, "Loaded at %pr\n", &resm);
                > -
                >         return 0;
                >
                >  err:
                > --
                > 2.17.1
                >








-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.ozlabs.org/pipermail/linux-aspeed/attachments/20190403/18ac1439/attachment-0001.htm>

^ permalink raw reply

* [PATCH v2] ARM: dts: Aspeed: Witherspoon: Update BMC partitioning
From: Cédric Le Goater @ 2019-04-03 17:02 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <1554310751-52446-1-git-send-email-anoo@linux.ibm.com>

On 4/3/19 6:59 PM, Adriana Kobylak wrote:
> From: "Edward A. James" <eajames@us.ibm.com>
> 
> Add simplified partitions for BMC and alternate flash. Include these by
> default in Witherspoon.

at last ! :

Reviewed-by: C?dric Le Goater <clg@kaod.org>

Thanks,

C.

> 
> Signed-off-by: Edward A. James <eajames@us.ibm.com>
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> Signed-off-by: Adriana Kobylak <anoo@us.ibm.com>
> ---
>  arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts | 37 ++++++++++++++++++++++--
>  1 file changed, 35 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts b/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
> index 1cdc96d..82f63aa 100644
> --- a/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
> +++ b/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
> @@ -194,14 +194,47 @@
>  		label = "bmc";
>  		m25p,fast-read;
>  		spi-max-frequency = <50000000>;
> -#include "openbmc-flash-layout.dtsi"
> +		partitions {
> +			#address-cells = < 1 >;
> +			#size-cells = < 1 >;
> +			compatible = "fixed-partitions";
> +			u-boot at 0 {
> +				reg = < 0 0x60000 >;
> +				label = "u-boot";
> +			};
> +			u-boot-env at 60000 {
> +				reg = < 0x60000 0x20000 >;
> +				label = "u-boot-env";
> +			};
> +			obmc-ubi at 80000 {
> +				reg = < 0x80000 0x1F80000 >;
> +				label = "obmc-ubi";
> +			};
> +		};
>  	};
>  
>  	flash at 1 {
>  		status = "okay";
> -		label = "alt";
> +		label = "alt-bmc";
>  		m25p,fast-read;
>  		spi-max-frequency = <50000000>;
> +		partitions {
> +			#address-cells = < 1 >;
> +			#size-cells = < 1 >;
> +			compatible = "fixed-partitions";
> +			u-boot at 0 {
> +				reg = < 0 0x60000 >;
> +				label = "alt-u-boot";
> +			};
> +			u-boot-env at 60000 {
> +				reg = < 0x60000 0x20000 >;
> +				label = "alt-u-boot-env";
> +			};
> +			obmc-ubi at 80000 {
> +				reg = < 0x80000 0x1F80000 >;
> +				label = "alt-obmc-ubi";
> +			};
> +		};
>  	};
>  };
>  
> 


^ permalink raw reply

* [PATCH v2] ARM: dts: Aspeed: Witherspoon: Update BMC partitioning
From: Adriana Kobylak @ 2019-04-03 16:59 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <962b866599ac4dfd866d7433b7ddb276@linux.vnet.ibm.com>

From: "Edward A. James" <eajames@us.ibm.com>

Add simplified partitions for BMC and alternate flash. Include these by
default in Witherspoon.

Signed-off-by: Edward A. James <eajames@us.ibm.com>
Signed-off-by: Joel Stanley <joel@jms.id.au>
Signed-off-by: Adriana Kobylak <anoo@us.ibm.com>
---
 arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts | 37 ++++++++++++++++++++++--
 1 file changed, 35 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts b/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
index 1cdc96d..82f63aa 100644
--- a/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
+++ b/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
@@ -194,14 +194,47 @@
 		label = "bmc";
 		m25p,fast-read;
 		spi-max-frequency = <50000000>;
-#include "openbmc-flash-layout.dtsi"
+		partitions {
+			#address-cells = < 1 >;
+			#size-cells = < 1 >;
+			compatible = "fixed-partitions";
+			u-boot at 0 {
+				reg = < 0 0x60000 >;
+				label = "u-boot";
+			};
+			u-boot-env at 60000 {
+				reg = < 0x60000 0x20000 >;
+				label = "u-boot-env";
+			};
+			obmc-ubi at 80000 {
+				reg = < 0x80000 0x1F80000 >;
+				label = "obmc-ubi";
+			};
+		};
 	};
 
 	flash at 1 {
 		status = "okay";
-		label = "alt";
+		label = "alt-bmc";
 		m25p,fast-read;
 		spi-max-frequency = <50000000>;
+		partitions {
+			#address-cells = < 1 >;
+			#size-cells = < 1 >;
+			compatible = "fixed-partitions";
+			u-boot at 0 {
+				reg = < 0 0x60000 >;
+				label = "alt-u-boot";
+			};
+			u-boot-env at 60000 {
+				reg = < 0x60000 0x20000 >;
+				label = "alt-u-boot-env";
+			};
+			obmc-ubi at 80000 {
+				reg = < 0x80000 0x1F80000 >;
+				label = "alt-obmc-ubi";
+			};
+		};
 	};
 };
 
-- 
1.8.3.1


^ permalink raw reply related

* [PATCH] ARM: dts: Aspeed: Witherspoon: Update BMC partitioning
From: Adriana Kobylak @ 2019-04-03 15:51 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <cd769312-732d-fb7a-ca50-e5b29eac235b@linux.ibm.com>

On 2019-03-27 10:10, Eddie James wrote:
> On 3/27/19 1:20 AM, Joel Stanley wrote:
>> On Tue, 12 Mar 2019 at 16:50, Adriana Kobylak <anoo@linux.ibm.com> 
>> wrote:
>>> From: "Edward A. James" <eajames@us.ibm.com>
>>> 
>>> Add simplified partitions for BMC and alternate flash. Include these 
>>> by
>>> default in Witherspoon.
>>> 
>>> Signed-off-by: Edward A. James <eajames@us.ibm.com>
>>> Signed-off-by: Joel Stanley <joel@jms.id.au>
>>> ---
>>>   .../boot/dts/aspeed-bmc-alt-opp-flash-layout-ubi.dtsi  | 18 
>>> ++++++++++++++++++
>>>   arch/arm/boot/dts/aspeed-bmc-opp-flash-layout-ubi.dtsi | 18 
>>> ++++++++++++++++++
>>>   arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts       |  3 ++-
>>>   3 files changed, 38 insertions(+), 1 deletion(-)
>>>   create mode 100644 
>>> arch/arm/boot/dts/aspeed-bmc-alt-opp-flash-layout-ubi.dtsi
>>>   create mode 100644 
>>> arch/arm/boot/dts/aspeed-bmc-opp-flash-layout-ubi.dtsi
>>> 
>>> diff --git 
>>> a/arch/arm/boot/dts/aspeed-bmc-alt-opp-flash-layout-ubi.dtsi 
>>> b/arch/arm/boot/dts/aspeed-bmc-alt-opp-flash-layout-ubi.dtsi
>>> new file mode 100644
>>> index 0000000..9277599
>>> --- /dev/null
>>> +++ b/arch/arm/boot/dts/aspeed-bmc-alt-opp-flash-layout-ubi.dtsi
>> As there are no other machines that use this layout, I think you
>> should have the layout in the dts file.
> 
> I think the idea was we might have more machines that use this layout
> in the future...
> 
Yeah that was the idea. Although since Witherspoon is the only machine
using this layout I can move it to its dts file, and the layout can be
moved out if/when another machine needs it. Will re-submit a v2.
>> 
>>> @@ -0,0 +1,18 @@
>>> +               label = "alt-bmc";
>>> +               partitions {
>>> +                               #address-cells = < 1 >;
>>> +                               #size-cells = < 1 >;
>>> +                               compatible = "fixed-partitions";
>>> +                               u-boot at 0 {
>>> +                                       reg = < 0 0x60000 >;
>>> +                                       label = "alt-u-boot";
>>> +                               };
>>> +                               u-boot-env at 60000 {
>>> +                                       reg = < 0x60000 0x20000 >;
>>> +                                       label = "alt-u-boot-env";
>>> +                               };
>>> +                               obmc-ubi at 80000 {
>>> +                                       reg = < 0x80000 0x1F80000 >;
>>> +                                       label = "alt-obmc-ubi";
>>> +                               };
>>> +               };
>>> diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-flash-layout-ubi.dtsi 
>>> b/arch/arm/boot/dts/aspeed-bmc-opp-flash-layout-ubi.dtsi
>>> new file mode 100644
>>> index 0000000..0059ad1
>>> --- /dev/null
>>> +++ b/arch/arm/boot/dts/aspeed-bmc-opp-flash-layout-ubi.dtsi
>>> @@ -0,0 +1,18 @@
>>> +               label = "bmc";
>>> +               partitions {
>>> +                               #address-cells = < 1 >;
>>> +                               #size-cells = < 1 >;
>>> +                               compatible = "fixed-partitions";
>>> +                               u-boot at 0 {
>>> +                                       reg = < 0 0x60000 >;
>>> +                                       label = "u-boot";
>>> +                               };
>>> +                               u-boot-env at 60000 {
>>> +                                       reg = < 0x60000 0x20000 >;
>>> +                                       label = "u-boot-env";
>>> +                               };
>>> +                               obmc-ubi at 80000 {
>>> +                                       reg = < 0x80000 0x1F80000 >;
>>> +                                       label = "obmc-ubi";
>>> +                               };
>>> +               };
>>> diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts 
>>> b/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
>>> index c51e3e8..058b9b7 100644
>>> --- a/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
>>> +++ b/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
>>> @@ -191,7 +191,7 @@
>>>                  label = "bmc";
>>>                  m25p,fast-read;
>>>                  spi-max-frequency = <50000000>;
>>> -#include "openbmc-flash-layout.dtsi"
>>> +#include "aspeed-bmc-opp-flash-layout-ubi.dtsi"
>>>          };
>>> 
>>>          flash at 1 {
>>> @@ -199,6 +199,7 @@
>>>                  label = "alt";
>>>                  m25p,fast-read;
>>>                  spi-max-frequency = <50000000>;
>>> +#include "aspeed-bmc-alt-opp-flash-layout-ubi.dtsi"
>>>          };
>>>   };
>>> 
>>> --
>>> 1.8.3.1
>>> 


^ permalink raw reply

* [PATCH 2/5] media: platform: Aspeed: Make reserved memory optional
From: Eddie James @ 2019-04-03 14:35 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <7effb2de-cc91-47af-88a2-a0075262e9c4@www.fastmail.com>


On 4/3/19 1:01 AM, Andrew Jeffery wrote:
>
> On Wed, 3 Apr 2019, at 04:55, Eddie James wrote:
>> Reserved memory doesn't need to be required; system memory would work
>> fine.
> I had to do a bit of legwork to understand what you were doing here. My
> understanding is that we allocate out of the default CMA region if the
> memory-region property isn't specified. Is that what you're expecting?
> Could be helpful to be a little less terse in the commit message.


Correct.


>
>> Signed-off-by: Eddie James <eajames@linux.ibm.com>
>> ---
>>   drivers/media/platform/aspeed-video.c | 6 +-----
>>   1 file changed, 1 insertion(+), 5 deletions(-)
>>
>> diff --git a/drivers/media/platform/aspeed-video.c
>> b/drivers/media/platform/aspeed-video.c
>> index 55c55a6..8144fe3 100644
>> --- a/drivers/media/platform/aspeed-video.c
>> +++ b/drivers/media/platform/aspeed-video.c
>> @@ -1608,11 +1608,7 @@ static int aspeed_video_init(struct aspeed_video
>> *video)
>>   		return PTR_ERR(video->vclk);
>>   	}
>>   
>> -	rc = of_reserved_mem_device_init(dev);
>> -	if (rc) {
>> -		dev_err(dev, "Unable to reserve memory\n");
>> -		return rc;
>> -	}
>> +	of_reserved_mem_device_init(dev);
> You're ignoring *all* errors here with the expectation that the cause is the
> missing memory-region property. However, other errors can propagate
> out of of_reserved_mem_device_init() - e.g. ENOMEM. Rather than remove
> error checking, I think you should explicitly test for ENODEV, which is what is
> returned if the memory-region property is absent.


But it doesn't matter if it fails for any reason, any DMA allocation 
should fall back to default CMA memory. In the case of ENOMEM or other 
errors, then the later calls to allocate DMA may fail and we can deal 
with it then.


Thanks,

Eddie


>
> Cheers,
>
> Andrew
>
>>   
>>   	rc = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
>>   	if (rc) {
>> -- 
>> 1.8.3.1
>>
>>


^ permalink raw reply

* [PATCH v3 3/3] MAINTAINERS: Add ASPEED BMC GFX DRM driver entry
From: Joel Stanley @ 2019-04-03  7:36 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <CACPK8XdLkG3TR3yLKCJuUJzw+Fonpdi+imKPnQjbZ8B=WEQ47g@mail.gmail.com>

On Wed, 3 Apr 2019 at 07:30, Joel Stanley <joel@jms.id.au> wrote:
>
> On Wed, 3 Apr 2019 at 07:11, Daniel Vetter <daniel@ffwll.ch> wrote:
> >
> > On Wed, Apr 03, 2019 at 10:49:09AM +1030, Joel Stanley wrote:
> > > This hardware is found inside ASPEED Baseboard Management Controller
> > > (BMC) system on chips. It is called the 'SOC Display Controller' or 'GFX'.
> > >
> > > Signed-off-by: Joel Stanley <joel@jms.id.au>
> > > ---
> > >  MAINTAINERS | 8 ++++++++
> > >  1 file changed, 8 insertions(+)
> > >
> > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > index c18f5f10cf91..c3ad730e26f3 100644
> > > --- a/MAINTAINERS
> > > +++ b/MAINTAINERS
> > > @@ -4909,6 +4909,14 @@ M:     Dave Airlie <airlied@redhat.com>
> > >  S:   Odd Fixes
> > >  F:   drivers/gpu/drm/ast/
> > >
> > > +DRM DRIVER FOR ASPEED BMC GFX
> > > +M:   Joel Stanley <joel@jms.id.au>
> > > +L:   linux-aspeed at lists.ozlabs.org
> > > +T:   git git://anongit.freedesktop.org/drm/drm-misc
> >
> > Do you have the fdo account request for drm-misc already?
>
> I don't. How does the process work?

I found it. Here's my request:

 https://gitlab.freedesktop.org/freedesktop/freedesktop/issues/141

^ permalink raw reply

* [PATCH v3 3/3] MAINTAINERS: Add ASPEED BMC GFX DRM driver entry
From: Joel Stanley @ 2019-04-03  7:30 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <20190403071115.GO2665@phenom.ffwll.local>

On Wed, 3 Apr 2019 at 07:11, Daniel Vetter <daniel@ffwll.ch> wrote:
>
> On Wed, Apr 03, 2019 at 10:49:09AM +1030, Joel Stanley wrote:
> > This hardware is found inside ASPEED Baseboard Management Controller
> > (BMC) system on chips. It is called the 'SOC Display Controller' or 'GFX'.
> >
> > Signed-off-by: Joel Stanley <joel@jms.id.au>
> > ---
> >  MAINTAINERS | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index c18f5f10cf91..c3ad730e26f3 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -4909,6 +4909,14 @@ M:     Dave Airlie <airlied@redhat.com>
> >  S:   Odd Fixes
> >  F:   drivers/gpu/drm/ast/
> >
> > +DRM DRIVER FOR ASPEED BMC GFX
> > +M:   Joel Stanley <joel@jms.id.au>
> > +L:   linux-aspeed at lists.ozlabs.org
> > +T:   git git://anongit.freedesktop.org/drm/drm-misc
>
> Do you have the fdo account request for drm-misc already?

I don't. How does the process work?

Cheers,

Joel

^ permalink raw reply

* [PATCH v3 3/3] MAINTAINERS: Add ASPEED BMC GFX DRM driver entry
From: Daniel Vetter @ 2019-04-03  7:11 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <20190403001909.31637-4-joel@jms.id.au>

On Wed, Apr 03, 2019 at 10:49:09AM +1030, Joel Stanley wrote:
> This hardware is found inside ASPEED Baseboard Management Controller
> (BMC) system on chips. It is called the 'SOC Display Controller' or 'GFX'.
> 
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> ---
>  MAINTAINERS | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index c18f5f10cf91..c3ad730e26f3 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -4909,6 +4909,14 @@ M:	Dave Airlie <airlied@redhat.com>
>  S:	Odd Fixes
>  F:	drivers/gpu/drm/ast/
>  
> +DRM DRIVER FOR ASPEED BMC GFX
> +M: 	Joel Stanley <joel@jms.id.au>
> +L:	linux-aspeed at lists.ozlabs.org
> +T:	git git://anongit.freedesktop.org/drm/drm-misc

Do you have the fdo account request for drm-misc already?
-Daniel

> +S:	Supported
> +F:	drivers/gpu/drm/aspeed/
> +F:	Documentation/devicetree/bindings/gpu/aspeed-gfx.txt
> +
>  DRM DRIVER FOR BOCHS VIRTUAL GPU
>  M:	Gerd Hoffmann <kraxel@redhat.com>
>  L:	virtualization at lists.linux-foundation.org
> -- 
> 2.20.1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

^ permalink raw reply

* Re: [PATCH 2/5] media: platform: Aspeed: Make reserved memory optional
From: Andrew Jeffery @ 2019-04-03  6:01 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <1554229504-5661-3-git-send-email-eajames@linux.ibm.com>



On Wed, 3 Apr 2019, at 04:55, Eddie James wrote:
> Reserved memory doesn't need to be required; system memory would work
> fine.

I had to do a bit of legwork to understand what you were doing here. My
understanding is that we allocate out of the default CMA region if the
memory-region property isn't specified. Is that what you're expecting?
Could be helpful to be a little less terse in the commit message.

> 
> Signed-off-by: Eddie James <eajames@linux.ibm.com>
> ---
>  drivers/media/platform/aspeed-video.c | 6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/drivers/media/platform/aspeed-video.c 
> b/drivers/media/platform/aspeed-video.c
> index 55c55a6..8144fe3 100644
> --- a/drivers/media/platform/aspeed-video.c
> +++ b/drivers/media/platform/aspeed-video.c
> @@ -1608,11 +1608,7 @@ static int aspeed_video_init(struct aspeed_video 
> *video)
>  		return PTR_ERR(video->vclk);
>  	}
>  
> -	rc = of_reserved_mem_device_init(dev);
> -	if (rc) {
> -		dev_err(dev, "Unable to reserve memory\n");
> -		return rc;
> -	}
> +	of_reserved_mem_device_init(dev);

You're ignoring *all* errors here with the expectation that the cause is the
missing memory-region property. However, other errors can propagate
out of of_reserved_mem_device_init() - e.g. ENOMEM. Rather than remove
error checking, I think you should explicitly test for ENODEV, which is what is
returned if the memory-region property is absent.

Cheers,

Andrew

>  
>  	rc = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
>  	if (rc) {
> -- 
> 1.8.3.1
> 
>

^ permalink raw reply

* [PATCH v3 3/3] MAINTAINERS: Add ASPEED BMC GFX DRM driver entry
From: Joel Stanley @ 2019-04-03  0:19 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <20190403001909.31637-1-joel@jms.id.au>

This hardware is found inside ASPEED Baseboard Management Controller
(BMC) system on chips. It is called the 'SOC Display Controller' or 'GFX'.

Signed-off-by: Joel Stanley <joel@jms.id.au>
---
 MAINTAINERS | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index c18f5f10cf91..c3ad730e26f3 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4909,6 +4909,14 @@ M:	Dave Airlie <airlied@redhat.com>
 S:	Odd Fixes
 F:	drivers/gpu/drm/ast/
 
+DRM DRIVER FOR ASPEED BMC GFX
+M: 	Joel Stanley <joel@jms.id.au>
+L:	linux-aspeed at lists.ozlabs.org
+T:	git git://anongit.freedesktop.org/drm/drm-misc
+S:	Supported
+F:	drivers/gpu/drm/aspeed/
+F:	Documentation/devicetree/bindings/gpu/aspeed-gfx.txt
+
 DRM DRIVER FOR BOCHS VIRTUAL GPU
 M:	Gerd Hoffmann <kraxel@redhat.com>
 L:	virtualization at lists.linux-foundation.org
-- 
2.20.1


^ permalink raw reply related

* [PATCH v3 2/3] drm: Add ASPEED GFX driver
From: Joel Stanley @ 2019-04-03  0:19 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <20190403001909.31637-1-joel@jms.id.au>

This driver is for the ASPEED BMC SoC's GFX display hardware. This
driver runs on the ARM based BMC systems, unlike the ast driver which
runs on a host CPU and is is for a PCI graphics device.

Signed-off-by: Joel Stanley <joel@jms.id.au>
Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: Noralf Tr?nnes <noralf@tronnes.org>
Reviewed-by: Sam Ravnborg <sam@ravnborg.org>
---
v3:
 Really remove drmP.h
 Remove legacy DRIVER_HAVE_IRQ
v2:
 Use drm_gem_fb_simple_display_pipe_prepare_fb
 Sort headers
 Remove drmP.h inclusion
 Replace gem callbacks with those suggested by Noralf
 Use drm_connector_cleanup
 Drop null check for embedddd crtc object
 Tweaks to spelling in kerneldoc
 Drop DRM_KMS_FB_HELPER to make fbdev emulation optional
 Remove ast2400 compatible as the ast2400 support is untested

Changes since RFC:
 drm_fbdev_cma_init -> drm_fb_cma_fbdev_init and use generic lastclose callback
 Use generic irq handling instead of drm_irq_install
 Add doc to driver
 Get rid of unncessary reads in irq enable/disable path
 Rebase on linux-next

---
 drivers/gpu/drm/Kconfig                  |   2 +
 drivers/gpu/drm/Makefile                 |   1 +
 drivers/gpu/drm/aspeed/Kconfig           |  14 ++
 drivers/gpu/drm/aspeed/Makefile          |   3 +
 drivers/gpu/drm/aspeed/aspeed_gfx.h      | 104 +++++++++
 drivers/gpu/drm/aspeed/aspeed_gfx_crtc.c | 241 ++++++++++++++++++++
 drivers/gpu/drm/aspeed/aspeed_gfx_drv.c  | 269 +++++++++++++++++++++++
 drivers/gpu/drm/aspeed/aspeed_gfx_out.c  |  42 ++++
 8 files changed, 676 insertions(+)
 create mode 100644 drivers/gpu/drm/aspeed/Kconfig
 create mode 100644 drivers/gpu/drm/aspeed/Makefile
 create mode 100644 drivers/gpu/drm/aspeed/aspeed_gfx.h
 create mode 100644 drivers/gpu/drm/aspeed/aspeed_gfx_crtc.c
 create mode 100644 drivers/gpu/drm/aspeed/aspeed_gfx_drv.c
 create mode 100644 drivers/gpu/drm/aspeed/aspeed_gfx_out.c

diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index 82bb221ec94e..b1ec8f85c2a8 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -335,6 +335,8 @@ source "drivers/gpu/drm/xen/Kconfig"
 
 source "drivers/gpu/drm/vboxvideo/Kconfig"
 
+source "drivers/gpu/drm/aspeed/Kconfig"
+
 # Keep legacy drivers last
 
 menuconfig DRM_LEGACY
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 0baf148e3687..df8835045310 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -110,3 +110,4 @@ obj-$(CONFIG_DRM_PL111) += pl111/
 obj-$(CONFIG_DRM_TVE200) += tve200/
 obj-$(CONFIG_DRM_XEN) += xen/
 obj-$(CONFIG_DRM_VBOXVIDEO) += vboxvideo/
+obj-$(CONFIG_DRM_ASPEED_GFX) += aspeed/
diff --git a/drivers/gpu/drm/aspeed/Kconfig b/drivers/gpu/drm/aspeed/Kconfig
new file mode 100644
index 000000000000..42b74d18a41b
--- /dev/null
+++ b/drivers/gpu/drm/aspeed/Kconfig
@@ -0,0 +1,14 @@
+config DRM_ASPEED_GFX
+	tristate "ASPEED BMC Display Controller"
+	depends on DRM && OF
+	select DRM_KMS_HELPER
+	select DRM_KMS_CMA_HELPER
+	select DRM_PANEL
+	select DMA_CMA
+	select CMA
+	select MFD_SYSCON
+	help
+	  Chose this option if you have an ASPEED AST2500 SOC Display
+	  Controller (aka GFX).
+
+	  If M is selected this module will be called aspeed_gfx.
diff --git a/drivers/gpu/drm/aspeed/Makefile b/drivers/gpu/drm/aspeed/Makefile
new file mode 100644
index 000000000000..6e194cd790d8
--- /dev/null
+++ b/drivers/gpu/drm/aspeed/Makefile
@@ -0,0 +1,3 @@
+aspeed_gfx-y := aspeed_gfx_drv.o aspeed_gfx_crtc.o aspeed_gfx_out.o
+
+obj-$(CONFIG_DRM_ASPEED_GFX) += aspeed_gfx.o
diff --git a/drivers/gpu/drm/aspeed/aspeed_gfx.h b/drivers/gpu/drm/aspeed/aspeed_gfx.h
new file mode 100644
index 000000000000..b7a986e49177
--- /dev/null
+++ b/drivers/gpu/drm/aspeed/aspeed_gfx.h
@@ -0,0 +1,104 @@
+// SPDX-License-Identifier: GPL-2.0+
+// Copyright 2018 IBM Corporation
+
+#include <drm/drm_device.h>
+#include <drm/drm_simple_kms_helper.h>
+
+struct aspeed_gfx {
+	void __iomem			*base;
+	struct clk			*clk;
+	struct reset_control		*rst;
+	struct regmap			*scu;
+
+	struct drm_simple_display_pipe	pipe;
+	struct drm_connector		connector;
+	struct drm_fbdev_cma		*fbdev;
+};
+
+int aspeed_gfx_create_pipe(struct drm_device *drm);
+int aspeed_gfx_create_output(struct drm_device *drm);
+
+#define CRT_CTRL1		0x60 /* CRT Control I */
+#define CRT_CTRL2		0x64 /* CRT Control II */
+#define CRT_STATUS		0x68 /* CRT Status */
+#define CRT_MISC		0x6c /* CRT Misc Setting */
+#define CRT_HORIZ0		0x70 /* CRT Horizontal Total & Display Enable End */
+#define CRT_HORIZ1		0x74 /* CRT Horizontal Retrace Start & End */
+#define CRT_VERT0		0x78 /* CRT Vertical Total & Display Enable End */
+#define CRT_VERT1		0x7C /* CRT Vertical Retrace Start & End */
+#define CRT_ADDR		0x80 /* CRT Display Starting Address */
+#define CRT_OFFSET		0x84 /* CRT Display Offset & Terminal Count */
+#define CRT_THROD		0x88 /* CRT Threshold */
+#define CRT_XSCALE		0x8C /* CRT Scaling-Up Factor */
+#define CRT_CURSOR0		0x90 /* CRT Hardware Cursor X & Y Offset */
+#define CRT_CURSOR1		0x94 /* CRT Hardware Cursor X & Y Position */
+#define CRT_CURSOR2		0x98 /* CRT Hardware Cursor Pattern Address */
+#define CRT_9C			0x9C
+#define CRT_OSD_H		0xA0 /* CRT OSD Horizontal Start/End */
+#define CRT_OSD_V		0xA4 /* CRT OSD Vertical Start/End */
+#define CRT_OSD_ADDR		0xA8 /* CRT OSD Pattern Address */
+#define CRT_OSD_DISP		0xAC /* CRT OSD Offset */
+#define CRT_OSD_THRESH		0xB0 /* CRT OSD Threshold & Alpha */
+#define CRT_B4			0xB4
+#define CRT_STS_V		0xB8 /* CRT Status V */
+#define CRT_SCRATCH		0xBC /* Scratchpad */
+#define CRT_BB0_ADDR		0xD0 /* CRT Display BB0 Starting Address */
+#define CRT_BB1_ADDR		0xD4 /* CRT Display BB1 Starting Address */
+#define CRT_BB_COUNT		0xD8 /* CRT Display BB Terminal Count */
+#define OSD_COLOR1		0xE0 /* OSD Color Palette Index 1 & 0 */
+#define OSD_COLOR2		0xE4 /* OSD Color Palette Index 3 & 2 */
+#define OSD_COLOR3		0xE8 /* OSD Color Palette Index 5 & 4 */
+#define OSD_COLOR4		0xEC /* OSD Color Palette Index 7 & 6 */
+#define OSD_COLOR5		0xF0 /* OSD Color Palette Index 9 & 8 */
+#define OSD_COLOR6		0xF4 /* OSD Color Palette Index 11 & 10 */
+#define OSD_COLOR7		0xF8 /* OSD Color Palette Index 13 & 12 */
+#define OSD_COLOR8		0xFC /* OSD Color Palette Index 15 & 14 */
+
+/* CTRL1 */
+#define CRT_CTRL_EN			BIT(0)
+#define CRT_CTRL_HW_CURSOR_EN		BIT(1)
+#define CRT_CTRL_OSD_EN			BIT(2)
+#define CRT_CTRL_INTERLACED		BIT(3)
+#define CRT_CTRL_COLOR_RGB565		(0 << 7)
+#define CRT_CTRL_COLOR_YUV444		(1 << 7)
+#define CRT_CTRL_COLOR_XRGB8888		(2 << 7)
+#define CRT_CTRL_COLOR_RGB888		(3 << 7)
+#define CRT_CTRL_COLOR_YUV444_2RGB	(5 << 7)
+#define CRT_CTRL_COLOR_YUV422		(7 << 7)
+#define CRT_CTRL_COLOR_MASK		GENMASK(9, 7)
+#define CRT_CTRL_HSYNC_NEGATIVE		BIT(16)
+#define CRT_CTRL_VSYNC_NEGATIVE		BIT(17)
+#define CRT_CTRL_VERTICAL_INTR_EN	BIT(30)
+#define CRT_CTRL_VERTICAL_INTR_STS	BIT(31)
+
+/* CTRL2 */
+#define CRT_CTRL_DAC_EN			BIT(0)
+#define CRT_CTRL_VBLANK_LINE(x)		(((x) << 20) & CRT_CTRL_VBLANK_LINE_MASK)
+#define CRT_CTRL_VBLANK_LINE_MASK	GENMASK(20, 31)
+
+/* CRT_HORIZ0 */
+#define CRT_H_TOTAL(x)			(x)
+#define CRT_H_DE(x)			((x) << 16)
+
+/* CRT_HORIZ1 */
+#define CRT_H_RS_START(x)		(x)
+#define CRT_H_RS_END(x)			((x) << 16)
+
+/* CRT_VIRT0 */
+#define CRT_V_TOTAL(x)			(x)
+#define CRT_V_DE(x)			((x) << 16)
+
+/* CRT_VIRT1 */
+#define CRT_V_RS_START(x)		(x)
+#define CRT_V_RS_END(x)			((x) << 16)
+
+/* CRT_OFFSET */
+#define CRT_DISP_OFFSET(x)		(x)
+#define CRT_TERM_COUNT(x)		((x) << 16)
+
+/* CRT_THROD */
+#define CRT_THROD_LOW(x)		(x)
+#define CRT_THROD_HIGH(x)		((x) << 8)
+
+/* Default Threshold Seting */
+#define G5_CRT_THROD_VAL	(CRT_THROD_LOW(0x24) | CRT_THROD_HIGH(0x3C))
diff --git a/drivers/gpu/drm/aspeed/aspeed_gfx_crtc.c b/drivers/gpu/drm/aspeed/aspeed_gfx_crtc.c
new file mode 100644
index 000000000000..15db9e426ec4
--- /dev/null
+++ b/drivers/gpu/drm/aspeed/aspeed_gfx_crtc.c
@@ -0,0 +1,241 @@
+// SPDX-License-Identifier: GPL-2.0+
+// Copyright 2018 IBM Corporation
+
+#include <linux/clk.h>
+#include <linux/reset.h>
+#include <linux/regmap.h>
+
+#include <drm/drm_crtc_helper.h>
+#include <drm/drm_device.h>
+#include <drm/drm_fb_cma_helper.h>
+#include <drm/drm_fourcc.h>
+#include <drm/drm_gem_cma_helper.h>
+#include <drm/drm_gem_framebuffer_helper.h>
+#include <drm/drm_panel.h>
+#include <drm/drm_simple_kms_helper.h>
+#include <drm/drm_vblank.h>
+
+#include "aspeed_gfx.h"
+
+static struct aspeed_gfx *
+drm_pipe_to_aspeed_gfx(struct drm_simple_display_pipe *pipe)
+{
+	return container_of(pipe, struct aspeed_gfx, pipe);
+}
+
+static int aspeed_gfx_set_pixel_fmt(struct aspeed_gfx *priv, u32 *bpp)
+{
+	struct drm_crtc *crtc = &priv->pipe.crtc;
+	struct drm_device *drm = crtc->dev;
+	const u32 format = crtc->primary->state->fb->format->format;
+	u32 ctrl1;
+
+	ctrl1 = readl(priv->base + CRT_CTRL1);
+	ctrl1 &= ~CRT_CTRL_COLOR_MASK;
+
+	switch (format) {
+	case DRM_FORMAT_RGB565:
+		dev_dbg(drm->dev, "Setting up RGB565 mode\n");
+		ctrl1 |= CRT_CTRL_COLOR_RGB565;
+		*bpp = 16;
+		break;
+	case DRM_FORMAT_XRGB8888:
+		dev_dbg(drm->dev, "Setting up XRGB8888 mode\n");
+		ctrl1 |= CRT_CTRL_COLOR_XRGB8888;
+		*bpp = 32;
+		break;
+	default:
+		dev_err(drm->dev, "Unhandled pixel format %08x\n", format);
+		return -EINVAL;
+	}
+
+	writel(ctrl1, priv->base + CRT_CTRL1);
+
+	return 0;
+}
+
+static void aspeed_gfx_enable_controller(struct aspeed_gfx *priv)
+{
+	u32 ctrl1 = readl(priv->base + CRT_CTRL1);
+	u32 ctrl2 = readl(priv->base + CRT_CTRL2);
+
+	/* SCU2C: set DAC source for display output to Graphics CRT (GFX) */
+	regmap_update_bits(priv->scu, 0x2c, BIT(16), BIT(16));
+
+	writel(ctrl1 | CRT_CTRL_EN, priv->base + CRT_CTRL1);
+	writel(ctrl2 | CRT_CTRL_DAC_EN, priv->base + CRT_CTRL2);
+}
+
+static void aspeed_gfx_disable_controller(struct aspeed_gfx *priv)
+{
+	u32 ctrl1 = readl(priv->base + CRT_CTRL1);
+	u32 ctrl2 = readl(priv->base + CRT_CTRL2);
+
+	writel(ctrl1 & ~CRT_CTRL_EN, priv->base + CRT_CTRL1);
+	writel(ctrl2 & ~CRT_CTRL_DAC_EN, priv->base + CRT_CTRL2);
+
+	regmap_update_bits(priv->scu, 0x2c, BIT(16), 0);
+}
+
+static void aspeed_gfx_crtc_mode_set_nofb(struct aspeed_gfx *priv)
+{
+	struct drm_display_mode *m = &priv->pipe.crtc.state->adjusted_mode;
+	u32 ctrl1, d_offset, t_count, bpp;
+	int err;
+
+	err = aspeed_gfx_set_pixel_fmt(priv, &bpp);
+	if (err)
+		return;
+
+#if 0
+	/* TODO: we have only been able to test with the 40MHz USB clock. The
+	 * clock is fixed, so we cannot adjust it here. */
+	clk_set_rate(priv->pixel_clk, m->crtc_clock * 1000);
+#endif
+
+	ctrl1 = readl(priv->base + CRT_CTRL1);
+	ctrl1 &= ~(CRT_CTRL_INTERLACED |
+			CRT_CTRL_HSYNC_NEGATIVE |
+			CRT_CTRL_VSYNC_NEGATIVE);
+
+	if (m->flags & DRM_MODE_FLAG_INTERLACE)
+		ctrl1 |= CRT_CTRL_INTERLACED;
+
+	if (!(m->flags & DRM_MODE_FLAG_PHSYNC))
+		ctrl1 |= CRT_CTRL_HSYNC_NEGATIVE;
+
+	if (!(m->flags & DRM_MODE_FLAG_PVSYNC))
+		ctrl1 |= CRT_CTRL_VSYNC_NEGATIVE;
+
+	writel(ctrl1, priv->base + CRT_CTRL1);
+
+	/* Horizontal timing */
+	writel(CRT_H_TOTAL(m->htotal - 1) | CRT_H_DE(m->hdisplay - 1),
+			priv->base + CRT_HORIZ0);
+	writel(CRT_H_RS_START(m->hsync_start - 1) | CRT_H_RS_END(m->hsync_end),
+			priv->base + CRT_HORIZ1);
+
+
+	/* Vertical timing */
+	writel(CRT_V_TOTAL(m->vtotal - 1) | CRT_V_DE(m->vdisplay - 1),
+			priv->base + CRT_VERT0);
+	writel(CRT_V_RS_START(m->vsync_start) | CRT_V_RS_END(m->vsync_end),
+			priv->base + CRT_VERT1);
+
+	/*
+	 * Display Offset: address difference between consecutive scan lines
+	 * Terminal Count: memory size of one scan line
+	 */
+	d_offset = m->hdisplay * bpp / 8;
+	t_count = (m->hdisplay * bpp + 127) / 128;
+	writel(CRT_DISP_OFFSET(d_offset) | CRT_TERM_COUNT(t_count),
+			priv->base + CRT_OFFSET);
+
+	/*
+	 * Threshold: FIFO thresholds of refill and stop (16 byte chunks
+	 * per line, rounded up)
+	 */
+	writel(G5_CRT_THROD_VAL, priv->base + CRT_THROD);
+}
+
+static void aspeed_gfx_pipe_enable(struct drm_simple_display_pipe *pipe,
+			      struct drm_crtc_state *crtc_state,
+			      struct drm_plane_state *plane_state)
+{
+	struct aspeed_gfx *priv = drm_pipe_to_aspeed_gfx(pipe);
+	struct drm_crtc *crtc = &pipe->crtc;
+
+	aspeed_gfx_crtc_mode_set_nofb(priv);
+	aspeed_gfx_enable_controller(priv);
+	drm_crtc_vblank_on(crtc);
+}
+
+static void aspeed_gfx_pipe_disable(struct drm_simple_display_pipe *pipe)
+{
+	struct aspeed_gfx *priv = drm_pipe_to_aspeed_gfx(pipe);
+	struct drm_crtc *crtc = &pipe->crtc;
+
+	drm_crtc_vblank_off(crtc);
+	aspeed_gfx_disable_controller(priv);
+}
+
+static void aspeed_gfx_pipe_update(struct drm_simple_display_pipe *pipe,
+				   struct drm_plane_state *plane_state)
+{
+	struct aspeed_gfx *priv = drm_pipe_to_aspeed_gfx(pipe);
+	struct drm_crtc *crtc = &pipe->crtc;
+	struct drm_framebuffer *fb = pipe->plane.state->fb;
+	struct drm_pending_vblank_event *event;
+	struct drm_gem_cma_object *gem;
+
+	spin_lock_irq(&crtc->dev->event_lock);
+	event = crtc->state->event;
+	if (event) {
+		crtc->state->event = NULL;
+
+		if (drm_crtc_vblank_get(crtc) == 0)
+			drm_crtc_arm_vblank_event(crtc, event);
+		else
+			drm_crtc_send_vblank_event(crtc, event);
+	}
+	spin_unlock_irq(&crtc->dev->event_lock);
+
+	if (!fb)
+		return;
+
+	gem = drm_fb_cma_get_gem_obj(fb, 0);
+	if (!gem)
+		return;
+	writel(gem->paddr, priv->base + CRT_ADDR);
+}
+
+static int aspeed_gfx_enable_vblank(struct drm_simple_display_pipe *pipe)
+{
+	struct aspeed_gfx *priv = drm_pipe_to_aspeed_gfx(pipe);
+	u32 reg = readl(priv->base + CRT_CTRL1);
+
+	/* Clear pending VBLANK IRQ */
+	writel(reg | CRT_CTRL_VERTICAL_INTR_STS, priv->base + CRT_CTRL1);
+
+	reg |= CRT_CTRL_VERTICAL_INTR_EN;
+	writel(reg, priv->base + CRT_CTRL1);
+
+	return 0;
+}
+
+static void aspeed_gfx_disable_vblank(struct drm_simple_display_pipe *pipe)
+{
+	struct aspeed_gfx *priv = drm_pipe_to_aspeed_gfx(pipe);
+	u32 reg = readl(priv->base + CRT_CTRL1);
+
+	reg &= ~CRT_CTRL_VERTICAL_INTR_EN;
+	writel(reg, priv->base + CRT_CTRL1);
+
+	/* Clear pending VBLANK IRQ */
+	writel(reg | CRT_CTRL_VERTICAL_INTR_STS, priv->base + CRT_CTRL1);
+}
+
+static struct drm_simple_display_pipe_funcs aspeed_gfx_funcs = {
+	.enable		= aspeed_gfx_pipe_enable,
+	.disable	= aspeed_gfx_pipe_disable,
+	.update		= aspeed_gfx_pipe_update,
+	.prepare_fb	= drm_gem_fb_simple_display_pipe_prepare_fb,
+	.enable_vblank	= aspeed_gfx_enable_vblank,
+	.disable_vblank	= aspeed_gfx_disable_vblank,
+};
+
+static const uint32_t aspeed_gfx_formats[] = {
+	DRM_FORMAT_XRGB8888,
+	DRM_FORMAT_RGB565,
+};
+
+int aspeed_gfx_create_pipe(struct drm_device *drm)
+{
+	struct aspeed_gfx *priv = drm->dev_private;
+
+	return drm_simple_display_pipe_init(drm, &priv->pipe, &aspeed_gfx_funcs,
+					    aspeed_gfx_formats,
+					    ARRAY_SIZE(aspeed_gfx_formats),
+					    NULL,
+					    &priv->connector);
+}
diff --git a/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c b/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c
new file mode 100644
index 000000000000..eeb22eccd1fc
--- /dev/null
+++ b/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c
@@ -0,0 +1,269 @@
+// SPDX-License-Identifier: GPL-2.0+
+// Copyright 2018 IBM Corporation
+
+#include <linux/clk.h>
+#include <linux/dma-mapping.h>
+#include <linux/irq.h>
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_reserved_mem.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/reset.h>
+
+#include <drm/drm_atomic_helper.h>
+#include <drm/drm_crtc_helper.h>
+#include <drm/drm_device.h>
+#include <drm/drm_fb_cma_helper.h>
+#include <drm/drm_fb_helper.h>
+#include <drm/drm_gem_cma_helper.h>
+#include <drm/drm_gem_framebuffer_helper.h>
+#include <drm/drm_probe_helper.h>
+#include <drm/drm_simple_kms_helper.h>
+#include <drm/drm_vblank.h>
+#include <drm/drm_drv.h>
+
+#include "aspeed_gfx.h"
+
+/**
+ * DOC: ASPEED GFX Driver
+ *
+ * This driver is for the ASPEED BMC SoC's 'GFX' display hardware, also called
+ * the 'SOC Display Controller' in the datasheet. This driver runs on the ARM
+ * based BMC systems, unlike the ast driver which runs on a host CPU and is for
+ * a PCIe graphics device.
+ *
+ * The AST2500 supports a total of 3 output paths:
+ *
+ *   1. VGA output, the output target can choose either or both to the DAC
+ *   or DVO interface.
+ *
+ *   2. Graphics CRT output, the output target can choose either or both to
+ *   the DAC or DVO interface.
+ *
+ *   3. Video input from DVO, the video input can be used for video engine
+ *   capture or DAC display output.
+ *
+ * Output options are selected in SCU2C.
+ *
+ * The "VGA mode" device is the PCI attached controller. The "Graphics CRT"
+ * is the ARM's internal display controller.
+ *
+ * The driver only supports a simple configuration consisting of a 40MHz
+ * pixel clock, fixed by hardware limitations, and the VGA output path.
+ *
+ * The driver was written with the 'AST2500 Software Programming Guide' v17,
+ * which is available under NDA from ASPEED.
+ */
+
+static const struct drm_mode_config_funcs aspeed_gfx_mode_config_funcs = {
+	.fb_create		= drm_gem_fb_create,
+	.atomic_check		= drm_atomic_helper_check,
+	.atomic_commit		= drm_atomic_helper_commit,
+};
+
+static void aspeed_gfx_setup_mode_config(struct drm_device *drm)
+{
+	drm_mode_config_init(drm);
+
+	drm->mode_config.min_width = 0;
+	drm->mode_config.min_height = 0;
+	drm->mode_config.max_width = 800;
+	drm->mode_config.max_height = 600;
+	drm->mode_config.funcs = &aspeed_gfx_mode_config_funcs;
+}
+
+static irqreturn_t aspeed_gfx_irq_handler(int irq, void *data)
+{
+	struct drm_device *drm = data;
+	struct aspeed_gfx *priv = drm->dev_private;
+	u32 reg;
+
+	reg = readl(priv->base + CRT_CTRL1);
+
+	if (reg & CRT_CTRL_VERTICAL_INTR_STS) {
+		drm_crtc_handle_vblank(&priv->pipe.crtc);
+		writel(reg, priv->base + CRT_CTRL1);
+		return IRQ_HANDLED;
+	}
+
+	return IRQ_NONE;
+}
+
+
+
+static int aspeed_gfx_load(struct drm_device *drm)
+{
+	struct platform_device *pdev = to_platform_device(drm->dev);
+	struct aspeed_gfx *priv;
+	struct resource *res;
+	int ret;
+
+	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+	drm->dev_private = priv;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	priv->base = devm_ioremap_resource(drm->dev, res);
+	if (IS_ERR(priv->base))
+		return PTR_ERR(priv->base);
+
+	priv->scu = syscon_regmap_lookup_by_compatible("aspeed,ast2500-scu");
+	if (IS_ERR(priv->scu)) {
+		dev_err(&pdev->dev, "failed to find SCU regmap\n");
+		return PTR_ERR(priv->scu);
+	}
+
+	ret = of_reserved_mem_device_init(drm->dev);
+	if (ret) {
+		dev_err(&pdev->dev,
+			"failed to initialize reserved mem: %d\n", ret);
+		return ret;
+	}
+
+	ret = dma_set_mask_and_coherent(drm->dev, DMA_BIT_MASK(32));
+	if (ret) {
+		dev_err(&pdev->dev, "failed to set DMA mask: %d\n", ret);
+		return ret;
+	}
+
+	priv->rst = devm_reset_control_get_exclusive(&pdev->dev, NULL);
+	if (IS_ERR(priv->rst)) {
+		dev_err(&pdev->dev,
+			"missing or invalid reset controller device tree entry");
+		return PTR_ERR(priv->rst);
+	}
+	reset_control_deassert(priv->rst);
+
+	priv->clk = devm_clk_get(drm->dev, NULL);
+	if (IS_ERR(priv->clk)) {
+		dev_err(&pdev->dev,
+			"missing or invalid clk device tree entry");
+		return PTR_ERR(priv->clk);
+	}
+	clk_prepare_enable(priv->clk);
+
+	/* Sanitize control registers */
+	writel(0, priv->base + CRT_CTRL1);
+	writel(0, priv->base + CRT_CTRL2);
+
+	aspeed_gfx_setup_mode_config(drm);
+
+	ret = drm_vblank_init(drm, 1);
+	if (ret < 0) {
+		dev_err(drm->dev, "Failed to initialise vblank\n");
+		return ret;
+	}
+
+	ret = aspeed_gfx_create_output(drm);
+	if (ret < 0) {
+		dev_err(drm->dev, "Failed to create outputs\n");
+		return ret;
+	}
+
+	ret = aspeed_gfx_create_pipe(drm);
+	if (ret < 0) {
+		dev_err(drm->dev, "Cannot setup simple display pipe\n");
+		return ret;
+	}
+
+	ret = devm_request_irq(drm->dev, platform_get_irq(pdev, 0),
+			       aspeed_gfx_irq_handler, 0, "aspeed gfx", drm);
+	if (ret < 0) {
+		dev_err(drm->dev, "Failed to install IRQ handler\n");
+		return ret;
+	}
+
+	drm_mode_config_reset(drm);
+
+	drm_fbdev_generic_setup(drm, 32);
+
+	return 0;
+}
+
+static void aspeed_gfx_unload(struct drm_device *drm)
+{
+	drm_kms_helper_poll_fini(drm);
+	drm_mode_config_cleanup(drm);
+
+	drm->dev_private = NULL;
+}
+
+DEFINE_DRM_GEM_CMA_FOPS(fops);
+
+static struct drm_driver aspeed_gfx_driver = {
+	.driver_features        = DRIVER_GEM | DRIVER_MODESET |
+				DRIVER_PRIME | DRIVER_ATOMIC,
+	.gem_create_object	= drm_cma_gem_create_object_default_funcs,
+	.dumb_create		= drm_gem_cma_dumb_create,
+	.prime_handle_to_fd	= drm_gem_prime_handle_to_fd,
+	.prime_fd_to_handle	= drm_gem_prime_fd_to_handle,
+	.gem_prime_import_sg_table = drm_gem_cma_prime_import_sg_table,
+	.gem_prime_mmap		= drm_gem_prime_mmap,
+	.fops = &fops,
+	.name = "aspeed-gfx-drm",
+	.desc = "ASPEED GFX DRM",
+	.date = "20180319",
+	.major = 1,
+	.minor = 0,
+};
+
+static const struct of_device_id aspeed_gfx_match[] = {
+	{ .compatible = "aspeed,ast2500-gfx" },
+	{ }
+};
+
+static int aspeed_gfx_probe(struct platform_device *pdev)
+{
+	struct drm_device *drm;
+	int ret;
+
+	drm = drm_dev_alloc(&aspeed_gfx_driver, &pdev->dev);
+	if (IS_ERR(drm))
+		return PTR_ERR(drm);
+
+	ret = aspeed_gfx_load(drm);
+	if (ret)
+		goto err_free;
+
+	ret = drm_dev_register(drm, 0);
+	if (ret)
+		goto err_unload;
+
+	return 0;
+
+err_unload:
+	aspeed_gfx_unload(drm);
+err_free:
+	drm_dev_put(drm);
+
+	return ret;
+}
+
+static int aspeed_gfx_remove(struct platform_device *pdev)
+{
+	struct drm_device *drm = platform_get_drvdata(pdev);
+
+	drm_dev_unregister(drm);
+	aspeed_gfx_unload(drm);
+	drm_dev_put(drm);
+
+	return 0;
+}
+
+static struct platform_driver aspeed_gfx_platform_driver = {
+	.probe		= aspeed_gfx_probe,
+	.remove		= aspeed_gfx_remove,
+	.driver = {
+		.name = "aspeed_gfx",
+		.of_match_table = aspeed_gfx_match,
+	},
+};
+
+module_platform_driver(aspeed_gfx_platform_driver);
+
+MODULE_AUTHOR("Joel Stanley <joel@jms.id.au>");
+MODULE_DESCRIPTION("ASPEED BMC DRM/KMS driver");
+MODULE_LICENSE("GPL");
diff --git a/drivers/gpu/drm/aspeed/aspeed_gfx_out.c b/drivers/gpu/drm/aspeed/aspeed_gfx_out.c
new file mode 100644
index 000000000000..67ee5fa10055
--- /dev/null
+++ b/drivers/gpu/drm/aspeed/aspeed_gfx_out.c
@@ -0,0 +1,42 @@
+// SPDX-License-Identifier: GPL-2.0+
+// Copyright 2018 IBM Corporation
+
+#include <drm/drm_atomic_helper.h>
+#include <drm/drm_connector.h>
+#include <drm/drm_crtc_helper.h>
+#include <drm/drm_probe_helper.h>
+
+#include "aspeed_gfx.h"
+
+static int aspeed_gfx_get_modes(struct drm_connector *connector)
+{
+	return drm_add_modes_noedid(connector, 800, 600);
+}
+
+static const struct
+drm_connector_helper_funcs aspeed_gfx_connector_helper_funcs = {
+	.get_modes = aspeed_gfx_get_modes,
+};
+
+static const struct drm_connector_funcs aspeed_gfx_connector_funcs = {
+	.fill_modes		= drm_helper_probe_single_connector_modes,
+	.destroy		= drm_connector_cleanup,
+	.reset			= drm_atomic_helper_connector_reset,
+	.atomic_duplicate_state	= drm_atomic_helper_connector_duplicate_state,
+	.atomic_destroy_state	= drm_atomic_helper_connector_destroy_state,
+};
+
+int aspeed_gfx_create_output(struct drm_device *drm)
+{
+	struct aspeed_gfx *priv = drm->dev_private;
+	int ret;
+
+	priv->connector.dpms = DRM_MODE_DPMS_OFF;
+	priv->connector.polled = 0;
+	drm_connector_helper_add(&priv->connector,
+				 &aspeed_gfx_connector_helper_funcs);
+	ret = drm_connector_init(drm, &priv->connector,
+				 &aspeed_gfx_connector_funcs,
+				 DRM_MODE_CONNECTOR_Unknown);
+	return ret;
+}
-- 
2.20.1


^ permalink raw reply related

* [PATCH v3 1/3] dt-bindings: gpu: Add ASPEED GFX bindings document
From: Joel Stanley @ 2019-04-03  0:19 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <20190403001909.31637-1-joel@jms.id.au>

This describes the ASPEED BMC SoC's display controller.

Signed-off-by: Joel Stanley <joel@jms.id.au>
Reviewed-by: Andrew Jeffery <andrew@aj.id.au>
---
v3:
 Add Andrew's reviewed-by

 .../devicetree/bindings/gpu/aspeed-gfx.txt    | 41 +++++++++++++++++++
 1 file changed, 41 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/gpu/aspeed-gfx.txt

diff --git a/Documentation/devicetree/bindings/gpu/aspeed-gfx.txt b/Documentation/devicetree/bindings/gpu/aspeed-gfx.txt
new file mode 100644
index 000000000000..a74033332668
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpu/aspeed-gfx.txt
@@ -0,0 +1,41 @@
+Device tree configuration for the GFX display deivce on the AST2500 SoCs.
+
+Required properties:
+  - compatible
+    * Must be one of the following:
+      + aspeed,ast2500-gfx
+      + aspeed,ast2400-gfx
+    * In addition, the ASPEED pinctrl bindings require the 'syscon' property to
+      be present
+
+  - reg: Physical base address and length of the GFX registers
+
+  - interrupts: interrupt number for the GFX device
+
+  - clocks: clock number used to generate the pixel clock
+
+  - resets: reset line that must be released to use the GFX device
+
+  - memory-region:
+    Phandle to a memory region to allocate from, as defined in
+    Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
+
+
+Example:
+
+gfx: display at 1e6e6000 {
+	compatible = "aspeed,ast2500-gfx", "syscon";
+	reg = <0x1e6e6000 0x1000>;
+	reg-io-width = <4>;
+	clocks = <&syscon ASPEED_CLK_GATE_D1CLK>;
+	resets = <&syscon ASPEED_RESET_CRT1>;
+	interrupts = <0x19>;
+	memory-region = <&gfx_memory>;
+};
+
+gfx_memory: framebuffer {
+	size = <0x01000000>;
+	alignment = <0x01000000>;
+	compatible = "shared-dma-pool";
+	reusable;
+};
-- 
2.20.1


^ permalink raw reply related

* [PATCH v3 0/3] drm: Add ASPEED BMC 'GFX' driver
From: Joel Stanley @ 2019-04-03  0:19 UTC (permalink / raw)
  To: linux-aspeed

v3: Fix up drmP and IRQ flag, thanks Sam
v2: Address review from Noralf and Daniel, add maintainers patch

This driver is for the ASPEED BMC SoC's GFX display hardware. This
driver runs on the ARM based BMC systems, unlike the ast driver which
runs on a host CPU and is is for a PCIe graphics device that happens to
live in the BMC's silicon, but is otherwise available for use by the
BMC.

Joel Stanley (3):
  dt-bindings: gpu: Add ASPEED GFX bindings document
  drm: Add ASPEED GFX driver
  MAINTAINERS: Add ASPEED BMC GFX DRM driver entry

 .../devicetree/bindings/gpu/aspeed-gfx.txt    |  41 +++
 MAINTAINERS                                   |   8 +
 drivers/gpu/drm/Kconfig                       |   2 +
 drivers/gpu/drm/Makefile                      |   1 +
 drivers/gpu/drm/aspeed/Kconfig                |  14 +
 drivers/gpu/drm/aspeed/Makefile               |   3 +
 drivers/gpu/drm/aspeed/aspeed_gfx.h           | 104 +++++++
 drivers/gpu/drm/aspeed/aspeed_gfx_crtc.c      | 241 ++++++++++++++++
 drivers/gpu/drm/aspeed/aspeed_gfx_drv.c       | 269 ++++++++++++++++++
 drivers/gpu/drm/aspeed/aspeed_gfx_out.c       |  42 +++
 10 files changed, 725 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/gpu/aspeed-gfx.txt
 create mode 100644 drivers/gpu/drm/aspeed/Kconfig
 create mode 100644 drivers/gpu/drm/aspeed/Makefile
 create mode 100644 drivers/gpu/drm/aspeed/aspeed_gfx.h
 create mode 100644 drivers/gpu/drm/aspeed/aspeed_gfx_crtc.c
 create mode 100644 drivers/gpu/drm/aspeed/aspeed_gfx_drv.c
 create mode 100644 drivers/gpu/drm/aspeed/aspeed_gfx_out.c

-- 
2.20.1


^ permalink raw reply

* [PATCH v2 2/3] drm: Add ASPEED GFX driver
From: Joel Stanley @ 2019-04-03  0:14 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <20190402062555.GA14723@ravnborg.org>

On Tue, 2 Apr 2019 at 06:26, Sam Ravnborg <sam@ravnborg.org> wrote:
>
> Hi Joel
>
> > index 000000000000..fb56e425bd48
> > --- /dev/null
> > +++ b/drivers/gpu/drm/aspeed/aspeed_gfx.h
> > @@ -0,0 +1,104 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +// Copyright 2018 IBM Corporation
> > +
> > +#include <drm/drmP.h>
> > +#include <drm/drm_simple_kms_helper.h>
>
> A drmP.h include was left here, can we have this removed too.
>
> > +
> > +static struct drm_driver aspeed_gfx_driver = {
> > +     .driver_features        = DRIVER_GEM | DRIVER_MODESET |
> > +                             DRIVER_PRIME | DRIVER_ATOMIC |
> > +                             DRIVER_HAVE_IRQ,
> DRIVER_HAVE_IRQ is obsolete and not needed anymore.
> See drm_drv.h for details.
>
> With these few things fixed:
> Reviewed-by: Sam Ravnborg <sam@ravnborg.org>

Thanks for the review. I've fixed those up now, and this time I'm sure
there's no drmP.h!

Cheers,

Joe

^ permalink raw reply

* [PATCH v8 2/2] drivers/misc: Add Aspeed P2A control driver
From: Patrick Venture @ 2019-04-02 21:25 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <eca7e634-375d-4413-868a-7a00f610d381@www.fastmail.com>

On Mon, Apr 1, 2019 at 8:36 PM Andrew Jeffery <andrew@aj.id.au> wrote:
>
>
>
> On Thu, 28 Mar 2019, at 07:52, Patrick Venture wrote:
> > The ASPEED AST2400, and AST2500 in some configurations include a
> > PCI-to-AHB MMIO bridge.  This bridge allows a server to read and write
> > in the BMC's physical address space.  This feature is especially useful
> > when using this bridge to send large files to the BMC.
> >
> > The host may use this to send down a firmware image by staging data at a
> > specific memory address, and in a coordinated effort with the BMC's
> > software stack and kernel, transmit the bytes.
> >
> > This driver enables the BMC to unlock the PCI bridge on demand, and
> > configure it via ioctl to allow the host to write bytes to an agreed
> > upon location.  In the primary use-case, the region to use is known
> > apriori on the BMC, and the host requests this information.  Once this
> > request is received, the BMC's software stack will enable the bridge and
> > the region and then using some software flow control (possibly via IPMI
> > packets), copy the bytes down.  Once the process is complete, the BMC
> > will disable the bridge and unset any region involved.
> >
> > The default behavior of this bridge when present is: enabled and all
> > regions marked read-write.  This driver will fix the regions to be
> > read-only and then disable the bridge entirely.
> >
> > The memory regions protected are:
> >  * BMC flash MMIO window
> >  * System flash MMIO windows
> >  * SOC IO (peripheral MMIO)
> >  * DRAM
> >
> > The DRAM region itself is all of DRAM and cannot be further specified.
> > Once the PCI bridge is enabled, the host can read all of DRAM, and if
> > the DRAM section is write-enabled, then it can write to all of it.
> >
> > Signed-off-by: Patrick Venture <venture@google.com>
> > ---
> > Changes for v8:
> >  - Promoted u32 address values to u64 to be compatible with either.
> > Changes for v7:
> > - Moved node under the syscon node and changed therefore how it grabs
> > the phandle for the regmap.
> > Changes for v6:
> > - Cleaned up documentation
> > - Added missing machine-readable copyright lines.
> > - Fixed over 80 chars instances.
> > - Changed error from invalid memory-region node to ENODEV.
> > Changes for v5:
> > - Fixup missing exit condition and remove extra jump.
> > Changes for v4:
> > - Added ioctl for reading back the memory-region configuration.
> > - Cleaned up some unused variables.
> > Changes for v3:
> > - Deleted unused read and write methods.
> > Changes for v2:
> > - Dropped unused reads.
> > - Moved call to disable bridge to before registering device.
> > - Switch from using regs to using a syscon regmap. <<< IN PROGRESS
> > - Updated the commit message. <<< TODO
> > - Updated the bit flipped for SCU180_ENP2A
> > - Dropped boolean region_specified variable.
> > - Renamed p2a_ctrl in _probe to misc_ctrl per suggestion.
> > - Renamed aspeed_p2a_region_search to aspeed_p2a_region_acquire
> > - Updated commit message.
> > ---
> >  drivers/misc/Kconfig                 |   8 +
> >  drivers/misc/Makefile                |   1 +
> >  drivers/misc/aspeed-p2a-ctrl.c       | 448 +++++++++++++++++++++++++++
> >  include/uapi/linux/aspeed-p2a-ctrl.h |  62 ++++
> >  4 files changed, 519 insertions(+)
> >  create mode 100644 drivers/misc/aspeed-p2a-ctrl.c
> >  create mode 100644 include/uapi/linux/aspeed-p2a-ctrl.h
> >
> > diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
> > index 42ab8ec92a046..3209ee020b153 100644
> > --- a/drivers/misc/Kconfig
> > +++ b/drivers/misc/Kconfig
> > @@ -496,6 +496,14 @@ config VEXPRESS_SYSCFG
> >         bus. System Configuration interface is one of the possible means
> >         of generating transactions on this bus.
> >
> > +config ASPEED_P2A_CTRL
> > +     depends on (ARCH_ASPEED || COMPILE_TEST) && REGMAP && MFD_SYSCON
> > +     tristate "Aspeed ast2400/2500 HOST P2A VGA MMIO to BMC bridge control"
> > +     help
> > +       Control Aspeed ast2400/2500 HOST P2A VGA MMIO to BMC mappings
> > through
> > +       ioctl()s, the driver also provides an interface for userspace
> > mappings to
> > +       a pre-defined region.
> > +
> >  config ASPEED_LPC_CTRL
> >       depends on (ARCH_ASPEED || COMPILE_TEST) && REGMAP && MFD_SYSCON
> >       tristate "Aspeed ast2400/2500 HOST LPC to BMC bridge control"
> > diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
> > index d5b7d3404dc78..c36239573a5ca 100644
> > --- a/drivers/misc/Makefile
> > +++ b/drivers/misc/Makefile
> > @@ -56,6 +56,7 @@ obj-$(CONFIG_VEXPRESS_SYSCFG)       += vexpress-syscfg.o
> >  obj-$(CONFIG_CXL_BASE)               += cxl/
> >  obj-$(CONFIG_ASPEED_LPC_CTRL)        += aspeed-lpc-ctrl.o
> >  obj-$(CONFIG_ASPEED_LPC_SNOOP)       += aspeed-lpc-snoop.o
> > +obj-$(CONFIG_ASPEED_P2A_CTRL)        += aspeed-p2a-ctrl.o
> >  obj-$(CONFIG_PCI_ENDPOINT_TEST)      += pci_endpoint_test.o
> >  obj-$(CONFIG_OCXL)           += ocxl/
> >  obj-y                                += cardreader/
> > diff --git a/drivers/misc/aspeed-p2a-ctrl.c
> > b/drivers/misc/aspeed-p2a-ctrl.c
> > new file mode 100644
> > index 0000000000000..06afbfe51a279
> > --- /dev/null
> > +++ b/drivers/misc/aspeed-p2a-ctrl.c
> > @@ -0,0 +1,448 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Copyright 2019 Google Inc
> > + *
> > + * This program is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU General Public License
> > + * as published by the Free Software Foundation; either version
> > + * 2 of the License, or (at your option) any later version.
> > + *
> > + * Provides a simple driver to control the ASPEED P2A interface which
> > allows
> > + * the host to read and write to various regions of the BMC's memory.
> > + */
> > +
> > +#include <linux/fs.h>
> > +#include <linux/io.h>
> > +#include <linux/mfd/syscon.h>
> > +#include <linux/miscdevice.h>
> > +#include <linux/mm.h>
> > +#include <linux/module.h>
> > +#include <linux/mutex.h>
> > +#include <linux/of_address.h>
> > +#include <linux/of_device.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/regmap.h>
> > +#include <linux/slab.h>
> > +#include <linux/uaccess.h>
> > +
> > +#include <linux/aspeed-p2a-ctrl.h>
> > +
> > +#define DEVICE_NAME  "aspeed-p2a-ctrl"
> > +
> > +/* SCU2C is a Misc. Control Register. */
> > +#define SCU2C 0x2c
> > +/* SCU180 is the PCIe Configuration Setting Control Register. */
> > +#define SCU180 0x180
> > +/* Bit 1 controls the P2A bridge, while bit 0 controls the entire VGA
> > device
> > + * on the PCI bus.
> > + */
> > +#define SCU180_ENP2A BIT(1)
> > +
> > +/* The ast2400/2500 both have six ranges. */
> > +#define P2A_REGION_COUNT 6
> > +
> > +struct region {
> > +     u64 min;
> > +     u64 max;
> > +     u32 bit;
> > +};
> > +
> > +struct aspeed_p2a_model_data {
> > +     /* min, max, bit */
> > +     struct region regions[P2A_REGION_COUNT];
> > +};
> > +
> > +struct aspeed_p2a_ctrl {
> > +     struct miscdevice miscdev;
> > +     struct regmap *regmap;
> > +
> > +     const struct aspeed_p2a_model_data *config;
> > +
> > +     /* Access to these needs to be locked, held via probe, mapping ioctl,
> > +      * and release, remove.
> > +      */
> > +     struct mutex tracking;
> > +     u32 readers;
> > +     u32 readerwriters[P2A_REGION_COUNT];
> > +
> > +     phys_addr_t mem_base;
> > +     resource_size_t mem_size;
> > +};
> > +
> > +struct aspeed_p2a_user {
> > +     struct file *file;
> > +     struct aspeed_p2a_ctrl *parent;
> > +
> > +     /* The entire memory space is opened for reading once the bridge is
> > +      * enabled, therefore this needs only to be tracked once per user.
> > +      * If any user has it open for read, the bridge must stay enabled.
> > +      */
> > +     u32 read;
> > +
> > +     /* Each entry of the array corresponds to a P2A Region.  If the user
> > +      * opens for read or readwrite, the reference goes up here.  On
> > +      * release, this array is walked and references adjusted accordingly.
> > +      */
> > +     u32 readwrite[P2A_REGION_COUNT];
> > +};
> > +
> > +static void aspeed_p2a_enable_bridge(struct aspeed_p2a_ctrl *p2a_ctrl)
> > +{
> > +     regmap_update_bits(p2a_ctrl->regmap,
> > +             SCU180, SCU180_ENP2A, SCU180_ENP2A);
> > +}
> > +
> > +static void aspeed_p2a_disable_bridge(struct aspeed_p2a_ctrl *p2a_ctrl)
> > +{
> > +     regmap_update_bits(p2a_ctrl->regmap, SCU180, SCU180_ENP2A, 0);
> > +}
> > +
> > +static int aspeed_p2a_mmap(struct file *file, struct vm_area_struct
> > *vma)
> > +{
> > +     struct aspeed_p2a_user *priv = file->private_data;
> > +     struct aspeed_p2a_ctrl *ctrl = priv->parent;
> > +
> > +     if (ctrl->mem_base == 0 && ctrl->mem_size == 0)
> > +             return -EINVAL;
> > +
> > +     unsigned long vsize = vma->vm_end - vma->vm_start;
> > +     pgprot_t prot = vma->vm_page_prot;
> > +
> > +     if (vma->vm_pgoff + vsize > ctrl->mem_base + ctrl->mem_size)
> > +             return -EINVAL;
> > +
> > +     /* ast2400/2500 AHB accesses are not cache coherent */
> > +     prot = pgprot_noncached(prot);
> > +
> > +     if (remap_pfn_range(vma, vma->vm_start,
> > +             (ctrl->mem_base >> PAGE_SHIFT) + vma->vm_pgoff,
> > +             vsize, prot))
> > +             return -EAGAIN;
> > +
> > +     return 0;
> > +}
> > +
> > +static bool aspeed_p2a_region_acquire(struct aspeed_p2a_user *priv,
> > +             struct aspeed_p2a_ctrl *ctrl,
> > +             struct aspeed_p2a_ctrl_mapping *map)
> > +{
> > +     int i;
> > +     u64 base, end;
> > +     bool matched = false;
> > +
> > +     base = map->addr;
> > +     end = map->addr + (map->length - 1);
> > +
> > +     /* If the value is a legal u32, it will find a match. */
> > +     for (i = 0; i < P2A_REGION_COUNT; i++) {
> > +             const struct region *curr = &ctrl->config->regions[i];
> > +
> > +             /* If the top of this region is lower than your base, skip it.
> > +              */
> > +             if (curr->max < base)
> > +                     continue;
> > +
> > +             /* If the bottom of this region is higher than your end, bail.
> > +              */
> > +             if (curr->min > end)
> > +                     break;
> > +
> > +             /* Lock this and update it, therefore it someone else is
> > +              * closing their file out, this'll preserve the increment.
> > +              */
> > +             mutex_lock(&ctrl->tracking);
> > +             ctrl->readerwriters[i] += 1;
> > +             mutex_unlock(&ctrl->tracking);
> > +
> > +             /* Track with the user, so when they close their file, we can
> > +              * decrement properly.
> > +              */
> > +             priv->readwrite[i] += 1;
> > +
> > +             /* Enable the region as read-write. */
> > +             regmap_update_bits(ctrl->regmap, SCU2C, curr->bit, 0);
> > +             matched = true;
> > +     }
> > +
> > +     return matched;
> > +}
> > +
> > +static long aspeed_p2a_ioctl(struct file *file, unsigned int cmd,
> > +             unsigned long data)
> > +{
> > +     struct aspeed_p2a_user *priv = file->private_data;
> > +     struct aspeed_p2a_ctrl *ctrl = priv->parent;
> > +     void __user *arg = (void __user *)data;
> > +     struct aspeed_p2a_ctrl_mapping map;
> > +
> > +     if (copy_from_user(&map, arg, sizeof(map)))
> > +             return -EFAULT;
> > +
> > +     switch (cmd) {
> > +     case ASPEED_P2A_CTRL_IOCTL_SET_WINDOW:
> > +             /* If they want a region to be read-only, since the entire
> > +              * region is read-only once enabled, we just need to track this
> > +              * user wants to read from the bridge, and if it's not enabled.
> > +              * Enable it.
> > +              */
> > +             if (map.flags == ASPEED_P2A_CTRL_READ_ONLY) {
> > +                     mutex_lock(&ctrl->tracking);
> > +                     ctrl->readers += 1;
> > +                     mutex_unlock(&ctrl->tracking);
> > +
> > +                     /* Track with the user, so when they close their file,
> > +                      * we can decrement properly.
> > +                      */
> > +                     priv->read += 1;
> > +             } else if (map.flags == ASPEED_P2A_CTRL_READWRITE) {
> > +                     /* If we don't acquire any region return error. */
> > +                     if (!aspeed_p2a_region_acquire(priv, ctrl, &map)) {
> > +                             return -EINVAL;
> > +                     }
> > +             } else {
> > +                     /* Invalid map flags. */
> > +                     return -EINVAL;
> > +             }
> > +
> > +             aspeed_p2a_enable_bridge(ctrl);
> > +             return 0;
> > +     case ASPEED_P2A_CTRL_IOCTL_GET_MEMORY_CONFIG:
> > +             /* This is a request for the memory-region and corresponding
> > +              * length that is used by the driver for mmap.
> > +              */
> > +
> > +             map.flags = 0;
> > +             map.addr = ctrl->mem_base;
> > +             map.length = ctrl->mem_size;
> > +
> > +             return copy_to_user(arg, &map, sizeof(map)) ? -EFAULT : 0;
> > +     }
> > +
> > +     return -EINVAL;
> > +}
> > +
> > +
> > +/*
> > + * When a user opens this file, we create a structure to track their
> > mappings.
> > + *
> > + * A user can map a region as read-only (bridge enabled), or
> > read-write (bit
> > + * flipped, and bridge enabled).  Either way, this tracking is used,
> > s.t. when
> > + * they release the device references are handled.
> > + *
> > + * The bridge is not enabled until a user calls an ioctl to map a
> > region,
> > + * simply opening the device does not enable it.
> > + */
> > +static int aspeed_p2a_open(struct inode *inode, struct file *file)
> > +{
> > +     struct aspeed_p2a_user *priv;
> > +
> > +     priv = kmalloc(sizeof(*priv), GFP_KERNEL);
> > +     if (!priv)
> > +             return -ENOMEM;
> > +
> > +     priv->file = file;
> > +     priv->read = 0;
> > +     memset(priv->readwrite, 0, sizeof(priv->readwrite));
> > +
> > +     /* The file's private_data is initialized to the p2a_ctrl. */
> > +     priv->parent = file->private_data;
> > +
> > +     /* Set the file's private_data to the user's data. */
> > +     file->private_data = priv;
> > +
> > +     return 0;
> > +}
> > +
> > +/*
> > + * This will close the users mappings.  It will go through what they
> > had opened
> > + * for readwrite, and decrement those counts.  If at the end, this is
> > the last
> > + * user, it'll close the bridge.
> > + */
> > +static int aspeed_p2a_release(struct inode *inode, struct file *file)
> > +{
> > +     int i;
> > +     u32 value;
> > +     u32 bits = 0;
> > +     bool open_regions = false;
> > +     struct aspeed_p2a_user *priv = file->private_data;
> > +
> > +     /* Lock others from changing these values until everything is updated
> > +      * in one pass.
> > +      */
> > +     mutex_lock(&priv->parent->tracking);
> > +
> > +     priv->parent->readers -= priv->read;
> > +
> > +     for (i = 0; i < P2A_REGION_COUNT; i++) {
> > +             priv->parent->readerwriters[i] -= priv->readwrite[i];
> > +
> > +             if (priv->parent->readerwriters[i] > 0)
> > +                     open_regions = true;
> > +             else
> > +                     bits |= priv->parent->config->regions[i].bit;
> > +     }
> > +
> > +     /* Setting a bit to 1 disables the region, so let's just OR with the
> > +      * above to disable any.
> > +      */
> > +
> > +     /* Note, if another user is trying to ioctl, they can't grab tracking,
> > +      * and therefore can't grab either register mutex.
> > +      * If another user is trying to close, they can't grab tracking
> > either.
> > +      */
> > +     regmap_update_bits(priv->parent->regmap, SCU2C, bits, bits);
> > +
> > +     /* If parent->readers is zero and open windows is 0, disable the
> > +      * bridge.
> > +      */
> > +     if (!open_regions && priv->parent->readers == 0)
> > +             aspeed_p2a_disable_bridge(priv->parent);
> > +
> > +     mutex_unlock(&priv->parent->tracking);
> > +
> > +     kfree(priv);
> > +
> > +     return 0;
> > +}
> > +
> > +static const struct file_operations aspeed_p2a_ctrl_fops = {
> > +     .owner = THIS_MODULE,
> > +     .mmap = aspeed_p2a_mmap,
> > +     .unlocked_ioctl = aspeed_p2a_ioctl,
> > +     .open = aspeed_p2a_open,
> > +     .release = aspeed_p2a_release,
> > +};
> > +
> > +/* The regions are controlled by SCU2C */
> > +static void aspeed_p2a_disable_all(struct aspeed_p2a_ctrl *p2a_ctrl)
> > +{
> > +     int i;
> > +     u32 value = 0;
> > +
> > +     for (i = 0; i < P2A_REGION_COUNT; i++)
> > +             value |= p2a_ctrl->config->regions[i].bit;
> > +
> > +     regmap_update_bits(p2a_ctrl->regmap, SCU2C, value, value);
> > +
> > +     /* Disable the bridge. */
> > +     aspeed_p2a_disable_bridge(p2a_ctrl);
> > +}
> > +
> > +static int aspeed_p2a_ctrl_probe(struct platform_device *pdev)
> > +{
> > +     struct aspeed_p2a_ctrl *misc_ctrl;
> > +     struct device *dev;
> > +     struct resource *res, resm;
> > +     struct device_node *node;
> > +     int rc = 0;
> > +
> > +     dev = &pdev->dev;
> > +
> > +     misc_ctrl = devm_kzalloc(dev, sizeof(*misc_ctrl), GFP_KERNEL);
> > +     if (!misc_ctrl)
> > +             return -ENOMEM;
> > +
> > +     mutex_init(&misc_ctrl->tracking);
> > +     misc_ctrl->readers = 0;
> > +     memset(misc_ctrl->readerwriters, 0, sizeof(misc_ctrl->readerwriters));
> > +
> > +     misc_ctrl->mem_size = 0;
> > +     misc_ctrl->mem_base = 0;
>
> This is a performance rather than a correctness nit, so I'm happy for it to be
> cleaned up in a follow-up patch, but if you're going to memset/assign a bunch
> of members to zero why not just use devm_kmalloc() instead? Or keep
> devm_kzalloc() and not do the memset()/assignments of 0 to the members?

I didn't use those because I didn't know about them and didn't dig too
far into allocation variations.  I'll fix this up later this week (I'm
OOO).

>
> > +
> > +     /* optional. */
> > +     node = of_parse_phandle(dev->of_node, "memory-region", 0);
> > +     if (node) {
> > +             rc = of_address_to_resource(node, 0, &resm);
> > +             of_node_put(node);
> > +             if (rc) {
> > +                     dev_err(dev, "Couldn't address to resource for reserved memory\n");
> > +                     return -ENODEV;
> > +             }
> > +
> > +             misc_ctrl->mem_size = resource_size(&resm);
> > +             misc_ctrl->mem_base = resm.start;
> > +     }
> > +
> > +     misc_ctrl->regmap = syscon_node_to_regmap(pdev->dev.parent->of_node);
>
> You're fetching the syscon from the parent node, but your bindings document
> requires the use of a syscon property. The bindings document is out of sync with
> the implementation.
>
> I believe Rob suggested making the node a child of the syscon (which is what
> you've implemented here), so it's the bindings document that should be fixed.
>
> > +     if (IS_ERR(misc_ctrl->regmap)) {
> > +             dev_err(dev, "Couldn't get regmap\n");
> > +             return -ENODEV;
> > +     }
> > +
> > +     misc_ctrl->config = of_device_get_match_data(dev);
> > +
> > +     dev_set_drvdata(&pdev->dev, misc_ctrl);
> > +
> > +     aspeed_p2a_disable_all(misc_ctrl);
> > +
> > +     misc_ctrl->miscdev.minor = MISC_DYNAMIC_MINOR;
> > +     misc_ctrl->miscdev.name = DEVICE_NAME;
> > +     misc_ctrl->miscdev.fops = &aspeed_p2a_ctrl_fops;
> > +     misc_ctrl->miscdev.parent = dev;
> > +
> > +     rc = misc_register(&misc_ctrl->miscdev);
> > +     if (rc)
> > +             dev_err(dev, "Unable to register device\n");
> > +
> > +     return rc;
> > +}
> > +
> > +static int aspeed_p2a_ctrl_remove(struct platform_device *pdev)
> > +{
> > +     struct aspeed_p2a_ctrl *p2a_ctrl = dev_get_drvdata(&pdev->dev);
> > +
> > +     misc_deregister(&p2a_ctrl->miscdev);
> > +
> > +     return 0;
> > +}
> > +
> > +#define SCU2C_DRAM   BIT(25)
> > +#define SCU2C_SPI    BIT(24)
> > +#define SCU2C_SOC    BIT(23)
> > +#define SCU2C_FLASH  BIT(22)
> > +
> > +static const struct aspeed_p2a_model_data ast2400_model_data = {
> > +     .regions = {
> > +             {0x00000000, 0x17FFFFFF, SCU2C_FLASH},
> > +             {0x18000000, 0x1FFFFFFF, SCU2C_SOC},
> > +             {0x20000000, 0x2FFFFFFF, SCU2C_FLASH},
> > +             {0x30000000, 0x3FFFFFFF, SCU2C_SPI},
> > +             {0x40000000, 0x5FFFFFFF, SCU2C_DRAM},
> > +             {0x60000000, 0xFFFFFFFF, SCU2C_SOC},
> > +     }
> > +};
> > +
> > +static const struct aspeed_p2a_model_data ast2500_model_data = {
> > +     .regions = {
> > +             {0x00000000, 0x0FFFFFFF, SCU2C_FLASH},
> > +             {0x10000000, 0x1FFFFFFF, SCU2C_SOC},
> > +             {0x20000000, 0x3FFFFFFF, SCU2C_FLASH},
> > +             {0x40000000, 0x5FFFFFFF, SCU2C_SOC},
> > +             {0x60000000, 0x7FFFFFFF, SCU2C_SPI},
> > +             {0x80000000, 0xFFFFFFFF, SCU2C_DRAM},
> > +     }
> > +};
> > +
> > +static const struct of_device_id aspeed_p2a_ctrl_match[] = {
> > +     { .compatible = "aspeed,ast2400-p2a-ctrl",
> > +       .data = &ast2400_model_data },
> > +     { .compatible = "aspeed,ast2500-p2a-ctrl",
> > +       .data = &ast2500_model_data },
> > +     { },
> > +};
> > +
> > +static struct platform_driver aspeed_p2a_ctrl_driver = {
> > +     .driver = {
> > +             .name           = DEVICE_NAME,
> > +             .of_match_table = aspeed_p2a_ctrl_match,
> > +     },
> > +     .probe = aspeed_p2a_ctrl_probe,
> > +     .remove = aspeed_p2a_ctrl_remove,
> > +};
> > +
> > +module_platform_driver(aspeed_p2a_ctrl_driver);
> > +
> > +MODULE_DEVICE_TABLE(of, aspeed_p2a_ctrl_match);
> > +MODULE_LICENSE("GPL");
> > +MODULE_AUTHOR("Patrick Venture <venture@google.com>");
> > +MODULE_DESCRIPTION("Control for aspeed 2400/2500 P2A VGA HOST to BMC
> > mappings");
>
> Reviewed-by: Andrew Jeffery <andrew@aj.id.au>
>
> > diff --git a/include/uapi/linux/aspeed-p2a-ctrl.h
> > b/include/uapi/linux/aspeed-p2a-ctrl.h
> > new file mode 100644
> > index 0000000000000..033355552a6e3
> > --- /dev/null
> > +++ b/include/uapi/linux/aspeed-p2a-ctrl.h
> > @@ -0,0 +1,62 @@
> > +/* SPDX-License-Identifier: GPL-2.0+ WITH Linux-syscall-note */
> > +/*
> > + * Copyright 2019 Google Inc
> > + *
> > + * This program is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU General Public License
> > + * as published by the Free Software Foundation; either version
> > + * 2 of the License, or (at your option) any later version.
> > + *
> > + * Provides a simple driver to control the ASPEED P2A interface which
> > allows
> > + * the host to read and write to various regions of the BMC's memory.
> > + */
> > +
> > +#ifndef _UAPI_LINUX_ASPEED_P2A_CTRL_H
> > +#define _UAPI_LINUX_ASPEED_P2A_CTRL_H
> > +
> > +#include <linux/ioctl.h>
> > +#include <linux/types.h>
> > +
> > +#define ASPEED_P2A_CTRL_READ_ONLY 0
> > +#define ASPEED_P2A_CTRL_READWRITE 1
> > +
> > +/*
> > + * This driver provides a mechanism for enabling or disabling the
> > read-write
> > + * property of specific windows into the ASPEED BMC's memory.
> > + *
> > + * A user can map a region of the BMC's memory as read-only or
> > read-write, with
> > + * the caveat that once any region is mapped, all regions are unlocked
> > for
> > + * reading.
> > + */
> > +
> > +/*
> > + * Unlock a region of BMC physical memory for access from the host.
> > + *
> > + * Also used to read back the optional memory-region configuration for
> > the
> > + * driver.
> > + */
> > +struct aspeed_p2a_ctrl_mapping {
> > +     __u64 addr;
> > +     __u32 length;
> > +     __u32 flags;
> > +};
> > +
> > +#define __ASPEED_P2A_CTRL_IOCTL_MAGIC 0xb3
> > +
> > +/*
> > + * This IOCTL is meant to configure a region or regions of memory
> > given a
> > + * starting address and length to be readable by the host, or
> > + * readable-writeable.
> > + */
> > +#define ASPEED_P2A_CTRL_IOCTL_SET_WINDOW
> > _IOW(__ASPEED_P2A_CTRL_IOCTL_MAGIC, \
> > +             0x00, struct aspeed_p2a_ctrl_mapping)
> > +
> > +/*
> > + * This IOCTL is meant to read back to the user the base address and
> > length of
> > + * the memory-region specified to the driver for use with mmap.
> > + */
> > +#define ASPEED_P2A_CTRL_IOCTL_GET_MEMORY_CONFIG \
> > +     _IOWR(__ASPEED_P2A_CTRL_IOCTL_MAGIC, \
> > +             0x01, struct aspeed_p2a_ctrl_mapping)
> > +
> > +#endif /* _UAPI_LINUX_ASPEED_P2A_CTRL_H */
> > --
> > 2.21.0.392.gf8f6787159e-goog
> >
> >

^ permalink raw reply

* [PATCH v8 1/2] dt-bindings: misc: aspeed-p2a-ctrl: add support
From: Patrick Venture @ 2019-04-02 21:24 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <e33bc539-2350-4b7d-9833-df270383b062@www.fastmail.com>

On Mon, Apr 1, 2019 at 8:42 PM Andrew Jeffery <andrew@aj.id.au> wrote:
>
> Hi Patrick,
>
> I held off on reviewing this until we'd hashed out what we needed in the driver.
>
> I have some comments below.
>
> On Sat, 30 Mar 2019, at 01:40, Patrick Venture wrote:
> > Document the ast2400, ast2500 PCI-to-AHB bridge control driver bindings.
> >
> > Signed-off-by: Patrick Venture <venture@google.com>
> > Reviewed-by: Rob Herring <robh@kernel.org>
> > ---
> > Changes for v8:
> > - None
> > Changes for v7:
> > - Moved node under the syscon node it requires
> > Changes for v6:
> > - None
> > Changes for v5:
> > - None
> > Changes for v4:
> > - None
> > Changes for v3:
> > - None
> > Changes for v2:
> > - Added comment about syscon required parameter.
> > ---
> >  .../bindings/misc/aspeed-p2a-ctrl.txt         | 48 +++++++++++++++++++
> >  1 file changed, 48 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/misc/aspeed-p2a-ctrl.txt
> >
> > diff --git a/Documentation/devicetree/bindings/misc/aspeed-p2a-ctrl.txt
> > b/Documentation/devicetree/bindings/misc/aspeed-p2a-ctrl.txt
> > new file mode 100644
> > index 0000000000000..088cc4e3dc54b
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/misc/aspeed-p2a-ctrl.txt
> > @@ -0,0 +1,48 @@
> > +======================================================================
> > +Device tree bindings for Aspeed AST2400/AST2500 PCI-to-AHB Bridge
> > Control Driver
> > +======================================================================
> > +
> > +The bridge is available on platforms with the VGA enabled on the
> > Aspeed device.
> > +In this case, the host has access to a 64KiB window into all of the
> > BMC's
> > +memory.  The BMC can disable this bridge.  If the bridge is enabled,
> > the host
> > +has read access to all the regions of memory, however the host only
> > has read
> > +and write access depending on a register controlled by the BMC.
> > +
> > +Required properties:
> > +===================
> > +
> > + - compatible: must be one of:
> > +     - "aspeed,ast2400-p2a-ctrl"
> > +     - "aspeed,ast2500-p2a-ctrl"
> > +
> > + - syscon: handle to syscon device node controlling PCI.
>
> The p2a-ctrl node is meant to be a child of the syscon. I noted this in my review
> of the associated driver - you need to remove the description of the syscon
> property.

Roger that, I'll take a hack at cleaning this up later this week (I'm OOO).

>
> > +
> > +Optional properties:
> > +===================
> > +
> > +- memory-region: A phandle to a reserved_memory region to be used for
> > the PCI
> > +             to AHB mapping
> > +
> > +The p2a-control node should be the child of a syscon node with the
> > required
> > +property:
> > +
> > +- compatible : Should be one of the following:
> > +             "aspeed,ast2400-scu", "syscon", "simple-mfd"
> > +             "aspeed,g4-scu", "syscon", "simple-mfd"
> > +             "aspeed,ast2500-scu", "syscon", "simple-mfd"
> > +             "aspeed,g5-scu", "syscon", "simple-mfd"
>
> The note above should go where you've described the syscon property above.
>
> Cheers,
>
> Andrew
>
> > +
> > +Example:
> > +
> > +g4 Example
> > +----------
> > +
> > +syscon: scu at 1e6e2000 {
> > +     compatible = "aspeed,ast2400-scu", "syscon", "simple-mfd";
> > +     reg = <0x1e6e2000 0x1a8>;
> > +
> > +     p2a: p2a-control {
> > +             compatible = "aspeed,ast2400-p2a-ctrl";
> > +             memory-region = <&reserved_memory>;
> > +     };
> > +};
> > --
> > 2.21.0.392.gf8f6787159e-goog
> >
> >

^ permalink raw reply

* [PATCH 3/5] media: dt-bindings: aspeed-video: Add missing memory-region property
From: Jae Hyun Yoo @ 2019-04-02 21:05 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <1554229504-5661-4-git-send-email-eajames@linux.ibm.com>

Hi Eddie,

On 4/2/2019 11:25 AM, Eddie James wrote:
> Missed documenting this property in the initial commit.
> 
> Signed-off-by: Eddie James <eajames@linux.ibm.com>
> ---
>   Documentation/devicetree/bindings/media/aspeed-video.txt | 6 ++++++
>   1 file changed, 6 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/media/aspeed-video.txt b/Documentation/devicetree/bindings/media/aspeed-video.txt
> index 78b464a..346c2d3 100644
> --- a/Documentation/devicetree/bindings/media/aspeed-video.txt
> +++ b/Documentation/devicetree/bindings/media/aspeed-video.txt
> @@ -14,6 +14,11 @@ Required properties:
>   			the VE
>    - interrupts:		the interrupt associated with the VE on this platform
>   
> +Optional properties:
> + - memory-region:
> +	phandle to a memory region to allocate from, as defined in
> +	Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
> +
>   Example:
>   
>   video-engine at 1e700000 {
> @@ -23,4 +28,5 @@ video-engine at 1e700000 {
>       clock-names = "vclk", "eclk";
>       resets = <&syscon ASPEED_RESET_VIDEO>;
>       interrupts = <7>;
> +    memory-region = <&video_engine_memory>

nit: a semicolon at the end of the line.

>   };
> 

^ permalink raw reply

* [PATCH 5/5] ARM: dts: aspeed-g5: Add video engine
From: Eddie James @ 2019-04-02 18:25 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <1554229504-5661-1-git-send-email-eajames@linux.ibm.com>

Add a node to describe the video engine on the AST2500.

Signed-off-by: Eddie James <eajames@linux.ibm.com>
---
 arch/arm/boot/dts/aspeed-g5.dtsi | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/arm/boot/dts/aspeed-g5.dtsi b/arch/arm/boot/dts/aspeed-g5.dtsi
index 85ed9db..c6d5edc 100644
--- a/arch/arm/boot/dts/aspeed-g5.dtsi
+++ b/arch/arm/boot/dts/aspeed-g5.dtsi
@@ -243,6 +243,16 @@
 				status = "disabled";
 			};
 
+			video: video at 1e700000 {
+				compatible = "aspeed,ast2500-video-engine";
+				reg = <0x1e700000 0x1000>;
+				clocks = <&syscon ASPEED_CLK_GATE_VCLK>,
+					 <&syscon ASPEED_CLK_GATE_ECLK>;
+				clock-names = "vclk", "eclk";
+				interrupts = <7>;
+				status = "disabled";
+			};
+
 			sram: sram at 1e720000 {
 				compatible = "mmio-sram";
 				reg = <0x1e720000 0x9000>;	// 36K
-- 
1.8.3.1


^ permalink raw reply related

* [PATCH 4/5] clk: Aspeed: Setup video engine clocking
From: Eddie James @ 2019-04-02 18:25 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <1554229504-5661-1-git-send-email-eajames@linux.ibm.com>

Add eclk mux and clock divider table. Also change the video engine reset
to the correct clock; it was previously on the video capture but needs
to be on the video engine clock.

Signed-off-by: Eddie James <eajames@linux.ibm.com>
---
 drivers/clk/clk-aspeed.c | 42 +++++++++++++++++++++++++++++++++++++++---
 1 file changed, 39 insertions(+), 3 deletions(-)

diff --git a/drivers/clk/clk-aspeed.c b/drivers/clk/clk-aspeed.c
index 5961367..42b4df6 100644
--- a/drivers/clk/clk-aspeed.c
+++ b/drivers/clk/clk-aspeed.c
@@ -87,10 +87,10 @@ struct aspeed_clk_gate {
 /* TODO: ask Aspeed about the actual parent data */
 static const struct aspeed_gate_data aspeed_gates[] = {
 	/*				 clk rst   name			parent	flags */
-	[ASPEED_CLK_GATE_ECLK] =	{  0, -1, "eclk-gate",		"eclk",	0 }, /* Video Engine */
+	[ASPEED_CLK_GATE_ECLK] =	{  0,  6, "eclk-gate",		"eclk",	0 }, /* Video Engine */
 	[ASPEED_CLK_GATE_GCLK] =	{  1,  7, "gclk-gate",		NULL,	0 }, /* 2D engine */
 	[ASPEED_CLK_GATE_MCLK] =	{  2, -1, "mclk-gate",		"mpll",	CLK_IS_CRITICAL }, /* SDRAM */
-	[ASPEED_CLK_GATE_VCLK] =	{  3,  6, "vclk-gate",		NULL,	0 }, /* Video Capture */
+	[ASPEED_CLK_GATE_VCLK] =	{  3, -1, "vclk-gate",		NULL,	0 }, /* Video Capture */
 	[ASPEED_CLK_GATE_BCLK] =	{  4,  8, "bclk-gate",		"bclk",	CLK_IS_CRITICAL }, /* PCIe/PCI */
 	[ASPEED_CLK_GATE_DCLK] =	{  5, -1, "dclk-gate",		NULL,	CLK_IS_CRITICAL }, /* DAC */
 	[ASPEED_CLK_GATE_REFCLK] =	{  6, -1, "refclk-gate",	"clkin", CLK_IS_CRITICAL },
@@ -113,6 +113,24 @@ struct aspeed_clk_gate {
 	[ASPEED_CLK_GATE_LHCCLK] =	{ 28, -1, "lhclk-gate",		"lhclk", 0 }, /* LPC master/LPC+ */
 };
 
+static const char * const eclk_parent_names[] = {
+	"mpll",
+	"hpll",
+	"dpll",
+};
+
+static const struct clk_div_table ast2500_eclk_div_table[] = {
+	{ 0x0, 2 },
+	{ 0x1, 2 },
+	{ 0x2, 3 },
+	{ 0x3, 4 },
+	{ 0x4, 5 },
+	{ 0x5, 6 },
+	{ 0x6, 7 },
+	{ 0x7, 8 },
+	{ 0 }
+};
+
 static const struct clk_div_table ast2500_mac_div_table[] = {
 	{ 0x0, 4 }, /* Yep, really. Aspeed confirmed this is correct */
 	{ 0x1, 4 },
@@ -192,18 +210,21 @@ static struct clk_hw *aspeed_ast2500_calc_pll(const char *name, u32 val)
 
 struct aspeed_clk_soc_data {
 	const struct clk_div_table *div_table;
+	const struct clk_div_table *eclk_div_table;
 	const struct clk_div_table *mac_div_table;
 	struct clk_hw *(*calc_pll)(const char *name, u32 val);
 };
 
 static const struct aspeed_clk_soc_data ast2500_data = {
 	.div_table = ast2500_div_table,
+	.eclk_div_table = ast2500_eclk_div_table,
 	.mac_div_table = ast2500_mac_div_table,
 	.calc_pll = aspeed_ast2500_calc_pll,
 };
 
 static const struct aspeed_clk_soc_data ast2400_data = {
 	.div_table = ast2400_div_table,
+	.eclk_div_table = ast2400_div_table,
 	.mac_div_table = ast2400_div_table,
 	.calc_pll = aspeed_ast2400_calc_pll,
 };
@@ -522,6 +543,22 @@ static int aspeed_clk_probe(struct platform_device *pdev)
 		return PTR_ERR(hw);
 	aspeed_clk_data->hws[ASPEED_CLK_24M] = hw;
 
+	hw = clk_hw_register_mux(dev, "eclk-mux", eclk_parent_names,
+				 ARRAY_SIZE(eclk_parent_names), 0,
+				 scu_base + ASPEED_CLK_SELECTION, 2, 0x3, 0,
+				 &aspeed_clk_lock);
+	if (IS_ERR(hw))
+		return PTR_ERR(hw);
+	aspeed_clk_data->hws[ASPEED_CLK_ECLK_MUX] = hw;
+
+	hw = clk_hw_register_divider_table(dev, "eclk", "eclk-mux", 0,
+					   scu_base + ASPEED_CLK_SELECTION, 28,
+					   3, 0, soc_data->eclk_div_table,
+					   &aspeed_clk_lock);
+	if (IS_ERR(hw))
+		return PTR_ERR(hw);
+	aspeed_clk_data->hws[ASPEED_CLK_ECLK] = hw;
+
 	/*
 	 * TODO: There are a number of clocks that not included in this driver
 	 * as more information is required:
@@ -531,7 +568,6 @@ static int aspeed_clk_probe(struct platform_device *pdev)
 	 *   RGMII
 	 *   RMII
 	 *   UART[1..5] clock source mux
-	 *   Video Engine (ECLK) mux and clock divider
 	 */
 
 	for (i = 0; i < ARRAY_SIZE(aspeed_gates); i++) {
-- 
1.8.3.1


^ permalink raw reply related

* [PATCH 3/5] media: dt-bindings: aspeed-video: Add missing memory-region property
From: Eddie James @ 2019-04-02 18:25 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <1554229504-5661-1-git-send-email-eajames@linux.ibm.com>

Missed documenting this property in the initial commit.

Signed-off-by: Eddie James <eajames@linux.ibm.com>
---
 Documentation/devicetree/bindings/media/aspeed-video.txt | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/media/aspeed-video.txt b/Documentation/devicetree/bindings/media/aspeed-video.txt
index 78b464a..346c2d3 100644
--- a/Documentation/devicetree/bindings/media/aspeed-video.txt
+++ b/Documentation/devicetree/bindings/media/aspeed-video.txt
@@ -14,6 +14,11 @@ Required properties:
 			the VE
  - interrupts:		the interrupt associated with the VE on this platform
 
+Optional properties:
+ - memory-region:
+	phandle to a memory region to allocate from, as defined in
+	Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
+
 Example:
 
 video-engine at 1e700000 {
@@ -23,4 +28,5 @@ video-engine at 1e700000 {
     clock-names = "vclk", "eclk";
     resets = <&syscon ASPEED_RESET_VIDEO>;
     interrupts = <7>;
+    memory-region = <&video_engine_memory>
 };
-- 
1.8.3.1


^ permalink raw reply related


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