2018-02-19 07:25:38

by Joel Stanley

[permalink] [raw]
Subject: [PATCH v2 0/3] misc: aspeed-lpc-ctrl fixes

Hi Greg,

Once Andrew has acked the bindings I think this is ready to go.

v2: Fix binding layout and add Rob's ack

These patches were developed when testing upstream Linux with OpenBMC on
Romulus. We need to ensure the LPC clock is enabled, now that the clock
driver turns off any unused clocks. We also need to enable the LPC
firmware cycles bit as we do not intend to upstream any mach-aspeed
hacks.

There was no existing binding document for the LPC host interface
controller, so I wrote one that includes the required clock description.

Joel Stanley (3):
dt-bindings: aspeed-lpc: Document LPC Host Interface Controller
misc: aspeed-lpc: Request and enable LPC clock
misc: aspeed-lpc-ctrl: Enable FWH and A2H bridge cycles

.../devicetree/bindings/mfd/aspeed-lpc.txt | 41 ++++++++++++++++++++
drivers/misc/aspeed-lpc-ctrl.c | 44 +++++++++++++++++++---
2 files changed, 80 insertions(+), 5 deletions(-)

--
2.15.1



2018-02-19 07:25:55

by Joel Stanley

[permalink] [raw]
Subject: [PATCH v2 1/3] dt-bindings: aspeed-lpc: Document LPC Host Interface Controller

The LPC Host Interface Controller is part of a BMC SoC that is used for
communication with the host.

Reviewed-by: Rob Herring <[email protected]>
Signed-off-by: Joel Stanley <[email protected]>
---
v2:
- Move the content to below the Host Node Children heading
- Add Rob's review tag
---
.../devicetree/bindings/mfd/aspeed-lpc.txt | 41 ++++++++++++++++++++++
1 file changed, 41 insertions(+)

diff --git a/Documentation/devicetree/bindings/mfd/aspeed-lpc.txt b/Documentation/devicetree/bindings/mfd/aspeed-lpc.txt
index 514d82ced95b..69aadee00d5f 100644
--- a/Documentation/devicetree/bindings/mfd/aspeed-lpc.txt
+++ b/Documentation/devicetree/bindings/mfd/aspeed-lpc.txt
@@ -109,9 +109,50 @@ lpc: lpc@1e789000 {
};
};

+BMC Node Children
+==================
+
+
Host Node Children
==================

+LPC Host Interface Controller
+-------------------
+
+The LPC Host Interface Controller manages functions exposed to the host such as
+LPC firmware hub cycles, configuration of the LPC-to-AHB mapping, UART
+management and bus snoop configuration.
+
+Required properties:
+
+- compatible: One of:
+ "aspeed,ast2400-lpc-ctrl";
+ "aspeed,ast2500-lpc-ctrl";
+
+- reg: contains offset/length values of the host interface controller
+ memory regions
+
+- clocks: contains a phandle to the syscon node describing the clocks.
+ There should then be one cell representing the clock to use
+
+- memory-region: A phandle to a reserved_memory region to be used for the LPC
+ to AHB mapping
+
+- flash: A phandle to the SPI flash controller containing the flash to
+ be exposed over the LPC to AHB mapping
+
+Example:
+
+lpc-host@80 {
+ lpc_ctrl: lpc-ctrl@0 {
+ compatible = "aspeed,ast2500-lpc-ctrl";
+ reg = <0x0 0x80>;
+ clocks = <&syscon ASPEED_CLK_GATE_LCLK>;
+ memory-region = <&flash_memory>;
+ flash = <&spi>;
+ };
+};
+
LPC Host Controller
-------------------

--
2.15.1


2018-02-19 07:26:30

by Joel Stanley

[permalink] [raw]
Subject: [PATCH v2 2/3] misc: aspeed-lpc: Request and enable LPC clock

The LPC device needs to ensure it's clock is enabled before it can do
anything.

In the past the clock was enabled and left running by u-boot, however
Linux now has an upstream clock driver that disables unused clocks.

