[V4,10/12] boot_constraint: Add support for Hisilicon platforms

Message ID facf1c5838468c8440240e2828a8e4982003a6f6.1509284255.git.viresh.kumar@linaro.org
State New
Headers show
Series
  • Untitled series #5557
Related show

Commit Message

Viresh Kumar Oct. 29, 2017, 1:48 p.m.
This adds boot constraint support for Hisilicon platforms. Currently
only one use case is supported: earlycon. One of the UART is enabled by
the bootloader and is used for early console in the kernel. The boot
constraint core handles it properly and removes constraints once the
serial device is probed by its driver.

This is tested on hi6220-hikey 96board.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>

---
 arch/arm64/Kconfig.platforms      |   1 +
 drivers/boot_constraints/Makefile |   2 +
 drivers/boot_constraints/hikey.c  | 145 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 148 insertions(+)
 create mode 100644 drivers/boot_constraints/hikey.c

-- 
2.15.0.rc1.236.g92ea95045093

Comments

Greg KH Dec. 13, 2017, 9:47 a.m. | #1
On Sun, Oct 29, 2017 at 07:18:58PM +0530, Viresh Kumar wrote:
> This adds boot constraint support for Hisilicon platforms. Currently

> only one use case is supported: earlycon. One of the UART is enabled by

> the bootloader and is used for early console in the kernel. The boot

> constraint core handles it properly and removes constraints once the

> serial device is probed by its driver.

> 

> This is tested on hi6220-hikey 96board.

> 

> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>

> ---

>  arch/arm64/Kconfig.platforms      |   1 +

>  drivers/boot_constraints/Makefile |   2 +

>  drivers/boot_constraints/hikey.c  | 145 ++++++++++++++++++++++++++++++++++++++

>  3 files changed, 148 insertions(+)

>  create mode 100644 drivers/boot_constraints/hikey.c

> 

> diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms

> index 6b54ee8c1262..265df4a088ab 100644

> --- a/arch/arm64/Kconfig.platforms

> +++ b/arch/arm64/Kconfig.platforms

> @@ -87,6 +87,7 @@ config ARCH_HISI

>  	select ARM_TIMER_SP804

>  	select HISILICON_IRQ_MBIGEN if PCI

>  	select PINCTRL

> +	select DEV_BOOT_CONSTRAINTS

>  	help

>  	  This enables support for Hisilicon ARMv8 SoC family

>  

> diff --git a/drivers/boot_constraints/Makefile b/drivers/boot_constraints/Makefile

> index 0d4f88bb767c..43c89d2458e9 100644

> --- a/drivers/boot_constraints/Makefile

> +++ b/drivers/boot_constraints/Makefile

> @@ -1,3 +1,5 @@

>  # Makefile for device boot constraints

>  

>  obj-y := clk.o deferrable_dev.o core.o pm.o serial.o supply.o

> +

> +obj-$(CONFIG_ARCH_HISI)			+= hikey.o

> diff --git a/drivers/boot_constraints/hikey.c b/drivers/boot_constraints/hikey.c

> new file mode 100644

> index 000000000000..5f69f9451d93

> --- /dev/null

> +++ b/drivers/boot_constraints/hikey.c

> @@ -0,0 +1,145 @@

> +/*

> + * This takes care of Hisilicon boot time device constraints, normally set by

> + * the Bootloader.

> + *

> + * Copyright (C) 2017 Linaro.

> + * Viresh Kumar <viresh.kumar@linaro.org>

> + *

> + * This file is released under the GPLv2.

> + */

> +

> +#include <linux/boot_constraint.h>

> +#include <linux/init.h>

> +#include <linux/kernel.h>

> +#include <linux/of.h>

> +

> +struct hikey_machine_constraints {

> +	struct dev_boot_constraint_of *dev_constraints;

> +	unsigned int count;

> +};

> +

> +static struct dev_boot_constraint_clk_info uart_iclk_info = {

> +	.name = "uartclk",

> +};

> +

> +static struct dev_boot_constraint_clk_info uart_pclk_info = {

> +	.name = "apb_pclk",

> +};

> +

