diff mbox series

[v4,1/2] ARM: dts: aspeed: Add KCS & lpc ctrl for Facebook

Message ID 20181212210352.1717617-1-vijaykhemka@fb.com
State Superseded, archived
Headers show
Series [v4,1/2] ARM: dts: aspeed: Add KCS & lpc ctrl for Facebook | expand

Commit Message

Vijay Khemka Dec. 12, 2018, 9:03 p.m. UTC 
Added kcs device and lpc ctrl device to enable LPC clock in Facebook
Tiogapass device tree.

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

Comments

Joel Stanley Dec. 13, 2018, 1:03 a.m. UTC | #1 
On Thu, 13 Dec 2018 at 07:34, Vijay Khemka <vijaykhemka@fb.com> wrote:
>
> Added kcs device and lpc ctrl device to enable LPC clock in Facebook
> Tiogapass device tree.

As we're going to re-spin, it's a good idea to have the KCS and LPC
changes in separate patches. I will take the KCS one straight away,
but the LPC one might need more work (see my comments below).

>
> Signed-off-by: Vijay Khemka <vijaykhemka@fb.com>
> ---
>  .../dts/aspeed-bmc-facebook-tiogapass.dts     | 30 +++++++++++++++++++
>  1 file changed, 30 insertions(+)
>
> diff --git a/arch/arm/boot/dts/aspeed-bmc-facebook-tiogapass.dts b/arch/arm/boot/dts/aspeed-bmc-facebook-tiogapass.dts
> index d9d7f208788a..9544fa2c1822 100644
> --- a/arch/arm/boot/dts/aspeed-bmc-facebook-tiogapass.dts
> +++ b/arch/arm/boot/dts/aspeed-bmc-facebook-tiogapass.dts
> @@ -21,6 +21,18 @@
>         memory@80000000 {
>                 reg = <0x80000000 0x20000000>;
>         };
> +
> +       reserved-memory {
> +               #address-cells = <1>;
> +               #size-cells = <1>;
> +               ranges;
> +
> +               flash_memory: region@98000000 {
> +                       no-map;
> +                       reg = <0x98000000 0x04000000>; /* 64M */
> +               };

Are you using this to expose some RAM over LPC? If not, we don't want this here.

It is a flaw in the design of the driver that the reserved memory
region is required. We should change the driver to make it optional.

As a workaround, we could add this patch to the tiogapass device tree
but make it smaller so you're not losing 64MB of RAM.

> +       };
> +
>         iio-hwmon {
>                 compatible = "iio-hwmon";
>                 oemname0 = "MB_P3V3";
> @@ -62,6 +74,12 @@
>         };
>  };
>
> +&lpc_ctrl {
> +       status = "okay";
> +       memory-region = <&flash_memory>;
> +       flash = <&spi1>;
> +};
> +
>  &uart1 {
>         // Host Console
>         status = "okay";
> @@ -75,6 +93,18 @@
>         status = "okay";
>  };
>
> +&kcs2 {
> +       // BMC KCS channel 2
> +       status = "okay";
> +       kcs_addr = <0xca8>;
> +};
> +
> +&kcs3 {
> +       // BMC KCS channel 3
> +       status = "okay";
> +       kcs_addr = <0xca2>;
> +};
> +
>  &mac0 {
>         status = "okay";
>
> --
> 2.17.1
>
Vijay Khemka Dec. 13, 2018, 7:18 p.m. UTC | #2 
On 12/12/18, 5:04 PM, "Joel Stanley" <joel@jms.id.au> wrote:

    On Thu, 13 Dec 2018 at 07:34, Vijay Khemka <vijaykhemka@fb.com> wrote:
    >
    > Added kcs device and lpc ctrl device to enable LPC clock in Facebook
    > Tiogapass device tree.
    
    As we're going to re-spin, it's a good idea to have the KCS and LPC
    changes in separate patches. I will take the KCS one straight away,
    but the LPC one might need more work (see my comments below).

Thanks Joel,
I will create different patches and resubmit again.
    
    >
    > Signed-off-by: Vijay Khemka <vijaykhemka@fb.com>
    > ---
    >  .../dts/aspeed-bmc-facebook-tiogapass.dts     | 30 +++++++++++++++++++
    >  1 file changed, 30 insertions(+)
    >
    > diff --git a/arch/arm/boot/dts/aspeed-bmc-facebook-tiogapass.dts b/arch/arm/boot/dts/aspeed-bmc-facebook-tiogapass.dts
    > index d9d7f208788a..9544fa2c1822 100644
    > --- a/arch/arm/boot/dts/aspeed-bmc-facebook-tiogapass.dts
    > +++ b/arch/arm/boot/dts/aspeed-bmc-facebook-tiogapass.dts
    > @@ -21,6 +21,18 @@
    >         memory@80000000 {
    >                 reg = <0x80000000 0x20000000>;
    >         };
    > +
    > +       reserved-memory {
    > +               #address-cells = <1>;
    > +               #size-cells = <1>;
    > +               ranges;
    > +
    > +               flash_memory: region@98000000 {
    > +                       no-map;
    > +                       reg = <0x98000000 0x04000000>; /* 64M */
    > +               };
    
    Are you using this to expose some RAM over LPC? If not, we don't want this here.
No, we are not using currently.
    
    It is a flaw in the design of the driver that the reserved memory
    region is required. We should change the driver to make it optional.
I agree with you we should make this change in driver.
    
    As a workaround, we could add this patch to the tiogapass device tree
    but make it smaller so you're not losing 64MB of RAM.
I will make it to 4K.
    
    > +       };
    > +
    >         iio-hwmon {
    >                 compatible = "iio-hwmon";
    >                 oemname0 = "MB_P3V3";
    > @@ -62,6 +74,12 @@
    >         };
    >  };
    >
    > +&lpc_ctrl {
    > +       status = "okay";
    > +       memory-region = <&flash_memory>;
    > +       flash = <&spi1>;
    > +};
    > +
    >  &uart1 {
    >         // Host Console
    >         status = "okay";
    > @@ -75,6 +93,18 @@
    >         status = "okay";
    >  };
    >
    > +&kcs2 {
    > +       // BMC KCS channel 2
    > +       status = "okay";
    > +       kcs_addr = <0xca8>;
    > +};
    > +
    > +&kcs3 {
    > +       // BMC KCS channel 3
    > +       status = "okay";
    > +       kcs_addr = <0xca2>;
    > +};
    > +
    >  &mac0 {
    >         status = "okay";
    >
    > --
    > 2.17.1
    >
diff mbox series

Patch

diff --git a/arch/arm/boot/dts/aspeed-bmc-facebook-tiogapass.dts b/arch/arm/boot/dts/aspeed-bmc-facebook-tiogapass.dts
index d9d7f208788a..9544fa2c1822 100644
--- a/arch/arm/boot/dts/aspeed-bmc-facebook-tiogapass.dts
+++ b/arch/arm/boot/dts/aspeed-bmc-facebook-tiogapass.dts
@@ -21,6 +21,18 @@ 
 	memory@80000000 {
 		reg = <0x80000000 0x20000000>;
 	};
+
+	reserved-memory {
+		#address-cells = <1>;
+		#size-cells = <1>;
+		ranges;
+
+		flash_memory: region@98000000 {
+			no-map;
+			reg = <0x98000000 0x04000000>; /* 64M */
+		};
+	};
+
 	iio-hwmon {
 		compatible = "iio-hwmon";
 		oemname0 = "MB_P3V3";
@@ -62,6 +74,12 @@ 
 	};
 };
 
+&lpc_ctrl {
+	status = "okay";
+	memory-region = <&flash_memory>;
+	flash = <&spi1>;
+};
+
 &uart1 {
 	// Host Console
 	status = "okay";
@@ -75,6 +93,18 @@ 
 	status = "okay";
 };
 
+&kcs2 {
+	// BMC KCS channel 2
+	status = "okay";
+	kcs_addr = <0xca8>;
+};
+
+&kcs3 {
+	// BMC KCS channel 3
+	status = "okay";
+	kcs_addr = <0xca2>;
+};
+
 &mac0 {
 	status = "okay";