Tested-by: Lei YU <[email protected]>
Reviewed-by: Andrew Jeffery <[email protected]>
Signed-off-by: Joel Stanley <[email protected]>
---
drivers/misc/aspeed-lpc-ctrl.c | 26 +++++++++++++++++++++++---
1 file changed, 23 insertions(+), 3 deletions(-)

diff --git a/drivers/misc/aspeed-lpc-ctrl.c b/drivers/misc/aspeed-lpc-ctrl.c
index b5439643f54b..1827b7aa6674 100644
--- a/drivers/misc/aspeed-lpc-ctrl.c
+++ b/drivers/misc/aspeed-lpc-ctrl.c
@@ -7,6 +7,7 @@
* 2 of the License, or (at your option) any later version.
*/

+#include <linux/clk.h>
#include <linux/mfd/syscon.h>
#include <linux/miscdevice.h>
#include <linux/mm.h>
@@ -26,6 +27,7 @@
struct aspeed_lpc_ctrl {
struct miscdevice miscdev;
struct regmap *regmap;
+ struct clk *clk;
phys_addr_t mem_base;
resource_size_t mem_size;
u32 pnor_size;
@@ -221,16 +223,33 @@ static int aspeed_lpc_ctrl_probe(struct platform_device *pdev)
return -ENODEV;
}

+ lpc_ctrl->clk = devm_clk_get(dev, NULL);
+ if (IS_ERR(lpc_ctrl->clk)) {
+ dev_err(dev, "couldn't get clock\n");
+ return PTR_ERR(lpc_ctrl->clk);
+ }
+ rc = clk_prepare_enable(lpc_ctrl->clk);
+ if (rc) {
+ dev_err(dev, "couldn't enable clock\n");
+ return rc;
+ }
+
lpc_ctrl->miscdev.minor = MISC_DYNAMIC_MINOR;
lpc_ctrl->miscdev.name = DEVICE_NAME;
lpc_ctrl->miscdev.fops = &aspeed_lpc_ctrl_fops;
lpc_ctrl->miscdev.parent = dev;
rc = misc_register(&lpc_ctrl->miscdev);
- if (rc)
+ if (rc) {
dev_err(dev, "Unable to register device\n");
- else
- dev_info(dev, "Loaded at %pr\n", &resm);
+ goto err;
+ }
+
+ dev_info(dev, "Loaded at %pr\n", &resm);
+
+ return 0;

+err:
+ clk_disable_unprepare(lpc_ctrl->clk);
return rc;
}

@@ -239,6 +258,7 @@ static int aspeed_lpc_ctrl_remove(struct platform_device *pdev)
struct aspeed_lpc_ctrl *lpc_ctrl = dev_get_drvdata(&pdev->dev);

misc_deregister(&lpc_ctrl->miscdev);
+ clk_disable_unprepare(lpc_ctrl->clk);

return 0;
}
--
2.15.1


2018-02-19 07:26:46

by Joel Stanley

[permalink] [raw]
Subject: [PATCH v2 3/3] misc: aspeed-lpc-ctrl: Enable FWH and A2H bridge cycles

To date this driver has relied on prevous state from out of tree hacks
and vendor u-boot trees in order to have the host be able to access
data over the LPC bus.

Now we explicitly enable the AHB to LPC bridge and FWH cycles from when
the user first configures the address to map. We chose to do this then
as before that time there is no way for the kernel to know where it is
safe to point the LPC window.

Tested-by: Lei YU <[email protected]>
Reviewed-by: Andrew Jeffery <[email protected]>
Signed-off-by: Joel Stanley <[email protected]>
---
drivers/misc/aspeed-lpc-ctrl.c | 18 ++++++++++++++++--
1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/drivers/misc/aspeed-lpc-ctrl.c b/drivers/misc/aspeed-lpc-ctrl.c
index 1827b7aa6674..a024f8042259 100644
--- a/drivers/misc/aspeed-lpc-ctrl.c
+++ b/drivers/misc/aspeed-lpc-ctrl.c
@@ -21,6 +21,10 @@

#define DEVICE_NAME "aspeed-lpc-ctrl"

+#define HICR5 0x0
+#define HICR5_ENL2H BIT(8)
+#define HICR5_ENFWH BIT(10)
+
#define HICR7 0x8
#define HICR8 0xc

@@ -155,8 +159,18 @@ static long aspeed_lpc_ctrl_ioctl(struct file *file, unsigned int cmd,
if (rc)
return rc;

- return regmap_write(lpc_ctrl->regmap, HICR8,
- (~(map.size - 1)) | ((map.size >> 16) - 1));
+ rc = regmap_write(lpc_ctrl->regmap, HICR8,
+ (~(map.size - 1)) | ((map.size >> 16) - 1));
+ if (rc)
+ return rc;
+
+ /*
+ * Enable LPC FHW cycles. This is required for the host to
+ * access the regions specified.
+ */
+ return regmap_update_bits(lpc_ctrl->regmap, HICR5,
+ HICR5_ENFWH | HICR5_ENL2H,
+ HICR5_ENFWH | HICR5_ENL2H);
}

return -EINVAL;
--
2.15.1


2018-02-19 23:21:03

by Cyril Bur

[permalink] [raw]
Subject: Re: [PATCH v2 3/3] misc: aspeed-lpc-ctrl: Enable FWH and A2H bridge cycles

On Mon, 2018-02-19 at 17:54 +1030, Joel Stanley wrote:
> To date this driver has relied on prevous state from out of tree hacks
> and vendor u-boot trees in order to have the host be able to access
> data over the LPC bus.
>
> Now we explicitly enable the AHB to LPC bridge and FWH cycles from when
> the user first configures the address to map. We chose to do this then
> as before that time there is no way for the kernel to know where it is
> safe to point the LPC window.
>
> Tested-by: Lei YU <[email protected]>
> Reviewed-by: Andrew Jeffery <[email protected]>
> Signed-off-by: Joel Stanley <[email protected]>

Reviewed-by: Cyril Bur <[email protected]>

> ---
> drivers/misc/aspeed-lpc-ctrl.c | 18 ++++++++++++++++--
> 1 file changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/misc/aspeed-lpc-ctrl.c b/drivers/misc/aspeed-lpc-ctrl.c
> index 1827b7aa6674..a024f8042259 100644
> --- a/drivers/misc/aspeed-lpc-ctrl.c
> +++ b/drivers/misc/aspeed-lpc-ctrl.c
> @@ -21,6 +21,10 @@
>
> #define DEVICE_NAME "aspeed-lpc-ctrl"
>
> +#define HICR5 0x0
> +#define HICR5_ENL2H BIT(8)
> +#define HICR5_ENFWH BIT(10)
> +
> #define HICR7 0x8
> #define HICR8 0xc
>
> @@ -155,8 +159,18 @@ static long aspeed_lpc_ctrl_ioctl(struct file *file, unsigned int cmd,
> if (rc)
> return rc;
>
> - return regmap_write(lpc_ctrl->regmap, HICR8,
> - (~(map.size - 1)) | ((map.size >> 16) - 1));
> + rc = regmap_write(lpc_ctrl->regmap, HICR8,
> + (~(map.size - 1)) | ((map.size >> 16) - 1));
> + if (rc)
> + return rc;
> +
> + /*
> + * Enable LPC FHW cycles. This is required for the host to
> + * access the regions specified.
> + */
> + return regmap_update_bits(lpc_ctrl->regmap, HICR5,
> + HICR5_ENFWH | HICR5_ENL2H,
> + HICR5_ENFWH | HICR5_ENL2H);
> }
>
> return -EINVAL;

2018-02-19 23:21:57

by Cyril Bur

[permalink] [raw]
Subject: Re: [PATCH v2 2/3] misc: aspeed-lpc: Request and enable LPC clock