> +static struct dev_boot_constraint hikey3660_uart_constraints[] = {

> +	{

> +		.type = DEV_BOOT_CONSTRAINT_CLK,

> +		.data = &uart_iclk_info,

> +	}, {

> +		.type = DEV_BOOT_CONSTRAINT_CLK,

> +		.data = &uart_pclk_info,

> +	},

> +};

> +

> +static const char * const uarts_hikey3660[] = {

> +	"serial@fff32000",	/* UART 6 */

> +};

> +

> +static struct dev_boot_constraint_of hikey3660_dev_constraints[] = {

> +	{

> +		.compat = "arm,pl011",

> +		.constraints = hikey3660_uart_constraints,

> +		.count = ARRAY_SIZE(hikey3660_uart_constraints),

> +

> +		.dev_names = uarts_hikey3660,

> +		.dev_names_count = ARRAY_SIZE(uarts_hikey3660),

> +	},

> +};

> +

> +static struct hikey_machine_constraints hikey3660_constraints = {

> +	.dev_constraints = hikey3660_dev_constraints,

> +	.count = ARRAY_SIZE(hikey3660_dev_constraints),

> +};

> +

> +static const char * const uarts_hikey6220[] = {

> +	"uart@f7113000",	/* UART 3 */

> +};

> +

> +static struct dev_boot_constraint_of hikey6220_dev_constraints[] = {

> +	{

> +		.compat = "arm,pl011",

> +		.constraints = hikey3660_uart_constraints,

> +		.count = ARRAY_SIZE(hikey3660_uart_constraints),

> +

> +		.dev_names = uarts_hikey6220,

> +		.dev_names_count = ARRAY_SIZE(uarts_hikey6220),

> +	},

> +};

> +

> +static struct hikey_machine_constraints hikey6220_constraints = {

> +	.dev_constraints = hikey6220_dev_constraints,

> +	.count = ARRAY_SIZE(hikey6220_dev_constraints),

> +};

> +

> +static struct dev_boot_constraint hikey3798cv200_uart_constraints[] = {

> +	{

> +		.type = DEV_BOOT_CONSTRAINT_CLK,

> +		.data = &uart_pclk_info,

> +	},

> +};

> +

> +static const char * const uarts_hikey3798cv200[] = {

> +	"serial@8b00000",	/* UART 0 */

> +};

> +

> +static struct dev_boot_constraint_of hikey3798cv200_dev_constraints[] = {

> +	{

> +		.compat = "arm,pl011",

> +		.constraints = hikey3798cv200_uart_constraints,

> +		.count = ARRAY_SIZE(hikey3798cv200_uart_constraints),

> +

> +		.dev_names = uarts_hikey3798cv200,

> +		.dev_names_count = ARRAY_SIZE(uarts_hikey3798cv200),

> +	},

> +};

> +

> +static struct hikey_machine_constraints hikey3798cv200_constraints = {

> +	.dev_constraints = hikey3798cv200_dev_constraints,

> +	.count = ARRAY_SIZE(hikey3798cv200_dev_constraints),

> +};

> +

> +static const struct of_device_id machines[] __initconst = {

> +	{ .compatible = "hisilicon,hi3660", .data = &hikey3660_constraints },

> +	{ .compatible = "hisilicon,hi3798cv200", .data = &hikey3798cv200_constraints },

> +	{ .compatible = "hisilicon,hi6220", .data = &hikey6220_constraints },

> +	{ }

> +};

> +

> +static int __init hikey_constraints_init(void)

> +{

> +	const struct hikey_machine_constraints *constraints;

> +	const struct of_device_id *match;

> +	struct device_node *np;

> +

> +	if (!boot_constraint_earlycon_enabled())

> +		return 0;

> +

> +	np = of_find_node_by_path("/");


What is this for?

> +	if (!np)

> +		return -ENODEV;

> +

> +	match = of_match_node(machines, np);

> +	of_node_put(np);

> +

> +	if (!match)

> +		return 0;

> +

> +	constraints = match->data;

> +	BUG_ON(!constraints);


Never crash the device for a driver configuration issue.  That's going
to be bad.

> +	dev_boot_constraint_add_deferrable_of(constraints->dev_constraints,

> +					      constraints->count);

> +

> +	return 0;

> +}

> +

> +/*

> + * The amba-pl011 driver registers itself from arch_initcall level. Setup the

> + * serial boot constraints before that in order not to miss any boot messages.

> + */

> +postcore_initcall_sync(hikey_constraints_init);


Now you have to worry about the bootconstraints earlycon being called
before/after your code.  That's another linking order dependancy you
just created.  It feels more complex for something so "simple" as
looking for the earlycon flag...

thanks,

greg k-h
Viresh Kumar Dec. 13, 2017, 10:13 a.m. | #2
On 13-12-17, 10:47, Greg Kroah-Hartman wrote:
> On Sun, Oct 29, 2017 at 07:18:58PM +0530, Viresh Kumar wrote:

> > +static const struct of_device_id machines[] __initconst = {

> > +	{ .compatible = "hisilicon,hi3660", .data = &hikey3660_constraints },

> > +	{ .compatible = "hisilicon,hi3798cv200", .data = &hikey3798cv200_constraints },

> > +	{ .compatible = "hisilicon,hi6220", .data = &hikey6220_constraints },

> > +	{ }

> > +};

> > +

> > +static int __init hikey_constraints_init(void)

> > +{

> > +	const struct hikey_machine_constraints *constraints;

> > +	const struct of_device_id *match;

> > +	struct device_node *np;

> > +

> > +	if (!boot_constraint_earlycon_enabled())

> > +		return 0;

> > +

> > +	np = of_find_node_by_path("/");

> 

> What is this for?


We need to match the above list of "machines" with the root node and "np" here
points to the root node.. and ...

> > +	if (!np)

> > +		return -ENODEV;

> > +

> > +	match = of_match_node(machines, np);


Its used here.

> > +	of_node_put(np);


> > +/*

> > + * The amba-pl011 driver registers itself from arch_initcall level. Setup the

> > + * serial boot constraints before that in order not to miss any boot messages.

> > + */

> > +postcore_initcall_sync(hikey_constraints_init);

> 

> Now you have to worry about the bootconstraints earlycon being called

> before/after your code.


For boot-constraints to work for any device, it is extremely important to add
the constraint before the device is probed by its driver, otherwise the driver
would end up re-configuring the resources. There is no other way then having
this order dependency here.

> That's another linking order dependancy you

> just created.  It feels more complex for something so "simple" as

> looking for the earlycon flag...


-- 
viresh

Patch

diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms
index 6b54ee8c1262..265df4a088ab 100644
--- a/arch/arm64/Kconfig.platforms
+++ b/arch/arm64/Kconfig.platforms
@@ -87,6 +87,7 @@  config ARCH_HISI
 	select ARM_TIMER_SP804
 	select HISILICON_IRQ_MBIGEN if PCI
 	select PINCTRL
+	select DEV_BOOT_CONSTRAINTS
 	help
 	  This enables support for Hisilicon ARMv8 SoC family
 
diff --git a/drivers/boot_constraints/Makefile b/drivers/boot_constraints/Makefile
index 0d4f88bb767c..43c89d2458e9 100644
--- a/drivers/boot_constraints/Makefile
+++ b/drivers/boot_constraints/Makefile
@@ -1,3 +1,5 @@ 
 # Makefile for device boot constraints
 
 obj-y := clk.o deferrable_dev.o core.o pm.o serial.o supply.o
+
+obj-$(CONFIG_ARCH_HISI)			+= hikey.o
diff --git a/drivers/boot_constraints/hikey.c b/drivers/boot_constraints/hikey.c
new file mode 100644
index 000000000000..5f69f9451d93
--- /dev/null
+++ b/drivers/boot_constraints/hikey.c
@@ -0,0 +1,145 @@ 
+/*
+ * This takes care of Hisilicon boot time device constraints, normally set by
+ * the Bootloader.
+ *
+ * Copyright (C) 2017 Linaro.
+ * Viresh Kumar <viresh.kumar@linaro.org>
+ *
+ * This file is released under the GPLv2.
+ */
+
+#include <linux/boot_constraint.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/of.h>
+
+struct hikey_machine_constraints {
+	struct dev_boot_constraint_of *dev_constraints;
+	unsigned int count;
+};
+
+static struct dev_boot_constraint_clk_info uart_iclk_info = {
+	.name = "uartclk",
+};
+
+static struct dev_boot_constraint_clk_info uart_pclk_info = {
+	.name = "apb_pclk",
+};
+
+static struct dev_boot_constraint hikey3660_uart_constraints[] = {
+	{
+		.type = DEV_BOOT_CONSTRAINT_CLK,
+		.data = &uart_iclk_info,
+	}, {
+		.type = DEV_BOOT_CONSTRAINT_CLK,
+		.data = &uart_pclk_info,
+	},
+};
+
+static const char * const uarts_hikey3660[] = {
+	"serial@fff32000",	/* UART 6 */
+};
+
+static struct dev_boot_constraint_of hikey3660_dev_constraints[] = {
+	{
+		.compat = "arm,pl011",
+		.constraints = hikey3660_uart_constraints,
+		.count = ARRAY_SIZE(hikey3660_uart_constraints),
+
+		.dev_names = uarts_hikey3660,
+		.dev_names_count = ARRAY_SIZE(uarts_hikey3660),
+	},
+};
+
+static struct hikey_machine_constraints hikey3660_constraints = {
+	.dev_constraints = hikey3660_dev_constraints,
+	.count = ARRAY_SIZE(hikey3660_dev_constraints),
+};
+
+static const char * const uarts_hikey6220[] = {
+	"uart@f7113000",	/* UART 3 */
+};
+
+static struct dev_boot_constraint_of hikey6220_dev_constraints[] = {
+	{
+		.compat = "arm,pl011",
+		.constraints = hikey3660_uart_constraints,
+		.count = ARRAY_SIZE(hikey3660_uart_constraints),
+
+		.dev_names = uarts_hikey6220,
+		.dev_names_count = ARRAY_SIZE(uarts_hikey6220),
+	},
+};
+
+static struct hikey_machine_constraints hikey6220_constraints = {
+	.dev_constraints = hikey6220_dev_constraints,
+	.count = ARRAY_SIZE(hikey6220_dev_constraints),
+};
+
+static struct dev_boot_constraint hikey3798cv200_uart_constraints[] = {
+	{
+		.type = DEV_BOOT_CONSTRAINT_CLK,
+		.data = &uart_pclk_info,
+	},
+};
+
+static const char * const uarts_hikey3798cv200[] = {
+	"serial@8b00000",	/* UART 0 */
+};
+
+static struct dev_boot_constraint_of hikey3798cv200_dev_constraints[] = {
+	{
+		.compat = "arm,pl011",
+		.constraints = hikey3798cv200_uart_constraints,
+		.count = ARRAY_SIZE(hikey3798cv200_uart_constraints),
+
+		.dev_names = uarts_hikey3798cv200,
+		.dev_names_count = ARRAY_SIZE(uarts_hikey3798cv200),
+	},
+};
+
+static struct hikey_machine_constraints hikey3798cv200_constraints = {
+	.dev_constraints = hikey3798cv200_dev_constraints,
+	.count = ARRAY_SIZE(hikey3798cv200_dev_constraints),
+};
+
+static const struct of_device_id machines[] __initconst = {
+	{ .compatible = "hisilicon,hi3660", .data = &hikey3660_constraints },
+	{ .compatible = "hisilicon,hi3798cv200", .data = &hikey3798cv200_constraints },
+	{ .compatible = "hisilicon,hi6220", .data = &hikey6220_constraints },
+	{ }
+};
+
+static int __init hikey_constraints_init(void)
+{
+	const struct hikey_machine_constraints *constraints;
+	const struct of_device_id *match;
+	struct device_node *np;
+
+	if (!boot_constraint_earlycon_enabled())
+		return 0;
+
+	np = of_find_node_by_path("/");
+	if (!np)
+		return -ENODEV;
+
+	match = of_match_node(machines, np);
+	of_node_put(np);
+
+	if (!match)
+		return 0;
+
+	constraints = match->data;
+	BUG_ON(!constraints);
+
+	dev_boot_constraint_add_deferrable_of(constraints->dev_constraints,
+					      constraints->count);
+
+	return 0;
+}
+
+/*
+ * The amba-pl011 driver registers itself from arch_initcall level. Setup the
+ * serial boot constraints before that in order not to miss any boot messages.
+ */
+postcore_initcall_sync(hikey_constraints_init);