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

2018-12-13 Thread Vijay Khemka 


On 12/12/18, 5:04 PM, "Joel Stanley"  wrote:

On Thu, 13 Dec 2018 at 07:34, Vijay Khemka  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 
> ---
>  .../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@8000 {
> reg = <0x8000 0x2000>;
> };
> +
> +   reserved-memory {
> +   #address-cells = <1>;
> +   #size-cells = <1>;
> +   ranges;
> +
> +   flash_memory: region@9800 {
> +   no-map;
> +   reg = <0x9800 0x0400>; /* 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 @@
> };
>  };
>
> +_ctrl {
> +   status = "okay";
> +   memory-region = <_memory>;
> +   flash = <>;
> +};
> +
>   {
> // Host Console
> status = "okay";
> @@ -75,6 +93,18 @@
> status = "okay";
>  };
>
> + {
> +   // BMC KCS channel 2
> +   status = "okay";
> +   kcs_addr = <0xca8>;
> +};
> +
> + {
> +   // BMC KCS channel 3
> +   status = "okay";
> +   kcs_addr = <0xca2>;
> +};
> +
>   {
> status = "okay";
>
> --
> 2.17.1
>

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

2018-12-12 Thread Joel Stanley 
On Thu, 13 Dec 2018 at 07:34, Vijay Khemka  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 
> ---
>  .../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@8000 {
> reg = <0x8000 0x2000>;
> };
> +
> +   reserved-memory {
> +   #address-cells = <1>;
> +   #size-cells = <1>;
> +   ranges;
> +
> +   flash_memory: region@9800 {
> +   no-map;
> +   reg = <0x9800 0x0400>; /* 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 @@
> };
>  };
>
> +_ctrl {
> +   status = "okay";
> +   memory-region = <_memory>;
> +   flash = <>;
> +};
> +
>   {
> // Host Console
> status = "okay";
> @@ -75,6 +93,18 @@
> status = "okay";
>  };
>
> + {
> +   // BMC KCS channel 2
> +   status = "okay";
> +   kcs_addr = <0xca8>;
> +};
> +
> + {
> +   // BMC KCS channel 3
> +   status = "okay";
> +   kcs_addr = <0xca2>;
> +};
> +
>   {
> status = "okay";
>
> --
> 2.17.1
>

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

2018-12-12 Thread Vijay Khemka 
Added kcs device and lpc ctrl device to enable LPC clock in Facebook
Tiogapass device tree.

Signed-off-by: Vijay Khemka 
---
 .../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@8000 {
reg = <0x8000 0x2000>;
};
+
+   reserved-memory {
+   #address-cells = <1>;
+   #size-cells = <1>;
+   ranges;
+
+   flash_memory: region@9800 {
+   no-map;
+   reg = <0x9800 0x0400>; /* 64M */
+   };
+   };
+
iio-hwmon {
compatible = "iio-hwmon";
oemname0 = "MB_P3V3";
@@ -62,6 +74,12 @@
};
 };
 
+_ctrl {
+   status = "okay";
+   memory-region = <_memory>;
+   flash = <>;
+};
+
  {
// Host Console
status = "okay";
@@ -75,6 +93,18 @@
status = "okay";
 };
 
+ {
+   // BMC KCS channel 2
+   status = "okay";
+   kcs_addr = <0xca8>;
+};
+
+ {
+   // BMC KCS channel 3
+   status = "okay";
+   kcs_addr = <0xca2>;
+};
+
  {
status = "okay";
 
-- 
2.17.1