On Mon, 2018-02-19 at 17:54 +1030, Joel Stanley wrote:
> The LPC device needs to ensure it's clock is enabled before it can do
> anything.
>
> In the past the clock was enabled and left running by u-boot, however
> Linux now has an upstream clock driver that disables unused clocks.
>
> Tested-by: Lei YU <[email protected]>
> Reviewed-by: Andrew Jeffery <[email protected]>
> Signed-off-by: Joel Stanley <[email protected]>

Reviewed-by: Cyril Bur <[email protected]>

> ---
> drivers/misc/aspeed-lpc-ctrl.c | 26 +++++++++++++++++++++++---
> 1 file changed, 23 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/misc/aspeed-lpc-ctrl.c b/drivers/misc/aspeed-lpc-ctrl.c
> index b5439643f54b..1827b7aa6674 100644
> --- a/drivers/misc/aspeed-lpc-ctrl.c
> +++ b/drivers/misc/aspeed-lpc-ctrl.c
> @@ -7,6 +7,7 @@
> * 2 of the License, or (at your option) any later version.
> */
>
> +#include <linux/clk.h>
> #include <linux/mfd/syscon.h>
> #include <linux/miscdevice.h>
> #include <linux/mm.h>
> @@ -26,6 +27,7 @@
> struct aspeed_lpc_ctrl {
> struct miscdevice miscdev;
> struct regmap *regmap;
> + struct clk *clk;
> phys_addr_t mem_base;
> resource_size_t mem_size;
> u32 pnor_size;
> @@ -221,16 +223,33 @@ static int aspeed_lpc_ctrl_probe(struct platform_device *pdev)
> return -ENODEV;
> }
>
> + lpc_ctrl->clk = devm_clk_get(dev, NULL);
> + if (IS_ERR(lpc_ctrl->clk)) {
> + dev_err(dev, "couldn't get clock\n");
> + return PTR_ERR(lpc_ctrl->clk);
> + }
> + rc = clk_prepare_enable(lpc_ctrl->clk);
> + if (rc) {
> + dev_err(dev, "couldn't enable clock\n");
> + return rc;
> + }
> +
> lpc_ctrl->miscdev.minor = MISC_DYNAMIC_MINOR;
> lpc_ctrl->miscdev.name = DEVICE_NAME;
> lpc_ctrl->miscdev.fops = &aspeed_lpc_ctrl_fops;
> lpc_ctrl->miscdev.parent = dev;
> rc = misc_register(&lpc_ctrl->miscdev);
> - if (rc)
> + if (rc) {
> dev_err(dev, "Unable to register device\n");
> - else
> - dev_info(dev, "Loaded at %pr\n", &resm);
> + goto err;
> + }
> +
> + dev_info(dev, "Loaded at %pr\n", &resm);
> +
> + return 0;
>
> +err:
> + clk_disable_unprepare(lpc_ctrl->clk);
> return rc;
> }
>
> @@ -239,6 +258,7 @@ static int aspeed_lpc_ctrl_remove(struct platform_device *pdev)
> struct aspeed_lpc_ctrl *lpc_ctrl = dev_get_drvdata(&pdev->dev);
>
> misc_deregister(&lpc_ctrl->miscdev);
> + clk_disable_unprepare(lpc_ctrl->clk);
>
> return 0;
> }

2018-03-07 13:14:51

by Lee Jones

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] dt-bindings: aspeed-lpc: Document LPC Host Interface Controller

On Mon, 19 Feb 2018, Joel Stanley wrote:

> The LPC Host Interface Controller is part of a BMC SoC that is used for
> communication with the host.
>
> Reviewed-by: Rob Herring <[email protected]>
> Signed-off-by: Joel Stanley <[email protected]>
> ---
> v2:
> - Move the content to below the Host Node Children heading
> - Add Rob's review tag
> ---
> .../devicetree/bindings/mfd/aspeed-lpc.txt | 41 ++++++++++++++++++++++
> 1 file changed, 41 insertions(+)

Applied, thanks.

--
Lee Jones
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog