* Re: [PATCH V2] pinctrl: qcom: Add msm8994 pinctrl driver
From: Bjorn Andersson @ 2016-10-29 18:06 UTC (permalink / raw)
To: Michael Scott
Cc: linux-gpio, linux-arm-msm, devicetree, linux-kernel,
Linus Walleij, Rob Herring, Mark Rutland, Andy Gross, David Brown,
Joonwoo Park, Jeremy McNicoll
In-Reply-To: <20161026233228.15219-1-michael.scott@linaro.org>
On Wed 26 Oct 16:32 PDT 2016, Michael Scott wrote:
> Initial pinctrl driver for QCOM msm8994 platforms.
>
> In order to continue the initial board support for QCOM msm8994/msm8992
> presented in patches from Jeremy McNicoll <jeremymc@redhat.com>, let's put
> a proper pinctrl driver in place.
>
> Currently, the DT for these platforms uses the msm8x74 pinctrl driver to enable
> basic UART. Beyond the first few pins the rest are different enough to justify
> it's own driver.
>
> Note: This driver is also be used by QCOM's msm8992 platform as it's TLM block
> is the same.
>
I'm not up to speed on the 8992/8994 story, but please add a
"qcom,msm8992-pinctrl" compatible to the list as well.
> - Initial formatting and style was taken from the msm8x74 pinctrl driver added
> by Björn Andersson <bjorn.andersson@linaro.org>
> - Data was then adjusted per QCOM MSM8994 documentation for Top Level Multiplexing
I think this is good enough. It looks reasonable and you based it on the
data sheet. Any fixes to the tables can be added incrementally.
> - Bindings documentation was based on qcom,msm8996-pinctrl.txt by
> Joonwoo Park <joonwoop@codeaurora.org> and then modified for msm8994 content
>
> Signed-off-by: Michael Scott <michael.scott@linaro.org>
Please add the compatible and you have my
Acked-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Regards,
Bjorn
^ permalink raw reply
* Re: [PATCH V2] pinctrl: qcom: Add msm8994 pinctrl driver
From: Michael Scott @ 2016-10-29 16:32 UTC (permalink / raw)
To: Linus Walleij, Bjorn Andersson
Cc: linux-gpio@vger.kernel.org, linux-arm-msm@vger.kernel.org,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
Rob Herring, Mark Rutland, Andy Gross, David Brown, Joonwoo Park,
Jeremy McNicoll, Stephen Boyd
In-Reply-To: <CACRpkda9fcUjtEuezEa-DdU_Q_OcE-kArRpY7RGp+aTFWWf=rA@mail.gmail.com>
Resending: The mailing lists bounced due me being silly and replying on
my phone.
Please reply to this email instead of the last one.
On 10/29/2016 01:27 AM, Linus Walleij wrote:
> On Thu, Oct 27, 2016 at 1:32 AM, Michael Scott <michael.scott@linaro.org> wrote:
>
>> Initial pinctrl driver for QCOM msm8994 platforms.
>>
>> In order to continue the initial board support for QCOM msm8994/msm8992
>> presented in patches from Jeremy McNicoll <jeremymc@redhat.com>, let's put
>> a proper pinctrl driver in place.
>>
>> Currently, the DT for these platforms uses the msm8x74 pinctrl driver to enable
>> basic UART. Beyond the first few pins the rest are different enough to justify
>> it's own driver.
>>
>> Note: This driver is also be used by QCOM's msm8992 platform as it's TLM block
>> is the same.
>>
>> - Initial formatting and style was taken from the msm8x74 pinctrl driver added
>> by Björn Andersson <bjorn.andersson@linaro.org>
>> - Data was then adjusted per QCOM MSM8994 documentation for Top Level Multiplexing
>> - Bindings documentation was based on qcom,msm8996-pinctrl.txt by
>> Joonwoo Park <joonwoop@codeaurora.org> and then modified for msm8994 content
>>
>> Signed-off-by: Michael Scott <michael.scott@linaro.org>
>> ---
>>
>> V1 -> V2: fixed missing FUNCTION(nav_pps) and removed 3 odd newlines between blsp_i2c4_groups and cci_timer0_groups
> Looks fine to me, just like the other Qcom drivers.
>
> I just want Björn Andersson's ACK before merging, Björn?
FYI: I asked Stephen if he could find someone else with tech docs to do
a data review. This may or may not happen depending on the internal
interest level and/or current work load. It will probably take a week
or so to get sorted out.
Worst case is no extra set of eyes on the data, and as issues are found
we can fix them in follow up patches. The driver works for the current
msm8994 patches submitted by Jeremy.
Michael Scott
>
> Yours,
> Linus Walleij
^ permalink raw reply
* Re: [PATCH V2] pinctrl: qcom: Add msm8994 pinctrl driver
From: Michael Scott @ 2016-10-29 16:26 UTC (permalink / raw)
To: Linus Walleij
Cc: Mark Rutland, linux-gpio, David Brown, Andy Gross,
Bjorn Andersson, Rob Herring, devicetree, Joonwoo Park,
Jeremy McNicoll, linux-arm-msm, linux-kernel, Stephen Boyd
In-Reply-To: <CACRpkda9fcUjtEuezEa-DdU_Q_OcE-kArRpY7RGp+aTFWWf=rA@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1926 bytes --]
On Oct 29, 2016 1:27 AM, "Linus Walleij" <linus.walleij@linaro.org> wrote:
>
> On Thu, Oct 27, 2016 at 1:32 AM, Michael Scott <michael.scott@linaro.org>
wrote:
>
> > Initial pinctrl driver for QCOM msm8994 platforms.
> >
> > In order to continue the initial board support for QCOM msm8994/msm8992
> > presented in patches from Jeremy McNicoll <jeremymc@redhat.com>, let's
put
> > a proper pinctrl driver in place.
> >
> > Currently, the DT for these platforms uses the msm8x74 pinctrl driver
to enable
> > basic UART. Beyond the first few pins the rest are different enough to
justify
> > it's own driver.
> >
> > Note: This driver is also be used by QCOM's msm8992 platform as it's
TLM block
> > is the same.
> >
> > - Initial formatting and style was taken from the msm8x74 pinctrl
driver added
> > by Björn Andersson <bjorn.andersson@linaro.org>
> > - Data was then adjusted per QCOM MSM8994 documentation for Top Level
Multiplexing
> > - Bindings documentation was based on qcom,msm8996-pinctrl.txt by
> > Joonwoo Park <joonwoop@codeaurora.org> and then modified for msm8994
content
> >
> > Signed-off-by: Michael Scott <michael.scott@linaro.org>
> > ---
> >
> > V1 -> V2: fixed missing FUNCTION(nav_pps) and removed 3 odd newlines
between blsp_i2c4_groups and cci_timer0_groups
>
> Looks fine to me, just like the other Qcom drivers.
>
> I just want Björn Andersson's ACK before merging, Björn?
FYI: I asked Stephen if he could find someone else with tech docs to do a
data review. This may or may not happen depending on the internal interest
level and/or current work load. It will probably take a week or so to get
sorted out.
Worst case is no extra set of eyes on the data, and as issues are found we
can fix them in follow up patches. The driver works for the current
msm8994 patches submitted by Jeremy.
Michael Scott
>
> Yours,
> Linus Walleij
[-- Attachment #2: Type: text/html, Size: 2657 bytes --]
^ permalink raw reply
* Re: [PATCH 2/2] clk: hisilicon: add CRG driver for Hi3516CV300 SoC
From: Hao Zhang @ 2016-10-29 11:36 UTC (permalink / raw)
To: Jiancheng Xue
Cc: Michael Turquette, Stephen Boyd, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
Mark Rutland, open list, open list:COMMON CLK FRAMEWORK,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
bin.chen-QSEj5FYQhm4dnm+yROfE0A, elder-QSEj5FYQhm4dnm+yROfE0A,
hermit.wangheming-C8/M+/jPZTeaMJb+Lgu22Q,
yanhaifeng-C8/M+/jPZTeaMJb+Lgu22Q, wenpan-C8/M+/jPZTeaMJb+Lgu22Q,
howell.yang-C8/M+/jPZTeaMJb+Lgu22Q
In-Reply-To: <1477721618-10809-3-git-send-email-xuejiancheng-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org>
Hi, is there any mainline kernel or uboot for hisilicon hi3520dv200 or
hi3520dv300? just want some base support to boot , anything close to
mainline is ok :)
2016-10-29 14:13 GMT+08:00 Jiancheng Xue <xuejiancheng-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org>:
> Add CRG driver for Hi3516CV300 SoC. CRG(Clock and Reset
> Generator) module generates clock and reset signals used
> by other module blocks on SoC.
>
> Signed-off-by: Pan Wen <wenpan-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org>
> ---
> drivers/clk/hisilicon/Kconfig | 8 +
> drivers/clk/hisilicon/Makefile | 1 +
> drivers/clk/hisilicon/crg-hi3516cv300.c | 330 ++++++++++++++++++++++++++
> include/dt-bindings/clock/hi3516cv300-clock.h | 48 ++++
> 4 files changed, 387 insertions(+)
> create mode 100644 drivers/clk/hisilicon/crg-hi3516cv300.c
> create mode 100644 include/dt-bindings/clock/hi3516cv300-clock.h
>
> diff --git a/drivers/clk/hisilicon/Kconfig b/drivers/clk/hisilicon/Kconfig
> index c41b6d2..f80773b 100644
> --- a/drivers/clk/hisilicon/Kconfig
> +++ b/drivers/clk/hisilicon/Kconfig
> @@ -1,3 +1,11 @@
> +config COMMON_CLK_HI3516CV300
> + tristate "HI3516CV300 Clock Driver"
> + depends on ARCH_HISI || COMPILE_TEST
> + select RESET_HISI
> + default ARCH_HISI
> + help
> + Build the clock driver for hi3516cv300.
> +
> config COMMON_CLK_HI3519
> tristate "Hi3519 Clock Driver"
> depends on ARCH_HISI || COMPILE_TEST
> diff --git a/drivers/clk/hisilicon/Makefile b/drivers/clk/hisilicon/Makefile
> index 5819891..4eec5e5 100644
> --- a/drivers/clk/hisilicon/Makefile
> +++ b/drivers/clk/hisilicon/Makefile
> @@ -7,6 +7,7 @@ obj-y += clk.o clkgate-separated.o clkdivider-hi6220.o
> obj-$(CONFIG_ARCH_HI3xxx) += clk-hi3620.o
> obj-$(CONFIG_ARCH_HIP04) += clk-hip04.o
> obj-$(CONFIG_ARCH_HIX5HD2) += clk-hix5hd2.o
> +obj-$(CONFIG_COMMON_CLK_HI3516CV300) += crg-hi3516cv300.o
> obj-$(CONFIG_COMMON_CLK_HI3519) += clk-hi3519.o
> obj-$(CONFIG_COMMON_CLK_HI3798CV200) += crg-hi3798cv200.o
> obj-$(CONFIG_COMMON_CLK_HI6220) += clk-hi6220.o
> diff --git a/drivers/clk/hisilicon/crg-hi3516cv300.c b/drivers/clk/hisilicon/crg-hi3516cv300.c
> new file mode 100644
> index 0000000..2007123
> --- /dev/null
> +++ b/drivers/clk/hisilicon/crg-hi3516cv300.c
> @@ -0,0 +1,330 @@
> +/*
> + * Hi3516CV300 Clock and Reset Generator Driver
> + *
> + * Copyright (c) 2016 HiSilicon Technologies Co., Ltd.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program. If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include <dt-bindings/clock/hi3516cv300-clock.h>
> +#include <linux/clk-provider.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include "clk.h"
> +#include "crg.h"
> +#include "reset.h"
> +
> +/* hi3516CV300 core CRG */
> +#define HI3516CV300_INNER_CLK_OFFSET 64
> +#define HI3516CV300_FIXED_3M 65
> +#define HI3516CV300_FIXED_6M 66
> +#define HI3516CV300_FIXED_24M 67
> +#define HI3516CV300_FIXED_49P5 68
> +#define HI3516CV300_FIXED_50M 69
> +#define HI3516CV300_FIXED_83P3M 70
> +#define HI3516CV300_FIXED_99M 71
> +#define HI3516CV300_FIXED_100M 72
> +#define HI3516CV300_FIXED_148P5M 73
> +#define HI3516CV300_FIXED_198M 74
> +#define HI3516CV300_FIXED_297M 75
> +#define HI3516CV300_UART_MUX 76
> +#define HI3516CV300_FMC_MUX 77
> +#define HI3516CV300_MMC0_MUX 78
> +#define HI3516CV300_MMC1_MUX 79
> +#define HI3516CV300_MMC2_MUX 80
> +#define HI3516CV300_MMC3_MUX 81
> +#define HI3516CV300_PWM_MUX 82
> +#define HI3516CV300_CRG_NR_CLKS 128
> +
> +static const struct hisi_fixed_rate_clock hi3516cv300_fixed_rate_clks[] = {
> + { HI3516CV300_FIXED_3M, "3m", NULL, 0, 3000000, },
> + { HI3516CV300_FIXED_6M, "6m", NULL, 0, 6000000, },
> + { HI3516CV300_FIXED_24M, "24m", NULL, 0, 24000000, },
> + { HI3516CV300_FIXED_49P5, "49.5m", NULL, 0, 49500000, },
> + { HI3516CV300_FIXED_50M, "50m", NULL, 0, 50000000, },
> + { HI3516CV300_FIXED_83P3M, "83.3m", NULL, 0, 83300000, },
> + { HI3516CV300_FIXED_99M, "99m", NULL, 0, 99000000, },
> + { HI3516CV300_FIXED_100M, "100m", NULL, 0, 100000000, },
> + { HI3516CV300_FIXED_148P5M, "148.5m", NULL, 0, 148500000, },
> + { HI3516CV300_FIXED_198M, "198m", NULL, 0, 198000000, },
> + { HI3516CV300_FIXED_297M, "297m", NULL, 0, 297000000, },
> + { HI3516CV300_APB_CLK, "apb", NULL, 0, 50000000, },
> +};
> +
> +static const char *const uart_mux_p[] = {"24m", "6m"};
> +static const char *const fmc_mux_p[] = {
> + "24m", "83.3m", "148.5m", "198m", "297m"
> +};
> +static const char *const mmc_mux_p[] = {"49.5m"};
> +static const char *const mmc2_mux_p[] = {"99m", "49.5m"};
> +static const char *const pwm_mux_p[] = {"3m", "50m", "24m", "24m"};
> +
> +static u32 uart_mux_table[] = {0, 1};
> +static u32 fmc_mux_table[] = {0, 1, 2, 3, 4};
> +static u32 mmc_mux_table[] = {0};
> +static u32 mmc2_mux_table[] = {0, 2};
> +static u32 pwm_mux_table[] = {0, 1, 2, 3};
> +
> +static const struct hisi_mux_clock hi3516cv300_mux_clks[] = {
> + { HI3516CV300_UART_MUX, "uart_mux", uart_mux_p, ARRAY_SIZE(uart_mux_p),
> + CLK_SET_RATE_PARENT, 0xe4, 19, 1, 0, uart_mux_table, },
> + { HI3516CV300_FMC_MUX, "fmc_mux", fmc_mux_p, ARRAY_SIZE(fmc_mux_p),
> + CLK_SET_RATE_PARENT, 0xc0, 2, 3, 0, fmc_mux_table, },
> + { HI3516CV300_MMC0_MUX, "mmc0_mux", mmc_mux_p, ARRAY_SIZE(mmc_mux_p),
> + CLK_SET_RATE_PARENT, 0xc4, 4, 2, 0, mmc_mux_table, },
> + { HI3516CV300_MMC1_MUX, "mmc1_mux", mmc_mux_p, ARRAY_SIZE(mmc_mux_p),
> + CLK_SET_RATE_PARENT, 0xc4, 12, 2, 0, mmc_mux_table, },
> + { HI3516CV300_MMC2_MUX, "mmc2_mux", mmc2_mux_p, ARRAY_SIZE(mmc2_mux_p),
> + CLK_SET_RATE_PARENT, 0xc4, 20, 2, 0, mmc2_mux_table, },
> + { HI3516CV300_MMC3_MUX, "mmc3_mux", mmc_mux_p, ARRAY_SIZE(mmc_mux_p),
> + CLK_SET_RATE_PARENT, 0xc8, 4, 2, 0, mmc_mux_table, },
> + { HI3516CV300_PWM_MUX, "pwm_mux", pwm_mux_p, ARRAY_SIZE(pwm_mux_p),
> + CLK_SET_RATE_PARENT, 0x38, 2, 2, 0, pwm_mux_table, },
> +};
> +
> +static const struct hisi_gate_clock hi3516cv300_gate_clks[] = {
> +
> + { HI3516CV300_UART0_CLK, "clk_uart0", "uart_mux", CLK_SET_RATE_PARENT,
> + 0xe4, 15, 0, },
> + { HI3516CV300_UART1_CLK, "clk_uart1", "uart_mux", CLK_SET_RATE_PARENT,
> + 0xe4, 16, 0, },
> + { HI3516CV300_UART2_CLK, "clk_uart2", "uart_mux", CLK_SET_RATE_PARENT,
> + 0xe4, 17, 0, },
> +
> + { HI3516CV300_SPI0_CLK, "clk_spi0", "100m", CLK_SET_RATE_PARENT,
> + 0xe4, 13, 0, },
> + { HI3516CV300_SPI1_CLK, "clk_spi1", "100m", CLK_SET_RATE_PARENT,
> + 0xe4, 14, 0, },
> +
> + { HI3516CV300_FMC_CLK, "clk_fmc", "fmc_mux", CLK_SET_RATE_PARENT,
> + 0xc0, 1, 0, },
> + { HI3516CV300_MMC0_CLK, "clk_mmc0", "mmc0_mux", CLK_SET_RATE_PARENT,
> + 0xc4, 1, 0, },
> + { HI3516CV300_MMC1_CLK, "clk_mmc1", "mmc1_mux", CLK_SET_RATE_PARENT,
> + 0xc4, 9, 0, },
> + { HI3516CV300_MMC2_CLK, "clk_mmc2", "mmc2_mux", CLK_SET_RATE_PARENT,
> + 0xc4, 17, 0, },
> + { HI3516CV300_MMC3_CLK, "clk_mmc3", "mmc3_mux", CLK_SET_RATE_PARENT,
> + 0xc8, 1, 0, },
> +
> + { HI3516CV300_ETH_CLK, "clk_eth", NULL, 0, 0xec, 1, 0, },
> +
> + { HI3516CV300_DMAC_CLK, "clk_dmac", NULL, 0, 0xd8, 5, 0, },
> + { HI3516CV300_PWM_CLK, "clk_pwm", "pwm_mux", CLK_SET_RATE_PARENT,
> + 0x38, 1, 0, },
> +
> + { HI3516CV300_USB2_BUS_CLK, "clk_usb2_bus", NULL, 0, 0xb8, 0, 0, },
> + { HI3516CV300_USB2_OHCI48M_CLK, "clk_usb2_ohci48m", NULL, 0,
> + 0xb8, 1, 0, },
> + { HI3516CV300_USB2_OHCI12M_CLK, "clk_usb2_ohci12m", NULL, 0,
> + 0xb8, 2, 0, },
> + { HI3516CV300_USB2_OTG_UTMI_CLK, "clk_usb2_otg_utmi", NULL, 0,
> + 0xb8, 3, 0, },
> + { HI3516CV300_USB2_HST_PHY_CLK, "clk_usb2_hst_phy", NULL, 0,
> + 0xb8, 4, 0, },
> + { HI3516CV300_USB2_UTMI0_CLK, "clk_usb2_utmi0", NULL, 0, 0xb8, 5, 0, },
> + { HI3516CV300_USB2_PHY_CLK, "clk_usb2_phy", NULL, 0, 0xb8, 7, 0, },
> +};
> +
> +static struct hisi_clock_data *hi3516cv300_clk_register(
> + struct platform_device *pdev)
> +{
> + struct hisi_clock_data *clk_data;
> + int ret;
> +
> + clk_data = hisi_clk_alloc(pdev, HI3516CV300_CRG_NR_CLKS);
> + if (!clk_data)
> + return ERR_PTR(-ENOMEM);
> +
> + ret = hisi_clk_register_fixed_rate(hi3516cv300_fixed_rate_clks,
> + ARRAY_SIZE(hi3516cv300_fixed_rate_clks), clk_data);
> + if (ret)
> + return ERR_PTR(ret);
> +
> + ret = hisi_clk_register_mux(hi3516cv300_mux_clks,
> + ARRAY_SIZE(hi3516cv300_mux_clks), clk_data);
> + if (ret)
> + goto unregister_fixed_rate;
> +
> + ret = hisi_clk_register_gate(hi3516cv300_gate_clks,
> + ARRAY_SIZE(hi3516cv300_gate_clks), clk_data);
> + if (ret)
> + goto unregister_mux;
> +
> + ret = of_clk_add_provider(pdev->dev.of_node,
> + of_clk_src_onecell_get, &clk_data->clk_data);
> + if (ret)
> + goto unregister_gate;
> +
> + return clk_data;
> +
> +unregister_gate:
> + hisi_clk_unregister_gate(hi3516cv300_gate_clks,
> + ARRAY_SIZE(hi3516cv300_gate_clks), clk_data);
> +unregister_mux:
> + hisi_clk_unregister_mux(hi3516cv300_mux_clks,
> + ARRAY_SIZE(hi3516cv300_mux_clks), clk_data);
> +unregister_fixed_rate:
> + hisi_clk_unregister_fixed_rate(hi3516cv300_fixed_rate_clks,
> + ARRAY_SIZE(hi3516cv300_fixed_rate_clks), clk_data);
> + return ERR_PTR(ret);
> +}
> +
> +static void hi3516cv300_clk_unregister(struct platform_device *pdev)
> +{
> + struct hisi_crg_dev *crg = platform_get_drvdata(pdev);
> +
> + of_clk_del_provider(pdev->dev.of_node);
> +
> + hisi_clk_unregister_gate(hi3516cv300_gate_clks,
> + ARRAY_SIZE(hi3516cv300_gate_clks), crg->clk_data);
> + hisi_clk_unregister_mux(hi3516cv300_mux_clks,
> + ARRAY_SIZE(hi3516cv300_mux_clks), crg->clk_data);
> + hisi_clk_unregister_fixed_rate(hi3516cv300_fixed_rate_clks,
> + ARRAY_SIZE(hi3516cv300_fixed_rate_clks), crg->clk_data);
> +}
> +
> +static const struct hisi_crg_funcs hi3516cv300_crg_funcs = {
> + .register_clks = hi3516cv300_clk_register,
> + .unregister_clks = hi3516cv300_clk_unregister,
> +};
> +
> +/* hi3516CV300 sysctrl CRG */
> +#define HI3516CV300_SYSCTRL_NR_CLKS 16
> +
> +static const char *wdt_mux_p[] __initconst = { "3m", "apb" };
> +static u32 wdt_mux_table[] = {0, 1};
> +
> +static const struct hisi_mux_clock hi3516cv300_sysctrl_mux_clks[] = {
> + { HI3516CV300_WDT_CLK, "wdt", wdt_mux_p, ARRAY_SIZE(wdt_mux_p),
> + CLK_SET_RATE_PARENT, 0x0, 23, 1, 0, wdt_mux_table, },
> +};
> +
> +static struct hisi_clock_data *hi3516cv300_sysctrl_clk_register(
> + struct platform_device *pdev)
> +{
> + struct hisi_clock_data *clk_data;
> + int ret;
> +
> + clk_data = hisi_clk_alloc(pdev, HI3516CV300_SYSCTRL_NR_CLKS);
> + if (!clk_data)
> + return ERR_PTR(-ENOMEM);
> +
> + ret = hisi_clk_register_mux(hi3516cv300_sysctrl_mux_clks,
> + ARRAY_SIZE(hi3516cv300_sysctrl_mux_clks), clk_data);
> + if (ret)
> + return ERR_PTR(ret);
> +
> +
> + ret = of_clk_add_provider(pdev->dev.of_node,
> + of_clk_src_onecell_get, &clk_data->clk_data);
> + if (ret)
> + goto unregister_mux;
> +
> + return clk_data;
> +
> +unregister_mux:
> + hisi_clk_unregister_mux(hi3516cv300_sysctrl_mux_clks,
> + ARRAY_SIZE(hi3516cv300_sysctrl_mux_clks), clk_data);
> + return ERR_PTR(ret);
> +}
> +
> +static void hi3516cv300_sysctrl_clk_unregister(struct platform_device *pdev)
> +{
> + struct hisi_crg_dev *crg = platform_get_drvdata(pdev);
> +
> + of_clk_del_provider(pdev->dev.of_node);
> +
> + hisi_clk_unregister_mux(hi3516cv300_sysctrl_mux_clks,
> + ARRAY_SIZE(hi3516cv300_sysctrl_mux_clks),
> + crg->clk_data);
> +}
> +
> +static const struct hisi_crg_funcs hi3516cv300_sysctrl_funcs = {
> + .register_clks = hi3516cv300_sysctrl_clk_register,
> + .unregister_clks = hi3516cv300_sysctrl_clk_unregister,
> +};
> +
> +static const struct of_device_id hi3516cv300_crg_match_table[] = {
> + {
> + .compatible = "hisilicon,hi3516cv300-crg",
> + .data = &hi3516cv300_crg_funcs
> + },
> + {
> + .compatible = "hisilicon,hi3516cv300-sysctrl",
> + .data = &hi3516cv300_sysctrl_funcs
> + },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, hi3516cv300_crg_match_table);
> +
> +static int hi3516cv300_crg_probe(struct platform_device *pdev)
> +{
> + struct hisi_crg_dev *crg;
> +
> + crg = devm_kmalloc(&pdev->dev, sizeof(*crg), GFP_KERNEL);
> + if (!crg)
> + return -ENOMEM;
> +
> + crg->funcs = of_device_get_match_data(&pdev->dev);
> + if (!crg->funcs)
> + return -ENOENT;
> +
> + crg->rstc = hisi_reset_init(pdev);
> + if (!crg->rstc)
> + return -ENOMEM;
> +
> + crg->clk_data = crg->funcs->register_clks(pdev);
> + if (IS_ERR(crg->clk_data)) {
> + hisi_reset_exit(crg->rstc);
> + return PTR_ERR(crg->clk_data);
> + }
> +
> + platform_set_drvdata(pdev, crg);
> + return 0;
> +}
> +
> +static int hi3516cv300_crg_remove(struct platform_device *pdev)
> +{
> + struct hisi_crg_dev *crg = platform_get_drvdata(pdev);
> +
> + hisi_reset_exit(crg->rstc);
> + crg->funcs->unregister_clks(pdev);
> + return 0;
> +}
> +
> +static struct platform_driver hi3516cv300_crg_driver = {
> + .probe = hi3516cv300_crg_probe,
> + .remove = hi3516cv300_crg_remove,
> + .driver = {
> + .name = "hi3516cv300-crg",
> + .of_match_table = hi3516cv300_crg_match_table,
> + },
> +};
> +
> +static int __init hi3516cv300_crg_init(void)
> +{
> + return platform_driver_register(&hi3516cv300_crg_driver);
> +}
> +core_initcall(hi3516cv300_crg_init);
> +
> +static void __exit hi3516cv300_crg_exit(void)
> +{
> + platform_driver_unregister(&hi3516cv300_crg_driver);
> +}
> +module_exit(hi3516cv300_crg_exit);
> +
> +MODULE_LICENSE("GPL v2");
> +MODULE_DESCRIPTION("HiSilicon Hi3516CV300 CRG Driver");
> diff --git a/include/dt-bindings/clock/hi3516cv300-clock.h b/include/dt-bindings/clock/hi3516cv300-clock.h
> new file mode 100644
> index 0000000..5ba51b8
> --- /dev/null
> +++ b/include/dt-bindings/clock/hi3516cv300-clock.h
> @@ -0,0 +1,48 @@
> +/*
> + * Copyright (c) 2016 HiSilicon Technologies Co., Ltd.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program. If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#ifndef __DTS_HI3516CV300_CLOCK_H
> +#define __DTS_HI3516CV300_CLOCK_H
> +
> +/* hi3516CV300 core CRG */
> +#define HI3516CV300_APB_CLK 0
> +#define HI3516CV300_UART0_CLK 1
> +#define HI3516CV300_UART1_CLK 2
> +#define HI3516CV300_UART2_CLK 3
> +#define HI3516CV300_SPI0_CLK 4
> +#define HI3516CV300_SPI1_CLK 5
> +#define HI3516CV300_FMC_CLK 6
> +#define HI3516CV300_MMC0_CLK 7
> +#define HI3516CV300_MMC1_CLK 8
> +#define HI3516CV300_MMC2_CLK 9
> +#define HI3516CV300_MMC3_CLK 10
> +#define HI3516CV300_ETH_CLK 11
> +#define HI3516CV300_ETH_MACIF_CLK 12
> +#define HI3516CV300_DMAC_CLK 13
> +#define HI3516CV300_PWM_CLK 14
> +#define HI3516CV300_USB2_BUS_CLK 15
> +#define HI3516CV300_USB2_OHCI48M_CLK 16
> +#define HI3516CV300_USB2_OHCI12M_CLK 17
> +#define HI3516CV300_USB2_OTG_UTMI_CLK 18
> +#define HI3516CV300_USB2_HST_PHY_CLK 19
> +#define HI3516CV300_USB2_UTMI0_CLK 20
> +#define HI3516CV300_USB2_PHY_CLK 21
> +
> +/* hi3516CV300 sysctrl CRG */
> +#define HI3516CV300_WDT_CLK 1
> +
> +#endif /* __DTS_HI3516CV300_CLOCK_H */
> --
> 1.9.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH] ARM: BCM5301X: Add back handler ignoring external imprecise aborts
From: Rafał Miłecki @ 2016-10-29 11:12 UTC (permalink / raw)
To: Florian Fainelli,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
Cc: Arnd Bergmann, Bjorn Helgaas, Lucas Stach, Hauke Mehrtens,
Jon Mason, Mark Rutland, Ray Jui, Scott Branden,
bcm-kernel-feedback-list-dY08KVG/lbpWk0Htik3J/w,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-pci-u79uwXL29TY76Z2rM5mHXA, Rafał Miłecki
From: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
Since early BCM5301X days we got abort handler that was removed by
commit 937b12306ea79 ("ARM: BCM5301X: remove workaround imprecise abort
fault handler"). It assumed we need to deal only with pending aborts
left by the bootloader. Unfortunately this isn't true for BCM5301X.
When probing PCI config space (device enumeration) it is expected to
have master aborts on the PCI bus. Most bridges don't forward (or they
allow disabling it) these errors onto the AXI/AMBA bus but not the
Northstar (BCM5301X) one.
iProc PCIe controller on Northstar seems to be some older one, without
a control register for errors forwarding. It means we need to workaround
this at platform level. All newer platforms are not affected by this
issue.
Signed-off-by: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
---
arch/arm/mach-bcm/bcm_5301x.c | 28 ++++++++++++++++++++++++++++
1 file changed, 28 insertions(+)
diff --git a/arch/arm/mach-bcm/bcm_5301x.c b/arch/arm/mach-bcm/bcm_5301x.c
index c8830a2..fe067f6 100644
--- a/arch/arm/mach-bcm/bcm_5301x.c
+++ b/arch/arm/mach-bcm/bcm_5301x.c
@@ -9,14 +9,42 @@
#include <asm/hardware/cache-l2x0.h>
#include <asm/mach/arch.h>
+#include <asm/siginfo.h>
+#include <asm/signal.h>
+
+#define FSR_EXTERNAL (1 << 12)
+#define FSR_READ (0 << 10)
+#define FSR_IMPRECISE 0x0406
static const char *const bcm5301x_dt_compat[] __initconst = {
"brcm,bcm4708",
NULL,
};
+static int bcm5301x_abort_handler(unsigned long addr, unsigned int fsr,
+ struct pt_regs *regs)
+{
+ /*
+ * We want to ignore aborts forwarded from the PCIe bus that are
+ * expected and shouldn't really be passed by the PCIe controller.
+ * The biggest disadvantage is the same FSR code may be reported when
+ * reading non-existing APB register and we shouldn't ignore that.
+ */
+ if (fsr == (FSR_EXTERNAL | FSR_READ | FSR_IMPRECISE))
+ return 0;
+
+ return 1;
+}
+
+static void __init bcm5301x_init_early(void)
+{
+ hook_fault_code(16 + 6, bcm5301x_abort_handler, SIGBUS, BUS_OBJERR,
+ "imprecise external abort");
+}
+
DT_MACHINE_START(BCM5301X, "BCM5301X")
.l2c_aux_val = 0,
.l2c_aux_mask = ~0,
.dt_compat = bcm5301x_dt_compat,
+ .init_early = bcm5301x_init_early,
MACHINE_END
--
2.9.3
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH v3 2/2] ARM: dts: sun6i: hummingbird-a31: Enable display output through VGA bridge
From: Chen-Yu Tsai @ 2016-10-29 11:06 UTC (permalink / raw)
To: Maxime Ripard, David Airlie, Rob Herring, Mark Rutland
Cc: Chen-Yu Tsai, dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw
In-Reply-To: <20161029110611.28951-1-wens-jdAy2FN1RRM@public.gmane.org>
The Hummingbird A31 board has a VGA DAC which converts RGB output
from the LCD interface to VGA analog signals.
Add nodes for the VGA DAC, its power supply, and enable this part
of the display pipeline.
Signed-off-by: Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org>
---
arch/arm/boot/dts/sun6i-a31-hummingbird.dts | 76 +++++++++++++++++++++++++++++
1 file changed, 76 insertions(+)
diff --git a/arch/arm/boot/dts/sun6i-a31-hummingbird.dts b/arch/arm/boot/dts/sun6i-a31-hummingbird.dts
index 9a74637f677f..1ab28b2108fe 100644
--- a/arch/arm/boot/dts/sun6i-a31-hummingbird.dts
+++ b/arch/arm/boot/dts/sun6i-a31-hummingbird.dts
@@ -63,6 +63,62 @@
stdout-path = "serial0:115200n8";
};
+ vga-connector {
+ compatible = "vga-connector";
+
+ port {
+ vga_con_in: endpoint {
+ remote-endpoint = <&vga_dac_out>;
+ };
+ };
+ };
+
+ vga-dac {
+ compatible = "dumb-vga-dac";
+ vdd-supply = <®_vga_3v3>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0>;
+
+ vga_dac_in: endpoint@0 {
+ reg = <0>;
+ remote-endpoint = <&tcon0_out_vga>;
+ };
+ };
+
+ port@1 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <1>;
+
+ vga_dac_out: endpoint@0 {
+ reg = <0>;
+ remote-endpoint = <&vga_con_in>;
+ };
+ };
+ };
+ };
+
+ reg_vga_3v3: vga_3v3_regulator {
+ compatible = "regulator-fixed";
+ pinctrl-names = "default";
+ pinctrl-0 = <&vga_3v3_enable_pin_hummingbird>;
+ regulator-name = "vga-3v3";
+ regulator-min-microvolt = <3300000>;
+ regulator-max-microvolt = <3300000>;
+ regulator-boot-on;
+ enable-active-high;
+ gpio = <&pio 7 25 GPIO_ACTIVE_HIGH>; /* PH25 */
+ };
+
wifi_pwrseq: wifi_pwrseq {
compatible = "mmc-pwrseq-simple";
reset-gpios = <&pio 6 10 GPIO_ACTIVE_LOW>; /* PG10 */
@@ -166,6 +222,13 @@
allwinner,pull = <SUN4I_PINCTRL_PULL_UP>;
};
+ vga_3v3_enable_pin_hummingbird: vga_3v3_enable_pin {
+ allwinner,pins = "PH25";
+ allwinner,function = "gpio_out";
+ allwinner,drive = <SUN4I_PINCTRL_10_MA>;
+ allwinner,pull = <SUN4I_PINCTRL_NO_PULL>;
+ };
+
wifi_reset_pin_hummingbird: wifi_reset_pin@0 {
allwinner,pins = "PG10";
allwinner,function = "gpio_out";
@@ -245,6 +308,19 @@
status = "okay";
};
+&tcon0 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&lcd0_rgb888_pins>;
+ status = "okay";
+};
+
+&tcon0_out {
+ tcon0_out_vga: endpoint@0 {
+ reg = <0>;
+ remote-endpoint = <&vga_dac_in>;
+ };
+};
+
&uart0 {
pinctrl-names = "default";
pinctrl-0 = <&uart0_pins_a>;
--
2.9.3
^ permalink raw reply related
* [PATCH v3 1/2] drm/bridge: dumb-vga-dac: Support a VDD regulator supply
From: Chen-Yu Tsai @ 2016-10-29 11:06 UTC (permalink / raw)
To: Maxime Ripard, David Airlie, Rob Herring, Mark Rutland
Cc: Chen-Yu Tsai, dri-devel, linux-arm-kernel, linux-kernel,
devicetree, linux-sunxi
In-Reply-To: <20161029110611.28951-1-wens@csie.org>
Some dumb VGA DACs are active components which require external power.
Add support for specifying a regulator as its power supply.
Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
.../bindings/display/bridge/dumb-vga-dac.txt | 2 ++
drivers/gpu/drm/bridge/dumb-vga-dac.c | 35 ++++++++++++++++++++++
2 files changed, 37 insertions(+)
diff --git a/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt b/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
index 003bc246a270..164cbb15f04c 100644
--- a/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
+++ b/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
@@ -16,6 +16,8 @@ graph bindings specified in Documentation/devicetree/bindings/graph.txt.
- Video port 0 for RGB input
- Video port 1 for VGA output
+Optional properties:
+- vdd-supply: Power supply for DAC
Example
-------
diff --git a/drivers/gpu/drm/bridge/dumb-vga-dac.c b/drivers/gpu/drm/bridge/dumb-vga-dac.c
index afec232185a7..59781e031220 100644
--- a/drivers/gpu/drm/bridge/dumb-vga-dac.c
+++ b/drivers/gpu/drm/bridge/dumb-vga-dac.c
@@ -12,6 +12,7 @@
#include <linux/module.h>
#include <linux/of_graph.h>
+#include <linux/regulator/consumer.h>
#include <drm/drmP.h>
#include <drm/drm_atomic_helper.h>
@@ -23,6 +24,7 @@ struct dumb_vga {
struct drm_connector connector;
struct i2c_adapter *ddc;
+ struct regulator *vdd;
};
static inline struct dumb_vga *
@@ -124,8 +126,33 @@ static int dumb_vga_attach(struct drm_bridge *bridge)
return 0;
}
+static void dumb_vga_enable(struct drm_bridge *bridge)
+{
+ struct dumb_vga *vga = drm_bridge_to_dumb_vga(bridge);
+ int ret;
+
+ if (!IS_ERR(vga->vdd)) {
+ ret = regulator_enable(vga->vdd);
+
+ if (ret) {
+ DRM_ERROR("Failed to enable vdd regulator: %d\n", ret);
+ return;
+ }
+ }
+}
+
+static void dumb_vga_disable(struct drm_bridge *bridge)
+{
+ struct dumb_vga *vga = drm_bridge_to_dumb_vga(bridge);
+
+ if (!IS_ERR(vga->vdd))
+ regulator_disable(vga->vdd);
+}
+
static const struct drm_bridge_funcs dumb_vga_bridge_funcs = {
.attach = dumb_vga_attach,
+ .enable = dumb_vga_enable,
+ .disable = dumb_vga_disable,
};
static struct i2c_adapter *dumb_vga_retrieve_ddc(struct device *dev)
@@ -169,6 +196,14 @@ static int dumb_vga_probe(struct platform_device *pdev)
return -ENOMEM;
platform_set_drvdata(pdev, vga);
+ vga->vdd = devm_regulator_get_optional(&pdev->dev, "vdd");
+ if (IS_ERR(vga->vdd)) {
+ ret = PTR_ERR(vga->vdd);
+ if (ret == -EPROBE_DEFER)
+ return -EPROBE_DEFER;
+ dev_dbg(&pdev->dev, "No vdd regulator found: %d\n", ret);
+ }
+
vga->ddc = dumb_vga_retrieve_ddc(&pdev->dev);
if (IS_ERR(vga->ddc)) {
if (PTR_ERR(vga->ddc) == -ENODEV) {
--
2.9.3
^ permalink raw reply related
* [PATCH v3 0/2] ARM: dts: sun6i: hummingbird-a31: Enable display output through VGA bridge
From: Chen-Yu Tsai @ 2016-10-29 11:06 UTC (permalink / raw)
To: Maxime Ripard, David Airlie, Rob Herring, Mark Rutland
Cc: Chen-Yu Tsai, dri-devel, linux-arm-kernel, linux-kernel,
devicetree, linux-sunxi
Hi,
This is v3 of my sun6i DRM/KMS display enablement series. Most of the
patches in v2 were merged, leaving only the dumb-vga-dac patch and the
hummingbird-a31 VGA output patch.
A few things were changed, mainly the DAC enable method from a GPIO
to a regulator supply.
Changes since v2:
- Changed the enable-gpio of dumb-vga-dac to vdd-supply regulator.
This better matches the hardware that I have: the DAC has a fixed
regulator dropping the voltage from the board-wide 5V to 3.3V the
DAC uses. The regulator is controlled through a GPIO pin.
- Renamed the node of the VGA connector from "vga" to "vga-connector".
- Renamed the node of the VGA DAC from "bridge" to "vga-dac".
Regards
ChenYu
Chen-Yu Tsai (2):
drm/bridge: dumb-vga-dac: Support a VDD regulator supply
ARM: dts: sun6i: hummingbird-a31: Enable display output through VGA
bridge
.../bindings/display/bridge/dumb-vga-dac.txt | 2 +
arch/arm/boot/dts/sun6i-a31-hummingbird.dts | 76 ++++++++++++++++++++++
drivers/gpu/drm/bridge/dumb-vga-dac.c | 35 ++++++++++
3 files changed, 113 insertions(+)
--
2.9.3
^ permalink raw reply
* Re: [PATCH v12 RESEND 0/4] generic TEE subsystem
From: Jens Wiklander @ 2016-10-29 9:46 UTC (permalink / raw)
To: Andrew F. Davis
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA, Greg Kroah-Hartman, Al Viro,
valentin.manea-hv44wF8Li93QT0dZR+AlfA,
jean-michel.delorme-qxv4g6HH51o, emmanuel.michel-qxv4g6HH51o,
javier-5MUHepqpBA1BDgjK7y7TUQ, Jason Gunthorpe, Mark Rutland,
Michal Simek, Rob Herring, Will Deacon, Arnd Bergmann,
Nishanth Menon, broonie-DgEjT+Ai2ygdnm+yROfE0A,
vlad.babchuk-Re5JQEeQqe8AvxtiuMwx3w
In-Reply-To: <1e532aeb-4944-62e4-c8c4-1e23438b92cd-l0cyMroinI0@public.gmane.org>
On Fri, Oct 28, 2016 at 10:43:24AM -0500, Andrew F. Davis wrote:
> On 10/28/2016 05:19 AM, Jens Wiklander wrote:
> > Hi,
> >
> > This patch set introduces a generic TEE subsystem. The TEE subsystem will
> > contain drivers for various TEE implementations. A TEE (Trusted Execution
> > Environment) is a trusted OS running in some secure environment, for
> > example, TrustZone on ARM CPUs, or a separate secure co-processor etc.
> >
> > Regarding use cases, TrustZone has traditionally been used for
> > offloading secure tasks to the secure world. Examples include:
> > - Secure key handling where the OS may or may not have direct access to key
> > material.
> > - E-commerce and payment technologies. Credentials, credit card numbers etc
> > could be stored in a more secure environment.
> > - Trusted User Interface (TUI) to ensure that no-one can snoop PIN-codes
> > etc.
> > - Secure boot to ensure that loaded binaries haven’t been tampered with.
> > It’s not strictly needed for secure boot, but you could enhance security
> > by leveraging a TEE during boot.
> > - Digital Rights Management (DRM), the studios provides content with
> > different resolution depending on the security of the device. Higher
> > security means higher resolution.
> >
> > A TEE could also be used in existing and new technologies. For example IMA
> > (Integrity Measurement Architecture) which has been in the kernel for quite
> > a while. Today you can enhance security by using a TPM-chip to sign the IMA
> > measurement list. This is something that you also could do by leveraging a
> > TEE.
> >
> > Another example could be in 2-factor authentication which is becoming
> > increasingly more important. FIDO (https://fidoalliance.org) for example
> > are using public key cryptography in their 2-factor authentication standard
> > (U2F). With FIDO, a private and public key pair will be generated for every
> > site you visit and the private key should never leave the local device.
> > This is an example where you could use secure storage in a TEE for the
> > private key.
> >
> > Today you will find a quite a few different out of tree implementations of
> > TEE drivers which tends to fragment the TEE ecosystem and development. We
> > think it would be a good idea to have a generic TEE driver integrated in
> > the kernel which would serve as a base for several different TEE solutions,
> > no matter if they are on-chip like TrustZone or if they are on a separate
> > crypto co-processor.
> >
> > To develop this TEE subsystem we have been using the open source TEE called
> > OP-TEE (https://github.com/OP-TEE/optee_os) and therefore this would be the
> > first TEE solution supported by this new subsystem. OP-TEE is a
> > GlobalPlatform compliant TEE, however this TEE subsystem is not limited to
> > only GlobalPlatform TEEs, instead we have tried to design it so that it
> > should work with other TEE solutions also.
> >
>
> The above is my biggest concern with this whole subsystem, to me it
> still feels very OPTEE specific. As much as I would love to believe
> OPTEE will be the end-all TEE, I'm sure we soon will start to see wider
> use of vendor TEEs (like TI's own legacy Trustzone thing we are hoping
> to depreciate with OPTEE moving forward), possibly Google's Trusty TEE,
> and whatever Intel/AMD are cooking up for x86.
I'd rather say that it's slightly GlobalPlatform specific, but a bit
more flexible.
>
> As we all know when things are upstreamed we lose the ability to make
> radical changes easily, especially to full subsystems. What happens when
> this framework, built with only one existing TEE, built by the one
> existing TEE's devs, is not as flexible as we need when other TEEs start
> rolling out?
Initially the TEE subsystem was much more flexible and was criticized
for that.
>
> Do we see this as a chicken and egg situation, or is there any harm
> beyond the pains of supporting an out-of-tree driver for a while, to
> wait until we have at least one other TEE to add to this subsystem
> before merging?
This proposal is the bare minimum to have something useful. On top of
this there's more things we'd like to add, for example an in-kernel API
for accessing the TEE and secure buffer handling. The way we're dealing
with shared memory need to be improved to better support multiple guests
communicating with one TEE.
What we can do now with the subsystem now is somewhat limited by the
fact that we're trying to upstream it and want to do that it in
manageable increments.
Thanks,
Jens
>
> This may also help with the perceived lack of reviewers for this series.
>
> Thanks,
> Andrew
>
> > "tee: generic TEE subsystem" brings in the generic TEE subsystem which
> > helps when writing a driver for a specific TEE, for example, OP-TEE.
> >
> > "tee: add OP-TEE driver" is an OP-TEE driver which uses the subsystem to do
> > its work.
> >
> > This patch set has been prepared in cooperation with Javier González who
> > proposed "Generic TrustZone Driver in Linux Kernel" patches 28 Nov 2014,
> > https://lwn.net/Articles/623380/ . We've since then changed the scope to
> > TEE instead of TrustZone.
> >
> > We have discussed the design on tee-dev-cunTk1MwBs8s++Sfvej+rw@public.gmane.org (archive at
> > https://lists.linaro.org/pipermail/tee-dev/) with people from other
> > companies, including Valentin Manea <valentin.manea-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>,
> > Emmanuel MICHEL <emmanuel.michel-qxv4g6HH51o@public.gmane.org>,
> > Jean-michel DELORME <jean-michel.delorme-qxv4g6HH51o@public.gmane.org>,
> > and Joakim Bech <joakim.bech-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>. Our main concern has been to
> > agree on something that is generic enough to support many different
> > TEEs while still keeping the interface together.
> >
> > v12-resend:
> > * Rebased on v4.9-rc2
> >
> > v12:
> > * Rebased on v4.8-rc5
> > * Addressed review comments from Andrew F. Davis
> > * Removed Acked-by: Andreas Dannenberg <dannenberg-l0cyMroinI0@public.gmane.org> as the
> > mail bounces
> > * Bugfix possible null dereference in error cleanup path of
> > optee_probe().
> > * Bugfix optee_from_msg_param() when calculating offset of memref
> > into a shared memory object
> >
> > v11:
> > * Rebased on v4.8-rc3
> > * Addressed review comments from Nishanth Menon
> > * Made the TEE framework available as a loadable module.
> > * Reviewed-by: Javier González <javier-5MUHepqpBA1BDgjK7y7TUQ@public.gmane.org>
> > * Zeroes shared memory on allocation to avoid information leakage
> > * Links shared memory objects to context to avoid stealing of shared memory
> > object from an unrelated process
> > * Allow RPC interruption if supplicant is unavailable
> >
> > v10:
> > * Rebased on v4.7-rc1
> > * Addressed private review comments from Nishanth Menon
> > * Optee driver only accepts one supplicant process on the privileged device
> > * Optee driver avoids long delayed releases of shm objects
> > * Added more comments on functions and structs
> >
> > v9:
> > * Rebased on v4.6-rc1
> > * Acked-by: Andreas Dannenberg <dannenberg-l0cyMroinI0@public.gmane.org>
> > * Addressed comments from Al Viro how file descriptors are passed to
> > user space
> > * Addressed comments from Randy Dunlap on documentation
> > * Changed license for include/uapi/linux/tee.h
> >
> > v8:
> > * Rebased on v4.5-rc3
> > * dt/bindings: add bindings for optee
> > Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> > * Fixes build error for X86
> > * Fixes spell error in "dt/bindings: add bindings for optee"
> >
> > v7:
> > * Rebased on v4.5-rc2
> > * Moved the ARM SMC Calling Convention support into a separate patch
> > set, which is now merged
> >
> > v6:
> > * Rebased on v4.3-rc7
> > * Changed smccc interface to let the compiler marshal most of the
> > parameters
> > * Added ARCH64 capability for smccc interface
> > * Changed the PSCI firmware calls (both arm and arm64) to use the new
> > generic smccc interface instead instead of own assembly functions.
> > * Move optee DT bindings to below arm/firmware
> > * Defines method for OP-TEE driver to call secure world in DT, smc or hvc
> > * Exposes implementation id of a TEE driver in sysfs
> > to easily spawn corresponding tee-supplicant when device is ready
> > * Update OP-TEE Message Protocol to better cope with fragmented physical
> > memory
> > * Read time directly from OP-TEE driver instead of forwarding the RPC
> > request to tee-supplicant
> >
> > v5:
> > * Replaced kref reference counting for the device with a size_t instead as
> > the counter is always protected by a mutex
> >
> > v4:
> > * Rebased on 4.1
> > * Redesigned the synchronization around entry exit of normal SMC
> > * Replaced rwsem on the driver instance with kref and completion since
> > rwsem wasn't intended to be used in this way
> > * Expanded the TEE_IOCTL_PARAM_ATTR_TYPE_MASK to make room for
> > future additional parameter types
> > * Documents TEE subsystem and OP-TEE driver
> > * Replaced TEE_IOC_CMD with TEE_IOC_OPEN_SESSION, TEE_IOC_INVOKE,
> > TEE_IOC_CANCEL and TEE_IOC_CLOSE_SESSION
> > * DT bindings in a separate patch
> > * Assembly parts moved to arch/arm and arch/arm64 respectively, in a
> > separate patch
> > * Redefined/clarified the meaning of OPTEE_SMC_SHM_CACHED
> > * Removed CMA usage to limit the scope of the patch set
> >
> > v3:
> > * Rebased on 4.1-rc3 (dma_buf_export() API change)
> > * A couple of small sparse fixes
> > * Documents bindings for OP-TEE driver
> > * Updated MAINTAINERS
> >
> > v2:
> > * Replaced the stubbed OP-TEE driver with a real OP-TEE driver
> > * Removed most APIs not needed by OP-TEE in current state
> > * Update Documentation/ioctl/ioctl-number.txt with correct path to tee.h
> > * Rename tee_shm_pool_alloc_cma() to tee_shm_pool_alloc()
> > * Moved tee.h into include/uapi/linux/
> > * Redefined tee.h IOCTL macros to be directly based on _IOR and friends
> > * Removed version info on the API to user space, a data blob which
> > can contain an UUID is left for user space to be able to tell which
> > protocol to use in TEE_IOC_CMD
> > * Changed user space exposed structures to only have types with __ prefix
> > * Dropped THIS_MODULE from tee_fops
> > * Reworked how the driver is registered and ref counted:
> > - moved from using an embedded struct miscdevice to an embedded struct
> > device.
> > - uses an struct rw_semaphore as synchronization for driver detachment
> > - uses alloc/register pattern from TPM
> >
> > Thanks,
> > Jens
> >
> > Jens Wiklander (4):
> > dt/bindings: add bindings for optee
> > tee: generic TEE subsystem
> > tee: add OP-TEE driver
> > Documentation: tee subsystem and op-tee driver
> >
> > Documentation/00-INDEX | 2 +
> > .../bindings/arm/firmware/linaro,optee-tz.txt | 31 +
> > .../devicetree/bindings/vendor-prefixes.txt | 1 +
> > Documentation/ioctl/ioctl-number.txt | 1 +
> > Documentation/tee.txt | 118 +++
> > MAINTAINERS | 13 +
> > drivers/Kconfig | 2 +
> > drivers/Makefile | 1 +
> > drivers/tee/Kconfig | 18 +
> > drivers/tee/Makefile | 5 +
> > drivers/tee/optee/Kconfig | 7 +
> > drivers/tee/optee/Makefile | 5 +
> > drivers/tee/optee/call.c | 435 ++++++++++
> > drivers/tee/optee/core.c | 598 ++++++++++++++
> > drivers/tee/optee/optee_msg.h | 435 ++++++++++
> > drivers/tee/optee/optee_private.h | 185 +++++
> > drivers/tee/optee/optee_smc.h | 446 ++++++++++
> > drivers/tee/optee/rpc.c | 404 +++++++++
> > drivers/tee/optee/supp.c | 273 +++++++
> > drivers/tee/tee_core.c | 901 +++++++++++++++++++++
> > drivers/tee/tee_private.h | 129 +++
> > drivers/tee/tee_shm.c | 357 ++++++++
> > drivers/tee/tee_shm_pool.c | 158 ++++
> > include/linux/tee_drv.h | 278 +++++++
> > include/uapi/linux/tee.h | 403 +++++++++
> > 25 files changed, 5206 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/arm/firmware/linaro,optee-tz.txt
> > create mode 100644 Documentation/tee.txt
> > create mode 100644 drivers/tee/Kconfig
> > create mode 100644 drivers/tee/Makefile
> > create mode 100644 drivers/tee/optee/Kconfig
> > create mode 100644 drivers/tee/optee/Makefile
> > create mode 100644 drivers/tee/optee/call.c
> > create mode 100644 drivers/tee/optee/core.c
> > create mode 100644 drivers/tee/optee/optee_msg.h
> > create mode 100644 drivers/tee/optee/optee_private.h
> > create mode 100644 drivers/tee/optee/optee_smc.h
> > create mode 100644 drivers/tee/optee/rpc.c
> > create mode 100644 drivers/tee/optee/supp.c
> > create mode 100644 drivers/tee/tee_core.c
> > create mode 100644 drivers/tee/tee_private.h
> > create mode 100644 drivers/tee/tee_shm.c
> > create mode 100644 drivers/tee/tee_shm_pool.c
> > create mode 100644 include/linux/tee_drv.h
> > create mode 100644 include/uapi/linux/tee.h
> >
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v3 1/6] pinctrl: sunxi: Deal with configless pins
From: Linus Walleij @ 2016-10-29 9:08 UTC (permalink / raw)
To: Maxime Ripard
Cc: Chen-Yu Tsai, linux-gpio@vger.kernel.org,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, Rob Herring
In-Reply-To: <ae812275bfb0f0ded2a73b571bfcecffcd747126.1476971126.git-series.maxime.ripard@free-electrons.com>
On Thu, Oct 20, 2016 at 3:49 PM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> Even though the our binding had the assumption that the allwinner,pull and
> allwinner,drive properties were optional, the code never took that into
> account.
>
> Fix that.
>
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> Acked-by: Chen-Yu Tsai <wens@csie.org>
Patch applied.
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH V2] pinctrl: qcom: Add msm8994 pinctrl driver
From: Linus Walleij @ 2016-10-29 8:27 UTC (permalink / raw)
To: Michael Scott, Bjorn Andersson
Cc: linux-gpio@vger.kernel.org, linux-arm-msm@vger.kernel.org,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
Rob Herring, Mark Rutland, Andy Gross, David Brown, Joonwoo Park,
Jeremy McNicoll
In-Reply-To: <20161026233228.15219-1-michael.scott@linaro.org>
On Thu, Oct 27, 2016 at 1:32 AM, Michael Scott <michael.scott@linaro.org> wrote:
> Initial pinctrl driver for QCOM msm8994 platforms.
>
> In order to continue the initial board support for QCOM msm8994/msm8992
> presented in patches from Jeremy McNicoll <jeremymc@redhat.com>, let's put
> a proper pinctrl driver in place.
>
> Currently, the DT for these platforms uses the msm8x74 pinctrl driver to enable
> basic UART. Beyond the first few pins the rest are different enough to justify
> it's own driver.
>
> Note: This driver is also be used by QCOM's msm8992 platform as it's TLM block
> is the same.
>
> - Initial formatting and style was taken from the msm8x74 pinctrl driver added
> by Björn Andersson <bjorn.andersson@linaro.org>
> - Data was then adjusted per QCOM MSM8994 documentation for Top Level Multiplexing
> - Bindings documentation was based on qcom,msm8996-pinctrl.txt by
> Joonwoo Park <joonwoop@codeaurora.org> and then modified for msm8994 content
>
> Signed-off-by: Michael Scott <michael.scott@linaro.org>
> ---
>
> V1 -> V2: fixed missing FUNCTION(nav_pps) and removed 3 odd newlines between blsp_i2c4_groups and cci_timer0_groups
Looks fine to me, just like the other Qcom drivers.
I just want Björn Andersson's ACK before merging, Björn?
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH v2 2/6] net: phy: broadcom: Add BCM54810 PHY entry
From: Andrew Lunn @ 2016-10-29 8:18 UTC (permalink / raw)
To: Jon Mason
Cc: David Miller, Rob Herring, Mark Rutland, Florian Fainelli,
devicetree, netdev, linux-kernel, bcm-kernel-feedback-list, rafal,
linux-arm-kernel
In-Reply-To: <1477688219-3871-3-git-send-email-jon.mason@broadcom.com>
On Fri, Oct 28, 2016 at 04:56:55PM -0400, Jon Mason wrote:
> The BCM54810 PHY requires some semi-unique configuration, which results
> in some additional configuration in addition to the standard config.
> Also, some users of the BCM54810 require the PHY lanes to be swapped.
> Since there is no way to detect this, add a device tree query to see if
> it is applicable.
>
> Inspired-by: Vikas Soni <vsoni@broadcom.com>
> Signed-off-by: Jon Mason <jon.mason@broadcom.com>
> ---
> drivers/net/phy/Kconfig | 2 +-
> drivers/net/phy/broadcom.c | 58 +++++++++++++++++++++++++++++++++++++++++++++-
> include/linux/brcmphy.h | 10 ++++++++
Hi Jon
The binding documentation is missing.
> + if (of_property_read_bool(np, "brcm,enet-phy-lane-swap")) {
> + /* Lane Swap - Undocumented register...magic! */
> + ret = bcm_phy_write_exp(phydev, MII_BCM54XX_EXP_SEL_ER + 0x9,
> + 0x11B);
> + if (ret < 0)
> + return ret;
> + }
> +
I wounder if this property could be made generic? What exactly are you
swapping? Rx and Tx lanes? Maybe we should add it to phy.txt?
Andrew
^ permalink raw reply
* [PATCH 12/12] of: Remove unused variable overlay_symbols
From: frowand.list @ 2016-10-29 6:26 UTC (permalink / raw)
To: Rob Herring, pantelis.antoniou, Pantelis Antoniou
Cc: devicetree, linux-kernel
In-Reply-To: <1477722392-32172-1-git-send-email-frowand.list@gmail.com>
From: Frank Rowand <frank.rowand@am.sony.com>
Remove unused pointer to node "__symbols__".
Signed-off-by: Frank Rowand <frank.rowand@am.sony.com>
---
drivers/of/resolver.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/of/resolver.c b/drivers/of/resolver.c
index 53353cc8f2bb..783bd09463b5 100644
--- a/drivers/of/resolver.c
+++ b/drivers/of/resolver.c
@@ -287,7 +287,7 @@ static int adjust_local_phandle_references(struct device_node *local_fixups,
int of_resolve_phandles(struct device_node *overlay)
{
struct device_node *child, *local_fixups, *refnode;
- struct device_node *tree_symbols, *overlay_symbols, *overlay_fixups;
+ struct device_node *tree_symbols, *overlay_fixups;
struct property *prop;
const char *refpath;
phandle phandle, phandle_delta;
@@ -317,12 +317,9 @@ int of_resolve_phandles(struct device_node *overlay)
if (err)
goto err_out;
- overlay_symbols = NULL;
overlay_fixups = NULL;
for_each_child_of_node(overlay, child) {
- if (!of_node_cmp(child->name, "__symbols__"))
- overlay_symbols = child;
if (!of_node_cmp(child->name, "__fixups__"))
overlay_fixups = child;
}
--
1.9.1
^ permalink raw reply related
* [PATCH 11/12] of: Move setting of pointer to beside test for non-null
From: frowand.list @ 2016-10-29 6:26 UTC (permalink / raw)
To: Rob Herring, pantelis.antoniou, Pantelis Antoniou
Cc: devicetree, linux-kernel
In-Reply-To: <1477722392-32172-1-git-send-email-frowand.list@gmail.com>
From: Frank Rowand <frank.rowand@am.sony.com>
Value of pointer was calculated in an earlier block than
where it was used. Move it down into the block where it
is used, immediately before where is is checked to be valid.
Signed-off-by: Frank Rowand <frank.rowand@am.sony.com>
---
drivers/of/resolver.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/of/resolver.c b/drivers/of/resolver.c
index eb78010c21a3..53353cc8f2bb 100644
--- a/drivers/of/resolver.c
+++ b/drivers/of/resolver.c
@@ -320,8 +320,6 @@ int of_resolve_phandles(struct device_node *overlay)
overlay_symbols = NULL;
overlay_fixups = NULL;
- tree_symbols = of_find_node_by_path("/__symbols__");
-
for_each_child_of_node(overlay, child) {
if (!of_node_cmp(child->name, "__symbols__"))
overlay_symbols = child;
@@ -334,6 +332,7 @@ int of_resolve_phandles(struct device_node *overlay)
goto out;
}
+ tree_symbols = of_find_node_by_path("/__symbols__");
if (!tree_symbols) {
pr_err("no symbols in root of device tree.\n");
err = -EINVAL;
--
1.9.1
^ permalink raw reply related
* [PATCH 10/12] of: Add back an error message, restructured
From: frowand.list @ 2016-10-29 6:26 UTC (permalink / raw)
To: Rob Herring, pantelis.antoniou, Pantelis Antoniou
Cc: devicetree, linux-kernel
In-Reply-To: <1477722392-32172-1-git-send-email-frowand.list@gmail.com>
From: Frank Rowand <frank.rowand@am.sony.com>
Add a single pr_err() to cover a range of errors that were reported
by several pr_err() that were removed earlier in this series.
Signed-off-by: Frank Rowand <frank.rowand@am.sony.com>
---
drivers/of/resolver.c | 18 ++++++++++++------
1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/drivers/of/resolver.c b/drivers/of/resolver.c
index f842dbd1585c..eb78010c21a3 100644
--- a/drivers/of/resolver.c
+++ b/drivers/of/resolver.c
@@ -293,13 +293,17 @@ int of_resolve_phandles(struct device_node *overlay)
phandle phandle, phandle_delta;
int err;
+ tree_symbols = NULL;
+
if (!overlay) {
pr_err("null overlay\n");
- return -EINVAL;
+ err = -EINVAL;
+ goto err_out;
}
if (!of_node_check_flag(overlay, OF_DETACHED)) {
pr_err("overlay not detached\n");
- return -EINVAL;
+ err = -EINVAL;
+ goto err_out;
}
phandle_delta = live_tree_max_phandle() + 1;
@@ -311,7 +315,7 @@ int of_resolve_phandles(struct device_node *overlay)
err = adjust_local_phandle_references(local_fixups, overlay, phandle_delta);
if (err)
- return err;
+ goto err_out;
overlay_symbols = NULL;
overlay_fixups = NULL;
@@ -333,7 +337,7 @@ int of_resolve_phandles(struct device_node *overlay)
if (!tree_symbols) {
pr_err("no symbols in root of device tree.\n");
err = -EINVAL;
- goto out;
+ goto err_out;
}
for_each_property_of_node(overlay_fixups, prop) {
@@ -345,12 +349,12 @@ int of_resolve_phandles(struct device_node *overlay)
err = of_property_read_string(tree_symbols,
prop->name, &refpath);
if (err)
- goto out;
+ goto err_out;
refnode = of_find_node_by_path(refpath);
if (!refnode) {
err = -ENOENT;
- goto out;
+ goto err_out;
}
phandle = refnode->phandle;
@@ -361,6 +365,8 @@ int of_resolve_phandles(struct device_node *overlay)
break;
}
+err_out:
+ pr_err("overlay phandle fixup failed: %d\n", err);
out:
of_node_put(tree_symbols);
--
1.9.1
^ permalink raw reply related
* [PATCH 09/12] of: Update comments to reflect changes and increase clarity
From: frowand.list @ 2016-10-29 6:26 UTC (permalink / raw)
To: Rob Herring, pantelis.antoniou, Pantelis Antoniou
Cc: devicetree, linux-kernel
In-Reply-To: <1477722392-32172-1-git-send-email-frowand.list@gmail.com>
From: Frank Rowand <frank.rowand@am.sony.com>
Update comments to better explain what functions are doing.
Signed-off-by: Frank Rowand <frank.rowand@am.sony.com>
---
drivers/of/resolver.c | 66 ++++++++++++++++++++++++++++++++++++---------------
1 file changed, 47 insertions(+), 19 deletions(-)
diff --git a/drivers/of/resolver.c b/drivers/of/resolver.c
index 76c09cb57eae..f842dbd1585c 100644
--- a/drivers/of/resolver.c
+++ b/drivers/of/resolver.c
@@ -50,9 +50,6 @@ static struct device_node *find_node_by_full_name(struct device_node *node,
return NULL;
}
-/*
- * Find live tree's maximum phandle value.
- */
static phandle live_tree_max_phandle(void)
{
struct device_node *node;
@@ -71,9 +68,6 @@ static phandle live_tree_max_phandle(void)
return phandle;
}
-/*
- * Adjust a subtree's phandle values by a given delta.
- */
static void adjust_overlay_phandles(struct device_node *overlay,
int phandle_delta)
{
@@ -81,9 +75,11 @@ static void adjust_overlay_phandles(struct device_node *overlay,
struct property *prop;
phandle phandle;
+ /* adjust node's phandle in node */
if (overlay->phandle != 0 && overlay->phandle != OF_PHANDLE_ILLEGAL)
overlay->phandle += phandle_delta;
+ /* copy adjusted phandle into *phandle properties */
for_each_property_of_node(overlay, prop) {
if (of_prop_cmp(prop->name, "phandle") &&
@@ -118,6 +114,7 @@ static int update_usages_of_a_phandle_reference(struct device_node *overlay,
return -ENOMEM;
memcpy(value, prop_fixup->value, prop_fixup->length);
+ /* prop_fixup contains a list of tuples of path:property_name:offset */
end = value + prop_fixup->length;
for (cur = value; cur < end; cur += len + 1) {
len = strlen(cur);
@@ -177,10 +174,14 @@ static int node_name_cmp(const struct device_node *dn1,
/*
* Adjust the local phandle references by the given phandle delta.
- * Assumes the existances of a __local_fixups__ node at the root.
- * Assumes that __of_verify_tree_phandle_references has been called.
- * Does not take any devtree locks so make sure you call this on a tree
- * which is at the detached state.
+ *
+ * Subtree @local_fixups, which is overlay node __local_fixups__,
+ * mirrors the fragment node structure at the root of the overlay.
+ *
+ * For each property in the fragments that contains a phandle reference,
+ * @local_fixups has a property of the same name that contains a list
+ * of offsets of the phandle reference(s) within the respective property
+ * value(s). The values at these offsets will be fixed up.
*/
static int adjust_local_phandle_references(struct device_node *local_fixups,
struct device_node *overlay, int phandle_delta)
@@ -225,6 +226,13 @@ static int adjust_local_phandle_references(struct device_node *local_fixups,
}
}
+ /*
+ * These nested loops recurse down two subtrees in parallel, where the
+ * node names in the two subtrees match.
+ *
+ * The roots of the subtrees are the overlay's __local_fixups__ node
+ * and the overlay's root node.
+ */
for_each_child_of_node(local_fixups, child) {
for_each_child_of_node(overlay, overlay_child)
@@ -244,17 +252,37 @@ static int adjust_local_phandle_references(struct device_node *local_fixups,
}
/**
- * of_resolve - Resolve the given node against the live tree.
+ * of_resolve_phandles - Relocate and resolve overlay against live tree
+ *
+ * @overlay: Pointer to devicetree overlay to relocate and resolve
+ *
+ * Modify (relocate) values of local phandles in @overlay to a range that
+ * does not conflict with the live expanded devicetree. Update references
+ * to the local phandles in @overlay. Update (resolve) phandle references
+ * in @overlay that refer to the live expanded devicetree.
+ *
+ * Phandle values in the live tree are in the range of
+ * 1 .. live_tree_max_phandle(). The range of phandle values in the overlay
+ * also begin with at 1. Adjust the phandle values in the overlay to begin
+ * at live_tree_max_phandle() + 1. Update references to the phandles to
+ * the adjusted phandle values.
+ *
+ * The name of each property in the "__fixups__" node in the overlay matches
+ * the name of a symbol (a label) in the live tree. The values of each
+ * property in the "__fixups__" node is a list of the property values in the
+ * overlay that need to be updated to contain the phandle reference
+ * corresponding to that symbol in the live tree. Update the references in
+ * the overlay with the phandle values in the live tree.
+ *
+ * @overlay must be detached.
*
- * @resolve: Node to resolve
+ * Resolving and applying @overlay to the live expanded devicetree must be
+ * protected by a mechanism to ensure that multiple overlays are processed
+ * in a single threaded manner so that multiple overlays will not relocate
+ * phandles to overlapping ranges. The mechanism to enforce this is not
+ * yet implemented.
*
- * Perform dynamic Device Tree resolution against the live tree
- * to the given node to resolve. This depends on the live tree
- * having a __symbols__ node, and the resolve node the __fixups__ &
- * __local_fixups__ nodes (if needed).
- * The result of the operation is a resolve node that it's contents
- * are fit to be inserted or operate upon the live tree.
- * Returns 0 on success or a negative error value on error.
+ * Return: %0 on success or a negative error value on error.
*/
int of_resolve_phandles(struct device_node *overlay)
{
--
1.9.1
^ permalink raw reply related
* [PATCH 08/12] of: Remove redundant size check
From: frowand.list @ 2016-10-29 6:26 UTC (permalink / raw)
To: Rob Herring, pantelis.antoniou, Pantelis Antoniou
Cc: devicetree, linux-kernel
In-Reply-To: <1477722392-32172-1-git-send-email-frowand.list@gmail.com>
From: Frank Rowand <frank.rowand@am.sony.com>
Remove a redundant check of buffer size.
Signed-off-by: Frank Rowand <frank.rowand@am.sony.com>
---
drivers/of/resolver.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/of/resolver.c b/drivers/of/resolver.c
index 708daca1d522..76c09cb57eae 100644
--- a/drivers/of/resolver.c
+++ b/drivers/of/resolver.c
@@ -216,7 +216,7 @@ static int adjust_local_phandle_references(struct device_node *local_fixups,
for (i = 0; i < count; i++) {
off = be32_to_cpu(((__be32 *)prop_fix->value)[i]);
- if (off >= prop->length || (off + 4) > prop->length)
+ if ((off + 4) > prop->length)
return -EINVAL;
phandle = be32_to_cpu(*(__be32 *)(prop->value + off));
--
1.9.1
^ permalink raw reply related
* [PATCH 07/12] of: Update structure of code to be clearer, also remove BUG_ON()
From: frowand.list @ 2016-10-29 6:26 UTC (permalink / raw)
To: Rob Herring, pantelis.antoniou, Pantelis Antoniou
Cc: devicetree, linux-kernel
In-Reply-To: <1477722392-32172-1-git-send-email-frowand.list@gmail.com>
From: Frank Rowand <frank.rowand@am.sony.com>
Remove BUG_ON(), which is frowned upon and not needed here.
Restructure to remove some excessive complexity.
Signed-off-by: Frank Rowand <frank.rowand@am.sony.com>
---
drivers/of/resolver.c | 48 +++++++++++++++++-------------------------------
1 file changed, 17 insertions(+), 31 deletions(-)
diff --git a/drivers/of/resolver.c b/drivers/of/resolver.c
index 0778747cdd58..708daca1d522 100644
--- a/drivers/of/resolver.c
+++ b/drivers/of/resolver.c
@@ -136,8 +136,8 @@ static int update_usages_of_a_phandle_reference(struct device_node *overlay,
err = -EINVAL;
goto err_fail;
}
-
*s++ = '\0';
+
err = kstrtoint(s, 10, &offset);
if (err)
goto err_fail;
@@ -219,11 +219,9 @@ static int adjust_local_phandle_references(struct device_node *local_fixups,
if (off >= prop->length || (off + 4) > prop->length)
return -EINVAL;
- if (phandle_delta) {
- phandle = be32_to_cpu(*(__be32 *)(prop->value + off));
- phandle += phandle_delta;
- *(__be32 *)(prop->value + off) = cpu_to_be32(phandle);
- }
+ phandle = be32_to_cpu(*(__be32 *)(prop->value + off));
+ phandle += phandle_delta;
+ *(__be32 *)(prop->value + off) = cpu_to_be32(phandle);
}
}
@@ -267,48 +265,36 @@ int of_resolve_phandles(struct device_node *overlay)
phandle phandle, phandle_delta;
int err;
- if (!overlay)
- pr_err("%s: null overlay\n", __func__);
- if (overlay && !of_node_check_flag(overlay, OF_DETACHED))
- pr_err("%s: node %s not detached\n", __func__,
- overlay->full_name);
- if (!overlay || !of_node_check_flag(overlay, OF_DETACHED))
+ if (!overlay) {
+ pr_err("null overlay\n");
+ return -EINVAL;
+ }
+ if (!of_node_check_flag(overlay, OF_DETACHED)) {
+ pr_err("overlay not detached\n");
return -EINVAL;
+ }
phandle_delta = live_tree_max_phandle() + 1;
adjust_overlay_phandles(overlay, phandle_delta);
- local_fixups = NULL;
for_each_child_of_node(overlay, local_fixups)
if (!of_node_cmp(local_fixups->name, "__local_fixups__"))
break;
- if (local_fixups != NULL) {
- err = adjust_local_phandle_references(local_fixups,
- overlay, 0);
- if (err)
- return err;
+ err = adjust_local_phandle_references(local_fixups, overlay, phandle_delta);
+ if (err)
+ return err;
- BUG_ON(adjust_local_phandle_references(local_fixups,
- overlay, phandle_delta));
- }
-
- tree_symbols = NULL;
overlay_symbols = NULL;
overlay_fixups = NULL;
tree_symbols = of_find_node_by_path("/__symbols__");
for_each_child_of_node(overlay, child) {
-
- if (!overlay_symbols && !of_node_cmp(child->name, "__symbols__"))
+ if (!of_node_cmp(child->name, "__symbols__"))
overlay_symbols = child;
-
- if (!overlay_fixups && !of_node_cmp(child->name, "__fixups__"))
+ if (!of_node_cmp(child->name, "__fixups__"))
overlay_fixups = child;
-
- if (overlay_symbols && overlay_fixups)
- break;
}
if (!overlay_fixups) {
@@ -317,7 +303,7 @@ int of_resolve_phandles(struct device_node *overlay)
}
if (!tree_symbols) {
- pr_err("%s: no symbols in root of device tree.\n", __func__);
+ pr_err("no symbols in root of device tree.\n");
err = -EINVAL;
goto out;
}
--
1.9.1
^ permalink raw reply related
* [PATCH 06/12] of: Rename variables to better reflect purpose or follow convention
From: frowand.list @ 2016-10-29 6:26 UTC (permalink / raw)
To: Rob Herring, pantelis.antoniou, Pantelis Antoniou
Cc: devicetree, linux-kernel
In-Reply-To: <1477722392-32172-1-git-send-email-frowand.list@gmail.com>
From: Frank Rowand <frank.rowand@am.sony.com>
Rename variables to better reflect what their purpose is. As a side
effect, this reduces the need for some of the comments previously
removed in this series.
Signed-off-by: Frank Rowand <frank.rowand@am.sony.com>
---
drivers/of/resolver.c | 172 +++++++++++++++++++++++++-------------------------
1 file changed, 85 insertions(+), 87 deletions(-)
diff --git a/drivers/of/resolver.c b/drivers/of/resolver.c
index 0ce38aa0ed3c..0778747cdd58 100644
--- a/drivers/of/resolver.c
+++ b/drivers/of/resolver.c
@@ -74,17 +74,17 @@ static phandle live_tree_max_phandle(void)
/*
* Adjust a subtree's phandle values by a given delta.
*/
-static void adjust_overlay_phandles(struct device_node *node,
+static void adjust_overlay_phandles(struct device_node *overlay,
int phandle_delta)
{
struct device_node *child;
struct property *prop;
phandle phandle;
- if (node->phandle != 0 && node->phandle != OF_PHANDLE_ILLEGAL)
- node->phandle += phandle_delta;
+ if (overlay->phandle != 0 && overlay->phandle != OF_PHANDLE_ILLEGAL)
+ overlay->phandle += phandle_delta;
- for_each_property_of_node(node, prop) {
+ for_each_property_of_node(overlay, prop) {
if (of_prop_cmp(prop->name, "phandle") &&
of_prop_cmp(prop->name, "linux,phandle"))
@@ -97,41 +97,40 @@ static void adjust_overlay_phandles(struct device_node *node,
if (phandle == OF_PHANDLE_ILLEGAL)
continue;
- *(uint32_t *)prop->value = cpu_to_be32(node->phandle);
+ *(uint32_t *)prop->value = cpu_to_be32(overlay->phandle);
}
- for_each_child_of_node(node, child)
+ for_each_child_of_node(overlay, child)
adjust_overlay_phandles(child, phandle_delta);
}
-static int update_usages_of_a_phandle_reference(struct device_node *node,
- struct property *rprop, int value)
+static int update_usages_of_a_phandle_reference(struct device_node *overlay,
+ struct property *prop_fixup, phandle phandle)
{
- phandle phandle;
struct device_node *refnode;
- struct property *sprop;
- char *propval, *propcur, *propend, *nodestr, *propstr, *s;
- int offset, propcurlen;
+ struct property *prop;
+ char *value, *cur, *end, *node_path, *prop_name, *s;
+ int offset, len;
int err = 0;
- propval = kmalloc(rprop->length, GFP_KERNEL);
- if (!propval)
+ value = kmalloc(prop_fixup->length, GFP_KERNEL);
+ if (!value)
return -ENOMEM;
- memcpy(propval, rprop->value, rprop->length);
+ memcpy(value, prop_fixup->value, prop_fixup->length);
- propend = propval + rprop->length;
- for (propcur = propval; propcur < propend; propcur += propcurlen + 1) {
- propcurlen = strlen(propcur);
+ end = value + prop_fixup->length;
+ for (cur = value; cur < end; cur += len + 1) {
+ len = strlen(cur);
- nodestr = propcur;
- s = strchr(propcur, ':');
+ node_path = cur;
+ s = strchr(cur, ':');
if (!s) {
err = -EINVAL;
goto err_fail;
}
*s++ = '\0';
- propstr = s;
+ prop_name = s;
s = strchr(s, ':');
if (!s) {
err = -EINVAL;
@@ -143,27 +142,26 @@ static int update_usages_of_a_phandle_reference(struct device_node *node,
if (err)
goto err_fail;
- refnode = find_node_by_full_name(node, nodestr);
+ refnode = find_node_by_full_name(overlay, node_path);
if (!refnode)
continue;
- for_each_property_of_node(refnode, sprop) {
- if (!of_prop_cmp(sprop->name, propstr))
+ for_each_property_of_node(refnode, prop) {
+ if (!of_prop_cmp(prop->name, prop_name))
break;
}
of_node_put(refnode);
- if (!sprop) {
+ if (!prop) {
err = -ENOENT;
goto err_fail;
}
- phandle = value;
- *(__be32 *)(sprop->value + offset) = cpu_to_be32(phandle);
+ *(__be32 *)(prop->value + offset) = cpu_to_be32(phandle);
}
err_fail:
- kfree(propval);
+ kfree(value);
return err;
}
@@ -184,61 +182,61 @@ static int node_name_cmp(const struct device_node *dn1,
* Does not take any devtree locks so make sure you call this on a tree
* which is at the detached state.
*/
-static int adjust_local_phandle_references(struct device_node *node,
- struct device_node *target, int phandle_delta)
+static int adjust_local_phandle_references(struct device_node *local_fixups,
+ struct device_node *overlay, int phandle_delta)
{
- struct device_node *child, *childtarget;
- struct property *rprop, *sprop;
+ struct device_node *child, *overlay_child;
+ struct property *prop_fix, *prop;
int err, i, count;
unsigned int off;
phandle phandle;
- if (!node)
+ if (!local_fixups)
return 0;
- for_each_property_of_node(node, rprop) {
+ for_each_property_of_node(local_fixups, prop_fix) {
/* skip properties added automatically */
- if (!of_prop_cmp(rprop->name, "name") ||
- !of_prop_cmp(rprop->name, "phandle") ||
- !of_prop_cmp(rprop->name, "linux,phandle"))
+ if (!of_prop_cmp(prop_fix->name, "name") ||
+ !of_prop_cmp(prop_fix->name, "phandle") ||
+ !of_prop_cmp(prop_fix->name, "linux,phandle"))
continue;
- if ((rprop->length % 4) != 0 || rprop->length == 0)
+ if ((prop_fix->length % 4) != 0 || prop_fix->length == 0)
return -EINVAL;
- count = rprop->length / sizeof(__be32);
+ count = prop_fix->length / sizeof(__be32);
- for_each_property_of_node(target, sprop) {
- if (!of_prop_cmp(sprop->name, rprop->name))
+ for_each_property_of_node(overlay, prop) {
+ if (!of_prop_cmp(prop->name, prop_fix->name))
break;
}
- if (!sprop)
+ if (!prop)
return -EINVAL;
for (i = 0; i < count; i++) {
- off = be32_to_cpu(((__be32 *)rprop->value)[i]);
- if (off >= sprop->length || (off + 4) > sprop->length)
+ off = be32_to_cpu(((__be32 *)prop_fix->value)[i]);
+ if (off >= prop->length || (off + 4) > prop->length)
return -EINVAL;
if (phandle_delta) {
- phandle = be32_to_cpu(*(__be32 *)(sprop->value + off));
+ phandle = be32_to_cpu(*(__be32 *)(prop->value + off));
phandle += phandle_delta;
- *(__be32 *)(sprop->value + off) = cpu_to_be32(phandle);
+ *(__be32 *)(prop->value + off) = cpu_to_be32(phandle);
}
}
}
- for_each_child_of_node(node, child) {
+ for_each_child_of_node(local_fixups, child) {
- for_each_child_of_node(target, childtarget)
- if (!node_name_cmp(child, childtarget))
+ for_each_child_of_node(overlay, overlay_child)
+ if (!node_name_cmp(child, overlay_child))
break;
- if (!childtarget)
+ if (!overlay_child)
return -EINVAL;
- err = adjust_local_phandle_references(child, childtarget,
+ err = adjust_local_phandle_references(child, overlay_child,
phandle_delta);
if (err)
return err;
@@ -260,78 +258,78 @@ static int adjust_local_phandle_references(struct device_node *node,
* are fit to be inserted or operate upon the live tree.
* Returns 0 on success or a negative error value on error.
*/
-int of_resolve_phandles(struct device_node *resolve)
+int of_resolve_phandles(struct device_node *overlay)
{
- struct device_node *child, *childroot, *refnode;
- struct device_node *root_sym, *resolve_sym, *resolve_fix;
- struct property *rprop;
+ struct device_node *child, *local_fixups, *refnode;
+ struct device_node *tree_symbols, *overlay_symbols, *overlay_fixups;
+ struct property *prop;
const char *refpath;
phandle phandle, phandle_delta;
int err;
- if (!resolve)
- pr_err("%s: null node\n", __func__);
- if (resolve && !of_node_check_flag(resolve, OF_DETACHED))
+ if (!overlay)
+ pr_err("%s: null overlay\n", __func__);
+ if (overlay && !of_node_check_flag(overlay, OF_DETACHED))
pr_err("%s: node %s not detached\n", __func__,
- resolve->full_name);
- if (!resolve || !of_node_check_flag(resolve, OF_DETACHED))
+ overlay->full_name);
+ if (!overlay || !of_node_check_flag(overlay, OF_DETACHED))
return -EINVAL;
phandle_delta = live_tree_max_phandle() + 1;
- adjust_overlay_phandles(resolve, phandle_delta);
+ adjust_overlay_phandles(overlay, phandle_delta);
- childroot = NULL;
- for_each_child_of_node(resolve, childroot)
- if (!of_node_cmp(childroot->name, "__local_fixups__"))
+ local_fixups = NULL;
+ for_each_child_of_node(overlay, local_fixups)
+ if (!of_node_cmp(local_fixups->name, "__local_fixups__"))
break;
- if (childroot != NULL) {
- err = adjust_local_phandle_references(childroot,
- resolve, 0);
+ if (local_fixups != NULL) {
+ err = adjust_local_phandle_references(local_fixups,
+ overlay, 0);
if (err)
return err;
- BUG_ON(adjust_local_phandle_references(childroot,
- resolve, phandle_delta));
+ BUG_ON(adjust_local_phandle_references(local_fixups,
+ overlay, phandle_delta));
}
- root_sym = NULL;
- resolve_sym = NULL;
- resolve_fix = NULL;
+ tree_symbols = NULL;
+ overlay_symbols = NULL;
+ overlay_fixups = NULL;
- root_sym = of_find_node_by_path("/__symbols__");
+ tree_symbols = of_find_node_by_path("/__symbols__");
- for_each_child_of_node(resolve, child) {
+ for_each_child_of_node(overlay, child) {
- if (!resolve_sym && !of_node_cmp(child->name, "__symbols__"))
- resolve_sym = child;
+ if (!overlay_symbols && !of_node_cmp(child->name, "__symbols__"))
+ overlay_symbols = child;
- if (!resolve_fix && !of_node_cmp(child->name, "__fixups__"))
- resolve_fix = child;
+ if (!overlay_fixups && !of_node_cmp(child->name, "__fixups__"))
+ overlay_fixups = child;
- if (resolve_sym && resolve_fix)
+ if (overlay_symbols && overlay_fixups)
break;
}
- if (!resolve_fix) {
+ if (!overlay_fixups) {
err = 0;
goto out;
}
- if (!root_sym) {
+ if (!tree_symbols) {
pr_err("%s: no symbols in root of device tree.\n", __func__);
err = -EINVAL;
goto out;
}
- for_each_property_of_node(resolve_fix, rprop) {
+ for_each_property_of_node(overlay_fixups, prop) {
/* skip properties added automatically */
- if (!of_prop_cmp(rprop->name, "name"))
+ if (!of_prop_cmp(prop->name, "name"))
continue;
- err = of_property_read_string(root_sym,
- rprop->name, &refpath);
+ err = of_property_read_string(tree_symbols,
+ prop->name, &refpath);
if (err)
goto out;
@@ -344,13 +342,13 @@ int of_resolve_phandles(struct device_node *resolve)
phandle = refnode->phandle;
of_node_put(refnode);
- err = update_usages_of_a_phandle_reference(resolve, rprop, phandle);
+ err = update_usages_of_a_phandle_reference(overlay, prop, phandle);
if (err)
break;
}
out:
- of_node_put(root_sym);
+ of_node_put(tree_symbols);
return err;
}
--
1.9.1
^ permalink raw reply related
* [PATCH 05/12] of: Remove prefix "__of_" from local function names
From: frowand.list @ 2016-10-29 6:26 UTC (permalink / raw)
To: Rob Herring, pantelis.antoniou, Pantelis Antoniou
Cc: devicetree, linux-kernel
In-Reply-To: <1477722392-32172-1-git-send-email-frowand.list@gmail.com>
From: Frank Rowand <frank.rowand@am.sony.com>
Remove "__of_" prefix from local function names. The pattern of
a leading "__" is used in drivers/of/ to signify a function that
must be called with a lock held. These functions do not fit
that pattern.
Signed-off-by: Frank Rowand <frank.rowand@am.sony.com>
---
drivers/of/resolver.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/of/resolver.c b/drivers/of/resolver.c
index 3d123b612789..0ce38aa0ed3c 100644
--- a/drivers/of/resolver.c
+++ b/drivers/of/resolver.c
@@ -28,7 +28,7 @@
* Find a node with the give full name by recursively following any of
* the child node links.
*/
-static struct device_node *__of_find_node_by_full_name(struct device_node *node,
+static struct device_node *find_node_by_full_name(struct device_node *node,
const char *full_name)
{
struct device_node *child, *found;
@@ -40,7 +40,7 @@ static struct device_node *__of_find_node_by_full_name(struct device_node *node,
return of_node_get(node);
for_each_child_of_node(node, child) {
- found = __of_find_node_by_full_name(child, full_name);
+ found = find_node_by_full_name(child, full_name);
if (found != NULL) {
of_node_put(child);
return found;
@@ -143,7 +143,7 @@ static int update_usages_of_a_phandle_reference(struct device_node *node,
if (err)
goto err_fail;
- refnode = __of_find_node_by_full_name(node, nodestr);
+ refnode = find_node_by_full_name(node, nodestr);
if (!refnode)
continue;
@@ -168,7 +168,7 @@ static int update_usages_of_a_phandle_reference(struct device_node *node,
}
/* compare nodes taking into account that 'name' strips out the @ part */
-static int __of_node_name_cmp(const struct device_node *dn1,
+static int node_name_cmp(const struct device_node *dn1,
const struct device_node *dn2)
{
const char *n1 = strrchr(dn1->full_name, '/') ? : "/";
@@ -232,7 +232,7 @@ static int adjust_local_phandle_references(struct device_node *node,
for_each_child_of_node(node, child) {
for_each_child_of_node(target, childtarget)
- if (!__of_node_name_cmp(child, childtarget))
+ if (!node_name_cmp(child, childtarget))
break;
if (!childtarget)
--
1.9.1
^ permalink raw reply related
* [PATCH 04/12] of: Rename functions to more accurately reflect what they do
From: frowand.list @ 2016-10-29 6:26 UTC (permalink / raw)
To: Rob Herring, pantelis.antoniou, Pantelis Antoniou
Cc: devicetree, linux-kernel
In-Reply-To: <1477722392-32172-1-git-send-email-frowand.list@gmail.com>
From: Frank Rowand <frank.rowand@am.sony.com>
Some function names are misleading or do not provide a good
sense of what they do. Rename the functions to ne more
informative.
Signed-off-by: Frank Rowand <frank.rowand@am.sony.com>
---
drivers/of/resolver.c | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/drivers/of/resolver.c b/drivers/of/resolver.c
index 31fd3800787a..3d123b612789 100644
--- a/drivers/of/resolver.c
+++ b/drivers/of/resolver.c
@@ -53,7 +53,7 @@ static struct device_node *__of_find_node_by_full_name(struct device_node *node,
/*
* Find live tree's maximum phandle value.
*/
-static phandle of_get_tree_max_phandle(void)
+static phandle live_tree_max_phandle(void)
{
struct device_node *node;
phandle phandle;
@@ -74,7 +74,7 @@ static phandle of_get_tree_max_phandle(void)
/*
* Adjust a subtree's phandle values by a given delta.
*/
-static void __of_adjust_tree_phandles(struct device_node *node,
+static void adjust_overlay_phandles(struct device_node *node,
int phandle_delta)
{
struct device_node *child;
@@ -101,10 +101,10 @@ static void __of_adjust_tree_phandles(struct device_node *node,
}
for_each_child_of_node(node, child)
- __of_adjust_tree_phandles(child, phandle_delta);
+ adjust_overlay_phandles(child, phandle_delta);
}
-static int __of_adjust_phandle_ref(struct device_node *node,
+static int update_usages_of_a_phandle_reference(struct device_node *node,
struct property *rprop, int value)
{
phandle phandle;
@@ -184,7 +184,7 @@ static int __of_node_name_cmp(const struct device_node *dn1,
* Does not take any devtree locks so make sure you call this on a tree
* which is at the detached state.
*/
-static int __of_adjust_tree_phandle_references(struct device_node *node,
+static int adjust_local_phandle_references(struct device_node *node,
struct device_node *target, int phandle_delta)
{
struct device_node *child, *childtarget;
@@ -238,7 +238,7 @@ static int __of_adjust_tree_phandle_references(struct device_node *node,
if (!childtarget)
return -EINVAL;
- err = __of_adjust_tree_phandle_references(child, childtarget,
+ err = adjust_local_phandle_references(child, childtarget,
phandle_delta);
if (err)
return err;
@@ -277,8 +277,8 @@ int of_resolve_phandles(struct device_node *resolve)
if (!resolve || !of_node_check_flag(resolve, OF_DETACHED))
return -EINVAL;
- phandle_delta = of_get_tree_max_phandle() + 1;
- __of_adjust_tree_phandles(resolve, phandle_delta);
+ phandle_delta = live_tree_max_phandle() + 1;
+ adjust_overlay_phandles(resolve, phandle_delta);
childroot = NULL;
for_each_child_of_node(resolve, childroot)
@@ -286,12 +286,12 @@ int of_resolve_phandles(struct device_node *resolve)
break;
if (childroot != NULL) {
- err = __of_adjust_tree_phandle_references(childroot,
+ err = adjust_local_phandle_references(childroot,
resolve, 0);
if (err)
return err;
- BUG_ON(__of_adjust_tree_phandle_references(childroot,
+ BUG_ON(adjust_local_phandle_references(childroot,
resolve, phandle_delta));
}
@@ -344,7 +344,7 @@ int of_resolve_phandles(struct device_node *resolve)
phandle = refnode->phandle;
of_node_put(refnode);
- err = __of_adjust_phandle_ref(resolve, rprop, phandle);
+ err = update_usages_of_a_phandle_reference(resolve, rprop, phandle);
if (err)
break;
}
--
1.9.1
^ permalink raw reply related
* [PATCH 03/12] of: Convert comparisons to zero or NULL to logical expressions
From: frowand.list @ 2016-10-29 6:26 UTC (permalink / raw)
To: Rob Herring, pantelis.antoniou, Pantelis Antoniou
Cc: devicetree, linux-kernel
In-Reply-To: <1477722392-32172-1-git-send-email-frowand.list@gmail.com>
From: Frank Rowand <frank.rowand@am.sony.com>
Convert comparisons to zero or NULL to logical expressions. A
small number of such comparisons remain where they provide more
clarity of the numeric nature of a variable.
Signed-off-by: Frank Rowand <frank.rowand@am.sony.com>
---
drivers/of/resolver.c | 42 ++++++++++++++++++++----------------------
1 file changed, 20 insertions(+), 22 deletions(-)
diff --git a/drivers/of/resolver.c b/drivers/of/resolver.c
index c61ba99a1792..31fd3800787a 100644
--- a/drivers/of/resolver.c
+++ b/drivers/of/resolver.c
@@ -33,10 +33,10 @@ static struct device_node *__of_find_node_by_full_name(struct device_node *node,
{
struct device_node *child, *found;
- if (node == NULL)
+ if (!node)
return NULL;
- if (of_node_cmp(node->full_name, full_name) == 0)
+ if (!of_node_cmp(node->full_name, full_name))
return of_node_get(node);
for_each_child_of_node(node, child) {
@@ -86,8 +86,8 @@ static void __of_adjust_tree_phandles(struct device_node *node,
for_each_property_of_node(node, prop) {
- if (of_prop_cmp(prop->name, "phandle") != 0 &&
- of_prop_cmp(prop->name, "linux,phandle") != 0)
+ if (of_prop_cmp(prop->name, "phandle") &&
+ of_prop_cmp(prop->name, "linux,phandle"))
continue;
if (prop->length < 4)
@@ -140,7 +140,7 @@ static int __of_adjust_phandle_ref(struct device_node *node,
*s++ = '\0';
err = kstrtoint(s, 10, &offset);
- if (err != 0)
+ if (err)
goto err_fail;
refnode = __of_find_node_by_full_name(node, nodestr);
@@ -148,7 +148,7 @@ static int __of_adjust_phandle_ref(struct device_node *node,
continue;
for_each_property_of_node(refnode, sprop) {
- if (of_prop_cmp(sprop->name, propstr) == 0)
+ if (!of_prop_cmp(sprop->name, propstr))
break;
}
of_node_put(refnode);
@@ -193,15 +193,15 @@ static int __of_adjust_tree_phandle_references(struct device_node *node,
unsigned int off;
phandle phandle;
- if (node == NULL)
+ if (!node)
return 0;
for_each_property_of_node(node, rprop) {
/* skip properties added automatically */
- if (of_prop_cmp(rprop->name, "name") == 0 ||
- of_prop_cmp(rprop->name, "phandle") == 0 ||
- of_prop_cmp(rprop->name, "linux,phandle") == 0)
+ if (!of_prop_cmp(rprop->name, "name") ||
+ !of_prop_cmp(rprop->name, "phandle") ||
+ !of_prop_cmp(rprop->name, "linux,phandle"))
continue;
if ((rprop->length % 4) != 0 || rprop->length == 0)
@@ -209,11 +209,11 @@ static int __of_adjust_tree_phandle_references(struct device_node *node,
count = rprop->length / sizeof(__be32);
for_each_property_of_node(target, sprop) {
- if (of_prop_cmp(sprop->name, rprop->name) == 0)
+ if (!of_prop_cmp(sprop->name, rprop->name))
break;
}
- if (sprop == NULL)
+ if (!sprop)
return -EINVAL;
for (i = 0; i < count; i++) {
@@ -232,7 +232,7 @@ static int __of_adjust_tree_phandle_references(struct device_node *node,
for_each_child_of_node(node, child) {
for_each_child_of_node(target, childtarget)
- if (__of_node_name_cmp(child, childtarget) == 0)
+ if (!__of_node_name_cmp(child, childtarget))
break;
if (!childtarget)
@@ -240,7 +240,7 @@ static int __of_adjust_tree_phandle_references(struct device_node *node,
err = __of_adjust_tree_phandle_references(child, childtarget,
phandle_delta);
- if (err != 0)
+ if (err)
return err;
}
@@ -282,13 +282,13 @@ int of_resolve_phandles(struct device_node *resolve)
childroot = NULL;
for_each_child_of_node(resolve, childroot)
- if (of_node_cmp(childroot->name, "__local_fixups__") == 0)
+ if (!of_node_cmp(childroot->name, "__local_fixups__"))
break;
if (childroot != NULL) {
err = __of_adjust_tree_phandle_references(childroot,
resolve, 0);
- if (err != 0)
+ if (err)
return err;
BUG_ON(__of_adjust_tree_phandle_references(childroot,
@@ -303,12 +303,10 @@ int of_resolve_phandles(struct device_node *resolve)
for_each_child_of_node(resolve, child) {
- if (!resolve_sym &&
- of_node_cmp(child->name, "__symbols__") == 0)
+ if (!resolve_sym && !of_node_cmp(child->name, "__symbols__"))
resolve_sym = child;
- if (!resolve_fix &&
- of_node_cmp(child->name, "__fixups__") == 0)
+ if (!resolve_fix && !of_node_cmp(child->name, "__fixups__"))
resolve_fix = child;
if (resolve_sym && resolve_fix)
@@ -329,12 +327,12 @@ int of_resolve_phandles(struct device_node *resolve)
for_each_property_of_node(resolve_fix, rprop) {
/* skip properties added automatically */
- if (of_prop_cmp(rprop->name, "name") == 0)
+ if (!of_prop_cmp(rprop->name, "name"))
continue;
err = of_property_read_string(root_sym,
rprop->name, &refpath);
- if (err != 0)
+ if (err)
goto out;
refnode = of_find_node_by_path(refpath);
--
1.9.1
^ permalink raw reply related
* [PATCH 02/12] of: Remove excessive printks to reduce clutter.
From: frowand.list @ 2016-10-29 6:26 UTC (permalink / raw)
To: Rob Herring, pantelis.antoniou, Pantelis Antoniou
Cc: devicetree, linux-kernel
In-Reply-To: <1477722392-32172-1-git-send-email-frowand.list@gmail.com>
From: Frank Rowand <frank.rowand@am.sony.com>
Remove extra debug and error printks. A single pr_err() will
be added at the end of this series to replace many of these
error messages.
Signed-off-by: Frank Rowand <frank.rowand@am.sony.com>
---
drivers/of/resolver.c | 53 ++++++++-------------------------------------------
1 file changed, 8 insertions(+), 45 deletions(-)
diff --git a/drivers/of/resolver.c b/drivers/of/resolver.c
index 4ff0220d7aa2..c61ba99a1792 100644
--- a/drivers/of/resolver.c
+++ b/drivers/of/resolver.c
@@ -115,11 +115,8 @@ static int __of_adjust_phandle_ref(struct device_node *node,
int err = 0;
propval = kmalloc(rprop->length, GFP_KERNEL);
- if (!propval) {
- pr_err("%s: Could not copy value of '%s'\n",
- __func__, rprop->name);
+ if (!propval)
return -ENOMEM;
- }
memcpy(propval, rprop->value, rprop->length);
propend = propval + rprop->length;
@@ -129,8 +126,6 @@ static int __of_adjust_phandle_ref(struct device_node *node,
nodestr = propcur;
s = strchr(propcur, ':');
if (!s) {
- pr_err("%s: Illegal symbol entry '%s' (1)\n",
- __func__, propcur);
err = -EINVAL;
goto err_fail;
}
@@ -139,26 +134,18 @@ static int __of_adjust_phandle_ref(struct device_node *node,
propstr = s;
s = strchr(s, ':');
if (!s) {
- pr_err("%s: Illegal symbol entry '%s' (2)\n",
- __func__, (char *)rprop->value);
err = -EINVAL;
goto err_fail;
}
*s++ = '\0';
err = kstrtoint(s, 10, &offset);
- if (err != 0) {
- pr_err("%s: Could get offset '%s'\n",
- __func__, (char *)rprop->value);
+ if (err != 0)
goto err_fail;
- }
refnode = __of_find_node_by_full_name(node, nodestr);
- if (!refnode) {
- pr_warn("%s: Could not find refnode '%s'\n",
- __func__, (char *)rprop->value);
+ if (!refnode)
continue;
- }
for_each_property_of_node(refnode, sprop) {
if (of_prop_cmp(sprop->name, propstr) == 0)
@@ -167,8 +154,6 @@ static int __of_adjust_phandle_ref(struct device_node *node,
of_node_put(refnode);
if (!sprop) {
- pr_err("%s: Could not find property '%s'\n",
- __func__, (char *)rprop->value);
err = -ENOENT;
goto err_fail;
}
@@ -219,11 +204,8 @@ static int __of_adjust_tree_phandle_references(struct device_node *node,
of_prop_cmp(rprop->name, "linux,phandle") == 0)
continue;
- if ((rprop->length % 4) != 0 || rprop->length == 0) {
- pr_err("%s: Illegal property (size) '%s' @%s\n",
- __func__, rprop->name, node->full_name);
+ if ((rprop->length % 4) != 0 || rprop->length == 0)
return -EINVAL;
- }
count = rprop->length / sizeof(__be32);
for_each_property_of_node(target, sprop) {
@@ -231,21 +213,13 @@ static int __of_adjust_tree_phandle_references(struct device_node *node,
break;
}
- if (sprop == NULL) {
- pr_err("%s: Could not find target property '%s' @%s\n",
- __func__, rprop->name, node->full_name);
+ if (sprop == NULL)
return -EINVAL;
- }
for (i = 0; i < count; i++) {
off = be32_to_cpu(((__be32 *)rprop->value)[i]);
- if (off >= sprop->length ||
- (off + 4) > sprop->length) {
- pr_err("%s: Illegal property '%s' @%s\n",
- __func__, rprop->name,
- node->full_name);
+ if (off >= sprop->length || (off + 4) > sprop->length)
return -EINVAL;
- }
if (phandle_delta) {
phandle = be32_to_cpu(*(__be32 *)(sprop->value + off));
@@ -261,11 +235,8 @@ static int __of_adjust_tree_phandle_references(struct device_node *node,
if (__of_node_name_cmp(child, childtarget) == 0)
break;
- if (!childtarget) {
- pr_err("%s: Could not find target child '%s' @%s\n",
- __func__, child->name, node->full_name);
+ if (!childtarget)
return -EINVAL;
- }
err = __of_adjust_tree_phandle_references(child, childtarget,
phandle_delta);
@@ -363,16 +334,11 @@ int of_resolve_phandles(struct device_node *resolve)
err = of_property_read_string(root_sym,
rprop->name, &refpath);
- if (err != 0) {
- pr_err("%s: Could not find symbol '%s'\n",
- __func__, rprop->name);
+ if (err != 0)
goto out;
- }
refnode = of_find_node_by_path(refpath);
if (!refnode) {
- pr_err("%s: Could not find node by path '%s'\n",
- __func__, refpath);
err = -ENOENT;
goto out;
}
@@ -380,9 +346,6 @@ int of_resolve_phandles(struct device_node *resolve)
phandle = refnode->phandle;
of_node_put(refnode);
- pr_debug("%s: %s phandle is 0x%08x\n",
- __func__, rprop->name, phandle);
-
err = __of_adjust_phandle_ref(resolve, rprop, phandle);
if (err)
break;
--
1.9.1
^ permalink raw reply related
* [PATCH 01/12] of: Remove comments that state the obvious, to reduce clutter
From: frowand.list @ 2016-10-29 6:26 UTC (permalink / raw)
To: Rob Herring, pantelis.antoniou, Pantelis Antoniou
Cc: devicetree, linux-kernel
In-Reply-To: <1477722392-32172-1-git-send-email-frowand.list@gmail.com>
From: Frank Rowand <frank.rowand@am.sony.com>
Remove comments that report what is obvious from the code.
Signed-off-by: Frank Rowand <frank.rowand@am.sony.com>
---
drivers/of/resolver.c | 31 ++-----------------------------
1 file changed, 2 insertions(+), 29 deletions(-)
diff --git a/drivers/of/resolver.c b/drivers/of/resolver.c
index 46325d6394cf..4ff0220d7aa2 100644
--- a/drivers/of/resolver.c
+++ b/drivers/of/resolver.c
@@ -36,7 +36,6 @@ static struct device_node *__of_find_node_by_full_name(struct device_node *node,
if (node == NULL)
return NULL;
- /* check */
if (of_node_cmp(node->full_name, full_name) == 0)
return of_node_get(node);
@@ -60,7 +59,6 @@ static phandle of_get_tree_max_phandle(void)
phandle phandle;
unsigned long flags;
- /* now search recursively */
raw_spin_lock_irqsave(&devtree_lock, flags);
phandle = 0;
for_each_of_allnodes(node) {
@@ -75,8 +73,6 @@ static phandle of_get_tree_max_phandle(void)
/*
* Adjust a subtree's phandle values by a given delta.
- * Makes sure not to just adjust the device node's phandle value,
- * but modify the phandle properties values as well.
*/
static void __of_adjust_tree_phandles(struct device_node *node,
int phandle_delta)
@@ -85,32 +81,25 @@ static void __of_adjust_tree_phandles(struct device_node *node,
struct property *prop;
phandle phandle;
- /* first adjust the node's phandle direct value */
if (node->phandle != 0 && node->phandle != OF_PHANDLE_ILLEGAL)
node->phandle += phandle_delta;
- /* now adjust phandle & linux,phandle values */
for_each_property_of_node(node, prop) {
- /* only look for these two */
if (of_prop_cmp(prop->name, "phandle") != 0 &&
of_prop_cmp(prop->name, "linux,phandle") != 0)
continue;
- /* must be big enough */
if (prop->length < 4)
continue;
- /* read phandle value */
phandle = be32_to_cpup(prop->value);
- if (phandle == OF_PHANDLE_ILLEGAL) /* unresolved */
+ if (phandle == OF_PHANDLE_ILLEGAL)
continue;
- /* adjust */
*(uint32_t *)prop->value = cpu_to_be32(node->phandle);
}
- /* now do the children recursively */
for_each_child_of_node(node, child)
__of_adjust_tree_phandles(child, phandle_delta);
}
@@ -125,7 +114,6 @@ static int __of_adjust_phandle_ref(struct device_node *node,
int offset, propcurlen;
int err = 0;
- /* make a copy */
propval = kmalloc(rprop->length, GFP_KERNEL);
if (!propval) {
pr_err("%s: Could not copy value of '%s'\n",
@@ -165,7 +153,6 @@ static int __of_adjust_phandle_ref(struct device_node *node,
goto err_fail;
}
- /* look into the resolve node for the full path */
refnode = __of_find_node_by_full_name(node, nodestr);
if (!refnode) {
pr_warn("%s: Could not find refnode '%s'\n",
@@ -173,7 +160,6 @@ static int __of_adjust_phandle_ref(struct device_node *node,
continue;
}
- /* now find the property */
for_each_property_of_node(refnode, sprop) {
if (of_prop_cmp(sprop->name, propstr) == 0)
break;
@@ -240,7 +226,6 @@ static int __of_adjust_tree_phandle_references(struct device_node *node,
}
count = rprop->length / sizeof(__be32);
- /* now find the target property */
for_each_property_of_node(target, sprop) {
if (of_prop_cmp(sprop->name, rprop->name) == 0)
break;
@@ -254,7 +239,6 @@ static int __of_adjust_tree_phandle_references(struct device_node *node,
for (i = 0; i < count; i++) {
off = be32_to_cpu(((__be32 *)rprop->value)[i]);
- /* make sure the offset doesn't overstep (even wrap) */
if (off >= sprop->length ||
(off + 4) > sprop->length) {
pr_err("%s: Illegal property '%s' @%s\n",
@@ -264,7 +248,6 @@ static int __of_adjust_tree_phandle_references(struct device_node *node,
}
if (phandle_delta) {
- /* adjust */
phandle = be32_to_cpu(*(__be32 *)(sprop->value + off));
phandle += phandle_delta;
*(__be32 *)(sprop->value + off) = cpu_to_be32(phandle);
@@ -320,22 +303,18 @@ int of_resolve_phandles(struct device_node *resolve)
if (resolve && !of_node_check_flag(resolve, OF_DETACHED))
pr_err("%s: node %s not detached\n", __func__,
resolve->full_name);
- /* the resolve node must exist, and be detached */
if (!resolve || !of_node_check_flag(resolve, OF_DETACHED))
return -EINVAL;
- /* first we need to adjust the phandles */
phandle_delta = of_get_tree_max_phandle() + 1;
__of_adjust_tree_phandles(resolve, phandle_delta);
- /* locate the local fixups */
childroot = NULL;
for_each_child_of_node(resolve, childroot)
if (of_node_cmp(childroot->name, "__local_fixups__") == 0)
break;
if (childroot != NULL) {
- /* resolve root is guaranteed to be the '/' */
err = __of_adjust_tree_phandle_references(childroot,
resolve, 0);
if (err != 0)
@@ -349,10 +328,8 @@ int of_resolve_phandles(struct device_node *resolve)
resolve_sym = NULL;
resolve_fix = NULL;
- /* this may fail (if no fixups are required) */
root_sym = of_find_node_by_path("/__symbols__");
- /* locate the symbols & fixups nodes on resolve */
for_each_child_of_node(resolve, child) {
if (!resolve_sym &&
@@ -363,18 +340,15 @@ int of_resolve_phandles(struct device_node *resolve)
of_node_cmp(child->name, "__fixups__") == 0)
resolve_fix = child;
- /* both found, don't bother anymore */
if (resolve_sym && resolve_fix)
break;
}
- /* we do allow for the case where no fixups are needed */
if (!resolve_fix) {
- err = 0; /* no error */
+ err = 0;
goto out;
}
- /* we need to fixup, but no root symbols... */
if (!root_sym) {
pr_err("%s: no symbols in root of device tree.\n", __func__);
err = -EINVAL;
@@ -415,7 +389,6 @@ int of_resolve_phandles(struct device_node *resolve)
}
out:
- /* NULL is handled by of_node_put as NOP */
of_node_put(root_sym);
return err;
--
1.9.1
^ permalink raw reply related
* [PATCH 00/12] of: Make drivers/of/resolver.c more readable
From: frowand.list-Re5JQEeQqe8AvxtiuMwx3w @ 2016-10-29 6:26 UTC (permalink / raw)
To: Rob Herring, pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w,
Pantelis Antoniou
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
From: Frank Rowand <frank.rowand-mEdOJwZ7QcZBDgjK7y7TUQ@public.gmane.org>
drivers/of/resolve.c is a bit difficult to read. Clean it up so
that review of future overlay related patches will be easier.
Most of the patches are intended to be reformatting, with no functional
change. Patches that are expected to have a functional change are:
Remove excessive printks to reduce clutter.
Update structure of code to be clearer, also remove BUG_ON()
Any functional change would reflect undefined behavior on bad overlay.
Some error message text modified.
BUG_ON() removed.
Add back an error message, restructured
The patches are grouped into sets of changes that are intended
to be easy to verify correctness through simple inspection.
Some of the individual patches have checkpatch warnings or errors.
But after all patches are applied, the number of errors and
warnings from running checkpatch against the entire file are
reduced to two line size warnings.
These patches are only tested via the unit tests. I do not have
expansion boards to test with real hardware.
changes from rfc to v1:
- Remove fewer one line comments
- Add more extensive header comment to of_resolve_phandles()
to explain the how and why of resolving phandles
- Update patch header comments
- Incorporated patch "Remove braces around single line blocks"
into the previous patch in the series
Frank Rowand (12):
of: Remove comments that state the obvious, to reduce clutter
of: Remove excessive printks to reduce clutter.
of: Convert comparisons to zero or NULL to logical expressions
of: Rename functions to more accurately reflect what they do
of: Remove prefix "__of_" from local function names
of: Rename variables to better reflect purpose or follow convention
of: Update structure of code to be clearer, also remove BUG_ON()
of: Remove redundant size check
of: Update comments to reflect changes and increase clarity
of: Add back an error message, restructured
of: Move setting of pointer to beside test for non-null
of: Remove unused variable overlay_symbols
drivers/of/resolver.c | 364 ++++++++++++++++++++++----------------------------
1 file changed, 156 insertions(+), 208 deletions(-)
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox