All of lore.kernel.org
 help / color / mirror / Atom feed
From: gregkh@linuxfoundation.org (Greg Kroah-Hartman)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH V4 10/12] boot_constraint: Add support for Hisilicon platforms
Date: Wed, 13 Dec 2017 10:47:18 +0100	[thread overview]
Message-ID: <20171213094718.GE13194@kroah.com> (raw)
In-Reply-To: <facf1c5838468c8440240e2828a8e4982003a6f6.1509284255.git.viresh.kumar@linaro.org>

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 at 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 at 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 at 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

WARNING: multiple messages have this Message-ID (diff)
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: Viresh Kumar <viresh.kumar@linaro.org>
Cc: Vincent Guittot <vincent.guittot@linaro.org>,
	Stephen Boyd <sboyd@codeaurora.org>,
	Rajendra Nayak <rnayak@codeaurora.org>,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, robdclark@gmail.com,
	s.hauer@pengutronix.de, l.stach@pengutronix.de,
	shawnguo@kernel.org, fabio.estevam@nxp.com, nm@ti.com,
	xuwei5@hisilicon.com, robh+dt@kernel.org
Subject: Re: [PATCH V4 10/12] boot_constraint: Add support for Hisilicon platforms
Date: Wed, 13 Dec 2017 10:47:18 +0100	[thread overview]
Message-ID: <20171213094718.GE13194@kroah.com> (raw)
In-Reply-To: <facf1c5838468c8440240e2828a8e4982003a6f6.1509284255.git.viresh.kumar@linaro.org>

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

  reply	other threads:[~2017-12-13  9:47 UTC|newest]

Thread overview: 85+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-10-29 13:48 [PATCH V4 00/12] drivers: Boot Constraints core Viresh Kumar
2017-10-29 13:48 ` Viresh Kumar
2017-10-29 13:48 ` Viresh Kumar
2017-10-29 13:48 ` [PATCH V4 01/12] of: platform: Add of_find_amba_device_by_node() Viresh Kumar
2017-10-29 13:48   ` Viresh Kumar
2017-10-29 13:48   ` Viresh Kumar
2017-10-29 13:48 ` [PATCH V4 02/12] of: platform: Make of_platform_bus_create() global Viresh Kumar
2017-10-29 13:48   ` Viresh Kumar
2017-10-29 13:48 ` [PATCH V4 03/12] drivers: Add boot constraints core Viresh Kumar
2017-10-29 13:48   ` Viresh Kumar
2017-12-13  9:42   ` Greg Kroah-Hartman
2017-12-13  9:42     ` Greg Kroah-Hartman
2017-12-13  9:59     ` Viresh Kumar
2017-12-13  9:59       ` Viresh Kumar
2017-12-13  9:42   ` Greg Kroah-Hartman
2017-12-13  9:42     ` Greg Kroah-Hartman
2017-12-13 10:00     ` Viresh Kumar
2017-12-13 10:00       ` Viresh Kumar
2017-10-29 13:48 ` [PATCH V4 04/12] boot_constraint: Add support for supply constraints Viresh Kumar
2017-10-29 13:48   ` Viresh Kumar
2017-12-13  9:48   ` Greg Kroah-Hartman
2017-12-13  9:48     ` Greg Kroah-Hartman
2017-10-29 13:48 ` [PATCH V4 05/12] boot_constraint: Add support for clk constraints Viresh Kumar
2017-10-29 13:48   ` Viresh Kumar
2017-12-13  9:48   ` Greg Kroah-Hartman
2017-12-13  9:48     ` Greg Kroah-Hartman
2017-10-29 13:48 ` [PATCH V4 06/12] boot_constraint: Add support for PM constraints Viresh Kumar
2017-10-29 13:48   ` Viresh Kumar
2017-12-13  9:48   ` Greg Kroah-Hartman
2017-12-13  9:48     ` Greg Kroah-Hartman
2017-10-29 13:48 ` [PATCH V4 07/12] boot_constraint: Add debugfs support Viresh Kumar
2017-10-29 13:48   ` Viresh Kumar
2017-10-29 15:09   ` Randy Dunlap
2017-10-29 15:09     ` Randy Dunlap
2017-10-30  3:37     ` Viresh Kumar
2017-10-30  3:37       ` Viresh Kumar
2017-10-30  3:43       ` Randy Dunlap
2017-10-30  3:43         ` Randy Dunlap
2017-12-13  9:50   ` Greg Kroah-Hartman
2017-12-13  9:50     ` Greg Kroah-Hartman
2017-10-29 13:48 ` [PATCH V4 08/12] boot_constraint: Manage deferrable constraints Viresh Kumar
2017-10-29 13:48   ` Viresh Kumar
2017-10-31 16:20   ` Rob Herring
2017-10-31 16:20     ` Rob Herring
2017-11-01  2:29     ` Viresh Kumar
2017-11-01  2:29       ` Viresh Kumar
2017-12-13  9:53   ` Greg Kroah-Hartman
2017-12-13  9:53     ` Greg Kroah-Hartman
2017-12-13 10:27     ` Viresh Kumar
2017-12-13 10:27       ` Viresh Kumar
2017-12-13 10:33       ` Russell King - ARM Linux
2017-12-13 10:33         ` Russell King - ARM Linux
2017-12-13 14:39         ` Viresh Kumar
2017-12-13 14:39           ` Viresh Kumar
2017-10-29 13:48 ` [PATCH V4 09/12] boot_constraint: Add earlycon helper Viresh Kumar
2017-10-29 13:48   ` Viresh Kumar
2017-12-13  9:44   ` Greg Kroah-Hartman
2017-12-13  9:44     ` Greg Kroah-Hartman
2017-12-14  5:22     ` Viresh Kumar
2017-12-14  5:22       ` Viresh Kumar
2017-10-29 13:48 ` [PATCH V4 10/12] boot_constraint: Add support for Hisilicon platforms Viresh Kumar
2017-10-29 13:48   ` Viresh Kumar
2017-12-13  9:47   ` Greg Kroah-Hartman [this message]
2017-12-13  9:47     ` Greg Kroah-Hartman
2017-12-13 10:13     ` Viresh Kumar
2017-12-13 10:13       ` Viresh Kumar
2017-10-29 13:48 ` [PATCH V4 11/12] boot_constraint: Add support for IMX platform Viresh Kumar
2017-10-29 13:48   ` Viresh Kumar
2017-11-03  8:58   ` Sascha Hauer
2017-11-03  8:58     ` Sascha Hauer
2017-10-29 13:49 ` [PATCH V4 12/12] boot_constraint: Add Qualcomm display controller constraints Viresh Kumar
2017-10-29 13:49   ` Viresh Kumar
2017-10-30 22:07 ` [PATCH V4 00/12] drivers: Boot Constraints core Rob Herring
2017-10-30 22:07   ` Rob Herring
2017-10-31 10:01   ` Viresh Kumar
2017-10-31 10:01     ` Viresh Kumar
2017-10-31 16:03     ` Rob Herring
2017-11-28  4:18 ` Viresh Kumar
2017-11-28  4:18   ` Viresh Kumar
2017-11-28  4:18   ` Viresh Kumar
2017-12-13  9:55   ` Greg Kroah-Hartman
2017-12-13  9:55     ` Greg Kroah-Hartman
2017-12-13 10:27     ` Viresh Kumar
2017-12-13 10:27       ` Viresh Kumar
2017-12-13 10:27       ` Viresh Kumar

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20171213094718.GE13194@kroah.com \
    --to=gregkh@linuxfoundation.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.